Re: [Xen-devel] PCI passthrough to QEMU traditional stubdom not working when option ROM present

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.10.16 at 15:56, <eshelton@xxxxxxxxx> wrote:
> On Mon, Oct 24, 2016 at 9:12 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>> On 24.10.16 at 15:01, <eshelton@xxxxxxxxx> wrote:
>>> On Mon, Oct 24, 2016 at 8:47 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>>>> On 24.10.16 at 14:30, <eshelton@xxxxxxxxx> wrote:
>>>>> As best as I can tell, the current code already writes out the proper
>>>>> value for sizing ("pci_write_long(d, reg, ~0)" in the above code
>>>>> snippet).
>>>>
>>>> No - for sizing a ROM BAR, only the address portion is supposed to
>>>> be written with all 1s as per the PCI spec. In particular it should be
>>>> quite clear that writing the enable bit with 1 is a bad thing when the
>>>> address portion is also written with all 1s.
>>>>
>>>> I'd like to note though that as of commit d2bd05d88d ("xen-pciback:
>>>> return proper values during BAR sizing") the handling there is more
>>>> relaxed - previously writes of 0xFFFFFFFE were the only ones
>>>> considered to be sizing ones.
>>>
>>> Rev 2.2 of the PCI spec, at 6.2.5.2, states that "configuration
>>> software can determine how much address space the device requires by
>>> writing a value of all 1' to the address portion of the register."
>>> Does the "address portion" refer to the upper 21 bits (meaning we
>>> should write 0xFFFFF800) or the upper 31 bits (meaning we should write
>>> 0xFFFFFFFE)?
>>
>> Clearly the upper 21 bits.
> 
> It's perhaps not all that clear, given that earlier pciback code
> expected 0xFFFFFFFE.

Well, the "clearly" referred to the PCI spec.

>>>  4.4.14 pciback code appears to expect the latter, and
>>> with the newer more relaxed pciback code it doesn't matter.  For
>>> purposes of fixing this bug in Xen, which do you consider to be the
>>> "correct" sizing write value for QEMU to be writing out when it sizes
>>> the expansion ROM?
>>
>> 0xFFFFF800 | (<read-val> & 0x7FE)
> 
> Excuse me for being a little dense, but I don't understand what is
> going on with the "| (<read-val> & 0x7FE)" portion above.  Should the
> fixed code be:
> 
> pci_write_long(d, reg, ~PCI_ROM_ADDRESS_MASK);
> 
> Or something else?

You first read the BAR, mask out the enable bit, or in the address
mask, and write out the resulting value.

Jan

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