| Message ID | 1351143725-2781-1-git-send-email-alex.hung@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
On Thu, Oct 25, 2012 at 1:42 PM, Alex Hung <alex.hung@canonical.com> wrote: > In PCI Firmware Specification, it claims "If the operating system does not natively > comprehend reserving the MMCFG region, the MMCFG region must be reserved by firmware." > However, Linux is an operating system that comprehend MMCFG, and therefore it is not > a problem for not declaring it. > > Signed-off-by: Alex Hung <alex.hung@canonical.com> > --- > src/acpi/mcfg/mcfg.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/acpi/mcfg/mcfg.c b/src/acpi/mcfg/mcfg.c > index 9dfd9fd..878c0f1 100644 > --- a/src/acpi/mcfg/mcfg.c > +++ b/src/acpi/mcfg/mcfg.c > @@ -200,7 +200,7 @@ static int mcfg_test1(fwts_framework *fw) > if ((memory_map_list != NULL) && > (!fwts_memory_map_is_reserved(memory_map_list, config->base_address))) { > > - fwts_failed(fw, LOG_LEVEL_MEDIUM, "MCFGMMIONotReserved", > + fwts_failed(fw, LOG_LEVEL_LOW, "MCFGMMIONotReserved", > "MCFG mmio config space at 0x%" PRIx64 > " is not reserved in the memory map table", > config->base_address); > -- > 1.7.9.5 > Acked-by: Keng-Yu Lin <kengyu@canonical.com>
On 25/10/12 06:42, Alex Hung wrote: > In PCI Firmware Specification, it claims "If the operating system does not natively > comprehend reserving the MMCFG region, the MMCFG region must be reserved by firmware." > However, Linux is an operating system that comprehend MMCFG, and therefore it is not > a problem for not declaring it. > > Signed-off-by: Alex Hung <alex.hung@canonical.com> > --- > src/acpi/mcfg/mcfg.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/acpi/mcfg/mcfg.c b/src/acpi/mcfg/mcfg.c > index 9dfd9fd..878c0f1 100644 > --- a/src/acpi/mcfg/mcfg.c > +++ b/src/acpi/mcfg/mcfg.c > @@ -200,7 +200,7 @@ static int mcfg_test1(fwts_framework *fw) > if ((memory_map_list != NULL) && > (!fwts_memory_map_is_reserved(memory_map_list, config->base_address))) { > > - fwts_failed(fw, LOG_LEVEL_MEDIUM, "MCFGMMIONotReserved", > + fwts_failed(fw, LOG_LEVEL_LOW, "MCFGMMIONotReserved", > "MCFG mmio config space at 0x%" PRIx64 > " is not reserved in the memory map table", > config->base_address); > I'm not 100% sure how significant this is. Can you take a look at what the kernel does if it isn't reserved by firmware? I think the relevant code is in arch/x86/pci/* Personally I don't mind dropping this to LOW, but I'd like to know what the kernel does nowadays for this issue. Colin
On 10/25/2012 04:48 PM, Colin Ian King wrote: > On 25/10/12 06:42, Alex Hung wrote: >> In PCI Firmware Specification, it claims "If the operating system does >> not natively >> comprehend reserving the MMCFG region, the MMCFG region must be >> reserved by firmware." >> However, Linux is an operating system that comprehend MMCFG, and >> therefore it is not >> a problem for not declaring it. >> >> Signed-off-by: Alex Hung <alex.hung@canonical.com> >> --- >> src/acpi/mcfg/mcfg.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/src/acpi/mcfg/mcfg.c b/src/acpi/mcfg/mcfg.c >> index 9dfd9fd..878c0f1 100644 >> --- a/src/acpi/mcfg/mcfg.c >> +++ b/src/acpi/mcfg/mcfg.c >> @@ -200,7 +200,7 @@ static int mcfg_test1(fwts_framework *fw) >> if ((memory_map_list != NULL) && >> (!fwts_memory_map_is_reserved(memory_map_list, >> config->base_address))) { >> >> - fwts_failed(fw, LOG_LEVEL_MEDIUM, "MCFGMMIONotReserved", >> + fwts_failed(fw, LOG_LEVEL_LOW, "MCFGMMIONotReserved", >> "MCFG mmio config space at 0x%" PRIx64 >> " is not reserved in the memory map table", >> config->base_address); >> > > I'm not 100% sure how significant this is. Can you take a look at what > the kernel does if it isn't reserved by firmware? I think the relevant > code is in arch/x86/pci/* > > Personally I don't mind dropping this to LOW, but I'd like to know what > the kernel does nowadays for this issue. > > Colin > > Hi Colin, Thanks for reminding. I checked mmconfig-shared.c and there is a function "pci_mmconfig_insert" to insert (reserve) the memory address in MCFG. It seems safe to lower the error level. Cheers, Alex Hung
On 25/10/12 11:17, Alex Hung wrote: > On 10/25/2012 04:48 PM, Colin Ian King wrote: >> On 25/10/12 06:42, Alex Hung wrote: >>> In PCI Firmware Specification, it claims "If the operating system does >>> not natively >>> comprehend reserving the MMCFG region, the MMCFG region must be >>> reserved by firmware." >>> However, Linux is an operating system that comprehend MMCFG, and >>> therefore it is not >>> a problem for not declaring it. >>> >>> Signed-off-by: Alex Hung <alex.hung@canonical.com> >>> --- >>> src/acpi/mcfg/mcfg.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/src/acpi/mcfg/mcfg.c b/src/acpi/mcfg/mcfg.c >>> index 9dfd9fd..878c0f1 100644 >>> --- a/src/acpi/mcfg/mcfg.c >>> +++ b/src/acpi/mcfg/mcfg.c >>> @@ -200,7 +200,7 @@ static int mcfg_test1(fwts_framework *fw) >>> if ((memory_map_list != NULL) && >>> (!fwts_memory_map_is_reserved(memory_map_list, >>> config->base_address))) { >>> >>> - fwts_failed(fw, LOG_LEVEL_MEDIUM, "MCFGMMIONotReserved", >>> + fwts_failed(fw, LOG_LEVEL_LOW, "MCFGMMIONotReserved", >>> "MCFG mmio config space at 0x%" PRIx64 >>> " is not reserved in the memory map table", >>> config->base_address); >>> >> >> I'm not 100% sure how significant this is. Can you take a look at what >> the kernel does if it isn't reserved by firmware? I think the relevant >> code is in arch/x86/pci/* >> >> Personally I don't mind dropping this to LOW, but I'd like to know what >> the kernel does nowadays for this issue. >> >> Colin >> >> > > Hi Colin, > > Thanks for reminding. > > I checked mmconfig-shared.c and there is a function > "pci_mmconfig_insert" to insert (reserve) the memory address in MCFG. It > seems safe to lower the error level. > > Cheers, > Alex Hung > Thanks Alex, I suspect I picked up this paranoid check from the original code derived from the linuxfirmwarekit. I'm happy to reduce this to a LOW failure. Acked-by: Colin Ian King <colin.king@canonical.com>
diff --git a/src/acpi/mcfg/mcfg.c b/src/acpi/mcfg/mcfg.c index 9dfd9fd..878c0f1 100644 --- a/src/acpi/mcfg/mcfg.c +++ b/src/acpi/mcfg/mcfg.c @@ -200,7 +200,7 @@ static int mcfg_test1(fwts_framework *fw) if ((memory_map_list != NULL) && (!fwts_memory_map_is_reserved(memory_map_list, config->base_address))) { - fwts_failed(fw, LOG_LEVEL_MEDIUM, "MCFGMMIONotReserved", + fwts_failed(fw, LOG_LEVEL_LOW, "MCFGMMIONotReserved", "MCFG mmio config space at 0x%" PRIx64 " is not reserved in the memory map table", config->base_address);
In PCI Firmware Specification, it claims "If the operating system does not natively comprehend reserving the MMCFG region, the MMCFG region must be reserved by firmware." However, Linux is an operating system that comprehend MMCFG, and therefore it is not a problem for not declaring it. Signed-off-by: Alex Hung <alex.hung@canonical.com> --- src/acpi/mcfg/mcfg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)