Re: qemu direct kernel boot on LoongArch

Re: qemu direct kernel boot on LoongArch

[maobibo]

Thomas,

Thanks for reporting this issue.

It is the problem of QEMU for LoongArch system, QEMU does not support 
booting vmlinux with elf format without UEFI bios now. We will add 
support to boot vmlinux directly on QEMU LoongArch side.

Regards
Bibo Mao

在 2023/10/10 上午4:13, Thomas Weißschuh 写道:
> Hi,
> 
> I'm trying to boot a LoongArch Linux via the direct kernel mode of qemu.
> ( qemu -kernel vmlinux -append "verbose" )
> This doesn't work so well.
> 
> A few problems I encoutered:
> 
> * Linux reads the cmdline from $a1 during boot, but qemu doesn't place it there.
> * Qemu puts a FDT at 1MiB but Linux doesn't use it.
> * Qemu does not put the cmdline into the FDT.
> * Linux panics in memory_present() because memblock_init() in
>    arch/loongarch/kernel/mem.c did not set up any memblocks.
> * Qemu has no support for initrds in direct kernel mode on LoongArch.
> 
> (All observations based on current git versions)
> 
> For the other architectures I'm using Linux/qemu with the direct kernel
> mode works nicely including cmdline and initrds.
> 
> Are these restrictions by design/intentional or is it just for somebody
> to do the necessary legwork?
> 
> The direct kernel mode is nice because it does not require the
> installation of the additional EFI binary.
> 
> 
> Thanks,
> Thomas
> 

Re: qemu direct kernel boot on LoongArch

[Jason A. Donenfeld]

On Tue, Oct 10, 2023 at 09:12:46AM +0800, maobibo wrote:
> Thomas,
> 
> Thanks for reporting this issue.
> 
> It is the problem of QEMU for LoongArch system, QEMU does not support 
> booting vmlinux with elf format without UEFI bios now. We will add 
> support to boot vmlinux directly on QEMU LoongArch side.

I ran into this same issue when trying to boot my test kernel for
Ruoyao's recent vDSO getrandom work. Is this something that was ever
addressed on the QEMU side?

Jason

Re: qemu direct kernel boot on LoongArch

[maobibo]

Jason,

With the latest qemu 9.1 version, elf format booting is supported. Qemu 
9.0 does not support it still, here is example command line.

./qemu-system-loongarch64  -m 4G -smp 2 --cpu la464 --machine virt 
-serial stdio   -monitor telnet:localhost:4495,server,nowait -kernel 
/root/linux_larch/vmlinux -initrd ramdisk  -append "root=/dev/ram 
rdinit=/sbin/init console=ttyS0,115200" --nographic

Regards
Bibo Mao

On 2024/9/5 上午11:12, Jason A. Donenfeld wrote:
> On Tue, Oct 10, 2023 at 09:12:46AM +0800, maobibo wrote:
>> Thomas,
>>
>> Thanks for reporting this issue.
>>
>> It is the problem of QEMU for LoongArch system, QEMU does not support
>> booting vmlinux with elf format without UEFI bios now. We will add
>> support to boot vmlinux directly on QEMU LoongArch side.
> 
> I ran into this same issue when trying to boot my test kernel for
> Ruoyao's recent vDSO getrandom work. Is this something that was ever
> addressed on the QEMU side?
> 
> Jason
> 

Re: qemu direct kernel boot on LoongArch

[Jason A. Donenfeld]

On Thu, Sep 5, 2024 at 5:45 AM maobibo <maobibo@loongson.cn> wrote:
>
> Jason,
>
> With the latest qemu 9.1 version, elf format booting is supported.

Thanks, I just figured this out too, about 4 minutes ago. Excellent.
And the 1G minimum ram limit is gone too.

Now working on how to trigger resets.

Jason

Re: qemu direct kernel boot on LoongArch

[Thomas Weißschuh]

On 2024-09-05 06:04:12+0000, Jason A. Donenfeld wrote:
> On Thu, Sep 5, 2024 at 5:45 AM maobibo <maobibo@loongson.cn> wrote:
> >
> > Jason,
> >
> > With the latest qemu 9.1 version, elf format booting is supported.
> 
> Thanks, I just figured this out too, about 4 minutes ago. Excellent.
> And the 1G minimum ram limit is gone too.
> 
> Now working on how to trigger resets.

With "reset" do you mean normal (non-panic) system shutdown/poweroff?
Since QEMU 9.1 and a recent kernel you can use the pvpanic device for
that in a cross-architecture way.


Thomas

Re: qemu direct kernel boot on LoongArch

[maobibo]

