x86: qemu: Adjust VGA initialization

As VGA option rom needs to run at C segment, although QEMU PAM emulation seems to only guard E/F segments, for correctness, move VGA initialization after PAM decode C/D/E/F segments.

Also since we already tested QEMU targets to differentiate I440FX and Q35 platforms, change to locate the VGA device via hardcoded b.d.f instead of dynamic search for its vendor id & device id pair.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/cpu/qemu/pci.c                 | 34 +++++++++++++++------------------
 arch/x86/include/asm/arch-qemu/device.h |  2 ++
 2 files changed, 17 insertions(+), 19 deletions(-)

Quoted 8 lines · Bin Meng

As VGA option rom needs to run at C segment, although QEMU PAM emulation seems to only guard E/F segments, for correctness, move VGA initialization after PAM decode C/D/E/F segments.

Also since we already tested QEMU targets to differentiate I440FX and Q35 platforms, change to locate the VGA device via hardcoded b.d.f instead of dynamic search for its vendor id & device id pair.

Quoted 6 lines

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/cpu/qemu/pci.c                 | 34 +++++++++++++++------------------
 arch/x86/include/asm/arch-qemu/device.h |  2 ++
 2 files changed, 17 insertions(+), 19 deletions(-)

Acked-by Simon Glass <[email protected]>

Writing 0xcb to I/O port 0xb2 (Advanced Power Management Control) causes U-Boot to hang on QEMU q35 target. Disable the writing for QEMU targets.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/cpu/coreboot/coreboot.c | 4 ----
 1 file changed, 4 deletions(-)

arch/x86/cpu/coreboot/coreboot.c +0 -5

index 4cdd0d4..2234cf0 100644
@@ -85,10 +85,6 @@ void board_final_cleanup(void)
                wrmsrl(MTRR_PHYS_MASK_MSR(top_mtrr), 0);
                mtrr_close(&state);
        }
-
-       /* Issue SMI to Coreboot to lock down ME and registers */
-       printf("Finalizing Coreboot\n");
-       outb(0xcb, 0xb2);
 }
 
 void panic_puts(const char *str)
--

1.8.2.1

Hi Bin,

Quoted 23 lines · Bin Meng

Writing 0xcb to I/O port 0xb2 (Advanced Power Management Control) causes U-Boot to hang on QEMU q35 target. Disable the writing for QEMU targets.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/cpu/coreboot/coreboot.c | 4 ----
 1 file changed, 4 deletions(-)

arch/x86/cpu/coreboot/coreboot.c +0 -4

index 4cdd0d4..2234cf0 100644
@@ -85,10 +85,6 @@ void board_final_cleanup(void)
                wrmsrl(MTRR_PHYS_MASK_MSR(top_mtrr), 0);
                mtrr_close(&state);
        }
-
-       /* Issue SMI to Coreboot to lock down ME and registers */
-       printf("Finalizing Coreboot\n");
-       outb(0xcb, 0xb2);
 }
 

Quoted 4 lines

arch/x86/cpu/coreboot/coreboot.c +0 -1

 void panic_puts(const char *str)
--

1.8.2.1

But how does this run with coreboot on platforms that need it? Should this be controlled by a device tree settings, perhaps?

Hi Simon,

Quoted 26 lines · Simon Glass

Hi Bin,

Quoted 23 lines · Bin Meng

Writing 0xcb to I/O port 0xb2 (Advanced Power Management Control) causes U-Boot to hang on QEMU q35 target. Disable the writing for QEMU targets.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/cpu/coreboot/coreboot.c | 4 ----
 1 file changed, 4 deletions(-)

arch/x86/cpu/coreboot/coreboot.c +0 -4

index 4cdd0d4..2234cf0 100644
@@ -85,10 +85,6 @@ void board_final_cleanup(void)
                wrmsrl(MTRR_PHYS_MASK_MSR(top_mtrr), 0);
                mtrr_close(&state);
        }
