qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: Jiahui Cen <cenjiahui@huawei.com>, qemu-devel@nongnu.org
Cc: xieyingtai@huawei.com, "Michael S. Tsirkin" <mst@redhat.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	"Ard Biesheuvel \(ARM address\)" <ard.biesheuvel@arm.com>,
	Igor Mammedov <imammedo@redhat.com>
Subject: Re: [PATCH] acpi/gpex: Inform os to keep firmware resource map
Date: Thu, 17 Dec 2020 18:23:59 +0100	[thread overview]
Message-ID: <6eb6b9f9-d1b5-b4da-d3e0-fe7c9aa6ab87@redhat.com> (raw)
In-Reply-To: <44529260-f89a-67b5-d6ab-3652376badcc@huawei.com>

On 12/17/20 14:52, Jiahui Cen wrote:
> +Laszlo
> 
> On 2020/12/17 21:29, Jiahui Cen wrote:
>> There may be some differences in pci resource assignment between guest os
>> and firmware.
>>
>> Eg. A Bridge with Bus [d2]
>>     -+-[0000:d2]---01.0-[d3]----01.0
>>
>>     where [d2:01.00] is a pcie-pci-bridge with BAR0 (mem, 64-bit, non-pref) [size=256]
>>           [d3:01.00] is a PCI Device with BAR0 (mem, 64-bit, pref) [size=128K]
>>                                           BAR4 (mem, 64-bit, pref) [size=64M]
>>
>>     In EDK2, the Resource Map would be:
>>         PciBus: Resource Map for Bridge [D2|01|00]
>>         Type = PMem64; Base = 0x8004000000;     Length = 0x4100000;     Alignment = 0x3FFFFFF
>>            Base = 0x8004000000; Length = 0x4000000;     Alignment = 0x3FFFFFF;  Owner = PCI [D3|01|00:20]
>>            Base = 0x8008000000; Length = 0x20000;       Alignment = 0x1FFFF;    Owner = PCI [D3|01|00:10]
>>         Type =  Mem64; Base = 0x8008100000;     Length = 0x100; Alignment = 0xFFF
>>
>>     While in Linux, kernel will use 0x2FFFFFF as the alignment to calculate
>>     the PMem64 size, which would be 0x6000000.
>>
>> The diffences could result in resource assignment failure.
>>
>> Using _DSM #5 method to inform guest os not to ignore the PCI configuration
>> that firmware has done at boot time could handle the differences.
>>
>> Signed-off-by: Jiahui Cen <cenjiahui@huawei.com>
>> ---
>>  hw/pci-host/gpex-acpi.c | 11 ++++++++++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
>> index 071aa11b5c..2b490f3379 100644
>> --- a/hw/pci-host/gpex-acpi.c
>> +++ b/hw/pci-host/gpex-acpi.c
>> @@ -112,10 +112,19 @@ static void acpi_dsdt_add_pci_osc(Aml *dev)
>>      UUID = aml_touuid("E5C937D0-3553-4D7A-9117-EA4D19C3434D");
>>      ifctx = aml_if(aml_equal(aml_arg(0), UUID));
>>      ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
>> -    uint8_t byte_list[1] = {1};
>> +    uint8_t byte_list[1] = {0x21};
>>      buf = aml_buffer(1, byte_list);
>>      aml_append(ifctx1, aml_return(buf));
>>      aml_append(ifctx, ifctx1);
>> +
>> +    /* PCI Firmware Specification 3.2
>> +     * 4.6.5. _DSM for Ignoring PCI Boot Configurations
>> +     * The UUID in _DSM in this context is
>> +     * {E5C937D0-3553-4D7A-9117-EA4D19C3434D}
>> +     */
>> +    ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(5)));
>> +    aml_append(ifctx1, aml_return(aml_int(0)));
>> +    aml_append(ifctx, ifctx1);
>>      aml_append(method, ifctx);
>>  
>>      byte_list[0] = 0;
>>
> 

Seems to make sense to me (I didn't realize we already had the _DSM
method with this GUID!), but now I'm not sure what to expect of the
guest kernel, in light of what Ard said. So if it works now, is that by
accident, or is it an intentional, fresh commit in the kernel? Like
a78cf9657ba5 ("PCI/ACPI: Evaluate PCI Boot Configuration _DSM", 2019-06-21)?

Benjamin: can you please tell us something about this Linux commit? What
was the motivation for it?

Hmm.... this commit seems to be a part of the following series:

a78cf9657ba5 PCI/ACPI: Evaluate PCI Boot Configuration _DSM
7ac0d094fbe9 PCI: Don't auto-realloc if we're preserving firmware config
3e8ba9686600 arm64: PCI: Allow resource reallocation if necessary
85dc04136e86 arm64: PCI: Preserve firmware configuration when desired

OK, after reading through the commit messages in those commits (esp.
7ac0d094fbe9), I think the Linux change was made exactly for the purpose
that we want it for -- stick with the firmware assignments.

Ard, does that seem right, or am I misunderstanding the kernel series?

Thanks
Laszlo



  reply	other threads:[~2020-12-17 17:40 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-17 13:29 Jiahui Cen
2020-12-17 13:52 ` Jiahui Cen
2020-12-17 17:23   ` Laszlo Ersek [this message]
2020-12-17 19:24     ` Ard Biesheuvel
2020-12-17 18:29 ` Michael S. Tsirkin
2020-12-18  5:56   ` Jiahui Cen
2020-12-17 20:04 ` Michael S. Tsirkin
2020-12-18  5:56   ` Jiahui Cen
2020-12-19 19:06     ` Michael S. Tsirkin
2020-12-21  1:12       ` Jiahui Cen
2021-07-22  5:22 ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6eb6b9f9-d1b5-b4da-d3e0-fe7c9aa6ab87@redhat.com \
    --to=lersek@redhat.com \
    --cc=ard.biesheuvel@arm.com \
    --cc=cenjiahui@huawei.com \
    --cc=imammedo@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=xieyingtai@huawei.com \
    --subject='Re: [PATCH] acpi/gpex: Inform os to keep firmware resource map' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).