On 2024/9/5 下午1:25, Thomas Weißschuh wrote:
> On 2024-09-05 06:04:12+0000, Jason A. Donenfeld wrote:
>> On Thu, Sep 5, 2024 at 5:45 AM maobibo <maobibo@loongson.cn> wrote:
>>>
>>> Jason,
>>>
>>> With the latest qemu 9.1 version, elf format booting is supported.
>>
>> Thanks, I just figured this out too, about 4 minutes ago. Excellent.
>> And the 1G minimum ram limit is gone too.
>>
>> Now working on how to trigger resets.
> 
> With "reset" do you mean normal (non-panic) system shutdown/poweroff?
> Since QEMU 9.1 and a recent kernel you can use the pvpanic device for
> that in a cross-architecture way.
LoongArch uses acpi GED device to reboot machine. Now there is no FDT 
table description and FDT driver for acpi GED device :(

We will investigate pvpanic method.

Regards
Bibo Mao
> 
> 
> Thomas
> 

Re: qemu direct kernel boot on LoongArch

[Jason A. Donenfeld]

On Thu, Sep 05, 2024 at 07:25:05AM +0200, Thomas Weißschuh wrote:
> On 2024-09-05 06:04:12+0000, Jason A. Donenfeld wrote:
> > On Thu, Sep 5, 2024 at 5:45 AM maobibo <maobibo@loongson.cn> wrote:
> > >
> > > Jason,
> > >
> > > With the latest qemu 9.1 version, elf format booting is supported.
> > 
> > Thanks, I just figured this out too, about 4 minutes ago. Excellent.
> > And the 1G minimum ram limit is gone too.
> > 
> > Now working on how to trigger resets.
> 
> With "reset" do you mean normal (non-panic) system shutdown/poweroff?
> Since QEMU 9.1 and a recent kernel you can use the pvpanic device for
> that in a cross-architecture way.

What I mean is that I need for userspace calling `reboot(RB_AUTOBOOT);`
to actually result in QEMU being told to reboot the system. Sounds like
that's not possible (yet?) in 9.1?

The last hurdle I'm having, in addition to that one, is getting the
second serial port working. The kernel test suite that I'm running in CI
for /dev/urandom development and wireguard development does `-serial
stdio -serial chardev:result`, where the first serial is the normal
output for logging, and the second one is the test result, so I can get
that from the VM via a clean channel. This works well on other
architectures, but on loongarch, it's failing to open /dev/ttyS1 the
same way that it can open /dev/ttyS0.

Re: qemu direct kernel boot on LoongArch

[Jason A. Donenfeld]

On Thu, Sep 05, 2024 at 02:11:32PM +0800, maobibo wrote:
> 
> 
> On 2024/9/5 下午1:25, Thomas Weißschuh wrote:
> > On 2024-09-05 06:04:12+0000, Jason A. Donenfeld wrote:
> >> On Thu, Sep 5, 2024 at 5:45 AM maobibo <maobibo@loongson.cn> wrote:
> >>>
> >>> Jason,
> >>>
> >>> With the latest qemu 9.1 version, elf format booting is supported.
> >>
> >> Thanks, I just figured this out too, about 4 minutes ago. Excellent.
> >> And the 1G minimum ram limit is gone too.
> >>
> >> Now working on how to trigger resets.
> > 
> > With "reset" do you mean normal (non-panic) system shutdown/poweroff?
> > Since QEMU 9.1 and a recent kernel you can use the pvpanic device for
> > that in a cross-architecture way.
> LoongArch uses acpi GED device to reboot machine. Now there is no FDT 
> table description and FDT driver for acpi GED device :(

So you mean that QEMU exposes it via ACPI, but not via FDT, and the
kernel doesn't know how to recognize it, even with ACPI?

Re: qemu direct kernel boot on LoongArch

[Thomas Weißschuh]

On 2024-09-05 16:53:55+0000, Jason A. Donenfeld wrote:
> On Thu, Sep 05, 2024 at 07:25:05AM +0200, Thomas Weißschuh wrote:
> > On 2024-09-05 06:04:12+0000, Jason A. Donenfeld wrote:
> > > On Thu, Sep 5, 2024 at 5:45 AM maobibo <maobibo@loongson.cn> wrote:
> > > >
> > > > Jason,
> > > >
> > > > With the latest qemu 9.1 version, elf format booting is supported.
> > > 
> > > Thanks, I just figured this out too, about 4 minutes ago. Excellent.
> > > And the 1G minimum ram limit is gone too.
> > > 
> > > Now working on how to trigger resets.
> > 
> > With "reset" do you mean normal (non-panic) system shutdown/poweroff?
> > Since QEMU 9.1 and a recent kernel you can use the pvpanic device for
> > that in a cross-architecture way.
> 
> What I mean is that I need for userspace calling `reboot(RB_AUTOBOOT);`
> to actually result in QEMU being told to reboot the system. Sounds like
> that's not possible (yet?) in 9.1?

With reboot(RB_POWER_OFF) this is indeed the exact usecase for pvpanic
shutdown support. Reboot is not supported, but maybe -action
shutdown=reboot works and is enough. It would also be trivial to extend
pvpanic for reboot support.

Use -device pvpanic-pci for QEMU and CONFIG_PVPANIC_PCI=y for Linux.
(That should work for most architectures)

Re: qemu direct kernel boot on LoongArch

[Jason A. Donenfeld]

On Thu, Sep 5, 2024 at 5:05 PM Thomas Weißschuh <thomas@t-8ch.de> wrote:
>
> On 2024-09-05 16:53:55+0000, Jason A. Donenfeld wrote:
> > On Thu, Sep 05, 2024 at 07:25:05AM +0200, Thomas Weißschuh wrote:
> > > On 2024-09-05 06:04:12+0000, Jason A. Donenfeld wrote:
> > > > On Thu, Sep 5, 2024 at 5:45 AM maobibo <maobibo@loongson.cn> wrote:
> > > > >
> > > > > Jason,
> > > > >
> > > > > With the latest qemu 9.1 version, elf format booting is supported.
> > > >
> > > > Thanks, I just figured this out too, about 4 minutes ago. Excellent.
> > > > And the 1G minimum ram limit is gone too.
> > > >
> > > > Now working on how to trigger resets.
> > >
> > > With "reset" do you mean normal (non-panic) system shutdown/poweroff?
> > > Since QEMU 9.1 and a recent kernel you can use the pvpanic device for
> > > that in a cross-architecture way.
> >
> > What I mean is that I need for userspace calling `reboot(RB_AUTOBOOT);`
> > to actually result in QEMU being told to reboot the system. Sounds like
> > that's not possible (yet?) in 9.1?
>
> With reboot(RB_POWER_OFF) this is indeed the exact usecase for pvpanic

I'm actually using reboot(RB_AUTOBOOT) wth QEMU's -no-reboot, because
that tends to be far more compatible with a greater number of
platforms, for example, x86 without acpi. Shucks.

Re: qemu direct kernel boot on LoongArch

[Thomas Weißschuh]

On 2024-09-05 17:07:22+0000, Jason A. Donenfeld wrote:
> On Thu, Sep 5, 2024 at 5:05 PM Thomas Weißschuh <thomas@t-8ch.de> wrote:
> >
> > On 2024-09-05 16:53:55+0000, Jason A. Donenfeld wrote:
> > > On Thu, Sep 05, 2024 at 07:25:05AM +0200, Thomas Weißschuh wrote:
> > > > On 2024-09-05 06:04:12+0000, Jason A. Donenfeld wrote:
> > > > > On Thu, Sep 5, 2024 at 5:45 AM maobibo <maobibo@loongson.cn> wrote:
> > > > > >
> > > > > > Jason,
> > > > > >
> > > > > > With the latest qemu 9.1 version, elf format booting is supported.
> > > > >
> > > > > Thanks, I just figured this out too, about 4 minutes ago. Excellent.
> > > > > And the 1G minimum ram limit is gone too.
> > > > >
> > > > > Now working on how to trigger resets.
> > > >
> > > > With "reset" do you mean normal (non-panic) system shutdown/poweroff?
> > > > Since QEMU 9.1 and a recent kernel you can use the pvpanic device for
> > > > that in a cross-architecture way.
> > >
> > > What I mean is that I need for userspace calling `reboot(RB_AUTOBOOT);`
> > > to actually result in QEMU being told to reboot the system. Sounds like
> > > that's not possible (yet?) in 9.1?
> >
> > With reboot(RB_POWER_OFF) this is indeed the exact usecase for pvpanic
> 
> I'm actually using reboot(RB_AUTOBOOT) wth QEMU's -no-reboot, because
> that tends to be far more compatible with a greater number of
> platforms, for example, x86 without acpi. Shucks.

You can check that both QEMU and the kernel support pvpanic shutdown
through sysfs and if so use reboot(RB_POWER_OFF); and
reboot(RB_AUTOBOOT) otherwise.

Re: qemu direct kernel boot on LoongArch

[Jason A. Donenfeld]

On Thu, Sep 5, 2024 at 5:16 PM Thomas Weißschuh <thomas@t-8ch.de> wrote:
>
> On 2024-09-05 17:07:22+0000, Jason A. Donenfeld wrote:
> > On Thu, Sep 5, 2024 at 5:05 PM Thomas Weißschuh <thomas@t-8ch.de> wrote:
> > >
> > > On 2024-09-05 16:53:55+0000, Jason A. Donenfeld wrote:
> > > > On Thu, Sep 05, 2024 at 07:25:05AM +0200, Thomas Weißschuh wrote:
> > > > > On 2024-09-05 06:04:12+0000, Jason A. Donenfeld wrote:
> > > > > > On Thu, Sep 5, 2024 at 5:45 AM maobibo <maobibo@loongson.cn> wrote:
> > > > > > >
> > > > > > > Jason,
> > > > > > >
> > > > > > > With the latest qemu 9.1 version, elf format booting is supported.
> > > > > >
> > > > > > Thanks, I just figured this out too, about 4 minutes ago. Excellent.
> > > > > > And the 1G minimum ram limit is gone too.
> > > > > >
> > > > > > Now working on how to trigger resets.
> > > > >
> > > > > With "reset" do you mean normal (non-panic) system shutdown/poweroff?
> > > > > Since QEMU 9.1 and a recent kernel you can use the pvpanic device for
> > > > > that in a cross-architecture way.
> > > >
> > > > What I mean is that I need for userspace calling `reboot(RB_AUTOBOOT);`
> > > > to actually result in QEMU being told to reboot the system. Sounds like
> > > > that's not possible (yet?) in 9.1?
> > >
> > > With reboot(RB_POWER_OFF) this is indeed the exact usecase for pvpanic
> >
> > I'm actually using reboot(RB_AUTOBOOT) wth QEMU's -no-reboot, because
> > that tends to be far more compatible with a greater number of
> > platforms, for example, x86 without acpi. Shucks.
>
> You can check that both QEMU and the kernel support pvpanic shutdown
> through sysfs and if so use reboot(RB_POWER_OFF); and
> reboot(RB_AUTOBOOT) otherwise.

I guess. But the whole idea is to bloat the code as little as possible
and use one interface for everything. Pushing that all up into
userspace is pretty icky.

It sounds like LoongArch already supports this via ACPI GED, but
there's some plumbing that needs to be done still. So maybe I'll just
wait for that.

Meanwhile, any idea about adding a second serial to the platform? I've
been futzing with it for a bit now to no avail.

Jason

Re: qemu direct kernel boot on LoongArch

[Jason A. Donenfeld]

On Thu, Sep 05, 2024 at 04:54:41PM +0200, Jason A. Donenfeld wrote:
> On Thu, Sep 05, 2024 at 02:11:32PM +0800, maobibo wrote:
> > 
> > 
> > On 2024/9/5 下午1:25, Thomas Weißschuh wrote:
> > > On 2024-09-05 06:04:12+0000, Jason A. Donenfeld wrote:
> > >> On Thu, Sep 5, 2024 at 5:45 AM maobibo <maobibo@loongson.cn> wrote:
> > >>>
> > >>> Jason,
> > >>>
> > >>> With the latest qemu 9.1 version, elf format booting is supported.
> > >>
> > >> Thanks, I just figured this out too, about 4 minutes ago. Excellent.
> > >> And the 1G minimum ram limit is gone too.
> > >>
> > >> Now working on how to trigger resets.
> > > 
> > > With "reset" do you mean normal (non-panic) system shutdown/poweroff?
> > > Since QEMU 9.1 and a recent kernel you can use the pvpanic device for
> > > that in a cross-architecture way.
> > LoongArch uses acpi GED device to reboot machine. Now there is no FDT 
> > table description and FDT driver for acpi GED device :(
> 
> So you mean that QEMU exposes it via ACPI, but not via FDT, and the
> kernel doesn't know how to recognize it, even with ACPI?

Ahh, it looks like QEMU exposes it via ACPI, but ACPI is broken (?) when
booting directly from ELF. So it needs to be exposed via FDT instead? Or
ACPI needs to be fixed with direct ELF boot?

Re: qemu direct kernel boot on LoongArch

[Thomas Weißschuh]

On 2024-09-05 17:18:07+0000, Jason A. Donenfeld wrote:
> On Thu, Sep 5, 2024 at 5:16 PM Thomas Weißschuh <thomas@t-8ch.de> wrote:
> >
> > On 2024-09-05 17:07:22+0000, Jason A. Donenfeld wrote:
> > > On Thu, Sep 5, 2024 at 5:05 PM Thomas Weißschuh <thomas@t-8ch.de> wrote:
> > > >
> > > > On 2024-09-05 16:53:55+0000, Jason A. Donenfeld wrote:
> > > > > On Thu, Sep 05, 2024 at 07:25:05AM +0200, Thomas Weißschuh wrote:
> > > > > > On 2024-09-05 06:04:12+0000, Jason A. Donenfeld wrote:
> > > > > > > On Thu, Sep 5, 2024 at 5:45 AM maobibo <maobibo@loongson.cn> wrote:
> > > > > > > >
> > > > > > > > Jason,
> > > > > > > >
> > > > > > > > With the latest qemu 9.1 version, elf format booting is supported.
> > > > > > >
> > > > > > > Thanks, I just figured this out too, about 4 minutes ago. Excellent.
> > > > > > > And the 1G minimum ram limit is gone too.
> > > > > > >
> > > > > > > Now working on how to trigger resets.
> > > > > >
> > > > > > With "reset" do you mean normal (non-panic) system shutdown/poweroff?
> > > > > > Since QEMU 9.1 and a recent kernel you can use the pvpanic device for
> > > > > > that in a cross-architecture way.
> > > > >
> > > > > What I mean is that I need for userspace calling `reboot(RB_AUTOBOOT);`
> > > > > to actually result in QEMU being told to reboot the system. Sounds like
> > > > > that's not possible (yet?) in 9.1?
> > > >
> > > > With reboot(RB_POWER_OFF) this is indeed the exact usecase for pvpanic
> > >
> > > I'm actually using reboot(RB_AUTOBOOT) wth QEMU's -no-reboot, because
> > > that tends to be far more compatible with a greater number of
> > > platforms, for example, x86 without acpi. Shucks.
> >
> > You can check that both QEMU and the kernel support pvpanic shutdown
> > through sysfs and if so use reboot(RB_POWER_OFF); and
> > reboot(RB_AUTOBOOT) otherwise.
> 
> I guess. But the whole idea is to bloat the code as little as possible
> and use one interface for everything. Pushing that all up into
> userspace is pretty icky.

If it works through ACPI everywhere then sure.

> It sounds like LoongArch already supports this via ACPI GED, but
> there's some plumbing that needs to be done still. So maybe I'll just
> wait for that.

Also sounds reasonable.

> Meanwhile, any idea about adding a second serial to the platform? I've
> been futzing with it for a bit now to no avail.

No idea, sorry.

Re: qemu direct kernel boot on LoongArch

[maobibo]

On 2024/9/5 下午11:49, Jason A. Donenfeld wrote:
> On Thu, Sep 05, 2024 at 04:54:41PM +0200, Jason A. Donenfeld wrote:
>> On Thu, Sep 05, 2024 at 02:11:32PM +0800, maobibo wrote:
>>>
>>>
>>> On 2024/9/5 下午1:25, Thomas Weißschuh wrote:
>>>> On 2024-09-05 06:04:12+0000, Jason A. Donenfeld wrote:
>>>>> On Thu, Sep 5, 2024 at 5:45 AM maobibo <maobibo@loongson.cn> wrote:
>>>>>>
>>>>>> Jason,
>>>>>>
>>>>>> With the latest qemu 9.1 version, elf format booting is supported.
>>>>>
>>>>> Thanks, I just figured this out too, about 4 minutes ago. Excellent.
>>>>> And the 1G minimum ram limit is gone too.
>>>>>
>>>>> Now working on how to trigger resets.
>>>>
>>>> With "reset" do you mean normal (non-panic) system shutdown/poweroff?
>>>> Since QEMU 9.1 and a recent kernel you can use the pvpanic device for
>>>> that in a cross-architecture way.
>>> LoongArch uses acpi GED device to reboot machine. Now there is no FDT
>>> table description and FDT driver for acpi GED device :(
>>
>> So you mean that QEMU exposes it via ACPI, but not via FDT, and the
>> kernel doesn't know how to recognize it, even with ACPI?
> 
> Ahh, it looks like QEMU exposes it via ACPI, but ACPI is broken (?) when
> booting directly from ELF. So it needs to be exposed via FDT instead? Or
> ACPI needs to be fixed with direct ELF boot?
We plant to expose ACPI GED via FDT interface with syscon method, and 
the reboot driver in linux kernel side is
drivers/power/reset/syscon-reboot.c.

The only difference is that ACPI GED register is accessed with 
byte-mode, it seems that it works well with draft version.

Regards
Bibo Mao
> 

Re: qemu direct kernel boot on LoongArch

[maobibo]

On 2024/9/6 上午12:03, Thomas Weißschuh wrote:
> On 2024-09-05 17:18:07+0000, Jason A. Donenfeld wrote:
>> On Thu, Sep 5, 2024 at 5:16 PM Thomas Weißschuh <thomas@t-8ch.de> wrote:
>>>
>>> On 2024-09-05 17:07:22+0000, Jason A. Donenfeld wrote:
>>>> On Thu, Sep 5, 2024 at 5:05 PM Thomas Weißschuh <thomas@t-8ch.de> wrote:
>>>>>
>>>>> On 2024-09-05 16:53:55+0000, Jason A. Donenfeld wrote:
>>>>>> On Thu, Sep 05, 2024 at 07:25:05AM +0200, Thomas Weißschuh wrote:
>>>>>>> On 2024-09-05 06:04:12+0000, Jason A. Donenfeld wrote:
>>>>>>>> On Thu, Sep 5, 2024 at 5:45 AM maobibo <maobibo@loongson.cn> wrote:
>>>>>>>>>
>>>>>>>>> Jason,
>>>>>>>>>
>>>>>>>>> With the latest qemu 9.1 version, elf format booting is supported.
>>>>>>>>
>>>>>>>> Thanks, I just figured this out too, about 4 minutes ago. Excellent.
>>>>>>>> And the 1G minimum ram limit is gone too.
>>>>>>>>
>>>>>>>> Now working on how to trigger resets.
>>>>>>>
>>>>>>> With "reset" do you mean normal (non-panic) system shutdown/poweroff?
>>>>>>> Since QEMU 9.1 and a recent kernel you can use the pvpanic device for
>>>>>>> that in a cross-architecture way.
>>>>>>
>>>>>> What I mean is that I need for userspace calling `reboot(RB_AUTOBOOT);`
>>>>>> to actually result in QEMU being told to reboot the system. Sounds like
>>>>>> that's not possible (yet?) in 9.1?
>>>>>
>>>>> With reboot(RB_POWER_OFF) this is indeed the exact usecase for pvpanic
>>>>
>>>> I'm actually using reboot(RB_AUTOBOOT) wth QEMU's -no-reboot, because
>>>> that tends to be far more compatible with a greater number of
>>>> platforms, for example, x86 without acpi. Shucks.
>>>
>>> You can check that both QEMU and the kernel support pvpanic shutdown
>>> through sysfs and if so use reboot(RB_POWER_OFF); and
>>> reboot(RB_AUTOBOOT) otherwise.
>>
>> I guess. But the whole idea is to bloat the code as little as possible
>> and use one interface for everything. Pushing that all up into
>> userspace is pretty icky.
> 
> If it works through ACPI everywhere then sure.
> 
>> It sounds like LoongArch already supports this via ACPI GED, but
>> there's some plumbing that needs to be done still. So maybe I'll just
>> wait for that.
> 
> Also sounds reasonable.
yeap, will submit a patch to expose ACPI GED pm interface with FDT method.

> 
>> Meanwhile, any idea about adding a second serial to the platform? I've
>> been futzing with it for a bit now to no avail.
> 
> No idea, sorry.
Will investigate the second serial method.

Regards
Bibo Mao

Re: qemu direct kernel boot on LoongArch

[Jason A. Donenfeld]

On Fri, Sep 6, 2024 at 3:14 AM maobibo <maobibo@loongson.cn> wrote:
> yeap, will submit a patch to expose ACPI GED pm interface with FDT method.

Clever trick. Works well.

>
> >
> >> Meanwhile, any idea about adding a second serial to the platform? I've
> >> been futzing with it for a bit now to no avail.
> >
> > No idea, sorry.
> Will investigate the second serial method.

Thank you very much.

Re: qemu direct kernel boot on LoongArch

[Jason A. Donenfeld]

On Fri, Sep 06, 2024 at 06:04:25AM +0200, Jason A. Donenfeld wrote:
> On Fri, Sep 6, 2024 at 3:14 AM maobibo <maobibo@loongson.cn> wrote:
> > yeap, will submit a patch to expose ACPI GED pm interface with FDT method.
> 
> Clever trick. Works well.
> 
> >
> > >
> > >> Meanwhile, any idea about adding a second serial to the platform? I've
> > >> been futzing with it for a bit now to no avail.
> > >
> > > No idea, sorry.
> > Will investigate the second serial method.
> 
> Thank you very much.

I did it. I'll send a patch.

[PATCH] hw/loongarch: virt: support up to 4 serial ports

[Jason A. Donenfeld]

In order to support additional channels of communication using
`-serial`, add several serial ports, up to the standard 4 generally
supported by the 8250 driver.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 hw/loongarch/virt.c        | 24 ++++++++++++++----------
 include/hw/pci-host/ls7a.h |  9 +++++----
 2 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index 4151fc5e0c..155678b684 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -319,10 +319,10 @@ static void fdt_add_ged_reset(LoongArchVirtMachineState *lvms)
 }
 
 static void fdt_add_uart_node(LoongArchVirtMachineState *lvms,
-                              uint32_t *pch_pic_phandle)
+                              uint32_t *pch_pic_phandle, hwaddr base,
+                              int irq, bool chosen)
 {
     char *nodename;
-    hwaddr base = VIRT_UART_BASE;
     hwaddr size = VIRT_UART_SIZE;
     MachineState *ms = MACHINE(lvms);
 
@@ -331,9 +331,9 @@ static void fdt_add_uart_node(LoongArchVirtMachineState *lvms,
     qemu_fdt_setprop_string(ms->fdt, nodename, "compatible", "ns16550a");
     qemu_fdt_setprop_cells(ms->fdt, nodename, "reg", 0x0, base, 0x0, size);
     qemu_fdt_setprop_cell(ms->fdt, nodename, "clock-frequency", 100000000);
-    qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
-    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
-                           VIRT_UART_IRQ - VIRT_GSI_BASE, 0x4);
+    if (chosen)
+        qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
+    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts", irq, 0x4);
     qemu_fdt_setprop_cell(ms->fdt, nodename, "interrupt-parent",
                           *pch_pic_phandle);
     g_free(nodename);
@@ -750,11 +750,15 @@ static void virt_devices_init(DeviceState *pch_pic,
     /* Add pcie node */
     fdt_add_pcie_node(lvms, pch_pic_phandle, pch_msi_phandle);
 
-    serial_mm_init(get_system_memory(), VIRT_UART_BASE, 0,
-                   qdev_get_gpio_in(pch_pic,
-                                    VIRT_UART_IRQ - VIRT_GSI_BASE),
-                   115200, serial_hd(0), DEVICE_LITTLE_ENDIAN);
-    fdt_add_uart_node(lvms, pch_pic_phandle);
+    for (i = 0; i < VIRT_UART_COUNT; ++i) {
+        hwaddr base = VIRT_UART_BASE + i * VIRT_UART_SIZE;
+        int irq = VIRT_UART_IRQ + i - VIRT_GSI_BASE;
+        serial_mm_init(get_system_memory(), base, 0,
+                       qdev_get_gpio_in(pch_pic, irq),
+                       115200, serial_hd(VIRT_UART_COUNT - 1 - i),
+                       DEVICE_LITTLE_ENDIAN);
+        fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == 0);
+    }
 
     /* Network init */
     pci_init_nic_devices(pci_bus, mc->default_nic);
diff --git a/include/hw/pci-host/ls7a.h b/include/hw/pci-host/ls7a.h
index cd7c9ec7bc..79d4ea8501 100644
--- a/include/hw/pci-host/ls7a.h
+++ b/include/hw/pci-host/ls7a.h
@@ -36,17 +36,18 @@
 #define VIRT_PCH_PIC_IRQ_NUM     32
 #define VIRT_GSI_BASE            64
 #define VIRT_DEVICE_IRQS         16
+#define VIRT_UART_COUNT          4
 #define VIRT_UART_IRQ            (VIRT_GSI_BASE + 2)
 #define VIRT_UART_BASE           0x1fe001e0
-#define VIRT_UART_SIZE           0X100
-#define VIRT_RTC_IRQ             (VIRT_GSI_BASE + 3)
+#define VIRT_UART_SIZE           0x100
+#define VIRT_RTC_IRQ             (VIRT_GSI_BASE + 6)
 #define VIRT_MISC_REG_BASE       (VIRT_PCH_REG_BASE + 0x00080000)
 #define VIRT_RTC_REG_BASE        (VIRT_MISC_REG_BASE + 0x00050100)
 #define VIRT_RTC_LEN             0x100
-#define VIRT_SCI_IRQ             (VIRT_GSI_BASE + 4)
+#define VIRT_SCI_IRQ             (VIRT_GSI_BASE + 7)
 
 #define VIRT_PLATFORM_BUS_BASEADDRESS   0x16000000
 #define VIRT_PLATFORM_BUS_SIZE          0x2000000
 #define VIRT_PLATFORM_BUS_NUM_IRQS      2
-#define VIRT_PLATFORM_BUS_IRQ           (VIRT_GSI_BASE + 5)
+#define VIRT_PLATFORM_BUS_IRQ           (VIRT_GSI_BASE + 8)
 #endif
-- 
2.46.0

Re: [PATCH] hw/loongarch: virt: support up to 4 serial ports

[maobibo]

On 2024/9/6 下午12:49, Jason A. Donenfeld wrote:
> In order to support additional channels of communication using
> `-serial`, add several serial ports, up to the standard 4 generally
> supported by the 8250 driver.
> 
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>   hw/loongarch/virt.c        | 24 ++++++++++++++----------
>   include/hw/pci-host/ls7a.h |  9 +++++----
>   2 files changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> index 4151fc5e0c..155678b684 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -319,10 +319,10 @@ static void fdt_add_ged_reset(LoongArchVirtMachineState *lvms)
>   }
>   
>   static void fdt_add_uart_node(LoongArchVirtMachineState *lvms,
> -                              uint32_t *pch_pic_phandle)
> +                              uint32_t *pch_pic_phandle, hwaddr base,
> +                              int irq, bool chosen)
>   {
>       char *nodename;
> -    hwaddr base = VIRT_UART_BASE;
>       hwaddr size = VIRT_UART_SIZE;
>       MachineState *ms = MACHINE(lvms);
>   
> @@ -331,9 +331,9 @@ static void fdt_add_uart_node(LoongArchVirtMachineState *lvms,
>       qemu_fdt_setprop_string(ms->fdt, nodename, "compatible", "ns16550a");
>       qemu_fdt_setprop_cells(ms->fdt, nodename, "reg", 0x0, base, 0x0, size);
>       qemu_fdt_setprop_cell(ms->fdt, nodename, "clock-frequency", 100000000);
> -    qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
> -    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
> -                           VIRT_UART_IRQ - VIRT_GSI_BASE, 0x4);
> +    if (chosen)
> +        qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
> +    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts", irq, 0x4);
>       qemu_fdt_setprop_cell(ms->fdt, nodename, "interrupt-parent",
>                             *pch_pic_phandle);
>       g_free(nodename);
> @@ -750,11 +750,15 @@ static void virt_devices_init(DeviceState *pch_pic,
>       /* Add pcie node */
>       fdt_add_pcie_node(lvms, pch_pic_phandle, pch_msi_phandle);
>   
> -    serial_mm_init(get_system_memory(), VIRT_UART_BASE, 0,
> -                   qdev_get_gpio_in(pch_pic,
> -                                    VIRT_UART_IRQ - VIRT_GSI_BASE),
> -                   115200, serial_hd(0), DEVICE_LITTLE_ENDIAN);
> -    fdt_add_uart_node(lvms, pch_pic_phandle);
> +    for (i = 0; i < VIRT_UART_COUNT; ++i) {
How about adding serial_hd(i) checking here, such as
   for (i = 0; (i < VIRT_UART_COUNT) && serial_hd(i); ++i) {

> +        hwaddr base = VIRT_UART_BASE + i * VIRT_UART_SIZE;
> +        int irq = VIRT_UART_IRQ + i - VIRT_GSI_BASE;
> +        serial_mm_init(get_system_memory(), base, 0,
> +                       qdev_get_gpio_in(pch_pic, irq),
> +                       115200, serial_hd(VIRT_UART_COUNT - 1 - i),
is it serial_hd(i) here rather than serial_hd(VIRT_UART_COUNT - 1 - i)? 
In general serial_hd(0) is default serial.

> +                       DEVICE_LITTLE_ENDIAN);
> +        fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == 0);
> +    }
>   
>       /* Network init */
>       pci_init_nic_devices(pci_bus, mc->default_nic);
> diff --git a/include/hw/pci-host/ls7a.h b/include/hw/pci-host/ls7a.h
> index cd7c9ec7bc..79d4ea8501 100644
> --- a/include/hw/pci-host/ls7a.h
> +++ b/include/hw/pci-host/ls7a.h
> @@ -36,17 +36,18 @@
>   #define VIRT_PCH_PIC_IRQ_NUM     32
>   #define VIRT_GSI_BASE            64
>   #define VIRT_DEVICE_IRQS         16
> +#define VIRT_UART_COUNT          4
>   #define VIRT_UART_IRQ            (VIRT_GSI_BASE + 2)
>   #define VIRT_UART_BASE           0x1fe001e0
> -#define VIRT_UART_SIZE           0X100
> -#define VIRT_RTC_IRQ             (VIRT_GSI_BASE + 3)
> +#define VIRT_UART_SIZE           0x100
> +#define VIRT_RTC_IRQ             (VIRT_GSI_BASE + 6)
>   #define VIRT_MISC_REG_BASE       (VIRT_PCH_REG_BASE + 0x00080000)
>   #define VIRT_RTC_REG_BASE        (VIRT_MISC_REG_BASE + 0x00050100)
>   #define VIRT_RTC_LEN             0x100
> -#define VIRT_SCI_IRQ             (VIRT_GSI_BASE + 4)
> +#define VIRT_SCI_IRQ             (VIRT_GSI_BASE + 7)
>   
>   #define VIRT_PLATFORM_BUS_BASEADDRESS   0x16000000
>   #define VIRT_PLATFORM_BUS_SIZE          0x2000000
>   #define VIRT_PLATFORM_BUS_NUM_IRQS      2
> -#define VIRT_PLATFORM_BUS_IRQ           (VIRT_GSI_BASE + 5)
> +#define VIRT_PLATFORM_BUS_IRQ           (VIRT_GSI_BASE + 8)
>   #endif
> 
By the way, serial port for acpi table should be refreshed also, such as
diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
index 2638f87434..5bd2a9beaa 100644
--- a/hw/loongarch/acpi-build.c
+++ b/hw/loongarch/acpi-build.c
@@ -31,6 +31,7 @@

  #include "hw/acpi/generic_event_device.h"
  #include "hw/pci-host/gpex.h"
+#include "sysemu/sysemu.h"
  #include "sysemu/tpm.h"
  #include "hw/platform-bus.h"
  #include "hw/acpi/aml-build.h"
@@ -252,23 +253,26 @@ struct AcpiBuildState {
      MemoryRegion *linker_mr;
  } AcpiBuildState;

-static void build_uart_device_aml(Aml *table)
+static void build_uart_device_aml(Aml *table, int index)
  {
      Aml *dev;
      Aml *crs;
      Aml *pkg0, *pkg1, *pkg2;
-    uint32_t uart_irq = VIRT_UART_IRQ;
+    uint32_t uart_irq;
+    uint64_t base;

+    uart_irq = VIRT_UART_IRQ + index;
+    base = VIRT_UART_BASE + index * VIRT_UART_SIZE;
      Aml *scope = aml_scope("_SB");
-    dev = aml_device("COMA");
+    dev = aml_device("COM%d", index);
      aml_append(dev, aml_name_decl("_HID", aml_string("PNP0501")));
-    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+    aml_append(dev, aml_name_decl("_UID", aml_int(index)));
      aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
      crs = aml_resource_template();
      aml_append(crs,
          aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
                           AML_NON_CACHEABLE, AML_READ_WRITE,
-                         0, VIRT_UART_BASE, VIRT_UART_BASE + 
VIRT_UART_SIZE - 1,
+                         0, base, base + VIRT_UART_SIZE - 1,
                           0, VIRT_UART_SIZE));
      aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL, 
AML_ACTIVE_HIGH,
                                    AML_SHARED, &uart_irq, 1));
@@ -405,10 +409,13 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, 
MachineState *machine)
      LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(machine);
      AcpiTable table = { .sig = "DSDT", .rev = 1, .oem_id = lvms->oem_id,
                          .oem_table_id = lvms->oem_table_id };