-
-       /* Issue SMI to Coreboot to lock down ME and registers */
-       printf("Finalizing Coreboot\n");
-       outb(0xcb, 0xb2);
 }
 

Quoted 8 lines

Quoted 4 lines

arch/x86/cpu/coreboot/coreboot.c +0 -1

 void panic_puts(const char *str)
--

1.8.2.1

But how does this run with coreboot on platforms that need it? Should this be controlled by a device tree settings, perhaps?

I am not sure if the lock down should be done by the coreboot in the first place. IMHO U-Boot as the coreboot payload should not touch such low-level stuff.

Hi Bin,

Quoted 38 lines · Bin Meng

Hi Simon,

Quoted 34 lines · Simon Glass

Hi Bin,

Quoted 25 lines · Bin Meng

Writing 0xcb to I/O port 0xb2 (Advanced Power Management Control) causes U-Boot to hang on QEMU q35 target. Disable the writing for QEMU targets.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/cpu/coreboot/coreboot.c | 4 ----
 1 file changed, 4 deletions(-)

arch/x86/cpu/coreboot/coreboot.c +0 -5

index 4cdd0d4..2234cf0 100644
@@ -85,10 +85,6 @@ void board_final_cleanup(void)
                wrmsrl(MTRR_PHYS_MASK_MSR(top_mtrr), 0);
                mtrr_close(&state);
        }
-
-       /* Issue SMI to Coreboot to lock down ME and registers */
-       printf("Finalizing Coreboot\n");
-       outb(0xcb, 0xb2);
 }
 
 void panic_puts(const char *str)
--

Quoted 2 lines

1.8.2.1

But how does this run with coreboot on platforms that need it? Should this be controlled by a device tree settings, perhaps?

Quoted 3 lines

I am not sure if the lock down should be done by the coreboot in the first place. IMHO U-Boot as the coreboot payload should not touch such low-level stuff.

Well it is telling coreboot that it is about to boot the kernel. I think this is an x86 thing and we should support it.

Hi Simon,

Quoted 45 lines · Simon Glass

Hi Bin,

Quoted 41 lines · Bin Meng

Hi Simon,

Quoted 26 lines · Simon Glass

Hi Bin,

Quoted 23 lines · Bin Meng

Writing 0xcb to I/O port 0xb2 (Advanced Power Management Control) causes U-Boot to hang on QEMU q35 target. Disable the writing for QEMU targets.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/cpu/coreboot/coreboot.c | 4 ----
 1 file changed, 4 deletions(-)

arch/x86/cpu/coreboot/coreboot.c +0 -4

index 4cdd0d4..2234cf0 100644
@@ -85,10 +85,6 @@ void board_final_cleanup(void)
                wrmsrl(MTRR_PHYS_MASK_MSR(top_mtrr), 0);
                mtrr_close(&state);
        }
-
-       /* Issue SMI to Coreboot to lock down ME and registers */
-       printf("Finalizing Coreboot\n");
-       outb(0xcb, 0xb2);
 }
 

Quoted 8 lines

Quoted 4 lines

arch/x86/cpu/coreboot/coreboot.c +0 -1

 void panic_puts(const char *str)
--

1.8.2.1

But how does this run with coreboot on platforms that need it? Should this be controlled by a device tree settings, perhaps?

I am not sure if the lock down should be done by the coreboot in the first place. IMHO U-Boot as the coreboot payload should not touch such low-level stuff.

Quoted 3 lines

Well it is telling coreboot that it is about to boot the kernel. I think this is an x86 thing and we should support it.

What happens if coreboot directly boot into the kernel? Does coreboot do the same 'outb(0xcb, 0xb2)' thing before jumping to kernel? My

understanding is that coreboot payload is to enhance coreboot's functionality as a system BIOS (either having legacy BIOS interface with the help of a SeaBIOS payload, or having UEFI interface with the help of a TinaoCore payload). coreboot itself does the mainboard initialization and payload does something that is not hardware dependent.

Hi Bin,

Quoted 48 lines · Bin Meng

