Re: [Xen-devel] [PATCH] xen/pass-through: ROM BAR handling adjustments

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 15 May 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@xxxxxxxx>
> 
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -248,7 +248,9 @@ 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) &&
> +    if ((index >= 0) && (val != 0) &&
> +        (((index != PCI_ROM_SLOT) ?
> +          val : (val | (uint32_t)~PCI_ROM_ADDRESS_MASK)) != XEN_PT_BAR_ALLF) 
> &&

The change seems looks good and in line with the commit message. But
this if statement looks like acrobatic circus to me now.


>          (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);
> --- a/hw/xen/xen_pt_config_init.c
> +++ b/hw/xen/xen_pt_config_init.c
> @@ -726,8 +726,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,

There is no need for the explicit cast, is there?


>          .init       = xen_pt_bar_reg_init,
>          .u.dw.read  = xen_pt_long_reg_read,
>          .u.dw.write = xen_pt_exp_rom_bar_reg_write,
> 
> 
> 

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