+    int i;

      acpi_table_begin(&table, table_data);
      dsdt = init_aml_allocator();
-    build_uart_device_aml(dsdt);
+    for (i = 0; (i < VIRT_UART_COUNT) && serial_hd(i); i++) {
+        build_uart_device_aml(dsdt, i);
+    }
      build_pci_device_aml(dsdt, lvms);
      build_la_ged_aml(dsdt, machine);
      build_flash_aml(dsdt, lvms);


Regards
Bibo Mao

Re: [PATCH] hw/loongarch: virt: support up to 4 serial ports

[Jason A. Donenfeld]

On Fri, Sep 06, 2024 at 04:34:53PM +0800, maobibo wrote:
> > +    for (i = 0; i < VIRT_UART_COUNT; ++i) {
> How about adding serial_hd(i) checking here, such as
>    for (i = 0; (i < VIRT_UART_COUNT) && serial_hd(i); ++i) {

That doesn't seem to do anything, unfortunately.

> 
> > +        hwaddr base = VIRT_UART_BASE + i * VIRT_UART_SIZE;
> > +        int irq = VIRT_UART_IRQ + i - VIRT_GSI_BASE;
> > +        serial_mm_init(get_system_memory(), base, 0,
> > +                       qdev_get_gpio_in(pch_pic, irq),
> > +                       115200, serial_hd(VIRT_UART_COUNT - 1 - i),
> is it serial_hd(i) here rather than serial_hd(VIRT_UART_COUNT - 1 - i)? 
> In general serial_hd(0) is default serial.

They've got to be added in reverse order. The chosen calculation needed
to be fixed though, in the line below:

> > +        fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == 0);

That now checks for the last index.

> By the way, serial port for acpi table should be refreshed also, such as

Thanks. Will send you a v2.

Jason

[PATCH v2] hw/loongarch: virt: support up to 4 serial ports

[Jason A. Donenfeld]

In order to support additional channels of communication using
`-serial`, add several serial ports, up to the standard 4 generally
supported by the 8250 driver.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
As I don't use ACPI, I haven't tested the ACPI part of this, which
Maobibo wrote.

 hw/loongarch/acpi-build.c  | 23 +++++++++++++++--------
 hw/loongarch/virt.c        | 24 ++++++++++++++----------
 include/hw/pci-host/ls7a.h |  9 +++++----
 3 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
index 2638f87434..2750c6e858 100644
--- a/hw/loongarch/acpi-build.c
+++ b/hw/loongarch/acpi-build.c
@@ -31,6 +31,7 @@
 
 #include "hw/acpi/generic_event_device.h"
 #include "hw/pci-host/gpex.h"
+#include "sysemu/sysemu.h"
 #include "sysemu/tpm.h"
 #include "hw/platform-bus.h"
 #include "hw/acpi/aml-build.h"
@@ -252,23 +253,27 @@ struct AcpiBuildState {
     MemoryRegion *linker_mr;
 } AcpiBuildState;
 
-static void build_uart_device_aml(Aml *table)
+static void build_uart_device_aml(Aml *table, int index)
 {
     Aml *dev;
     Aml *crs;
     Aml *pkg0, *pkg1, *pkg2;
-    uint32_t uart_irq = VIRT_UART_IRQ;
-
-    Aml *scope = aml_scope("_SB");
-    dev = aml_device("COMA");
+    Aml *scope;
+    uint32_t uart_irq;
+    uint64_t base;
+
+    uart_irq = VIRT_UART_IRQ + index;
+    base = VIRT_UART_BASE + index * VIRT_UART_SIZE;
+    scope = aml_scope("_SB");
+    dev = aml_device("COM%d", VIRT_UART_COUNT - 1 - index);
     aml_append(dev, aml_name_decl("_HID", aml_string("PNP0501")));
-    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+    aml_append(dev, aml_name_decl("_UID", aml_int(VIRT_UART_COUNT - 1 - index)));
     aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
     crs = aml_resource_template();
     aml_append(crs,
         aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
                          AML_NON_CACHEABLE, AML_READ_WRITE,
-                         0, VIRT_UART_BASE, VIRT_UART_BASE + VIRT_UART_SIZE - 1,
+                         0, base, base + VIRT_UART_SIZE - 1,
                          0, VIRT_UART_SIZE));
     aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
                                   AML_SHARED, &uart_irq, 1));