Hi Simon,

Quoted 45 lines · Simon Glass

Hi Bin,

Quoted 41 lines · Bin Meng

Hi Simon,

Quoted 26 lines · Simon Glass

Hi Bin,

Quoted 23 lines · Bin Meng

Writing 0xcb to I/O port 0xb2 (Advanced Power Management Control) causes U-Boot to hang on QEMU q35 target. Disable the writing for QEMU targets.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/cpu/coreboot/coreboot.c | 4 ----
 1 file changed, 4 deletions(-)

arch/x86/cpu/coreboot/coreboot.c +0 -4

index 4cdd0d4..2234cf0 100644
@@ -85,10 +85,6 @@ void board_final_cleanup(void)
                wrmsrl(MTRR_PHYS_MASK_MSR(top_mtrr), 0);
                mtrr_close(&state);
        }
-
-       /* Issue SMI to Coreboot to lock down ME and registers */
-       printf("Finalizing Coreboot\n");
-       outb(0xcb, 0xb2);
 }
 

Quoted 8 lines

Quoted 4 lines

arch/x86/cpu/coreboot/coreboot.c +0 -1

 void panic_puts(const char *str)
--

1.8.2.1

But how does this run with coreboot on platforms that need it? Should this be controlled by a device tree settings, perhaps?

I am not sure if the lock down should be done by the coreboot in the first place. IMHO U-Boot as the coreboot payload should not touch such low-level stuff.

Quoted 6 lines

Quoted 3 lines

Well it is telling coreboot that it is about to boot the kernel. I think this is an x86 thing and we should support it.

What happens if coreboot directly boot into the kernel? Does coreboot do the same 'outb(0xcb, 0xb2)' thing before jumping to kernel? My

Kind of, although in that case it is just a function call within coreboot.

Quoted 6 lines

understanding is that coreboot payload is to enhance coreboot's functionality as a system BIOS (either having legacy BIOS interface with the help of a SeaBIOS payload, or having UEFI interface with the help of a TinaoCore payload). coreboot itself does the mainboard initialization and payload does something that is not hardware dependent.

Right, but we need to tell coreboot that we have finished with the ME / registers, etc. It can't lock them before calling U-Boot since we may adjust them.

Hi Simon,

Quoted 67 lines · Simon Glass

Hi Bin,

Quoted 48 lines · Bin Meng

Hi Simon,

Quoted 45 lines · Simon Glass

Hi Bin,

Quoted 41 lines · Bin Meng

Hi Simon,

Quoted 26 lines · Simon Glass

Hi Bin,

Quoted 23 lines · Bin Meng

Writing 0xcb to I/O port 0xb2 (Advanced Power Management Control) causes U-Boot to hang on QEMU q35 target. Disable the writing for QEMU targets.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/cpu/coreboot/coreboot.c | 4 ----
 1 file changed, 4 deletions(-)

arch/x86/cpu/coreboot/coreboot.c +0 -4

index 4cdd0d4..2234cf0 100644
@@ -85,10 +85,6 @@ void board_final_cleanup(void)
                wrmsrl(MTRR_PHYS_MASK_MSR(top_mtrr), 0);
                mtrr_close(&state);
        }
-
-       /* Issue SMI to Coreboot to lock down ME and registers */
-       printf("Finalizing Coreboot\n");
-       outb(0xcb, 0xb2);
 }
 

Quoted 8 lines

Quoted 4 lines

arch/x86/cpu/coreboot/coreboot.c +0 -1

 void panic_puts(const char *str)
--

1.8.2.1

But how does this run with coreboot on platforms that need it? Should this be controlled by a device tree settings, perhaps?

I am not sure if the lock down should be done by the coreboot in the first place. IMHO U-Boot as the coreboot payload should not touch such low-level stuff.

Quoted 6 lines

Quoted 3 lines

Well it is telling coreboot that it is about to boot the kernel. I think this is an x86 thing and we should support it.

