From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Stabellini Subject: Re: [PATCH 4/6] xen/pass-through: correctly deal with RW1C bits Date: Tue, 16 Jun 2015 15:19:58 +0100 Message-ID: References: <5571AA3B020000780008152E@mail.emea.novell.com> <5571AC2E020000780008156B@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Z4rjB-0004nJ-Fz for xen-devel@lists.xenproject.org; Tue, 16 Jun 2015 14:20:37 +0000 In-Reply-To: <5571AC2E020000780008156B@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel , qemu-devel@nongnu.org, Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On Fri, 5 Jun 2015, Jan Beulich wrote: > Introduce yet another mask for them, so that the generic routine can > handle them, at once rendering xen_pt_pmcsr_reg_write() superfluous. > > Signed-off-by: Jan Beulich > > --- a/qemu/upstream/hw/xen/xen_pt.h > +++ b/qemu/upstream/hw/xen/xen_pt.h > @@ -105,6 +105,8 @@ struct XenPTRegInfo { > uint32_t res_mask; > /* reg read only field mask (ON:RO/ROS, OFF:other) */ > uint32_t ro_mask; > + /* reg read/write-1-clear field mask (ON:RW1C/RW1CS, OFF:other) */ > + uint32_t rw1c_mask; > /* reg emulate field mask (ON:emu, OFF:passthrough) */ > uint32_t emu_mask; > xen_pt_conf_reg_init init; > --- a/qemu/upstream/hw/xen/xen_pt_config_init.c > +++ b/qemu/upstream/hw/xen/xen_pt_config_init.c > @@ -176,7 +176,8 @@ static int xen_pt_byte_reg_write(XenPCIP > cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask); > > /* create value for writing to I/O device register */ > - *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); > + *val = XEN_PT_MERGE_VALUE(*val, dev_value & ~reg->rw1c_mask, > + throughable_mask); > > return 0; > } > @@ -193,7 +194,8 @@ static int xen_pt_word_reg_write(XenPCIP > cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask); > > /* create value for writing to I/O device register */ > - *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); > + *val = XEN_PT_MERGE_VALUE(*val, dev_value & ~reg->rw1c_mask, > + throughable_mask); > > return 0; > } > @@ -210,7 +212,8 @@ static int xen_pt_long_reg_write(XenPCIP > cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask); > > /* create value for writing to I/O device register */ > - *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); > + *val = XEN_PT_MERGE_VALUE(*val, dev_value & ~reg->rw1c_mask, > + throughable_mask); > > return 0; > } > @@ -611,6 +614,7 @@ static XenPTRegInfo xen_pt_emu_reg_heade > .init_val = 0x0000, > .res_mask = 0x0007, > .ro_mask = 0x06F8, > + .rw1c_mask = 0xF900, > .emu_mask = 0x0010, > .init = xen_pt_status_reg_init, > .u.w.read = xen_pt_word_reg_read, > @@ -910,6 +914,7 @@ static XenPTRegInfo xen_pt_emu_reg_pcie[ > .size = 2, > .res_mask = 0xFFC0, > .ro_mask = 0x0030, > + .rw1c_mask = 0x000F, > .init = xen_pt_common_reg_init, > .u.w.read = xen_pt_word_reg_read, > .u.w.write = xen_pt_word_reg_write, > @@ -930,6 +935,7 @@ static XenPTRegInfo xen_pt_emu_reg_pcie[ > .offset = PCI_EXP_LNKSTA, > .size = 2, > .ro_mask = 0x3FFF, > + .rw1c_mask = 0xC000, > .init = xen_pt_common_reg_init, > .u.w.read = xen_pt_word_reg_read, > .u.w.write = xen_pt_word_reg_write, > @@ -966,26 +972,6 @@ static XenPTRegInfo xen_pt_emu_reg_pcie[ > * Power Management Capability > */ > > -/* write Power Management Control/Status register */ > -static int xen_pt_pmcsr_reg_write(XenPCIPassthroughState *s, > - XenPTReg *cfg_entry, uint16_t *val, > - uint16_t dev_value, uint16_t valid_mask) > -{ > - XenPTRegInfo *reg = cfg_entry->reg; > - uint16_t writable_mask = 0; > - uint16_t throughable_mask = get_throughable_mask(s, reg, valid_mask); > - > - /* modify emulate register */ > - writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask; > - cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask); > - > - /* create value for writing to I/O device register */ > - *val = XEN_PT_MERGE_VALUE(*val, dev_value & ~PCI_PM_CTRL_PME_STATUS, > - throughable_mask); > - > - return 0; > -} > - > /* Power Management Capability reg static information table */ > static XenPTRegInfo xen_pt_emu_reg_pm[] = { > /* Next Pointer reg */ > @@ -1016,11 +1002,12 @@ static XenPTRegInfo xen_pt_emu_reg_pm[] > .size = 2, > .init_val = 0x0008, > .res_mask = 0x00F0, > - .ro_mask = 0xE10C, > + .ro_mask = 0x610C, > + .rw1c_mask = 0x8000, > .emu_mask = 0x810B, > .init = xen_pt_common_reg_init, > .u.w.read = xen_pt_word_reg_read, > - .u.w.write = xen_pt_pmcsr_reg_write, > + .u.w.write = xen_pt_word_reg_write, > }, > { > .size = 0, I can see that the code change doesn't cause a change in behaviour for PCI_PM_CTRL, but it does for PCI_STATUS, PCI_EXP_DEVSTA and PCI_EXP_LNKSTA. Please explain why in the commit message.