@@ -401,6 +406,7 @@ static void acpi_dsdt_add_tpm(Aml *scope, LoongArchVirtMachineState *vms)
 static void
 build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine)
 {
+    int i;
     Aml *dsdt, *scope, *pkg;
     LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(machine);
     AcpiTable table = { .sig = "DSDT", .rev = 1, .oem_id = lvms->oem_id,
@@ -408,7 +414,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine)
 
     acpi_table_begin(&table, table_data);
     dsdt = init_aml_allocator();
-    build_uart_device_aml(dsdt);
+    for (i = 0; i < VIRT_UART_COUNT; ++i)
+        build_uart_device_aml(dsdt, i);
     build_pci_device_aml(dsdt, lvms);
     build_la_ged_aml(dsdt, machine);
     build_flash_aml(dsdt, lvms);
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index 4151fc5e0c..6e8608874c 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -319,10 +319,10 @@ static void fdt_add_ged_reset(LoongArchVirtMachineState *lvms)
 }
 
 static void fdt_add_uart_node(LoongArchVirtMachineState *lvms,
-                              uint32_t *pch_pic_phandle)
+                              uint32_t *pch_pic_phandle, hwaddr base,
+                              int irq, bool chosen)
 {
     char *nodename;
-    hwaddr base = VIRT_UART_BASE;
     hwaddr size = VIRT_UART_SIZE;
     MachineState *ms = MACHINE(lvms);
 
@@ -331,9 +331,9 @@ static void fdt_add_uart_node(LoongArchVirtMachineState *lvms,
     qemu_fdt_setprop_string(ms->fdt, nodename, "compatible", "ns16550a");
     qemu_fdt_setprop_cells(ms->fdt, nodename, "reg", 0x0, base, 0x0, size);
     qemu_fdt_setprop_cell(ms->fdt, nodename, "clock-frequency", 100000000);
-    qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
-    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
-                           VIRT_UART_IRQ - VIRT_GSI_BASE, 0x4);
+    if (chosen)
+        qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
+    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts", irq, 0x4);
     qemu_fdt_setprop_cell(ms->fdt, nodename, "interrupt-parent",
                           *pch_pic_phandle);
     g_free(nodename);
@@ -750,11 +750,15 @@ static void virt_devices_init(DeviceState *pch_pic,
     /* Add pcie node */
     fdt_add_pcie_node(lvms, pch_pic_phandle, pch_msi_phandle);
 
-    serial_mm_init(get_system_memory(), VIRT_UART_BASE, 0,
-                   qdev_get_gpio_in(pch_pic,
-                                    VIRT_UART_IRQ - VIRT_GSI_BASE),
-                   115200, serial_hd(0), DEVICE_LITTLE_ENDIAN);
-    fdt_add_uart_node(lvms, pch_pic_phandle);
+    for (i = 0; i < VIRT_UART_COUNT; ++i) {
+        hwaddr base = VIRT_UART_BASE + i * VIRT_UART_SIZE;
+        int irq = VIRT_UART_IRQ + i - VIRT_GSI_BASE;
+        serial_mm_init(get_system_memory(), base, 0,
+                       qdev_get_gpio_in(pch_pic, irq),
+                       115200, serial_hd(VIRT_UART_COUNT - 1 - i),
+                       DEVICE_LITTLE_ENDIAN);
+        fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == VIRT_UART_COUNT - 1 - i);
+    }
 
     /* Network init */
     pci_init_nic_devices(pci_bus, mc->default_nic);
diff --git a/include/hw/pci-host/ls7a.h b/include/hw/pci-host/ls7a.h
index cd7c9ec7bc..79d4ea8501 100644
--- a/include/hw/pci-host/ls7a.h
+++ b/include/hw/pci-host/ls7a.h
@@ -36,17 +36,18 @@
 #define VIRT_PCH_PIC_IRQ_NUM     32
 #define VIRT_GSI_BASE            64
 #define VIRT_DEVICE_IRQS         16
+#define VIRT_UART_COUNT          4
 #define VIRT_UART_IRQ            (VIRT_GSI_BASE + 2)
 #define VIRT_UART_BASE           0x1fe001e0
-#define VIRT_UART_SIZE           0X100
-#define VIRT_RTC_IRQ             (VIRT_GSI_BASE + 3)
+#define VIRT_UART_SIZE           0x100
+#define VIRT_RTC_IRQ             (VIRT_GSI_BASE + 6)
 #define VIRT_MISC_REG_BASE       (VIRT_PCH_REG_BASE + 0x00080000)
 #define VIRT_RTC_REG_BASE        (VIRT_MISC_REG_BASE + 0x00050100)
 #define VIRT_RTC_LEN             0x100
-#define VIRT_SCI_IRQ             (VIRT_GSI_BASE + 4)
+#define VIRT_SCI_IRQ             (VIRT_GSI_BASE + 7)
 
 #define VIRT_PLATFORM_BUS_BASEADDRESS   0x16000000
 #define VIRT_PLATFORM_BUS_SIZE          0x2000000
 #define VIRT_PLATFORM_BUS_NUM_IRQS      2
-#define VIRT_PLATFORM_BUS_IRQ           (VIRT_GSI_BASE + 5)
+#define VIRT_PLATFORM_BUS_IRQ           (VIRT_GSI_BASE + 8)
 #endif
-- 
2.46.0

Re: [PATCH v2] hw/loongarch: virt: support up to 4 serial ports

[maobibo]

Hi Jason,

It works well with ELF kernel, however it fails to boot with UEFI BIOS. 
Maybe it is problem of UEFI BIOS, can we create UART in reverse order? 
so that it can work well on both ELF kernel and UEFI BIOS.

Also for develops they usually use as earlycon with command line 
-serial stdio --append "... earlycon=uart,mmio,0x1fe001e0", this 
requires to uart with address 0x1fe001e0 as the first serial port.

Just small code base your patch like this:

-    for (i = 0; i < VIRT_UART_COUNT; ++i) {
+    for (i = VIRT_UART_COUNT - 1; i >= 0; i--) {
          hwaddr base = VIRT_UART_BASE + i * VIRT_UART_SIZE;
          int irq = VIRT_UART_IRQ + i - VIRT_GSI_BASE;
          serial_mm_init(get_system_memory(), base, 0,
                         qdev_get_gpio_in(pch_pic, irq),
-                       115200, serial_hd(VIRT_UART_COUNT - 1 - i),
+                       115200, serial_hd(i),
                         DEVICE_LITTLE_ENDIAN);
-        fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == 
VIRT_UART_COUNT - 1 - i);
+        fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == 0);
      }

