Re: [Xen-devel] [PATCH 4/6] xen/pass-through: correctly deal with RW1C bits

2015-06-16 Thread Stefano Stabellini
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 jbeul...@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   = 0x,
  .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.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 4/6] xen/pass-through: correctly deal with RW1C bits

2015-06-16 Thread Jan Beulich
 On 16.06.15 at 16:19, stefano.stabell...@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


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


[Xen-devel] [PATCH 4/6] xen/pass-through: correctly deal with RW1C bits

2015-06-05 Thread Jan Beulich
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 jbeul...@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   = 0x,
 .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,


xen/pass-through: correctly deal with RW1C bits

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 jbeul...@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,