[SeaBIOS] [PATCH] resume: reset mmconfig to zero

Omar Berrow posted 1 patch 1 month ago
src/hw/pci.c | 2 +-
src/resume.c | 9 +++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
[SeaBIOS] [PATCH] resume: reset mmconfig to zero
Posted by Omar Berrow 1 month ago
I apologize for the previous thing sent on the mailing list.
On wake it would attempt to use the ECAM space to restore access to
ECAM, which wouldn't work because ECAM is disabled after reset.

Signed-off-by: Omar Berrow <omarkberrow@gmail.com>
---
 src/hw/pci.c | 2 +-
 src/resume.c | 9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/hw/pci.c b/src/hw/pci.c
index 8eda84b2..dfe4d92a 100644
--- a/src/hw/pci.c
+++ b/src/hw/pci.c
@@ -14,7 +14,7 @@
 #define PORT_PCI_CMD           0x0cf8
 #define PORT_PCI_DATA          0x0cfc

-static u32 mmconfig;
+volatile u32 mmconfig;

 static void *mmconfig_addr(u16 bdf, u32 addr)
 {
diff --git a/src/resume.c b/src/resume.c
index fb0b8a89..10b873c3 100644
--- a/src/resume.c
+++ b/src/resume.c
@@ -96,6 +96,15 @@ s3_resume(void)
         return;
     }

+    make_bios_writable();
+    // reset mmconfig to make sure that we don't use PCIe to
+    // resume PCIe
+    dprintf(1, "resetting mmconfig\n");
+    extern volatile u32 mmconfig;
+    mmconfig = 0;
+    dprintf(1, "mmconfig: 0x%x\n", mmconfig);
+//    make_bios_readonly();
+
     pic_setup();
     smm_setup();
     smp_resume();
-- 
2.45.2
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH] resume: reset mmconfig to zero
Posted by Gerd Hoffmann 3 weeks, 5 days ago
On Sat, Nov 02, 2024 at 08:23:33PM -0400, Omar Berrow wrote:
> I apologize for the previous thing sent on the mailing list.
> On wake it would attempt to use the ECAM space to restore access to
> ECAM, which wouldn't work because ECAM is disabled after reset.

> +    mmconfig = 0;

How about adding this line to the start of the pci_resume() function?

take care,
  Gerd

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH] resume: reset mmconfig to zero
Posted by Omar Berrow 3 weeks, 3 days ago
> How about adding this line to the start of the pci_resume() function?

I see no problem with this. However, I cannot test this for around a week
or two.
I will get back to you as soon as I can test this.

Sincerely,
Omar.

On Tue., Nov. 12, 2024, 4:02 p.m. Gerd Hoffmann, <kraxel@redhat.com> wrote:

> On Sat, Nov 02, 2024 at 08:23:33PM -0400, Omar Berrow wrote:
> > I apologize for the previous thing sent on the mailing list.
> > On wake it would attempt to use the ECAM space to restore access to
> > ECAM, which wouldn't work because ECAM is disabled after reset.
>
> > +    mmconfig = 0;
>
> How about adding this line to the start of the pci_resume() function?
>
> take care,
>   Gerd
>
>
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH] resume: reset mmconfig to zero
Posted by Omar Berrow via SeaBIOS 2 weeks, 1 day ago
Putting the line at the beginning to the start of the pci_resume()
ended up working just the same as in the s3_resume() function.

Sincerely,
Omar.

On Thu, Nov 14, 2024 at 7:45 AM Omar Berrow <omarkberrow@gmail.com> wrote:
>
> > How about adding this line to the start of the pci_resume() function?
>
> I see no problem with this. However, I cannot test this for around a week or two.
> I will get back to you as soon as I can test this.
>
> Sincerely,
> Omar.
>
> On Tue., Nov. 12, 2024, 4:02 p.m. Gerd Hoffmann, <kraxel@redhat.com> wrote:
>>
>> On Sat, Nov 02, 2024 at 08:23:33PM -0400, Omar Berrow wrote:
>> > I apologize for the previous thing sent on the mailing list.
>> > On wake it would attempt to use the ECAM space to restore access to
>> > ECAM, which wouldn't work because ECAM is disabled after reset.
>>
>> > +    mmconfig = 0;
>>
>> How about adding this line to the start of the pci_resume() function?
>>
>> take care,
>>   Gerd
>>
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH] resume: reset mmconfig to zero
Posted by Gerd Hoffmann 1 week, 6 days ago
On Sat, Nov 23, 2024 at 06:43:23PM -0500, Omar Berrow wrote:
> Putting the line at the beginning to the start of the pci_resume()
> ended up working just the same as in the s3_resume() function.

Good.  Could you resent the updated patch to the list?

thanks,
  Gerd

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH] resume: reset mmconfig to zero
Posted by Paul Menzel 1 month ago
Dear Omar,


Thank you for your patch.

Am 03.11.24 um 01:23 schrieb Omar Berrow:
> I apologize for the previous thing sent on the mailing list.

No problem. For some tooling it’s useful to mark the iteration with `git 
format-patch`’s switch `-v/--reroll-count`.

Your comment would also go below the `---` line, so it’s not part of the 
commit message, when applying your patch.

> On wake it would attempt to use the ECAM space to restore access to
> ECAM, which wouldn't work because ECAM is disabled after reset.

Nice find. Is there a way to reproduce your issue, for example with QEMU?

Also maybe mention the new log messages.

> Signed-off-by: Omar Berrow <omarkberrow@gmail.com>
> ---
>   src/hw/pci.c | 2 +-
>   src/resume.c | 9 +++++++++
>   2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/src/hw/pci.c b/src/hw/pci.c
> index 8eda84b2..dfe4d92a 100644
> --- a/src/hw/pci.c
> +++ b/src/hw/pci.c
> @@ -14,7 +14,7 @@
>   #define PORT_PCI_CMD           0x0cf8
>   #define PORT_PCI_DATA          0x0cfc
> 
> -static u32 mmconfig;
> +volatile u32 mmconfig;
> 
>   static void *mmconfig_addr(u16 bdf, u32 addr)
>   {
> diff --git a/src/resume.c b/src/resume.c
> index fb0b8a89..10b873c3 100644
> --- a/src/resume.c
> +++ b/src/resume.c
> @@ -96,6 +96,15 @@ s3_resume(void)
>           return;
>       }
> 
> +    make_bios_writable();
> +    // reset mmconfig to make sure that we don't use PCIe to
> +    // resume PCIe
> +    dprintf(1, "resetting mmconfig\n");

Should the log level be increased to 3, or is it important to have it in 
the logs?

> +    extern volatile u32 mmconfig;
> +    mmconfig = 0;
> +    dprintf(1, "mmconfig: 0x%x\n", mmconfig);
> +//    make_bios_readonly();

What’s the downside of not making the BIOS read-only?

> +
>       pic_setup();
>       smm_setup();
>       smp_resume();


Kind regards,

Paul
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org