Regards
Bibo Mao

On 2024/9/6 下午10:31, Jason A. Donenfeld wrote:
> In order to support additional channels of communication using
> `-serial`, add several serial ports, up to the standard 4 generally
> supported by the 8250 driver.
> 
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
> As I don't use ACPI, I haven't tested the ACPI part of this, which
> Maobibo wrote.
> 
>   hw/loongarch/acpi-build.c  | 23 +++++++++++++++--------
>   hw/loongarch/virt.c        | 24 ++++++++++++++----------
>   include/hw/pci-host/ls7a.h |  9 +++++----
>   3 files changed, 34 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
> index 2638f87434..2750c6e858 100644
> --- a/hw/loongarch/acpi-build.c
> +++ b/hw/loongarch/acpi-build.c
> @@ -31,6 +31,7 @@
>   
>   #include "hw/acpi/generic_event_device.h"
>   #include "hw/pci-host/gpex.h"
> +#include "sysemu/sysemu.h"
>   #include "sysemu/tpm.h"
>   #include "hw/platform-bus.h"
>   #include "hw/acpi/aml-build.h"
> @@ -252,23 +253,27 @@ struct AcpiBuildState {
>       MemoryRegion *linker_mr;
>   } AcpiBuildState;
>   
> -static void build_uart_device_aml(Aml *table)
> +static void build_uart_device_aml(Aml *table, int index)
>   {
>       Aml *dev;
>       Aml *crs;
>       Aml *pkg0, *pkg1, *pkg2;
> -    uint32_t uart_irq = VIRT_UART_IRQ;
> -
> -    Aml *scope = aml_scope("_SB");
> -    dev = aml_device("COMA");
> +    Aml *scope;
> +    uint32_t uart_irq;
> +    uint64_t base;
> +
> +    uart_irq = VIRT_UART_IRQ + index;
> +    base = VIRT_UART_BASE + index * VIRT_UART_SIZE;
> +    scope = aml_scope("_SB");
> +    dev = aml_device("COM%d", VIRT_UART_COUNT - 1 - index);
>       aml_append(dev, aml_name_decl("_HID", aml_string("PNP0501")));
> -    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> +    aml_append(dev, aml_name_decl("_UID", aml_int(VIRT_UART_COUNT - 1 - index)));
>       aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
>       crs = aml_resource_template();
>       aml_append(crs,
>           aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
>                            AML_NON_CACHEABLE, AML_READ_WRITE,
> -                         0, VIRT_UART_BASE, VIRT_UART_BASE + VIRT_UART_SIZE - 1,
> +                         0, base, base + VIRT_UART_SIZE - 1,
>                            0, VIRT_UART_SIZE));
>       aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
>                                     AML_SHARED, &uart_irq, 1));
> @@ -401,6 +406,7 @@ static void acpi_dsdt_add_tpm(Aml *scope, LoongArchVirtMachineState *vms)
>   static void
>   build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>   {
> +    int i;
>       Aml *dsdt, *scope, *pkg;
>       LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(machine);
>       AcpiTable table = { .sig = "DSDT", .rev = 1, .oem_id = lvms->oem_id,
> @@ -408,7 +414,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>   
>       acpi_table_begin(&table, table_data);
>       dsdt = init_aml_allocator();
> -    build_uart_device_aml(dsdt);
> +    for (i = 0; i < VIRT_UART_COUNT; ++i)
> +        build_uart_device_aml(dsdt, i);
>       build_pci_device_aml(dsdt, lvms);
>       build_la_ged_aml(dsdt, machine);
>       build_flash_aml(dsdt, lvms);
> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> index 4151fc5e0c..6e8608874c 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -319,10 +319,10 @@ static void fdt_add_ged_reset(LoongArchVirtMachineState *lvms)
>   }
>   
>   static void fdt_add_uart_node(LoongArchVirtMachineState *lvms,
> -                              uint32_t *pch_pic_phandle)
> +                              uint32_t *pch_pic_phandle, hwaddr base,
> +                              int irq, bool chosen)
>   {
>       char *nodename;
> -    hwaddr base = VIRT_UART_BASE;
>       hwaddr size = VIRT_UART_SIZE;
>       MachineState *ms = MACHINE(lvms);
>   
> @@ -331,9 +331,9 @@ static void fdt_add_uart_node(LoongArchVirtMachineState *lvms,
>       qemu_fdt_setprop_string(ms->fdt, nodename, "compatible", "ns16550a");
>       qemu_fdt_setprop_cells(ms->fdt, nodename, "reg", 0x0, base, 0x0, size);
>       qemu_fdt_setprop_cell(ms->fdt, nodename, "clock-frequency", 100000000);
> -    qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
> -    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
> -                           VIRT_UART_IRQ - VIRT_GSI_BASE, 0x4);
> +    if (chosen)
> +        qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
> +    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts", irq, 0x4);
>       qemu_fdt_setprop_cell(ms->fdt, nodename, "interrupt-parent",
>                             *pch_pic_phandle);
>       g_free(nodename);
> @@ -750,11 +750,15 @@ static void virt_devices_init(DeviceState *pch_pic,
>       /* Add pcie node */
>       fdt_add_pcie_node(lvms, pch_pic_phandle, pch_msi_phandle);
>   
> -    serial_mm_init(get_system_memory(), VIRT_UART_BASE, 0,
> -                   qdev_get_gpio_in(pch_pic,
> -                                    VIRT_UART_IRQ - VIRT_GSI_BASE),
> -                   115200, serial_hd(0), DEVICE_LITTLE_ENDIAN);
> -    fdt_add_uart_node(lvms, pch_pic_phandle);
> +    for (i = 0; i < VIRT_UART_COUNT; ++i) {
> +        hwaddr base = VIRT_UART_BASE + i * VIRT_UART_SIZE;
> +        int irq = VIRT_UART_IRQ + i - VIRT_GSI_BASE;
> +        serial_mm_init(get_system_memory(), base, 0,
> +                       qdev_get_gpio_in(pch_pic, irq),
> +                       115200, serial_hd(VIRT_UART_COUNT - 1 - i),
> +                       DEVICE_LITTLE_ENDIAN);
> +        fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == VIRT_UART_COUNT - 1 - i);
> +    }
>   
>       /* Network init */
>       pci_init_nic_devices(pci_bus, mc->default_nic);
> diff --git a/include/hw/pci-host/ls7a.h b/include/hw/pci-host/ls7a.h
> index cd7c9ec7bc..79d4ea8501 100644
> --- a/include/hw/pci-host/ls7a.h
> +++ b/include/hw/pci-host/ls7a.h
> @@ -36,17 +36,18 @@
>   #define VIRT_PCH_PIC_IRQ_NUM     32
>   #define VIRT_GSI_BASE            64
>   #define VIRT_DEVICE_IRQS         16
> +#define VIRT_UART_COUNT          4
>   #define VIRT_UART_IRQ            (VIRT_GSI_BASE + 2)
>   #define VIRT_UART_BASE           0x1fe001e0
> -#define VIRT_UART_SIZE           0X100
> -#define VIRT_RTC_IRQ             (VIRT_GSI_BASE + 3)
> +#define VIRT_UART_SIZE           0x100
> +#define VIRT_RTC_IRQ             (VIRT_GSI_BASE + 6)
>   #define VIRT_MISC_REG_BASE       (VIRT_PCH_REG_BASE + 0x00080000)
>   #define VIRT_RTC_REG_BASE        (VIRT_MISC_REG_BASE + 0x00050100)
>   #define VIRT_RTC_LEN             0x100
> -#define VIRT_SCI_IRQ             (VIRT_GSI_BASE + 4)
> +#define VIRT_SCI_IRQ             (VIRT_GSI_BASE + 7)
>   
>   #define VIRT_PLATFORM_BUS_BASEADDRESS   0x16000000
>   #define VIRT_PLATFORM_BUS_SIZE          0x2000000
>   #define VIRT_PLATFORM_BUS_NUM_IRQS      2
> -#define VIRT_PLATFORM_BUS_IRQ           (VIRT_GSI_BASE + 5)
> +#define VIRT_PLATFORM_BUS_IRQ           (VIRT_GSI_BASE + 8)
>   #endif
> 

[PATCH v3] hw/loongarch: virt: support up to 4 serial ports

[Jason A. Donenfeld]

In order to support additional channels of communication using
`-serial`, add several serial ports, up to the standard 4 generally
supported by the 8250 driver.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 hw/loongarch/acpi-build.c  | 23 +++++++++++++++--------
 hw/loongarch/virt.c        | 23 +++++++++++++----------
 include/hw/pci-host/ls7a.h |  9 +++++----
 3 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
index 3912c8d307..459d2b5f84 100644
--- a/hw/loongarch/acpi-build.c
+++ b/hw/loongarch/acpi-build.c
@@ -31,6 +31,7 @@
 
 #include "hw/acpi/generic_event_device.h"
 #include "hw/pci-host/gpex.h"
+#include "sysemu/sysemu.h"
 #include "sysemu/tpm.h"
 #include "hw/platform-bus.h"
 #include "hw/acpi/aml-build.h"
@@ -290,23 +291,27 @@ struct AcpiBuildState {
     MemoryRegion *linker_mr;
 } AcpiBuildState;
 
-static void build_uart_device_aml(Aml *table)
+static void build_uart_device_aml(Aml *table, int index)
 {
     Aml *dev;
     Aml *crs;
     Aml *pkg0, *pkg1, *pkg2;
-    uint32_t uart_irq = VIRT_UART_IRQ;
-
-    Aml *scope = aml_scope("_SB");
-    dev = aml_device("COMA");
+    Aml *scope;
+    uint32_t uart_irq;
+    uint64_t base;
+
+    uart_irq = VIRT_UART_IRQ + index;
+    base = VIRT_UART_BASE + index * VIRT_UART_SIZE;
+    scope = aml_scope("_SB");
+    dev = aml_device("COM%d", index);
     aml_append(dev, aml_name_decl("_HID", aml_string("PNP0501")));
-    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+    aml_append(dev, aml_name_decl("_UID", aml_int(index)));
     aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
     crs = aml_resource_template();
     aml_append(crs,
         aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
                          AML_NON_CACHEABLE, AML_READ_WRITE,
-                         0, VIRT_UART_BASE, VIRT_UART_BASE + VIRT_UART_SIZE - 1,
+                         0, base, base + VIRT_UART_SIZE - 1,
                          0, VIRT_UART_SIZE));
     aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
                                   AML_SHARED, &uart_irq, 1));