What happens if coreboot directly boot into the kernel? Does coreboot do the same 'outb(0xcb, 0xb2)' thing before jumping to kernel? My

Kind of, although in that case it is just a function call within coreboot.

Quoted 6 lines

understanding is that coreboot payload is to enhance coreboot's functionality as a system BIOS (either having legacy BIOS interface with the help of a SeaBIOS payload, or having UEFI interface with the help of a TinaoCore payload). coreboot itself does the mainboard initialization and payload does something that is not hardware dependent.

Quoted 4 lines

Right, but we need to tell coreboot that we have finished with the ME / registers, etc. It can't lock them before calling U-Boot since we may adjust them.

Will use a config option in the device tree in v2.

Hi Bin,

Quoted 70 lines · Bin Meng

Hi Simon,

Quoted 67 lines · Simon Glass

Hi Bin,

Quoted 48 lines · Bin Meng

Hi Simon,

Quoted 45 lines · Simon Glass

Hi Bin,

Quoted 41 lines · Bin Meng

Hi Simon,

Quoted 26 lines · Simon Glass

Hi Bin,

Quoted 23 lines · Bin Meng

Writing 0xcb to I/O port 0xb2 (Advanced Power Management Control) causes U-Boot to hang on QEMU q35 target. Disable the writing for QEMU targets.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/cpu/coreboot/coreboot.c | 4 ----
 1 file changed, 4 deletions(-)

arch/x86/cpu/coreboot/coreboot.c +0 -4

index 4cdd0d4..2234cf0 100644
@@ -85,10 +85,6 @@ void board_final_cleanup(void)
                wrmsrl(MTRR_PHYS_MASK_MSR(top_mtrr), 0);
                mtrr_close(&state);
        }
-
-       /* Issue SMI to Coreboot to lock down ME and registers */
-       printf("Finalizing Coreboot\n");
-       outb(0xcb, 0xb2);
 }
 

Quoted 8 lines

Quoted 4 lines

arch/x86/cpu/coreboot/coreboot.c +0 -1

 void panic_puts(const char *str)
--

1.8.2.1

But how does this run with coreboot on platforms that need it? Should this be controlled by a device tree settings, perhaps?

I am not sure if the lock down should be done by the coreboot in the first place. IMHO U-Boot as the coreboot payload should not touch such low-level stuff.

Quoted 6 lines

Quoted 3 lines

Well it is telling coreboot that it is about to boot the kernel. I think this is an x86 thing and we should support it.

What happens if coreboot directly boot into the kernel? Does coreboot do the same 'outb(0xcb, 0xb2)' thing before jumping to kernel? My

Kind of, although in that case it is just a function call within coreboot.

Quoted 6 lines

understanding is that coreboot payload is to enhance coreboot's functionality as a system BIOS (either having legacy BIOS interface with the help of a SeaBIOS payload, or having UEFI interface with the help of a TinaoCore payload). coreboot itself does the mainboard initialization and payload does something that is not hardware dependent.

Quoted 6 lines

Quoted 4 lines

Right, but we need to tell coreboot that we have finished with the ME / registers, etc. It can't lock them before calling U-Boot since we may adjust them.

Will use a config option in the device tree in v2.

OK thanks. When you resend can you please rebase on x86/master?

Clean up arch/x86/cpu/coreboot.c to fix several cosmetic issues.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/cpu/coreboot/coreboot.c  | 26 ++------------------------
 arch/x86/include/asm/u-boot-x86.h |  1 -
 2 files changed, 2 insertions(+), 25 deletions(-)

Although the two qemu-x86 targets (i440fx and q35) share a lot in common, they still have something that cannot easily handled in one place (like different configurations, different properties in the device tree). Split to create two dedicated board configuration and device tree files and make the i440fx be the default build target.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/dts/Makefile                             |  3 +-
 arch/x86/dts/qemu-x86_i440fx.dts                  | 34 +++++++++++++++
 arch/x86/dts/{qemu-x86.dts => qemu-x86_q35.dts}   |  2 +-
 board/coreboot/coreboot/Kconfig                   |  4 +-
 board/emulation/qemu-x86/Kconfig                  | 19 +++++++--
 configs/qemu-x86_defconfig                        |  1 -
 doc/README.x86                                    | 13 +++++-
 include/configs/{qemu-x86.h => qemu-x86_i440fx.h} | 20 ++-------
 include/configs/qemu-x86_q35.h                    | 52 +++++++++++++++++++++++
 9 files changed, 122 insertions(+), 26 deletions(-)
 create mode 100644 arch/x86/dts/qemu-x86_i440fx.dts
 rename arch/x86/dts/{qemu-x86.dts => qemu-x86_q35.dts} (95%)
 rename include/configs/{qemu-x86.h => qemu-x86_i440fx.h} (78%)
 create mode 100644 include/configs/qemu-x86_q35.h

Hi Bin,

Quoted 9 lines · Bin Meng

Although the two qemu-x86 targets (i440fx and q35) share a lot in common, they still have something that cannot easily handled in one place (like different configurations, different properties in the device tree). Split to create two dedicated board configuration and device tree files and make the i440fx be the default build target.

Signed-off-by Bin Meng <[email protected]>

---

Quoted 14 lines

 arch/x86/dts/Makefile                             |  3 +-
 arch/x86/dts/qemu-x86_i440fx.dts                  | 34 +++++++++++++++
 arch/x86/dts/{qemu-x86.dts => qemu-x86_q35.dts}   |  2 +-
 board/coreboot/coreboot/Kconfig                   |  4 +-
 board/emulation/qemu-x86/Kconfig                  | 19 +++++++--
 configs/qemu-x86_defconfig                        |  1 -
 doc/README.x86                                    | 13 +++++-
 include/configs/{qemu-x86.h => qemu-x86_i440fx.h} | 20 ++-------
 include/configs/qemu-x86_q35.h                    | 52 +++++++++++++++++++++++
 9 files changed, 122 insertions(+), 26 deletions(-)
 create mode 100644 arch/x86/dts/qemu-x86_i440fx.dts
 rename arch/x86/dts/{qemu-x86.dts => qemu-x86_q35.dts} (95%)
 rename include/configs/{qemu-x86.h => qemu-x86_i440fx.h} (78%)
 create mode 100644 include/configs/qemu-x86_q35.h

Do we need a separate config file? It would be good if all the changes were in the device tree so that we don't need a separate config. Or at least that the configs are the same except for the device tree.

Hi Simon,

Quoted 27 lines · Simon Glass

Hi Bin,

Quoted 9 lines · Bin Meng

Although the two qemu-x86 targets (i440fx and q35) share a lot in common, they still have something that cannot easily handled in one place (like different configurations, different properties in the device tree). Split to create two dedicated board configuration and device tree files and make the i440fx be the default build target.

Signed-off-by Bin Meng <[email protected]>

---

Quoted 14 lines

 arch/x86/dts/Makefile                             |  3 +-
 arch/x86/dts/qemu-x86_i440fx.dts                  | 34 +++++++++++++++
 arch/x86/dts/{qemu-x86.dts => qemu-x86_q35.dts}   |  2 +-
 board/coreboot/coreboot/Kconfig                   |  4 +-
 board/emulation/qemu-x86/Kconfig                  | 19 +++++++--
 configs/qemu-x86_defconfig                        |  1 -
 doc/README.x86                                    | 13 +++++-
 include/configs/{qemu-x86.h => qemu-x86_i440fx.h} | 20 ++-------
 include/configs/qemu-x86_q35.h                    | 52 +++++++++++++++++++++++
 9 files changed, 122 insertions(+), 26 deletions(-)
 create mode 100644 arch/x86/dts/qemu-x86_i440fx.dts
 rename arch/x86/dts/{qemu-x86.dts => qemu-x86_q35.dts} (95%)
 rename include/configs/{qemu-x86.h => qemu-x86_i440fx.h} (78%)
 create mode 100644 include/configs/qemu-x86_q35.h

Quoted 4 lines

Do we need a separate config file? It would be good if all the changes were in the device tree so that we don't need a separate config. Or at least that the configs are the same except for the device tree.

So far the only difference between two separate config files are the ATA/SATA settings. i440fx has legacy IDE support while q35 has the AHCI support. We can enable them both in just one config files, however turning on legacy IDE support on q35 causes significant boot delay as the legacy IDE driver has some big timeout in probing the attached devices. Do you think this is something we are tolerant of? If yes, I can just do separate device trees.

Hi Bin,

Quoted 35 lines · Bin Meng

Hi Simon,

Quoted 27 lines · Simon Glass

Hi Bin,

Quoted 9 lines · Bin Meng

Although the two qemu-x86 targets (i440fx and q35) share a lot in common, they still have something that cannot easily handled in one place (like different configurations, different properties in the device tree). Split to create two dedicated board configuration and device tree files and make the i440fx be the default build target.

Signed-off-by Bin Meng <[email protected]>

---

Quoted 14 lines

 arch/x86/dts/Makefile                             |  3 +-
 arch/x86/dts/qemu-x86_i440fx.dts                  | 34 +++++++++++++++
 arch/x86/dts/{qemu-x86.dts => qemu-x86_q35.dts}   |  2 +-
 board/coreboot/coreboot/Kconfig                   |  4 +-
 board/emulation/qemu-x86/Kconfig                  | 19 +++++++--
 configs/qemu-x86_defconfig                        |  1 -
 doc/README.x86                                    | 13 +++++-
 include/configs/{qemu-x86.h => qemu-x86_i440fx.h} | 20 ++-------
 include/configs/qemu-x86_q35.h                    | 52 +++++++++++++++++++++++
 9 files changed, 122 insertions(+), 26 deletions(-)
 create mode 100644 arch/x86/dts/qemu-x86_i440fx.dts
 rename arch/x86/dts/{qemu-x86.dts => qemu-x86_q35.dts} (95%)
 rename include/configs/{qemu-x86.h => qemu-x86_i440fx.h} (78%)
 create mode 100644 include/configs/qemu-x86_q35.h

Quoted 4 lines

Do we need a separate config file? It would be good if all the changes were in the device tree so that we don't need a separate config. Or at least that the configs are the same except for the device tree.

Quoted 7 lines

So far the only difference between two separate config files are the ATA/SATA settings. i440fx has legacy IDE support while q35 has the AHCI support. We can enable them both in just one config files, however turning on legacy IDE support on q35 causes significant boot delay as the legacy IDE driver has some big timeout in probing the attached devices. Do you think this is something we are tolerant of? If yes, I can just do separate device trees.

I think it is OK. But another option would be to add an IDE node to the device tree and check it when CONFIG_OF_CONTROL is defined...

Hi Simon,

Quoted 46 lines · Simon Glass

Hi Bin,

Quoted 42 lines · Bin Meng

Hi Simon,

Quoted 27 lines · Simon Glass

Hi Bin,

Quoted 9 lines · Bin Meng

Although the two qemu-x86 targets (i440fx and q35) share a lot in common, they still have something that cannot easily handled in one place (like different configurations, different properties in the device tree). Split to create two dedicated board configuration and device tree files and make the i440fx be the default build target.

Signed-off-by Bin Meng <[email protected]>

---

Quoted 14 lines

 arch/x86/dts/Makefile                             |  3 +-
 arch/x86/dts/qemu-x86_i440fx.dts                  | 34 +++++++++++++++
 arch/x86/dts/{qemu-x86.dts => qemu-x86_q35.dts}   |  2 +-
 board/coreboot/coreboot/Kconfig                   |  4 +-
 board/emulation/qemu-x86/Kconfig                  | 19 +++++++--
 configs/qemu-x86_defconfig                        |  1 -
 doc/README.x86                                    | 13 +++++-
 include/configs/{qemu-x86.h => qemu-x86_i440fx.h} | 20 ++-------
 include/configs/qemu-x86_q35.h                    | 52 +++++++++++++++++++++++
 9 files changed, 122 insertions(+), 26 deletions(-)
 create mode 100644 arch/x86/dts/qemu-x86_i440fx.dts
 rename arch/x86/dts/{qemu-x86.dts => qemu-x86_q35.dts} (95%)
 rename include/configs/{qemu-x86.h => qemu-x86_i440fx.h} (78%)
 create mode 100644 include/configs/qemu-x86_q35.h

Quoted 4 lines

Do we need a separate config file? It would be good if all the changes were in the device tree so that we don't need a separate config. Or at least that the configs are the same except for the device tree.

So far the only difference between two separate config files are the ATA/SATA settings. i440fx has legacy IDE support while q35 has the AHCI support. We can enable them both in just one config files, however turning on legacy IDE support on q35 causes significant boot delay as the legacy IDE driver has some big timeout in probing the attached devices. Do you think this is something we are tolerant of? If yes, I can just do separate device trees.

Quoted 3 lines

I think it is OK. But another option would be to add an IDE node to the device tree and check it when CONFIG_OF_CONTROL is defined...

I feel that we need convert all block drivers to driver model, so that the driver can probe and initialize IDE/AHCI based on device tree node. But I guess that's a long term goal?

Hi Bin,

Quoted 53 lines · Bin Meng

Hi Simon,

Quoted 49 lines · Simon Glass

Hi Bin,

Quoted 35 lines · Bin Meng

Hi Simon,

Quoted 27 lines · Simon Glass

Hi Bin,

Quoted 9 lines · Bin Meng

Although the two qemu-x86 targets (i440fx and q35) share a lot in common, they still have something that cannot easily handled in one place (like different configurations, different properties in the device tree). Split to create two dedicated board configuration and device tree files and make the i440fx be the default build target.

Signed-off-by Bin Meng <[email protected]>

---

Quoted 14 lines

 arch/x86/dts/Makefile                             |  3 +-
 arch/x86/dts/qemu-x86_i440fx.dts                  | 34 +++++++++++++++
 arch/x86/dts/{qemu-x86.dts => qemu-x86_q35.dts}   |  2 +-
 board/coreboot/coreboot/Kconfig                   |  4 +-
 board/emulation/qemu-x86/Kconfig                  | 19 +++++++--
 configs/qemu-x86_defconfig                        |  1 -
 doc/README.x86                                    | 13 +++++-
 include/configs/{qemu-x86.h => qemu-x86_i440fx.h} | 20 ++-------
 include/configs/qemu-x86_q35.h                    | 52 +++++++++++++++++++++++
 9 files changed, 122 insertions(+), 26 deletions(-)
 create mode 100644 arch/x86/dts/qemu-x86_i440fx.dts
 rename arch/x86/dts/{qemu-x86.dts => qemu-x86_q35.dts} (95%)
 rename include/configs/{qemu-x86.h => qemu-x86_i440fx.h} (78%)
 create mode 100644 include/configs/qemu-x86_q35.h

Quoted 4 lines

Do we need a separate config file? It would be good if all the changes were in the device tree so that we don't need a separate config. Or at least that the configs are the same except for the device tree.

Quoted 7 lines

So far the only difference between two separate config files are the ATA/SATA settings. i440fx has legacy IDE support while q35 has the AHCI support. We can enable them both in just one config files, however turning on legacy IDE support on q35 causes significant boot delay as the legacy IDE driver has some big timeout in probing the attached devices. Do you think this is something we are tolerant of? If yes, I can just do separate device trees.

I think it is OK. But another option would be to add an IDE node to the device tree and check it when CONFIG_OF_CONTROL is defined...

Quoted 3 lines

I feel that we need convert all block drivers to driver model, so that the driver can probe and initialize IDE/AHCI based on device tree node. But I guess that's a long term goal?

Yes, perhaps latter in the year. I did create a simple MMC uclass that uses the block driver library just as now, and it seems easy enough. I suppose you could do the same with IDE. But in the meantime just some sort of DT config is good enough to avoid your device.

Hi Simon,

Quoted 56 lines · Simon Glass

Hi Bin,

Quoted 53 lines · Bin Meng

Hi Simon,

Quoted 49 lines · Simon Glass

Hi Bin,

Quoted 35 lines · Bin Meng

Hi Simon,

Quoted 27 lines · Simon Glass

Hi Bin,

Quoted 9 lines · Bin Meng

Although the two qemu-x86 targets (i440fx and q35) share a lot in common, they still have something that cannot easily handled in one place (like different configurations, different properties in the device tree). Split to create two dedicated board configuration and device tree files and make the i440fx be the default build target.

Signed-off-by Bin Meng <[email protected]>

---

Quoted 14 lines

 arch/x86/dts/Makefile                             |  3 +-
 arch/x86/dts/qemu-x86_i440fx.dts                  | 34 +++++++++++++++
 arch/x86/dts/{qemu-x86.dts => qemu-x86_q35.dts}   |  2 +-
 board/coreboot/coreboot/Kconfig                   |  4 +-
 board/emulation/qemu-x86/Kconfig                  | 19 +++++++--
 configs/qemu-x86_defconfig                        |  1 -
 doc/README.x86                                    | 13 +++++-
 include/configs/{qemu-x86.h => qemu-x86_i440fx.h} | 20 ++-------
 include/configs/qemu-x86_q35.h                    | 52 +++++++++++++++++++++++
 9 files changed, 122 insertions(+), 26 deletions(-)
 create mode 100644 arch/x86/dts/qemu-x86_i440fx.dts
 rename arch/x86/dts/{qemu-x86.dts => qemu-x86_q35.dts} (95%)
 rename include/configs/{qemu-x86.h => qemu-x86_i440fx.h} (78%)
 create mode 100644 include/configs/qemu-x86_q35.h

Quoted 4 lines

Do we need a separate config file? It would be good if all the changes were in the device tree so that we don't need a separate config. Or at least that the configs are the same except for the device tree.

Quoted 7 lines

So far the only difference between two separate config files are the ATA/SATA settings. i440fx has legacy IDE support while q35 has the AHCI support. We can enable them both in just one config files, however turning on legacy IDE support on q35 causes significant boot delay as the legacy IDE driver has some big timeout in probing the attached devices. Do you think this is something we are tolerant of? If yes, I can just do separate device trees.

I think it is OK. But another option would be to add an IDE node to the device tree and check it when CONFIG_OF_CONTROL is defined...

Quoted 9 lines

Quoted 3 lines

I feel that we need convert all block drivers to driver model, so that the driver can probe and initialize IDE/AHCI based on device tree node. But I guess that's a long term goal?

Yes, perhaps latter in the year. I did create a simple MMC uclass that uses the block driver library just as now, and it seems easy enough. I suppose you could do the same with IDE. But in the meantime just some sort of DT config is good enough to avoid your device.

I am not sure I understand your point. Do you mean for now I need modify common/cmd_ide.c::ide_init() to probe the existence of an IDE node in the device tree? I am afraid that's a lot of work too since there are many boards use the IDE driver.

Support QEMU PIRQ routing via device tree on both i440fx and q35 platforms. With this commit, Linux booting on QEMU from U-Boot has working ATA/SATA, USB and ethernet.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/cpu/coreboot/coreboot.c  |  5 +++++
 arch/x86/cpu/qemu/qemu.c          |  8 ++++++++
 arch/x86/dts/qemu-x86_i440fx.dts  | 16 ++++++++++++++++
 arch/x86/dts/qemu-x86_q35.dts     | 32 ++++++++++++++++++++++++++++++++
 configs/qemu-x86_defconfig        |  1 +
 include/configs/qemu-x86_i440fx.h |  1 +
 include/configs/qemu-x86_q35.h    |  1 +
 7 files changed, 64 insertions(+)