diff mbox

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

Message ID 5571AC2E020000780008156B@mail.emea.novell.com
State New
Headers show

Commit Message

Jan Beulich June 5, 2015, 12:03 p.m. UTC 
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>
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 <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,

Comments

Stefano Stabellini June 16, 2015, 2:19 p.m. UTC | #1 
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.
Jan Beulich June 16, 2015, 2:38 p.m. UTC | #2 
>>> 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
diff mbox

Patch

--- 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,