@@ -439,6 +444,7 @@ static void acpi_dsdt_add_tpm(Aml *scope, LoongArchVirtMachineState *vms)
 static void
 build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine)
 {
+    int i;
     Aml *dsdt, *scope, *pkg;
     LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(machine);
     AcpiTable table = { .sig = "DSDT", .rev = 1, .oem_id = lvms->oem_id,
@@ -446,7 +452,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine)
 
     acpi_table_begin(&table, table_data);
     dsdt = init_aml_allocator();
-    build_uart_device_aml(dsdt);
+    for (i = VIRT_UART_COUNT; i --> 0;)
+        build_uart_device_aml(dsdt, i);
     build_pci_device_aml(dsdt, lvms);
     build_la_ged_aml(dsdt, machine);
     build_flash_aml(dsdt, lvms);
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index 4151fc5e0c..b9bd88d3f4 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -319,10 +319,10 @@ static void fdt_add_ged_reset(LoongArchVirtMachineState *lvms)
 }
 
 static void fdt_add_uart_node(LoongArchVirtMachineState *lvms,
-                              uint32_t *pch_pic_phandle)
+                              uint32_t *pch_pic_phandle, hwaddr base,
+                              int irq, bool chosen)
 {
     char *nodename;
-    hwaddr base = VIRT_UART_BASE;
     hwaddr size = VIRT_UART_SIZE;
     MachineState *ms = MACHINE(lvms);
 
@@ -331,9 +331,9 @@ static void fdt_add_uart_node(LoongArchVirtMachineState *lvms,
     qemu_fdt_setprop_string(ms->fdt, nodename, "compatible", "ns16550a");
     qemu_fdt_setprop_cells(ms->fdt, nodename, "reg", 0x0, base, 0x0, size);
     qemu_fdt_setprop_cell(ms->fdt, nodename, "clock-frequency", 100000000);
-    qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
-    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
-                           VIRT_UART_IRQ - VIRT_GSI_BASE, 0x4);
+    if (chosen)
+        qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
+    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts", irq, 0x4);
     qemu_fdt_setprop_cell(ms->fdt, nodename, "interrupt-parent",
                           *pch_pic_phandle);
     g_free(nodename);
@@ -750,11 +750,14 @@ static void virt_devices_init(DeviceState *pch_pic,
     /* Add pcie node */
     fdt_add_pcie_node(lvms, pch_pic_phandle, pch_msi_phandle);
 
-    serial_mm_init(get_system_memory(), VIRT_UART_BASE, 0,
-                   qdev_get_gpio_in(pch_pic,
-                                    VIRT_UART_IRQ - VIRT_GSI_BASE),
-                   115200, serial_hd(0), DEVICE_LITTLE_ENDIAN);
-    fdt_add_uart_node(lvms, pch_pic_phandle);
+    for (i = VIRT_UART_COUNT; i --> 0;) {
+        hwaddr base = VIRT_UART_BASE + i * VIRT_UART_SIZE;
+        int irq = VIRT_UART_IRQ + i - VIRT_GSI_BASE;
+        serial_mm_init(get_system_memory(), base, 0,
+                       qdev_get_gpio_in(pch_pic, irq),
+                       115200, serial_hd(i), DEVICE_LITTLE_ENDIAN);
+        fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == 0);
+    }
 
     /* Network init */
     pci_init_nic_devices(pci_bus, mc->default_nic);
diff --git a/include/hw/pci-host/ls7a.h b/include/hw/pci-host/ls7a.h
index cd7c9ec7bc..79d4ea8501 100644
--- a/include/hw/pci-host/ls7a.h
+++ b/include/hw/pci-host/ls7a.h
@@ -36,17 +36,18 @@
 #define VIRT_PCH_PIC_IRQ_NUM     32
 #define VIRT_GSI_BASE            64
 #define VIRT_DEVICE_IRQS         16
+#define VIRT_UART_COUNT          4
 #define VIRT_UART_IRQ            (VIRT_GSI_BASE + 2)
 #define VIRT_UART_BASE           0x1fe001e0
-#define VIRT_UART_SIZE           0X100
-#define VIRT_RTC_IRQ             (VIRT_GSI_BASE + 3)
+#define VIRT_UART_SIZE           0x100
+#define VIRT_RTC_IRQ             (VIRT_GSI_BASE + 6)
 #define VIRT_MISC_REG_BASE       (VIRT_PCH_REG_BASE + 0x00080000)
 #define VIRT_RTC_REG_BASE        (VIRT_MISC_REG_BASE + 0x00050100)
 #define VIRT_RTC_LEN             0x100
-#define VIRT_SCI_IRQ             (VIRT_GSI_BASE + 4)
+#define VIRT_SCI_IRQ             (VIRT_GSI_BASE + 7)
 
 #define VIRT_PLATFORM_BUS_BASEADDRESS   0x16000000
 #define VIRT_PLATFORM_BUS_SIZE          0x2000000
 #define VIRT_PLATFORM_BUS_NUM_IRQS      2
-#define VIRT_PLATFORM_BUS_IRQ           (VIRT_GSI_BASE + 5)
+#define VIRT_PLATFORM_BUS_IRQ           (VIRT_GSI_BASE + 8)
 #endif
-- 
2.46.0

Re: [PATCH v2] hw/loongarch: virt: support up to 4 serial ports

[Jason A. Donenfeld]

On Sat, Sep 07, 2024 at 11:37:09AM +0800, maobibo wrote:
> Hi Jason,
> 
> It works well with ELF kernel, however it fails to boot with UEFI BIOS. 
> Maybe it is problem of UEFI BIOS, can we create UART in reverse order? 
> so that it can work well on both ELF kernel and UEFI BIOS.
> 
> Also for develops they usually use as earlycon with command line 
> -serial stdio --append "... earlycon=uart,mmio,0x1fe001e0", this 
> requires to uart with address 0x1fe001e0 as the first serial port.
> 
> Just small code base your patch like this:
> 
> -    for (i = 0; i < VIRT_UART_COUNT; ++i) {
> +    for (i = VIRT_UART_COUNT - 1; i >= 0; i--) {
>           hwaddr base = VIRT_UART_BASE + i * VIRT_UART_SIZE;
>           int irq = VIRT_UART_IRQ + i - VIRT_GSI_BASE;
>           serial_mm_init(get_system_memory(), base, 0,
>                          qdev_get_gpio_in(pch_pic, irq),
> -                       115200, serial_hd(VIRT_UART_COUNT - 1 - i),
> +                       115200, serial_hd(i),
>                          DEVICE_LITTLE_ENDIAN);
> -        fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == 
> VIRT_UART_COUNT - 1 - i);
> +        fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == 0);
>       }

