[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [XEN PATCH v3] hvmloader: Enable MMIO and I/O decode, after all resource allocation

On 14/04/2020 15:12, Jan Beulich wrote:
> On 14.04.2020 13:12, Harsha Shamsundara Havanur wrote:
>> It was observed that PCI MMIO and/or IO BARs were programmed with
>> memory and I/O decodes (bits 0 and 1 of PCI COMMAND register) enabled,
>> during PCI setup phase. This resulted in incorrect memory mapping as
>> soon as the lower half of the 64 bit bar is programmed.
>> This displaced any RAM mappings under 4G. After the
>> upper half is programmed PCI memory mapping is restored to its
>> intended high mem location, but the RAM displaced is not restored.
>> The OS then continues to boot and function until it tries to access
>> the displaced RAM at which point it suffers a page fault and crashes.
>> This patch address the issue by deferring enablement of memory and
>> I/O decode in command register until all the resources, like interrupts
>> I/O and/or MMIO BARs for all the PCI device functions are programmed,
>> in the descending order of memory requested.
>> Signed-off-by: Harsha Shamsundara Havanur <havanur@xxxxxxxxxx>
>> Reviewed-by: Paul Durrant <pdurrant@xxxxxxxxxx>
>> Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

I have not acked this patch.  My comment on v1 was correctly your
mis-spelling of "Ack-by" for Paul's tag.  I apologise if that wasn't
terribly clear.

> You've fixed bugs, implying you need to drop tags covering the code
> which was fixed. As said on v2, imo you should have dropped the tags
> then already.
>> --- a/tools/firmware/hvmloader/pci.c
>> +++ b/tools/firmware/hvmloader/pci.c
>> @@ -84,6 +84,7 @@ void pci_setup(void)
>>      uint32_t vga_devfn = 256;
>>      uint16_t class, vendor_id, device_id;
>>      unsigned int bar, pin, link, isa_irq;
>> +    uint8_t pci_devfn_decode_type[256] = {};
> I'm still waiting for a reply on my v1 comment on the stack
> consumption of this, suggesting two 256-bit bitmaps instead.

256 bytes of stack space isn't something to worry about.  Definitely not
for the complexity of using bitmaps instead.




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.