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

[Jan Beulich]

>>> On 05.06.15 at 13:32, <address@hidden> wrote:
>> --- 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.

I think the alternative of splitting it up into multiple if()-s would not
be any better readable.

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

There is - PCI_ROM_ADDRESS_MASK being wider than 32 bits the
assignment could cause compiler warning otherwise (which I think
I actually ran into.

Jan