Fixed: https://lore.kernel.org/all/20240907143439.2792924-1-Jason@zx2c4.com/

Re: [PATCH v2] hw/loongarch: virt: support up to 4 serial ports

[Jason A. Donenfeld]

On Sat, Sep 07, 2024 at 04:44:45PM +0200, Jason A. Donenfeld wrote:
> On Sat, Sep 07, 2024 at 11:37:09AM +0800, maobibo wrote:
> > Hi Jason,
> > 
> > It works well with ELF kernel, however it fails to boot with UEFI BIOS. 
> > Maybe it is problem of UEFI BIOS, can we create UART in reverse order? 
> > so that it can work well on both ELF kernel and UEFI BIOS.
> > 
> > Also for develops they usually use as earlycon with command line 
> > -serial stdio --append "... earlycon=uart,mmio,0x1fe001e0", this 
> > requires to uart with address 0x1fe001e0 as the first serial port.
> > 
> > Just small code base your patch like this:
> > 
> > -    for (i = 0; i < VIRT_UART_COUNT; ++i) {
> > +    for (i = VIRT_UART_COUNT - 1; i >= 0; i--) {
> >           hwaddr base = VIRT_UART_BASE + i * VIRT_UART_SIZE;
> >           int irq = VIRT_UART_IRQ + i - VIRT_GSI_BASE;
> >           serial_mm_init(get_system_memory(), base, 0,
> >                          qdev_get_gpio_in(pch_pic, irq),
> > -                       115200, serial_hd(VIRT_UART_COUNT - 1 - i),
> > +                       115200, serial_hd(i),
> >                          DEVICE_LITTLE_ENDIAN);
> > -        fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == 
> > VIRT_UART_COUNT - 1 - i);
> > +        fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == 0);
> >       }
> 
> Fixed: https://lore.kernel.org/all/20240907143439.2792924-1-Jason@zx2c4.com/

