[v2] xen/pass-through: ROM BAR handling adjustments

Expecting the ROM BAR to be written with an all ones value when sizing
the region is wrong - the low bit has another meaning (enable/disable)
and bits 1..10 are reserved. The PCI spec also mandates writing all
ones to just the address portion of the register.

Use suitable constants also for initializing the ROM BAR register field
description.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Convert complex if() into simpler ones.
xen/pass-through: ROM BAR handling adjustments

Expecting the ROM BAR to be written with an all ones value when sizing
the region is wrong - the low bit has another meaning (enable/disable)
and bits 1..10 are reserved. The PCI spec also mandates writing all
ones to just the address portion of the register.

Use suitable constants also for initializing the ROM BAR register field
description.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Convert complex if() into simpler ones.

--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -249,10 +249,18 @@ static void xen_pt_pci_write_config(PCID
 
     /* check unused BAR register */
     index = xen_pt_bar_offset_to_index(addr);
-    if ((index >= 0) && (val > 0 && val < XEN_PT_BAR_ALLF) &&
-        (s->bases[index].bar_flag == XEN_PT_BAR_FLAG_UNUSED)) {
-        XEN_PT_WARN(d, "Guest attempt to set address to unused Base Address "
-                    "Register. (addr: 0x%02x, len: %d)\n", addr, len);
+    if ((index >= 0) && (val != 0)) {
+        uint32_t chk = val;
+
+        if (index == PCI_ROM_SLOT)
+            chk |= (uint32_t)~PCI_ROM_ADDRESS_MASK;
+
+        if ((chk != XEN_PT_BAR_ALLF) &&
+            (s->bases[index].bar_flag == XEN_PT_BAR_FLAG_UNUSED)) {
+            XEN_PT_WARN(d, "Guest attempt to set address to unused "
+                        "Base Address Register. (addr: 0x%02x, len: %d)\n",
+                        addr, len);
+        }
     }
 
     /* find register group entry */
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -729,8 +729,8 @@ static XenPTRegInfo xen_pt_emu_reg_heade
         .offset     = PCI_ROM_ADDRESS,
         .size       = 4,
         .init_val   = 0x00000000,
-        .ro_mask    = 0x000007FE,
-        .emu_mask   = 0xFFFFF800,
+        .ro_mask    = ~PCI_ROM_ADDRESS_MASK & ~PCI_ROM_ADDRESS_ENABLE,
+        .emu_mask   = (uint32_t)PCI_ROM_ADDRESS_MASK,
         .init       = xen_pt_bar_reg_init,
         .u.dw.read  = xen_pt_long_reg_read,
         .u.dw.write = xen_pt_exp_rom_bar_reg_write,

On Mon, 8 Jun 2015, Jan Beulich wrote:
> Expecting the ROM BAR to be written with an all ones value when sizing
> the region is wrong - the low bit has another meaning (enable/disable)
> and bits 1..10 are reserved. The PCI spec also mandates writing all
> ones to just the address portion of the register.
> 
> Use suitable constants also for initializing the ROM BAR register field
> description.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Added to my queue


> v2: Convert complex if() into simpler ones.
> 
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -249,10 +249,18 @@ static void xen_pt_pci_write_config(PCID
>  
>      /* check unused BAR register */
>      index = xen_pt_bar_offset_to_index(addr);
> -    if ((index >= 0) && (val > 0 && val < XEN_PT_BAR_ALLF) &&
> -        (s->bases[index].bar_flag == XEN_PT_BAR_FLAG_UNUSED)) {
> -        XEN_PT_WARN(d, "Guest attempt to set address to unused Base Address "
> -                    "Register. (addr: 0x%02x, len: %d)\n", addr, len);
> +    if ((index >= 0) && (val != 0)) {
> +        uint32_t chk = val;
> +
> +        if (index == PCI_ROM_SLOT)
> +            chk |= (uint32_t)~PCI_ROM_ADDRESS_MASK;
> +
> +        if ((chk != XEN_PT_BAR_ALLF) &&
> +            (s->bases[index].bar_flag == XEN_PT_BAR_FLAG_UNUSED)) {
> +            XEN_PT_WARN(d, "Guest attempt to set address to unused "
> +                        "Base Address Register. (addr: 0x%02x, len: %d)\n",
> +                        addr, len);
> +        }
>      }
>  
>      /* find register group entry */
> --- a/hw/xen/xen_pt_config_init.c
> +++ b/hw/xen/xen_pt_config_init.c
> @@ -729,8 +729,8 @@ static XenPTRegInfo xen_pt_emu_reg_heade
>          .offset     = PCI_ROM_ADDRESS,
>          .size       = 4,
>          .init_val   = 0x00000000,
> -        .ro_mask    = 0x000007FE,
> -        .emu_mask   = 0xFFFFF800,
> +        .ro_mask    = ~PCI_ROM_ADDRESS_MASK & ~PCI_ROM_ADDRESS_ENABLE,
> +        .emu_mask   = (uint32_t)PCI_ROM_ADDRESS_MASK,
>          .init       = xen_pt_bar_reg_init,
>          .u.dw.read  = xen_pt_long_reg_read,
>          .u.dw.write = xen_pt_exp_rom_bar_reg_write,
> 
> 
> 
>