Re: [edk2] [Patch 0/4] MdeModulePkg/PciBus Do not improperly degrade resource
On 05/24/16 17:29, Ni, Ruiyu wrote: > Laszlo, > I am terribly sorry about that! For this patch serials, I didn't noticed your > r-b. That's why I offline asked Jeff to send his r-b. I can add your tested > by tag, there is no reason to ignore your review by! > > For the dependency patch, I forgot your review by mail after a weekend. > > I will search related mails when committing patches to avoid ignoring mails > again.(Outlook doesn't organize these mails from this mail list in thread > style) Ah, sorry. In this case I'll clearly blame Outlook. It is plain impossible to work with patches and review feedback if the MUA doesn't support threading. Barring a MUA switch on your side, I guess I could recommend reviewing the patch series thread on GMANE (in the mailing list archive) one last time, before committing the patches. GMANE has a great threading WebUI, which would likely provide you with all the feedback for a patch series, in a nice centralized view. Thanks, Laszlo >> 在 2016年5月24日,下午8:28,Laszlo Ersek 写道: >> >> Ray, >> >>> On 05/18/16 13:35, Laszlo Ersek wrote: >>> >>> For #1 through #3: >>> Reviewed-by: Laszlo Ersek >> >> [snip] >> >>> - Anyway I'll leave the above two points up to your consideration. For >>> patch #4 too: >>> Reviewed-by: Laszlo Ersek >> >> I would *greatly* appreciate if you didn't ignore my (positive) reviews. >> >> Just because I'm not an official package maintainer for MdeModulePkg, I >> do expect that my Reviewed-by tags be picked up, when I make the effort >> to review MdeModulePkg patches. >> >> I have just noticed that you didn't pick up my above Reviewed-by tags, >> for commits >> >> cf81d5a68052 MdeModulePkg/PciBus: use better name for local variables. >> 48495aae386a MdeModulePkg/PciBus: Remove unused fields in PCI_BAR >> ea669c1ba331 MdeModulePkg/PciBus: Use shorter global variable name >> 05070c1b471b MdeModulePkg/PciBus: do not improperly degrade resource >> >> I see that you picked up my Tested-by tag (for all of the patches in >> this series), but I didn't just test these patches, I also reviewed them. >> >> I see the exact same occur in commit >> >> 0b58c4894dad MdeModulePkg/PciHostBridgeDxe: Add CpuArch protocol >> dependency >> >> I reviewed that patch on the list, but the committed version does not >> have my R-b. >> >> In general, if you receive *any* kind of feedback tag from anyone in the >> community, it is more or less your "duty" to preserve those tags when >> you commit the patch. If you squander reviews that you get (even if they >> are positive reviews), that's a big dis-incentive for future feedback. >> >> Thanks >> Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch 0/4] MdeModulePkg/PciBus Do not improperly degrade resource
Laszlo, I am terribly sorry about that! For this patch serials, I didn't noticed your r-b. That's why I offline asked Jeff to send his r-b. I can add your tested by tag, there is no reason to ignore your review by! For the dependency patch, I forgot your review by mail after a weekend. I will search related mails when committing patches to avoid ignoring mails again.(Outlook doesn't organize these mails from this mail list in thread style) Thanks, Ray > 在 2016年5月24日,下午8:28,Laszlo Ersek 写道: > > Ray, > >> On 05/18/16 13:35, Laszlo Ersek wrote: >> >> For #1 through #3: >> Reviewed-by: Laszlo Ersek > > [snip] > >> - Anyway I'll leave the above two points up to your consideration. For patch >> #4 too: >> Reviewed-by: Laszlo Ersek > > I would *greatly* appreciate if you didn't ignore my (positive) reviews. > > Just because I'm not an official package maintainer for MdeModulePkg, I > do expect that my Reviewed-by tags be picked up, when I make the effort > to review MdeModulePkg patches. > > I have just noticed that you didn't pick up my above Reviewed-by tags, > for commits > > cf81d5a68052 MdeModulePkg/PciBus: use better name for local variables. > 48495aae386a MdeModulePkg/PciBus: Remove unused fields in PCI_BAR > ea669c1ba331 MdeModulePkg/PciBus: Use shorter global variable name > 05070c1b471b MdeModulePkg/PciBus: do not improperly degrade resource > > I see that you picked up my Tested-by tag (for all of the patches in > this series), but I didn't just test these patches, I also reviewed them. > > I see the exact same occur in commit > > 0b58c4894dad MdeModulePkg/PciHostBridgeDxe: Add CpuArch protocol > dependency > > I reviewed that patch on the list, but the committed version does not > have my R-b. > > In general, if you receive *any* kind of feedback tag from anyone in the > community, it is more or less your "duty" to preserve those tags when > you commit the patch. If you squander reviews that you get (even if they > are positive reviews), that's a big dis-incentive for future feedback. > > Thanks > Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch 0/4] MdeModulePkg/PciBus Do not improperly degrade resource
Ray, On 05/18/16 13:35, Laszlo Ersek wrote: > For #1 through #3: > Reviewed-by: Laszlo Ersek [snip] > - Anyway I'll leave the above two points up to your consideration. For patch > #4 too: > Reviewed-by: Laszlo Ersek I would *greatly* appreciate if you didn't ignore my (positive) reviews. Just because I'm not an official package maintainer for MdeModulePkg, I do expect that my Reviewed-by tags be picked up, when I make the effort to review MdeModulePkg patches. I have just noticed that you didn't pick up my above Reviewed-by tags, for commits cf81d5a68052 MdeModulePkg/PciBus: use better name for local variables. 48495aae386a MdeModulePkg/PciBus: Remove unused fields in PCI_BAR ea669c1ba331 MdeModulePkg/PciBus: Use shorter global variable name 05070c1b471b MdeModulePkg/PciBus: do not improperly degrade resource I see that you picked up my Tested-by tag (for all of the patches in this series), but I didn't just test these patches, I also reviewed them. I see the exact same occur in commit 0b58c4894dad MdeModulePkg/PciHostBridgeDxe: Add CpuArch protocol dependency I reviewed that patch on the list, but the committed version does not have my R-b. In general, if you receive *any* kind of feedback tag from anyone in the community, it is more or less your "duty" to preserve those tags when you commit the patch. If you squander reviews that you get (even if they are positive reviews), that's a big dis-incentive for future feedback. Thanks Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch 0/4] MdeModulePkg/PciBus Do not improperly degrade resource
Reviewed-by: Jeff Fan -Original Message- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni Sent: Tuesday, May 17, 2016 10:04 AM To: edk2-devel@lists.01.org Cc: Ni, Ruiyu Subject: [edk2] [Patch 0/4] MdeModulePkg/PciBus Do not improperly degrade resource The patch serials refined the PciBus code and adds a new feature following PI spec 1.4a to not improperly degrade resource. Ruiyu Ni (4): MdeModulePkg/PciBus: use better name for local variables. MdeModulePkg/PciBus: Remove unused fields in PCI_BAR MdeModulePkg/PciBus: Use shorter global variable name MdeModulePkg/PciBus: do not improperly degrade resource MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c| 6 +-- MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h| 5 +- .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 58 +- .../Bus/Pci/PciBusDxe/PciResourceSupport.c | 26 ++ 4 files changed, 65 insertions(+), 30 deletions(-) -- 2.7.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch 0/4] MdeModulePkg/PciBus Do not improperly degrade resource
On 05/18/16 17:08, Ni, Ruiyu wrote: > I agree to put ecr number in commit message. Series Tested-by: Laszlo Ersek I'll soon submit a patch that enables this new functionality for OvmfPkg. Thank you! Laszlo >> 在 2016年5月18日,下午7:35,Laszlo Ersek 写道: >> >> On 05/18/16 10:16, Ni, Ruiyu wrote: >>>> -Original Message- >>>> From: Laszlo Ersek [mailto:ler...@redhat.com] >>>> Sent: Tuesday, May 17, 2016 8:23 PM >>>> To: Ni, Ruiyu >>>> Cc: edk2-de...@ml01.01.org >>>> Subject: Re: [edk2] [Patch 0/4] MdeModulePkg/PciBus Do not improperly >>>> degrade resource >>>> >>>>> On 05/17/16 04:03, Ruiyu Ni wrote: >>>>> The patch serials refined the PciBus code and adds a new feature following >>>>> PI spec 1.4a to not improperly degrade resource. >>>>> >>>>> Ruiyu Ni (4): >>>>> MdeModulePkg/PciBus: use better name for local variables. >>>>> MdeModulePkg/PciBus: Remove unused fields in PCI_BAR >>>>> MdeModulePkg/PciBus: Use shorter global variable name >>>>> MdeModulePkg/PciBus: do not improperly degrade resource >>>>> >>>>> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c| 6 +-- >>>>> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h| 5 +- >>>>> .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 58 >>>>> +- >>>>> .../Bus/Pci/PciBusDxe/PciResourceSupport.c | 26 ++ >>>>> 4 files changed, 65 insertions(+), 30 deletions(-) >>>> >>>> * Please add the following reference to patch #4: >>>> >>>> https://mantis.uefi.org/mantis/view.php?id=1529 >>> >>> I think it might not be proper to include the member only >>> web page in the commit message. >> >> I disagree -- I think the mantis ticket number should always be included in >> a commit message, if the patch is related to a Mantis ticket. Sharing just >> the number of the mantis ticket does not disclose any information that is >> not also publicly available from the UEFI spec itself (the UEFI spec starts >> with a long changelog of Mantis tickets). >> >> The contents of the mantis ticket *are* private, but for actually reading a >> ticket, general UEFI membership and the appropriate WG membership are >> required too. So, capturing the mantis ticket number in the commit message >> helps contributors that have access to Mantis, without disclosing sensitive >> information to those who haven't. >> >> Personally I frequently go to Mantis to understand the background of UEFI or >> PI spec changes better. The ECRs attached to Mantis tickets often describe >> use cases that are not carried over to the spec, but are important >> background. >> >> In this specific case, the PI spec 1.4a, Volume 5, has the following in its >> changelog: >> >> 1529 address space granularity errata >> >> >>>> * I'm interested in this work primarily due to GPU assignment to >>>> QEMU/KVM virtual machines. So I built OVMF applied with these patches, >>>> and checked if they made a difference for the PCI resource map, with an >>>> Nvidia GTX750 assigned to the guest. >>>> >>>> There's no difference; the 256MB BAR is still allocated under 4GB. >>>> >>>> * Apparently, EFI_INCOMPATIBLE_PCI_DEVICE_SUPPORT_PROTOCOL must be a >>>> singleton protocol; it should come from the platform firmware, not from >>>> the option ROM. In that case, how could OVMF implement this protocol, so >>>> that such a GPU BAR would be allocated high? >>> >>> Option ROM depends on the PCI resource assignment so it’s like a >>> chicken-egg problem if we let option rom produce such protocol. >>> Singleton is enough I think. >> >> I agree. >> >>>> Looking at the CheckDevice() spec, I guess we could hard-code some PCI >>>> vendor and device IDs, and set AddrSpaceGranularity to 64. However, >>>> that's the only thing we should do; I don't think we should do the rest >>>> of the PCI BAR probing. >>> >>> PCI BAR probing is still needed. We do not change the granularity >>> for 32bit BAR. >>>> >>>> ... The only thing I'd like to say to the resource allocator is, >>>> >>>> If CSM is disabled, then please don't degrade the MMIO64 BARs of this >>>> device, just be
Re: [edk2] [Patch 0/4] MdeModulePkg/PciBus Do not improperly degrade resource
I agree to put ecr number in commit message. Thanks, Ray > 在 2016年5月18日,下午7:35,Laszlo Ersek 写道: > > On 05/18/16 10:16, Ni, Ruiyu wrote: >>> -Original Message- >>> From: Laszlo Ersek [mailto:ler...@redhat.com] >>> Sent: Tuesday, May 17, 2016 8:23 PM >>> To: Ni, Ruiyu >>> Cc: edk2-de...@ml01.01.org >>> Subject: Re: [edk2] [Patch 0/4] MdeModulePkg/PciBus Do not improperly >>> degrade resource >>> >>>> On 05/17/16 04:03, Ruiyu Ni wrote: >>>> The patch serials refined the PciBus code and adds a new feature following >>>> PI spec 1.4a to not improperly degrade resource. >>>> >>>> Ruiyu Ni (4): >>>> MdeModulePkg/PciBus: use better name for local variables. >>>> MdeModulePkg/PciBus: Remove unused fields in PCI_BAR >>>> MdeModulePkg/PciBus: Use shorter global variable name >>>> MdeModulePkg/PciBus: do not improperly degrade resource >>>> >>>> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c| 6 +-- >>>> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h| 5 +- >>>> .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 58 >>>> +- >>>> .../Bus/Pci/PciBusDxe/PciResourceSupport.c | 26 ++ >>>> 4 files changed, 65 insertions(+), 30 deletions(-) >>> >>> * Please add the following reference to patch #4: >>> >>> https://mantis.uefi.org/mantis/view.php?id=1529 >> >> I think it might not be proper to include the member only >> web page in the commit message. > > I disagree -- I think the mantis ticket number should always be included in a > commit message, if the patch is related to a Mantis ticket. Sharing just the > number of the mantis ticket does not disclose any information that is not > also publicly available from the UEFI spec itself (the UEFI spec starts with > a long changelog of Mantis tickets). > > The contents of the mantis ticket *are* private, but for actually reading a > ticket, general UEFI membership and the appropriate WG membership are > required too. So, capturing the mantis ticket number in the commit message > helps contributors that have access to Mantis, without disclosing sensitive > information to those who haven't. > > Personally I frequently go to Mantis to understand the background of UEFI or > PI spec changes better. The ECRs attached to Mantis tickets often describe > use cases that are not carried over to the spec, but are important background. > > In this specific case, the PI spec 1.4a, Volume 5, has the following in its > changelog: > > 1529 address space granularity errata > > >>> * I'm interested in this work primarily due to GPU assignment to >>> QEMU/KVM virtual machines. So I built OVMF applied with these patches, >>> and checked if they made a difference for the PCI resource map, with an >>> Nvidia GTX750 assigned to the guest. >>> >>> There's no difference; the 256MB BAR is still allocated under 4GB. >>> >>> * Apparently, EFI_INCOMPATIBLE_PCI_DEVICE_SUPPORT_PROTOCOL must be a >>> singleton protocol; it should come from the platform firmware, not from >>> the option ROM. In that case, how could OVMF implement this protocol, so >>> that such a GPU BAR would be allocated high? >> >> Option ROM depends on the PCI resource assignment so it’s like a >> chicken-egg problem if we let option rom produce such protocol. >> Singleton is enough I think. > > I agree. > >>> Looking at the CheckDevice() spec, I guess we could hard-code some PCI >>> vendor and device IDs, and set AddrSpaceGranularity to 64. However, >>> that's the only thing we should do; I don't think we should do the rest >>> of the PCI BAR probing. >> >> PCI BAR probing is still needed. We do not change the granularity >> for 32bit BAR. >>> >>> ... The only thing I'd like to say to the resource allocator is, >>> >>> If CSM is disabled, then please don't degrade the MMIO64 BARs of this >>> device, just because it has an oprom. >>> >>> (In fact, the device is not an incompatible PCI device.) >> >> I agree. But another rule is MdeModulePkg cannot depend >> on IntelFrameworkPkg. PciBusDxe is in MdeModulePkg; LegacyBios >> protocol is defined in IntelFrameworkPkg. > > That's fine; I'm not suggesting that PciBusDxe itself look for the legacy > BIOS protocol. Instead, platform code in OvmfPkg could look for the lega
Re: [edk2] [Patch 0/4] MdeModulePkg/PciBus Do not improperly degrade resource
On 05/18/16 10:16, Ni, Ruiyu wrote: >>-Original Message- >>From: Laszlo Ersek [mailto:ler...@redhat.com] >>Sent: Tuesday, May 17, 2016 8:23 PM >>To: Ni, Ruiyu >>Cc: edk2-de...@ml01.01.org >>Subject: Re: [edk2] [Patch 0/4] MdeModulePkg/PciBus Do not improperly degrade >>resource >> >>On 05/17/16 04:03, Ruiyu Ni wrote: >>> The patch serials refined the PciBus code and adds a new feature following >>> PI spec 1.4a to not improperly degrade resource. >>> >>> Ruiyu Ni (4): >>> MdeModulePkg/PciBus: use better name for local variables. >>> MdeModulePkg/PciBus: Remove unused fields in PCI_BAR >>> MdeModulePkg/PciBus: Use shorter global variable name >>> MdeModulePkg/PciBus: do not improperly degrade resource >>> >>> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c| 6 +-- >>> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h| 5 +- >>> .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 58 >>> +- >>> .../Bus/Pci/PciBusDxe/PciResourceSupport.c | 26 ++ >>> 4 files changed, 65 insertions(+), 30 deletions(-) >>> >> >>* Please add the following reference to patch #4: >> >>https://mantis.uefi.org/mantis/view.php?id=1529 > > I think it might not be proper to include the member only > web page in the commit message. I disagree -- I think the mantis ticket number should always be included in a commit message, if the patch is related to a Mantis ticket. Sharing just the number of the mantis ticket does not disclose any information that is not also publicly available from the UEFI spec itself (the UEFI spec starts with a long changelog of Mantis tickets). The contents of the mantis ticket *are* private, but for actually reading a ticket, general UEFI membership and the appropriate WG membership are required too. So, capturing the mantis ticket number in the commit message helps contributors that have access to Mantis, without disclosing sensitive information to those who haven't. Personally I frequently go to Mantis to understand the background of UEFI or PI spec changes better. The ECRs attached to Mantis tickets often describe use cases that are not carried over to the spec, but are important background. In this specific case, the PI spec 1.4a, Volume 5, has the following in its changelog: 1529 address space granularity errata >>* I'm interested in this work primarily due to GPU assignment to >>QEMU/KVM virtual machines. So I built OVMF applied with these patches, >>and checked if they made a difference for the PCI resource map, with an >>Nvidia GTX750 assigned to the guest. >> >>There's no difference; the 256MB BAR is still allocated under 4GB. >> >>* Apparently, EFI_INCOMPATIBLE_PCI_DEVICE_SUPPORT_PROTOCOL must be a >>singleton protocol; it should come from the platform firmware, not from >>the option ROM. In that case, how could OVMF implement this protocol, so >>that such a GPU BAR would be allocated high? > > Option ROM depends on the PCI resource assignment so it’s like a > chicken-egg problem if we let option rom produce such protocol. > Singleton is enough I think. I agree. >>Looking at the CheckDevice() spec, I guess we could hard-code some PCI >>vendor and device IDs, and set AddrSpaceGranularity to 64. However, >>that's the only thing we should do; I don't think we should do the rest >>of the PCI BAR probing. > > PCI BAR probing is still needed. We do not change the granularity > for 32bit BAR. >> >>... The only thing I'd like to say to the resource allocator is, >> >> If CSM is disabled, then please don't degrade the MMIO64 BARs of this >> device, just because it has an oprom. >> >>(In fact, the device is not an incompatible PCI device.) > > I agree. But another rule is MdeModulePkg cannot depend > on IntelFrameworkPkg. PciBusDxe is in MdeModulePkg; LegacyBios > protocol is defined in IntelFrameworkPkg. That's fine; I'm not suggesting that PciBusDxe itself look for the legacy BIOS protocol. Instead, platform code in OvmfPkg could look for the legacy BIOS protocol (or determine CSM presence in another way), and then instruct PciBusDxe through a well-defined interface not to degrade the 64-bit MMIO BARs. My interest is in such a well-defined interface. >>* By following the links in Mantis #1529 (recursively), I arrived at a >>message that points to the following driver: >> >> MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe >> >>OVMF does not include this driver at the moment. It is a small driver >>and I guess we could for
Re: [edk2] [Patch 0/4] MdeModulePkg/PciBus Do not improperly degrade resource
64F5B107-317A-4857-9D71-5C4165493076 Regards, Ray >-Original Message- >From: Laszlo Ersek [mailto:ler...@redhat.com] >Sent: Tuesday, May 17, 2016 8:23 PM >To: Ni, Ruiyu >Cc: edk2-de...@ml01.01.org >Subject: Re: [edk2] [Patch 0/4] MdeModulePkg/PciBus Do not improperly degrade >resource > >On 05/17/16 04:03, Ruiyu Ni wrote: >> The patch serials refined the PciBus code and adds a new feature following >> PI spec 1.4a to not improperly degrade resource. >> >> Ruiyu Ni (4): >> MdeModulePkg/PciBus: use better name for local variables. >> MdeModulePkg/PciBus: Remove unused fields in PCI_BAR >> MdeModulePkg/PciBus: Use shorter global variable name >> MdeModulePkg/PciBus: do not improperly degrade resource >> >> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c| 6 +-- >> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h| 5 +- >> .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 58 >> +- >> .../Bus/Pci/PciBusDxe/PciResourceSupport.c | 26 ++ >> 4 files changed, 65 insertions(+), 30 deletions(-) >> > >* Please add the following reference to patch #4: > >https://mantis.uefi.org/mantis/view.php?id=1529 I think it might not be proper to include the member only web page in the commit message. > >* I'm interested in this work primarily due to GPU assignment to >QEMU/KVM virtual machines. So I built OVMF applied with these patches, >and checked if they made a difference for the PCI resource map, with an >Nvidia GTX750 assigned to the guest. > >There's no difference; the 256MB BAR is still allocated under 4GB. > >* Apparently, EFI_INCOMPATIBLE_PCI_DEVICE_SUPPORT_PROTOCOL must be a >singleton protocol; it should come from the platform firmware, not from >the option ROM. In that case, how could OVMF implement this protocol, so >that such a GPU BAR would be allocated high? Option ROM depends on the PCI resource assignment so it's like a chicken-egg problem if we let option rom produce such protocol. Singleton is enough I think. > >Looking at the CheckDevice() spec, I guess we could hard-code some PCI >vendor and device IDs, and set AddrSpaceGranularity to 64. However, >that's the only thing we should do; I don't think we should do the rest >of the PCI BAR probing. PCI BAR probing is still needed. We do not change the granularity for 32bit BAR. > >... The only thing I'd like to say to the resource allocator is, > > If CSM is disabled, then please don't degrade the MMIO64 BARs of this > device, just because it has an oprom. > >(In fact, the device is not an incompatible PCI device.) I agree. But another rule is MdeModulePkg cannot depend on IntelFrameworkPkg. PciBusDxe is in MdeModulePkg; LegacyBios protocol is defined in IntelFrameworkPkg. > >* By following the links in Mantis #1529 (recursively), I arrived at a >message that points to the following driver: > > MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe > >OVMF does not include this driver at the moment. It is a small driver >and I guess we could fork it for OvmfPkg, but how should we fill in the >"mIncompatiblePciDeviceList" array? > Try to use below value to disable all MMIO degrade. Not tested. GLOBAL_REMOVE_IF_UNREFERENCED UINT64 mIncompatiblePciDeviceList[] = { // // DEVICE_INF_TAG, // PCI_DEVICE_ID (VendorID, DeviceID, Revision, SubVendorId, SubDeviceId), // DEVICE_RES_TAG, // ResType, GFlag , SFlag, Granularity, RangeMin, // RangeMax, Offset, AddrLen // DEVICE_INF_TAG, PCI_DEVICE_ID(DEVICE_ID_NOCARE, DEVICE_ID_NOCARE, DEVICE_ID_NOCARE, DEVICE_ID_NOCARE, DEVICE_ID_NOCARE), DEVICE_RES_TAG, PCI_BAR_TYPE_MEM, PCI_ACPI_UNUSED, PCI_ACPI_UNUSED, 64, PCI_ACPI_UNUSED, PCI_BAR_OLD_ALIGN, PCI_BAR_ALL, PCI_BAR_NOCHANGE, >Thanks >Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch 0/4] MdeModulePkg/PciBus Do not improperly degrade resource
On 05/17/16 04:03, Ruiyu Ni wrote: > The patch serials refined the PciBus code and adds a new feature following > PI spec 1.4a to not improperly degrade resource. > > Ruiyu Ni (4): > MdeModulePkg/PciBus: use better name for local variables. > MdeModulePkg/PciBus: Remove unused fields in PCI_BAR > MdeModulePkg/PciBus: Use shorter global variable name > MdeModulePkg/PciBus: do not improperly degrade resource > > MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c| 6 +-- > MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h| 5 +- > .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 58 > +- > .../Bus/Pci/PciBusDxe/PciResourceSupport.c | 26 ++ > 4 files changed, 65 insertions(+), 30 deletions(-) > * Please add the following reference to patch #4: https://mantis.uefi.org/mantis/view.php?id=1529 * I'm interested in this work primarily due to GPU assignment to QEMU/KVM virtual machines. So I built OVMF applied with these patches, and checked if they made a difference for the PCI resource map, with an Nvidia GTX750 assigned to the guest. There's no difference; the 256MB BAR is still allocated under 4GB. * Apparently, EFI_INCOMPATIBLE_PCI_DEVICE_SUPPORT_PROTOCOL must be a singleton protocol; it should come from the platform firmware, not from the option ROM. In that case, how could OVMF implement this protocol, so that such a GPU BAR would be allocated high? Looking at the CheckDevice() spec, I guess we could hard-code some PCI vendor and device IDs, and set AddrSpaceGranularity to 64. However, that's the only thing we should do; I don't think we should do the rest of the PCI BAR probing. ... The only thing I'd like to say to the resource allocator is, If CSM is disabled, then please don't degrade the MMIO64 BARs of this device, just because it has an oprom. (In fact, the device is not an incompatible PCI device.) * By following the links in Mantis #1529 (recursively), I arrived at a message that points to the following driver: MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe OVMF does not include this driver at the moment. It is a small driver and I guess we could fork it for OvmfPkg, but how should we fill in the "mIncompatiblePciDeviceList" array? Thanks Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel