| Message ID | 20230531070229.334124-2-wangyuquan1236@phytium.com.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/arm/sbsa-ref: use XHCI to replace EHCI | expand |
On Wed, May 31, 2023 at 03:02:29PM +0800, wangyuquan1236@phytium.com.cn wrote: > From: Yuquan Wang <wangyuquan1236@phytium.com.cn> > > The current sbsa-ref cannot use EHCI controller which is only > able to do 32-bit DMA, since sbsa-ref doesn't have RAM above 4GB. > Hence, this uses XHCI to provide a usb controller with 64-bit > DMA capablity instead of EHCI. > Should this be below 4G? Also has EHCI never worked, or has it worked in some modes and so this change should be versioned? Graeme > Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn> > Change-Id: I1376f8bbc0e25dcd9d8a22b6e061cb56b3486394 > --- > hw/arm/sbsa-ref.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c > index 792371fdce..f9c0647353 100644 > --- a/hw/arm/sbsa-ref.c > +++ b/hw/arm/sbsa-ref.c > @@ -81,7 +81,7 @@ enum { > SBSA_SECURE_UART_MM, > SBSA_SECURE_MEM, > SBSA_AHCI, > - SBSA_EHCI, > + SBSA_XHCI, > }; > > struct SBSAMachineState { > @@ -118,7 +118,7 @@ static const MemMapEntry sbsa_ref_memmap[] = { > [SBSA_SMMU] = { 0x60050000, 0x00020000 }, > /* Space here reserved for more SMMUs */ > [SBSA_AHCI] = { 0x60100000, 0x00010000 }, > - [SBSA_EHCI] = { 0x60110000, 0x00010000 }, > + [SBSA_XHCI] = { 0x60110000, 0x00010000 }, > /* Space here reserved for other devices */ > [SBSA_PCIE_PIO] = { 0x7fff0000, 0x00010000 }, > /* 32-bit address PCIE MMIO space */ > @@ -138,7 +138,7 @@ static const int sbsa_ref_irqmap[] = { > [SBSA_SECURE_UART] = 8, > [SBSA_SECURE_UART_MM] = 9, > [SBSA_AHCI] = 10, > - [SBSA_EHCI] = 11, > + [SBSA_XHCI] = 11, > [SBSA_SMMU] = 12, /* ... to 15 */ > [SBSA_GWDT_WS0] = 16, > }; > @@ -558,12 +558,12 @@ static void create_ahci(const SBSAMachineState *sms) > } > } > > -static void create_ehci(const SBSAMachineState *sms) > +static void create_xhci(const SBSAMachineState *sms) > { > - hwaddr base = sbsa_ref_memmap[SBSA_EHCI].base; > - int irq = sbsa_ref_irqmap[SBSA_EHCI]; > + hwaddr base = sbsa_ref_memmap[SBSA_XHCI].base; > + int irq = sbsa_ref_irqmap[SBSA_XHCI]; > > - sysbus_create_simple("platform-ehci-usb", base, > + sysbus_create_simple("sysbus-xhci", base, > qdev_get_gpio_in(sms->gic, irq)); > } > > @@ -785,7 +785,7 @@ static void sbsa_ref_init(MachineState *machine) > > create_ahci(sms); > > - create_ehci(sms); > + create_xhci(sms); > > create_pcie(sms); > > -- > 2.34.1 > >
On Wed, 31 May 2023 at 15:58, Graeme Gregory <graeme@xora.org.uk> wrote: > > On Wed, May 31, 2023 at 03:02:29PM +0800, wangyuquan1236@phytium.com.cn wrote: > > From: Yuquan Wang <wangyuquan1236@phytium.com.cn> > > > > The current sbsa-ref cannot use EHCI controller which is only > > able to do 32-bit DMA, since sbsa-ref doesn't have RAM above 4GB. > > Hence, this uses XHCI to provide a usb controller with 64-bit > > DMA capablity instead of EHCI. > > > > Should this be below 4G? It would be pretty disruptive to try to rearrange the memory map to put RAM below 4GB at this point, though in theory it's possible I guess. (I have a vague recollection that there was some reason the RAM was all put above 4GB, but can't find anything about that in my email archives. Perhaps Leif remembers?) > Also has EHCI never worked, or has it worked in some modes and so this > change should be versioned? AIUI, EHCI has never worked and can never have worked, because this board's RAM is all above 4G and the QEMU EHCI controller implementation only allows DMA descriptors with 32-bit addresses. Looking back at the archives, it seems we discussed XHCI vs EHCI when the sbsa-ref board went in, and the conclusion was that XHCI would be better. But there wasn't a sysbus XHCI device at that point, so we ended up committing the sbsa-ref board with EHCI and a plan to switch to XHCI when the sysbus-xhci device was done, which we then forgot about: https://mail.gnu.org/archive/html/qemu-arm/2018-11/msg00638.html thanks -- PMM
W dniu 31.05.2023 o 17:27, Peter Maydell pisze: > On Wed, 31 May 2023 at 15:58, Graeme Gregory <graeme@xora.org.uk> wrote: >> On Wed, May 31, 2023 at 03:02:29PM +0800, wangyuquan1236@phytium.com.cn wrote: >>> From: Yuquan Wang <wangyuquan1236@phytium.com.cn> >>> >>> The current sbsa-ref cannot use EHCI controller which is only >>> able to do 32-bit DMA, since sbsa-ref doesn't have RAM above 4GB. >>> Hence, this uses XHCI to provide a usb controller with 64-bit >>> DMA capablity instead of EHCI. >> Should this be below 4G? > It would be pretty disruptive to try to rearrange the memory > map to put RAM below 4GB at this point, though in theory it's > possible I guess. (I have a vague recollection that there was > some reason the RAM was all put above 4GB, but can't find > anything about that in my email archives. Perhaps Leif remembers?) I thought that memory starts at 40bit was to not use Cortex-A53 cpu with sbsa-ref. Nowadays it also removed Cortex-A76. > Looking back at the archives, it seems we discussed XHCI vs > EHCI when the sbsa-ref board went in, and the conclusion was > that XHCI would be better. But there wasn't a sysbus XHCI device > at that point, so we ended up committing the sbsa-ref board > with EHCI and a plan to switch to XHCI when the sysbus-xhci > device was done, which we then forgot about: > https://mail.gnu.org/archive/html/qemu-arm/2018-11/msg00638.html Moving from EHCI to XHCI on sysbus requires also firmware changes. Or we can just add "qemu-xhci" pcie card and have USB running.
On 2023-05-31 16:27, Peter Maydell wrote: > On Wed, 31 May 2023 at 15:58, Graeme Gregory <graeme@xora.org.uk> wrote: >>> The current sbsa-ref cannot use EHCI controller which is only >>> able to do 32-bit DMA, since sbsa-ref doesn't have RAM above 4GB. >>> Hence, this uses XHCI to provide a usb controller with 64-bit >>> DMA capablity instead of EHCI. >> >> Should this be below 4G? > > It would be pretty disruptive to try to rearrange the memory > map to put RAM below 4GB at this point, though in theory it's > possible I guess. (I have a vague recollection that there was > some reason the RAM was all put above 4GB, but can't find > anything about that in my email archives. Perhaps Leif remembers?) I think Graeme was just pointing out a typo in Marcin's email. Yeah, we're not changing the DRAM base at this stage. I think the reason we put no RAM below 4GB was simply that we had several real-world platforms where that was true, and given the intended use-case for the platform, we explicitly wanted to trigger issues those platforms might encounter. >> Also has EHCI never worked, or has it worked in some modes and so this >> change should be versioned? > > AIUI, EHCI has never worked and can never have worked, because > this board's RAM is all above 4G and the QEMU EHCI controller > implementation only allows DMA descriptors with 32-bit addresses. > > Looking back at the archives, it seems we discussed XHCI vs > EHCI when the sbsa-ref board went in, and the conclusion was > that XHCI would be better. But there wasn't a sysbus XHCI device > at that point, so we ended up committing the sbsa-ref board > with EHCI and a plan to switch to XHCI when the sysbus-xhci > device was done, which we then forgot about: > https://mail.gnu.org/archive/html/qemu-arm/2018-11/msg00638.html Ah, thanks! That explains why we did the thing that made no sense :) To skip the migration hazard, my prefernece is we just leave the EHCI device in for now, and add a separate XHCI on PCIe. We can drop the EHCI device at some point in the future. / Leif
On Wed, 31 May 2023, at 5:36 PM, Leif Lindholm wrote: > On 2023-05-31 16:27, Peter Maydell wrote: >> On Wed, 31 May 2023 at 15:58, Graeme Gregory <graeme@xora.org.uk> wrote: >>>> The current sbsa-ref cannot use EHCI controller which is only >>>> able to do 32-bit DMA, since sbsa-ref doesn't have RAM above 4GB. >>>> Hence, this uses XHCI to provide a usb controller with 64-bit >>>> DMA capablity instead of EHCI. >>> >>> Should this be below 4G? >> >> It would be pretty disruptive to try to rearrange the memory >> map to put RAM below 4GB at this point, though in theory it's >> possible I guess. (I have a vague recollection that there was >> some reason the RAM was all put above 4GB, but can't find >> anything about that in my email archives. Perhaps Leif remembers?) > > I think Graeme was just pointing out a typo in Marcin's email. > Yes the typo! Graeme > Yeah, we're not changing the DRAM base at this stage. > I think the reason we put no RAM below 4GB was simply that we had > several real-world platforms where that was true, and given the intended > use-case for the platform, we explicitly wanted to trigger issues those > platforms might encounter. > >>> Also has EHCI never worked, or has it worked in some modes and so this >>> change should be versioned? >> >> AIUI, EHCI has never worked and can never have worked, because >> this board's RAM is all above 4G and the QEMU EHCI controller >> implementation only allows DMA descriptors with 32-bit addresses. >> >> Looking back at the archives, it seems we discussed XHCI vs >> EHCI when the sbsa-ref board went in, and the conclusion was >> that XHCI would be better. But there wasn't a sysbus XHCI device >> at that point, so we ended up committing the sbsa-ref board >> with EHCI and a plan to switch to XHCI when the sysbus-xhci >> device was done, which we then forgot about: >> https://mail.gnu.org/archive/html/qemu-arm/2018-11/msg00638.html > > Ah, thanks! That explains why we did the thing that made no sense :) > > To skip the migration hazard, my prefernece is we just leave the EHCI > device in for now, and add a separate XHCI on PCIe. We can drop the > EHCI device at some point in the future. > > / > Leif
Hi Leif, > On Jun 1, 2023, at 00:36, Leif Lindholm <quic_llindhol@quicinc.com> wrote: > > On 2023-05-31 16:27, Peter Maydell wrote: >> On Wed, 31 May 2023 at 15:58, Graeme Gregory <graeme@xora.org.uk> wrote: >>>> The current sbsa-ref cannot use EHCI controller which is only >>>> able to do 32-bit DMA, since sbsa-ref doesn't have RAM above 4GB. >>>> Hence, this uses XHCI to provide a usb controller with 64-bit >>>> DMA capablity instead of EHCI. >>> >>> Should this be below 4G? >> It would be pretty disruptive to try to rearrange the memory >> map to put RAM below 4GB at this point, though in theory it's >> possible I guess. (I have a vague recollection that there was >> some reason the RAM was all put above 4GB, but can't find >> anything about that in my email archives. Perhaps Leif remembers?) > > I think Graeme was just pointing out a typo in Marcin's email. > > Yeah, we're not changing the DRAM base at this stage. > I think the reason we put no RAM below 4GB was simply that we had several real-world platforms where that was true, and given the intended use-case for the platform, we explicitly wanted to trigger issues those platforms might encounter. > >>> Also has EHCI never worked, or has it worked in some modes and so this >>> change should be versioned? >> AIUI, EHCI has never worked and can never have worked, because >> this board's RAM is all above 4G and the QEMU EHCI controller >> implementation only allows DMA descriptors with 32-bit addresses. >> Looking back at the archives, it seems we discussed XHCI vs >> EHCI when the sbsa-ref board went in, and the conclusion was >> that XHCI would be better. But there wasn't a sysbus XHCI device >> at that point, so we ended up committing the sbsa-ref board >> with EHCI and a plan to switch to XHCI when the sysbus-xhci >> device was done, which we then forgot about: >> https://mail.gnu.org/archive/html/qemu-arm/2018-11/msg00638.html > > Ah, thanks! That explains why we did the thing that made no sense :) > > To skip the migration hazard, my prefernece is we just leave the EHCI device in for now, and add a separate XHCI on PCIe. We can drop the > EHCI device at some point in the future. What about introducing another SMMU for all the platform devices on sbsa-ref? I was thinking over this solution for some time. If that can be feasible, we then also have a prototype of IOMMU for platform device. Regards, Baozi.
On Wed, 31 May 2023 at 17:37, Leif Lindholm <quic_llindhol@quicinc.com> wrote: > > On 2023-05-31 16:27, Peter Maydell wrote: > > On Wed, 31 May 2023 at 15:58, Graeme Gregory <graeme@xora.org.uk> wrote: > >>> The current sbsa-ref cannot use EHCI controller which is only > >>> able to do 32-bit DMA, since sbsa-ref doesn't have RAM above 4GB. > >>> Hence, this uses XHCI to provide a usb controller with 64-bit > >>> DMA capablity instead of EHCI. > >> > >> Should this be below 4G? > > > > It would be pretty disruptive to try to rearrange the memory > > map to put RAM below 4GB at this point, though in theory it's > > possible I guess. (I have a vague recollection that there was > > some reason the RAM was all put above 4GB, but can't find > > anything about that in my email archives. Perhaps Leif remembers?) > > I think Graeme was just pointing out a typo in Marcin's email. > > Yeah, we're not changing the DRAM base at this stage. > I think the reason we put no RAM below 4GB was simply that we had > several real-world platforms where that was true, and given the intended > use-case for the platform, we explicitly wanted to trigger issues those > platforms might encounter. > > >> Also has EHCI never worked, or has it worked in some modes and so this > >> change should be versioned? > > > > AIUI, EHCI has never worked and can never have worked, because > > this board's RAM is all above 4G and the QEMU EHCI controller > > implementation only allows DMA descriptors with 32-bit addresses. > > > > Looking back at the archives, it seems we discussed XHCI vs > > EHCI when the sbsa-ref board went in, and the conclusion was > > that XHCI would be better. But there wasn't a sysbus XHCI device > > at that point, so we ended up committing the sbsa-ref board > > with EHCI and a plan to switch to XHCI when the sysbus-xhci > > device was done, which we then forgot about: > > https://mail.gnu.org/archive/html/qemu-arm/2018-11/msg00638.html > > Ah, thanks! That explains why we did the thing that made no sense :) > > To skip the migration hazard, my prefernece is we just leave the EHCI > device in for now, and add a separate XHCI on PCIe. We can drop the > EHCI device at some point in the future. Why PCIe for the XHCI and not sysbus? At the time the board was originally added the argument was in favour of using a sysbus USB controller (you can see Ard making that point in the linked archive thread). thanks -- PMM
W dniu 1.06.2023 o 17:01, Peter Maydell pisze: > On Wed, 31 May 2023 at 17:37, Leif Lindholm <quic_llindhol@quicinc.com> wrote: >> Ah, thanks! That explains why we did the thing that made no sense :) >> >> To skip the migration hazard, my prefernece is we just leave the EHCI >> device in for now, and add a separate XHCI on PCIe. We can drop the >> EHCI device at some point in the future. > > Why PCIe for the XHCI and not sysbus? At the time the board > was originally added the argument was in favour of using > a sysbus USB controller (you can see Ard making that point > in the linked archive thread). So something like below? I only tested does system boot into Debian. To make it work also changes to EDK2 would be needed to list XHCI controller in DSDT. "info qtree" in QEMU monitor lists controller with usb devices attached (added them by cli arguments). From 8f5af99a670be226a1dfc5b06cbdd3eff4841d27 Mon Sep 17 00:00:00 2001 From: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org> Date: Thu, 1 Jun 2023 17:27:24 +0200 Subject: [PATCH] WIP: arm/sbsa-ref: add XHCI on sysbus EHCI controller is not working as it requires 32-bit memory. XHCI one should work fine. --- hw/arm/sbsa-ref.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c index de21200ff9..0bc87abbf4 100644 --- a/hw/arm/sbsa-ref.c +++ b/hw/arm/sbsa-ref.c @@ -83,6 +83,7 @@ enum { SBSA_SECURE_MEM, SBSA_AHCI, SBSA_EHCI, + SBSA_XHCI, }; struct SBSAMachineState { @@ -120,6 +121,7 @@ static const MemMapEntry sbsa_ref_memmap[] = { /* Space here reserved for more SMMUs */ [SBSA_AHCI] = { 0x60100000, 0x00010000 }, [SBSA_EHCI] = { 0x60110000, 0x00010000 }, + [SBSA_XHCI] = { 0x60120000, 0x00010000 }, /* Space here reserved for other devices */ [SBSA_PCIE_PIO] = { 0x7fff0000, 0x00010000 }, /* 32-bit address PCIE MMIO space */ @@ -142,6 +144,7 @@ static const int sbsa_ref_irqmap[] = { [SBSA_EHCI] = 11, [SBSA_SMMU] = 12, /* ... to 15 */ [SBSA_GWDT_WS0] = 16, + [SBSA_XHCI] = 17, }; static const char * const valid_cpus[] = { @@ -575,6 +578,15 @@ static void create_ahci(const SBSAMachineState *sms) } } +static void create_xhci(const SBSAMachineState *sms) +{ + hwaddr base = sbsa_ref_memmap[SBSA_XHCI].base; + int irq = sbsa_ref_irqmap[SBSA_XHCI]; + + sysbus_create_simple("sysbus-xhci", base, + qdev_get_gpio_in(sms->gic, irq)); +} + static void create_ehci(const SBSAMachineState *sms) { hwaddr base = sbsa_ref_memmap[SBSA_EHCI].base; @@ -804,6 +816,7 @@ static void sbsa_ref_init(MachineState *machine) create_ahci(sms); create_ehci(sms); + create_xhci(sms); create_pcie(sms);
On Thu, 1 Jun 2023 at 16:30, Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org> wrote: > > W dniu 1.06.2023 o 17:01, Peter Maydell pisze: > > On Wed, 31 May 2023 at 17:37, Leif Lindholm <quic_llindhol@quicinc.com> wrote: > > >> Ah, thanks! That explains why we did the thing that made no sense :) > >> > >> To skip the migration hazard, my prefernece is we just leave the EHCI > >> device in for now, and add a separate XHCI on PCIe. We can drop the > >> EHCI device at some point in the future. > > > > Why PCIe for the XHCI and not sysbus? At the time the board > > was originally added the argument was in favour of using > > a sysbus USB controller (you can see Ard making that point > > in the linked archive thread). > > So something like below? I only tested does system boot into Debian. > To make it work also changes to EDK2 would be needed to list XHCI > controller in DSDT. Yes, and you also want to drop the useless EHCI controller, and (as you note) both these things mean firmware changes so presumably that's a version-bump event? -- PMM
W dniu 1.06.2023 o 18:39, Peter Maydell pisze: >> So something like below? I only tested does system boot into Debian. >> To make it work also changes to EDK2 would be needed to list XHCI >> controller in DSDT. > Yes, and you also want to drop the useless EHCI controller, > and (as you note) both these things mean firmware changes > so presumably that's a version-bump event? First we need to get two patches through TF-A to get current information from QEMU. And it takes time. https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/20871 https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/20953
+Ard On Thu, Jun 01, 2023 at 16:01:43 +0100, Peter Maydell wrote: > > >> Also has EHCI never worked, or has it worked in some modes and so this > > >> change should be versioned? > > > > > > AIUI, EHCI has never worked and can never have worked, because > > > this board's RAM is all above 4G and the QEMU EHCI controller > > > implementation only allows DMA descriptors with 32-bit addresses. > > > > > > Looking back at the archives, it seems we discussed XHCI vs > > > EHCI when the sbsa-ref board went in, and the conclusion was > > > that XHCI would be better. But there wasn't a sysbus XHCI device > > > at that point, so we ended up committing the sbsa-ref board > > > with EHCI and a plan to switch to XHCI when the sysbus-xhci > > > device was done, which we then forgot about: > > > https://mail.gnu.org/archive/html/qemu-arm/2018-11/msg00638.html > > > > Ah, thanks! That explains why we did the thing that made no sense :) > > > > To skip the migration hazard, my prefernece is we just leave the EHCI > > device in for now, and add a separate XHCI on PCIe. We can drop the > > EHCI device at some point in the future. > > Why PCIe for the XHCI and not sysbus? At the time the board > was originally added the argument was in favour of using > a sysbus USB controller (you can see Ard making that point > in the linked archive thread). The original argument was that having the device on the sysbus 1) enabled codepaths we wanted to exercise and 2) more closely resembled the development systems available at the time. 1 still applies, but I'm not sure 2 does. Ard? / Leif
On Thu, 1 Jun 2023 at 20:00, Leif Lindholm <quic_llindhol@quicinc.com> wrote: > > +Ard > > On Thu, Jun 01, 2023 at 16:01:43 +0100, Peter Maydell wrote: > > > >> Also has EHCI never worked, or has it worked in some modes and so this > > > >> change should be versioned? > > > > > > > > AIUI, EHCI has never worked and can never have worked, because > > > > this board's RAM is all above 4G and the QEMU EHCI controller > > > > implementation only allows DMA descriptors with 32-bit addresses. > > > > > > > > Looking back at the archives, it seems we discussed XHCI vs > > > > EHCI when the sbsa-ref board went in, and the conclusion was > > > > that XHCI would be better. But there wasn't a sysbus XHCI device > > > > at that point, so we ended up committing the sbsa-ref board > > > > with EHCI and a plan to switch to XHCI when the sysbus-xhci > > > > device was done, which we then forgot about: > > > > https://mail.gnu.org/archive/html/qemu-arm/2018-11/msg00638.html > > > > > > Ah, thanks! That explains why we did the thing that made no sense :) > > > > > > To skip the migration hazard, my prefernece is we just leave the EHCI > > > device in for now, and add a separate XHCI on PCIe. We can drop the > > > EHCI device at some point in the future. > > > > Why PCIe for the XHCI and not sysbus? At the time the board > > was originally added the argument was in favour of using > > a sysbus USB controller (you can see Ard making that point > > in the linked archive thread). > > The original argument was that having the device on the sysbus > 1) enabled codepaths we wanted to exercise and > 2) more closely resembled the development systems available at the > time. > > 1 still applies, but I'm not sure 2 does. Ard? > It was always primarily about #1. This was also the reason for putting all DRAM above 4G. I'm surprised that the EHCI never worked, though - I don't see the point in keeping it in that case.
Hi, Leif On Thu, 1 Jun 2023 18:59:56 +0100, Leif Lindholm wrote: > > +Ard > > On Thu, Jun 01, 2023 at 16:01:43 +0100, Peter Maydell wrote: > > > >> Also has EHCI never worked, or has it worked in some modes and so this > > > >> change should be versioned? > > > > > > > > AIUI, EHCI has never worked and can never have worked, because > > > > this board's RAM is all above 4G and the QEMU EHCI controller > > > > implementation only allows DMA descriptors with 32-bit addresses. > > > > > > > > Looking back at the archives, it seems we discussed XHCI vs > > > > EHCI when the sbsa-ref board went in, and the conclusion was > > > > that XHCI would be better. But there wasn't a sysbus XHCI device > > > > at that point, so we ended up committing the sbsa-ref board > > > > with EHCI and a plan to switch to XHCI when the sysbus-xhci > > > > device was done, which we then forgot about: > > > > https://mail.gnu.org/archive/html/qemu-arm/2018-11/msg00638.html > > > > > > Ah, thanks! That explains why we did the thing that made no sense :) > > > > > > To skip the migration hazard, my prefernece is we just leave the EHCI > > > device in for now, and add a separate XHCI on PCIe. We can drop the > > > EHCI device at some point in the future. > > > > Why PCIe for the XHCI and not sysbus? At the time the board > > was originally added the argument was in favour of using > > a sysbus USB controller (you can see Ard making that point > > in the linked archive thread). > > The original argument was that having the device on the sysbus > 1) enabled codepaths we wanted to exercise and Sorry, for my poor engineering experience, I am confused about the meaning of "enabled codepaths" here. Is it like a code target that to realize the original purpose of this board ? Yuquan 信息安全声明：本邮件包含信息归发件人所在组织所有,发件人所在组织对该邮件拥有所有权利。请接收者注意保密,未经发件人书面许可,不得向任何第三方组织和个人透露本邮件所含信息。 Information Security Notice: The information contained in this mail is solely property of the sender's organization.This mail communication is confidential.Recipients named above are obligated to maintain secrecy and are not permitted to disclose the contents of this communication to others.
Hi Yuquan, On Fri, Jun 02, 2023 at 11:24:11 +0800, Yuquan Wang wrote: > > > > To skip the migration hazard, my prefernece is we just leave the EHCI > > > > device in for now, and add a separate XHCI on PCIe. We can drop the > > > > EHCI device at some point in the future. > > > > > > Why PCIe for the XHCI and not sysbus? At the time the board > > > was originally added the argument was in favour of using > > > a sysbus USB controller (you can see Ard making that point > > > in the linked archive thread). > > > > The original argument was that having the device on the sysbus > > 1) enabled codepaths we wanted to exercise and > > Sorry, for my poor engineering experience, I am confused about the meaning > of "enabled codepaths" here. Is it like a code target that to realize the > original purpose of this board ? It is a bit of a convoluted term. sbsa-ref isn't a normal platform. We are using it as a vehicle for developing and verifying common firmware and software for sbsa (or now SystemReady SR) compliant platforms. This means that we ideally want it to expose *permitted* but not necessarily ideal behaviours, so that the parts of software that deals with those situations get frequently exercised (enabled). It's code coverage for the hw-interacting pieces of code. / Leif
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c index 792371fdce..f9c0647353 100644 --- a/hw/arm/sbsa-ref.c +++ b/hw/arm/sbsa-ref.c @@ -81,7 +81,7 @@ enum { SBSA_SECURE_UART_MM, SBSA_SECURE_MEM, SBSA_AHCI, - SBSA_EHCI, + SBSA_XHCI, }; struct SBSAMachineState { @@ -118,7 +118,7 @@ static const MemMapEntry sbsa_ref_memmap[] = { [SBSA_SMMU] = { 0x60050000, 0x00020000 }, /* Space here reserved for more SMMUs */ [SBSA_AHCI] = { 0x60100000, 0x00010000 }, - [SBSA_EHCI] = { 0x60110000, 0x00010000 }, + [SBSA_XHCI] = { 0x60110000, 0x00010000 }, /* Space here reserved for other devices */ [SBSA_PCIE_PIO] = { 0x7fff0000, 0x00010000 }, /* 32-bit address PCIE MMIO space */ @@ -138,7 +138,7 @@ static const int sbsa_ref_irqmap[] = { [SBSA_SECURE_UART] = 8, [SBSA_SECURE_UART_MM] = 9, [SBSA_AHCI] = 10, - [SBSA_EHCI] = 11, + [SBSA_XHCI] = 11, [SBSA_SMMU] = 12, /* ... to 15 */ [SBSA_GWDT_WS0] = 16, }; @@ -558,12 +558,12 @@ static void create_ahci(const SBSAMachineState *sms) } } -static void create_ehci(const SBSAMachineState *sms) +static void create_xhci(const SBSAMachineState *sms) { - hwaddr base = sbsa_ref_memmap[SBSA_EHCI].base; - int irq = sbsa_ref_irqmap[SBSA_EHCI]; + hwaddr base = sbsa_ref_memmap[SBSA_XHCI].base; + int irq = sbsa_ref_irqmap[SBSA_XHCI]; - sysbus_create_simple("platform-ehci-usb", base, + sysbus_create_simple("sysbus-xhci", base, qdev_get_gpio_in(sms->gic, irq)); } @@ -785,7 +785,7 @@ static void sbsa_ref_init(MachineState *machine) create_ahci(sms); - create_ehci(sms); + create_xhci(sms); create_pcie(sms);