Message ID | 5571AC2E020000780008156B@mail.emea.novell.com |
---|---|
State | New |
Headers | show |
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 <jbeulich@suse.com> > > --- 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.
>>> On 16.06.15 at 16:19, <stefano.stabellini@eu.citrix.com> wrote: > On Fri, 5 Jun 2015, Jan Beulich wrote: >> @@ -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. I'm not sure what you're after in a patch titled "correctly deal with RW1C bits". Jan
--- 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,