By the way, I don't know who is picking patches into which development
tree from the list, but this is starting to be kind of a lot of patches,
so I've queued them all up here:

   https://git.zx2c4.com/qemu/log/?h=loongarch64

In case that makes it easier for whoever the maintainer is to pick them.

Re: [PATCH v3] hw/loongarch: virt: support up to 4 serial ports

[maobibo]

On 2024/9/7 下午10:34, Jason A. Donenfeld wrote:
> In order to support additional channels of communication using
> `-serial`, add several serial ports, up to the standard 4 generally
> supported by the 8250 driver.
> 
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>   hw/loongarch/acpi-build.c  | 23 +++++++++++++++--------
>   hw/loongarch/virt.c        | 23 +++++++++++++----------
>   include/hw/pci-host/ls7a.h |  9 +++++----
>   3 files changed, 33 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
> index 3912c8d307..459d2b5f84 100644
> --- a/hw/loongarch/acpi-build.c
> +++ b/hw/loongarch/acpi-build.c
> @@ -31,6 +31,7 @@
>   
>   #include "hw/acpi/generic_event_device.h"
>   #include "hw/pci-host/gpex.h"
> +#include "sysemu/sysemu.h"
>   #include "sysemu/tpm.h"
>   #include "hw/platform-bus.h"
>   #include "hw/acpi/aml-build.h"
> @@ -290,23 +291,27 @@ struct AcpiBuildState {
>       MemoryRegion *linker_mr;
>   } AcpiBuildState;
>   
> -static void build_uart_device_aml(Aml *table)
> +static void build_uart_device_aml(Aml *table, int index)
>   {
>       Aml *dev;
>       Aml *crs;
>       Aml *pkg0, *pkg1, *pkg2;
> -    uint32_t uart_irq = VIRT_UART_IRQ;
> -
> -    Aml *scope = aml_scope("_SB");
> -    dev = aml_device("COMA");
> +    Aml *scope;
> +    uint32_t uart_irq;
> +    uint64_t base;
> +
> +    uart_irq = VIRT_UART_IRQ + index;
> +    base = VIRT_UART_BASE + index * VIRT_UART_SIZE;
> +    scope = aml_scope("_SB");
> +    dev = aml_device("COM%d", index);
>       aml_append(dev, aml_name_decl("_HID", aml_string("PNP0501")));
> -    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> +    aml_append(dev, aml_name_decl("_UID", aml_int(index)));
>       aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
>       crs = aml_resource_template();
>       aml_append(crs,
>           aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
>                            AML_NON_CACHEABLE, AML_READ_WRITE,
> -                         0, VIRT_UART_BASE, VIRT_UART_BASE + VIRT_UART_SIZE - 1,
> +                         0, base, base + VIRT_UART_SIZE - 1,
>                            0, VIRT_UART_SIZE));
>       aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
>                                     AML_SHARED, &uart_irq, 1));
> @@ -439,6 +444,7 @@ static void acpi_dsdt_add_tpm(Aml *scope, LoongArchVirtMachineState *vms)
>   static void
>   build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>   {
> +    int i;
>       Aml *dsdt, *scope, *pkg;
>       LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(machine);
>       AcpiTable table = { .sig = "DSDT", .rev = 1, .oem_id = lvms->oem_id,
> @@ -446,7 +452,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>   
>       acpi_table_begin(&table, table_data);
>       dsdt = init_aml_allocator();
> -    build_uart_device_aml(dsdt);
> +    for (i = VIRT_UART_COUNT; i --> 0;)
> +        build_uart_device_aml(dsdt, i);
>       build_pci_device_aml(dsdt, lvms);
>       build_la_ged_aml(dsdt, machine);
>       build_flash_aml(dsdt, lvms);
> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> index 4151fc5e0c..b9bd88d3f4 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -319,10 +319,10 @@ static void fdt_add_ged_reset(LoongArchVirtMachineState *lvms)
>   }
>   
>   static void fdt_add_uart_node(LoongArchVirtMachineState *lvms,
> -                              uint32_t *pch_pic_phandle)
> +                              uint32_t *pch_pic_phandle, hwaddr base,
> +                              int irq, bool chosen)
>   {
>       char *nodename;
> -    hwaddr base = VIRT_UART_BASE;
>       hwaddr size = VIRT_UART_SIZE;
>       MachineState *ms = MACHINE(lvms);
>   
> @@ -331,9 +331,9 @@ static void fdt_add_uart_node(LoongArchVirtMachineState *lvms,
>       qemu_fdt_setprop_string(ms->fdt, nodename, "compatible", "ns16550a");
>       qemu_fdt_setprop_cells(ms->fdt, nodename, "reg", 0x0, base, 0x0, size);
>       qemu_fdt_setprop_cell(ms->fdt, nodename, "clock-frequency", 100000000);
> -    qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
> -    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
> -                           VIRT_UART_IRQ - VIRT_GSI_BASE, 0x4);
> +    if (chosen)
> +        qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
> +    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts", irq, 0x4);
>       qemu_fdt_setprop_cell(ms->fdt, nodename, "interrupt-parent",
>                             *pch_pic_phandle);
>       g_free(nodename);
> @@ -750,11 +750,14 @@ static void virt_devices_init(DeviceState *pch_pic,
>       /* Add pcie node */
>       fdt_add_pcie_node(lvms, pch_pic_phandle, pch_msi_phandle);
>   
> -    serial_mm_init(get_system_memory(), VIRT_UART_BASE, 0,
> -                   qdev_get_gpio_in(pch_pic,
> -                                    VIRT_UART_IRQ - VIRT_GSI_BASE),
> -                   115200, serial_hd(0), DEVICE_LITTLE_ENDIAN);
> -    fdt_add_uart_node(lvms, pch_pic_phandle);
> +    for (i = VIRT_UART_COUNT; i --> 0;) {
> +        hwaddr base = VIRT_UART_BASE + i * VIRT_UART_SIZE;
> +        int irq = VIRT_UART_IRQ + i - VIRT_GSI_BASE;
> +        serial_mm_init(get_system_memory(), base, 0,
> +                       qdev_get_gpio_in(pch_pic, irq),
> +                       115200, serial_hd(i), DEVICE_LITTLE_ENDIAN);
> +        fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == 0);
> +    }
>   
>       /* Network init */
>       pci_init_nic_devices(pci_bus, mc->default_nic);
> diff --git a/include/hw/pci-host/ls7a.h b/include/hw/pci-host/ls7a.h
> index cd7c9ec7bc..79d4ea8501 100644
> --- a/include/hw/pci-host/ls7a.h
> +++ b/include/hw/pci-host/ls7a.h
> @@ -36,17 +36,18 @@
>   #define VIRT_PCH_PIC_IRQ_NUM     32
>   #define VIRT_GSI_BASE            64
>   #define VIRT_DEVICE_IRQS         16
> +#define VIRT_UART_COUNT          4
>   #define VIRT_UART_IRQ            (VIRT_GSI_BASE + 2)
>   #define VIRT_UART_BASE           0x1fe001e0
> -#define VIRT_UART_SIZE           0X100
> -#define VIRT_RTC_IRQ             (VIRT_GSI_BASE + 3)
> +#define VIRT_UART_SIZE           0x100
> +#define VIRT_RTC_IRQ             (VIRT_GSI_BASE + 6)
>   #define VIRT_MISC_REG_BASE       (VIRT_PCH_REG_BASE + 0x00080000)
>   #define VIRT_RTC_REG_BASE        (VIRT_MISC_REG_BASE + 0x00050100)
>   #define VIRT_RTC_LEN             0x100
> -#define VIRT_SCI_IRQ             (VIRT_GSI_BASE + 4)
> +#define VIRT_SCI_IRQ             (VIRT_GSI_BASE + 7)
>   
>   #define VIRT_PLATFORM_BUS_BASEADDRESS   0x16000000
>   #define VIRT_PLATFORM_BUS_SIZE          0x2000000
>   #define VIRT_PLATFORM_BUS_NUM_IRQS      2
> -#define VIRT_PLATFORM_BUS_IRQ           (VIRT_GSI_BASE + 5)
> +#define VIRT_PLATFORM_BUS_IRQ           (VIRT_GSI_BASE + 8)
>   #endif
> 
Tested-by: Bibo Mao <maobibo@loongson.cn>