Re: [PATCH] hw/passthrough: Prevent QEMU from mapping PCI option ROM at address 0

["Jan Beulich" <JBeulich@suse.com>]

>>> On 12.05.14 at 15:26, <malcolm.crossley@citrix.com> wrote:
> On 12/05/14 14:09, Jan Beulich wrote:
>>>>> On 12.05.14 at 14:42, <malcolm.crossley@citrix.com> wrote:
>>> The PCI option ROM BAR uses the LSB to indicate if the BAR is enabled.
>>> The AMD graphics driver sets the address bit's of the BAR to 0 but leaves 
> the
>>> LSB set to 1. Whilst this is not good practice, QEMU should be ignoring the
>>> non address parts of the BAR.
>>>
>>> This patch adds masking of the non address parts of the BAR before comparing
>>> the address to 0.
>>> ---
>>>  hw/pass-through.c |    2 +-
>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/hw/pass-through.c b/hw/pass-through.c
>>> index 304c438..7d6aefc 100644
>>> --- a/hw/pass-through.c
>>> +++ b/hw/pass-through.c
>>> @@ -2208,7 +2208,7 @@ static void pt_bar_mapping_one(struct pt_dev *ptdev, 
> int bar, int io_enable,
>>>      }
>>>  
>>>      /* prevent guest software mapping memory resource to 00000000h */
>>> -    if ((base->bar_flag == PT_BAR_FLAG_MEM) && (r_addr == 0))
>>> +    if ((base->bar_flag == PT_BAR_FLAG_MEM) && ((r_addr & 
> PCI_BASE_ADDRESS_MEM_MASK) == 0))
>> 
>> You talk of the low bit, but mask off the low 4 - how does that fit
>> together? Didn't you rather mean PCI_ROM_ADDRESS_MASK &
>> ~PCI_ROM_ADDRESS_ENABLE in text and code?
> 
> The description provides an example of a driver setting the lower bits
> of the BAR.
> 
> The intent of the fix is to ensure no BAR is mapped address 0 which is
> achieved by ensuring only the address bits of the BAR are used for the
> comparison with 0.

But the address bits here are bits 11-31, not 1-31 or 4-31.

Jan