[SeaBIOS] Re: [PATCH 3/4] csm: Move pci_probe_devices() from InitializeYourself phase to PreparToBoot phase
+// On physical hardware, some PCI devices are not detectable when +// UEFI invokes our InitializeYourself() function. But they +// are guaranteed to be available before UEFI invokes our +// PreparToBoot() function. +if (CONFIG_CSM && !CONFIG_QEMU_HARDWARE) +pci_probe_devices(); + dprintf(3, "PrepareToBoot table %04x:%04x\n", regs->es, regs->bx); struct e820entry *p = (void *)csm_compat_table.E820Pointer; >>> >>> Why don’t we want to do the same for QEMU? >> >> This problem was only seen on physical hardware, so I wanted this >> fix to be applied only to physical hardware. > >Yes, I understand, but less code “complexity” and having the same >behavior on QEMU and real hardware is preferred. The maintainer Gerd >replied yesterday. Did you get his reply (Message-ID: ><2021043917.i6tsrnaymcly5...@sirius.home.kraxel.org>)? Thanks Paul and sorry for late catch-up. Yes I saw Gerd's reply and we agreed to keep this change common to both QEMU and bare metal. > > >Kind regards, > >Paul >___ >SeaBIOS mailing list -- seabios@seabios.org >To unsubscribe send an email to seabios-le...@seabios.org ___ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] Re: [PATCH 3/4] csm: Move pci_probe_devices() from InitializeYourself phase to PreparToBoot phase
>On Thu, Dec 31, 2020 at 07:08:25PM -0800, Bin Gao wrote:
>> On physical hardware, some PCI devices are not detectable when
>> UEFI invokes our InitializeYourself() function. But they
>> are guaranteed to be available before UEFI invokes our
>> PreparToBoot() function.
>>
>> Signed-off-by: Bin Gao
>> ---
>> src/fw/csm.c | 11 ++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/fw/csm.c b/src/fw/csm.c
>> index f64bc73..1bd821e 100644
>> --- a/src/fw/csm.c
>> +++ b/src/fw/csm.c
>> @@ -64,7 +64,9 @@ static void
>> csm_maininit(struct bregs *regs)
>> {
>> interface_init();
>> -pci_probe_devices();
>> +
>> +if (CONFIG_CSM && CONFIG_QEMU_HARDWARE)
>> +pci_probe_devices();
>>
>> csm_compat_table.PnPInstallationCheckSegment = SEG_BIOS;
>> csm_compat_table.PnPInstallationCheckOffset = get_pnp_offset();
>> @@ -144,6 +146,13 @@ handle_csm_0002(struct bregs *regs)
>> return;
>> }
>>
>> +// On physical hardware, some PCI devices are not detectable when
>> +// UEFI invokes our InitializeYourself() function. But they
>> +// are guaranteed to be available before UEFI invokes our
>> +// PreparToBoot() function.
>> +if (CONFIG_CSM && !CONFIG_QEMU_HARDWARE)
>> +pci_probe_devices();
>
>This looks like a hack. I'm not sure what's being attempted, but a
>more scalable solution will be needed.
As the commit message described, the goal is to make sure all the pci devices
can be found in pci_probe_devices(). Due to the BIOS internal logic, some PCI
devices may not be visible yet when InitializeYourself() is called. Thus we
wanted
to run pci_probe_devices() later, i.e. in PrepareToBoot() instead of in
InitializeYourself().
As Paul and Gerd suggested, I will remove CONFIG_QEMU_HARDWARE and let the code
be common for both qemu and bare metal.
>
>-Kevin
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] Re: [PATCH 3/4] csm: Move pci_probe_devices() from InitializeYourself phase to PreparToBoot phase
On Thu, Dec 31, 2020 at 07:08:25PM -0800, Bin Gao wrote:
> On physical hardware, some PCI devices are not detectable when
> UEFI invokes our InitializeYourself() function. But they
> are guaranteed to be available before UEFI invokes our
> PreparToBoot() function.
>
> Signed-off-by: Bin Gao
> ---
> src/fw/csm.c | 11 ++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/src/fw/csm.c b/src/fw/csm.c
> index f64bc73..1bd821e 100644
> --- a/src/fw/csm.c
> +++ b/src/fw/csm.c
> @@ -64,7 +64,9 @@ static void
> csm_maininit(struct bregs *regs)
> {
> interface_init();
> -pci_probe_devices();
> +
> +if (CONFIG_CSM && CONFIG_QEMU_HARDWARE)
> +pci_probe_devices();
>
> csm_compat_table.PnPInstallationCheckSegment = SEG_BIOS;
> csm_compat_table.PnPInstallationCheckOffset = get_pnp_offset();
> @@ -144,6 +146,13 @@ handle_csm_0002(struct bregs *regs)
> return;
> }
>
> +// On physical hardware, some PCI devices are not detectable when
> +// UEFI invokes our InitializeYourself() function. But they
> +// are guaranteed to be available before UEFI invokes our
> +// PreparToBoot() function.
> +if (CONFIG_CSM && !CONFIG_QEMU_HARDWARE)
> +pci_probe_devices();
This looks like a hack. I'm not sure what's being attempted, but a
more scalable solution will be needed.
-Kevin
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] Re: [PATCH 3/4] csm: Move pci_probe_devices() from InitializeYourself phase to PreparToBoot phase
Dear Bin, Am 13.01.21 um 04:43 schrieb Gao, Bin: Am 01.01.21 um 04:08 schrieb Bin Gao: […] +// On physical hardware, some PCI devices are not detectable when +// UEFI invokes our InitializeYourself() function. But they +// are guaranteed to be available before UEFI invokes our +// PreparToBoot() function. +if (CONFIG_CSM && !CONFIG_QEMU_HARDWARE) +pci_probe_devices(); + dprintf(3, "PrepareToBoot table %04x:%04x\n", regs->es, regs->bx); struct e820entry *p = (void *)csm_compat_table.E820Pointer; Why don’t we want to do the same for QEMU? This problem was only seen on physical hardware, so I wanted this fix to be applied only to physical hardware. Yes, I understand, but less code “complexity” and having the same behavior on QEMU and real hardware is preferred. The maintainer Gerd replied yesterday. Did you get his reply (Message-ID: <2021043917.i6tsrnaymcly5...@sirius.home.kraxel.org>)? Kind regards, Paul ___ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] Re: [PATCH 3/4] csm: Move pci_probe_devices() from InitializeYourself phase to PreparToBoot phase
>
>
>Dear Bin,
>
>Thank you for your patches.
>
>Am 01.01.21 um 04:08 schrieb Bin Gao:
>> On physical hardware, some PCI devices are not detectable when
>> UEFI invokes our InitializeYourself() function.
>
>It would be great, if you gave a concrete example of hardware (board,
>firmware version and PCI devices).
>
Will add this formation in my next revision.
>> But they are guaranteed to be available before UEFI invokes our
>> PreparToBoot() function.
>
>Nit: Small typo (also in summary and comment below): Prepar*e*ToBoot.
Will fix in next revision.
>
>> Signed-off-by: Bin Gao
>> ---
>> src/fw/csm.c | 11 ++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/fw/csm.c b/src/fw/csm.c
>> index f64bc73..1bd821e 100644
>> --- a/src/fw/csm.c
>> +++ b/src/fw/csm.c
>> @@ -64,7 +64,9 @@ static void
>> csm_maininit(struct bregs *regs)
>> {
>> interface_init();
>> -pci_probe_devices();
>> +
>> +if (CONFIG_CSM && CONFIG_QEMU_HARDWARE)
>> +pci_probe_devices();
>>
>> csm_compat_table.PnPInstallationCheckSegment = SEG_BIOS;
>> csm_compat_table.PnPInstallationCheckOffset = get_pnp_offset();
>> @@ -144,6 +146,13 @@ handle_csm_0002(struct bregs *regs)
>> return;
>> }
>>
>> +// On physical hardware, some PCI devices are not detectable when
>> +// UEFI invokes our InitializeYourself() function. But they
>> +// are guaranteed to be available before UEFI invokes our
>> +// PreparToBoot() function.
>> +if (CONFIG_CSM && !CONFIG_QEMU_HARDWARE)
>> +pci_probe_devices();
>> +
>> dprintf(3, "PrepareToBoot table %04x:%04x\n", regs->es, regs->bx);
>>
>> struct e820entry *p = (void *)csm_compat_table.E820Pointer;
>
>Why don’t we want to do the same for QEMU?
This problem was only seen on physical hardware, so I wanted this fix to be
applied only to physical hardware.
>
>
>Kind regards,
>
>Paul
>___
>SeaBIOS mailing list -- seabios@seabios.org
>To unsubscribe send an email to seabios-le...@seabios.org
>
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] Re: [PATCH 3/4] csm: Move pci_probe_devices() from InitializeYourself phase to PreparToBoot phase
Hi, > > +// On physical hardware, some PCI devices are not detectable when > > +// UEFI invokes our InitializeYourself() function. But they > > +// are guaranteed to be available before UEFI invokes our > > +// PreparToBoot() function. > > +if (CONFIG_CSM && !CONFIG_QEMU_HARDWARE) > > +pci_probe_devices(); > > + > > dprintf(3, "PrepareToBoot table %04x:%04x\n", regs->es, regs->bx); > > struct e820entry *p = (void *)csm_compat_table.E820Pointer; > > Why don’t we want to do the same for QEMU? I think so, there is no reason why qemu should be handled in a different way. IIRC I've even mentioned that before. Do you guys read the review comments before submitting a new version of the patch set? take care, Gerd ___ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] Re: [PATCH 3/4] csm: Move pci_probe_devices() from InitializeYourself phase to PreparToBoot phase
Dear Bin,
Thank you for your patches.
Am 01.01.21 um 04:08 schrieb Bin Gao:
On physical hardware, some PCI devices are not detectable when
UEFI invokes our InitializeYourself() function.
It would be great, if you gave a concrete example of hardware (board,
firmware version and PCI devices).
But they are guaranteed to be available before UEFI invokes our
PreparToBoot() function.
Nit: Small typo (also in summary and comment below): Prepar*e*ToBoot.
Signed-off-by: Bin Gao
---
src/fw/csm.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/fw/csm.c b/src/fw/csm.c
index f64bc73..1bd821e 100644
--- a/src/fw/csm.c
+++ b/src/fw/csm.c
@@ -64,7 +64,9 @@ static void
csm_maininit(struct bregs *regs)
{
interface_init();
-pci_probe_devices();
+
+if (CONFIG_CSM && CONFIG_QEMU_HARDWARE)
+pci_probe_devices();
csm_compat_table.PnPInstallationCheckSegment = SEG_BIOS;
csm_compat_table.PnPInstallationCheckOffset = get_pnp_offset();
@@ -144,6 +146,13 @@ handle_csm_0002(struct bregs *regs)
return;
}
+// On physical hardware, some PCI devices are not detectable when
+// UEFI invokes our InitializeYourself() function. But they
+// are guaranteed to be available before UEFI invokes our
+// PreparToBoot() function.
+if (CONFIG_CSM && !CONFIG_QEMU_HARDWARE)
+pci_probe_devices();
+
dprintf(3, "PrepareToBoot table %04x:%04x\n", regs->es, regs->bx);
struct e820entry *p = (void *)csm_compat_table.E820Pointer;
Why don’t we want to do the same for QEMU?
Kind regards,
Paul
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
