* [PATCH v2 kvmtool] riscv: Move serial and rtc from IO port space to MMIO area.
@ 2023-02-01 16:01 Rajnesh Kanwal
2023-02-01 16:35 ` Andre Przywara
0 siblings, 1 reply; 6+ messages in thread
From: Rajnesh Kanwal @ 2023-02-01 16:01 UTC (permalink / raw)
To: apatel, atishp, andre.przywara, alexandru.elisei, kvm; +Cc: Rajnesh Kanwal
The default serial and rtc IO region overlaps with PCI IO bar
region leading bar 0 activation to fail. Moving these devices
to MMIO region similar to ARM.
Given serial has been moved from 0x3f8 to 0x10000000, this
requires us to now pass earlycon=uart8250,mmio,0x10000000
from cmdline rather than earlycon=uart8250,mmio,0x3f8.
Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
---
v2: Added further details in the commit message regarding the
UART address change required in kernel cmdline parameter.
v1: https://www.spinics.net/lists/kvm/msg301835.html
hw/rtc.c | 3 +++
hw/serial.c | 4 ++++
riscv/include/kvm/kvm-arch.h | 10 ++++++++--
3 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/hw/rtc.c b/hw/rtc.c
index 9b8785a..da696e1 100644
--- a/hw/rtc.c
+++ b/hw/rtc.c
@@ -9,6 +9,9 @@
#if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
#define RTC_BUS_TYPE DEVICE_BUS_MMIO
#define RTC_BASE_ADDRESS ARM_RTC_MMIO_BASE
+#elif defined(CONFIG_RISCV)
+#define RTC_BUS_TYPE DEVICE_BUS_MMIO
+#define RTC_BASE_ADDRESS RISCV_RTC_MMIO_BASE
#else
/* PORT 0070-007F - CMOS RAM/RTC (REAL TIME CLOCK) */
#define RTC_BUS_TYPE DEVICE_BUS_IOPORT
diff --git a/hw/serial.c b/hw/serial.c
index 3d53362..b6263a0 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -17,6 +17,10 @@
#define serial_iobase(nr) (ARM_UART_MMIO_BASE + (nr) * 0x1000)
#define serial_irq(nr) (32 + (nr))
#define SERIAL8250_BUS_TYPE DEVICE_BUS_MMIO
+#elif defined(CONFIG_RISCV)
+#define serial_iobase(nr) (RISCV_UART_MMIO_BASE + (nr) * 0x1000)
+#define serial_irq(nr) (1 + (nr))
+#define SERIAL8250_BUS_TYPE DEVICE_BUS_MMIO
#else
#define serial_iobase_0 (KVM_IOPORT_AREA + 0x3f8)
#define serial_iobase_1 (KVM_IOPORT_AREA + 0x2f8)
diff --git a/riscv/include/kvm/kvm-arch.h b/riscv/include/kvm/kvm-arch.h
index 3f96d00..620c796 100644
--- a/riscv/include/kvm/kvm-arch.h
+++ b/riscv/include/kvm/kvm-arch.h
@@ -11,7 +11,7 @@
#define RISCV_IOPORT 0x00000000ULL
#define RISCV_IOPORT_SIZE SZ_64K
#define RISCV_IRQCHIP 0x08000000ULL
-#define RISCV_IRQCHIP_SIZE SZ_128M
+#define RISCV_IRQCHIP_SIZE SZ_128M
#define RISCV_MMIO 0x10000000ULL
#define RISCV_MMIO_SIZE SZ_512M
#define RISCV_PCI 0x30000000ULL
@@ -35,10 +35,16 @@
#define RISCV_MAX_MEMORY(kvm) RISCV_LOMAP_MAX_MEMORY
#endif
+#define RISCV_UART_MMIO_BASE RISCV_MMIO
+#define RISCV_UART_MMIO_SIZE 0x10000
+
+#define RISCV_RTC_MMIO_BASE (RISCV_UART_MMIO_BASE + RISCV_UART_MMIO_SIZE)
+#define RISCV_RTC_MMIO_SIZE 0x10000
+
#define KVM_IOPORT_AREA RISCV_IOPORT
#define KVM_PCI_CFG_AREA RISCV_PCI
#define KVM_PCI_MMIO_AREA (KVM_PCI_CFG_AREA + RISCV_PCI_CFG_SIZE)
-#define KVM_VIRTIO_MMIO_AREA RISCV_MMIO
+#define KVM_VIRTIO_MMIO_AREA (RISCV_RTC_MMIO_BASE + RISCV_UART_MMIO_SIZE)
#define KVM_IOEVENTFD_HAS_PIO 0
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 kvmtool] riscv: Move serial and rtc from IO port space to MMIO area.
2023-02-01 16:01 [PATCH v2 kvmtool] riscv: Move serial and rtc from IO port space to MMIO area Rajnesh Kanwal
@ 2023-02-01 16:35 ` Andre Przywara
2023-02-01 17:04 ` Rajnesh Kanwal
0 siblings, 1 reply; 6+ messages in thread
From: Andre Przywara @ 2023-02-01 16:35 UTC (permalink / raw)
To: Rajnesh Kanwal, apatel, atishp; +Cc: alexandru.elisei, kvm, Will Deacon
On Wed, 1 Feb 2023 16:01:37 +0000
Rajnesh Kanwal <rkanwal@rivosinc.com> wrote:
Hi,
> The default serial and rtc IO region overlaps with PCI IO bar
> region leading bar 0 activation to fail. Moving these devices
> to MMIO region similar to ARM.
>
> Given serial has been moved from 0x3f8 to 0x10000000, this
> requires us to now pass earlycon=uart8250,mmio,0x10000000
> from cmdline rather than earlycon=uart8250,mmio,0x3f8.
Doesn't it work either way with just "earlycon"? At least on the ARM side
it then finds the UART type and base address by following the DT's
stdout-path property. This would not only make this more robust, but also
more VMM agnostic.
Also, Atish, Anup: can one of you please provide a Reviewed-by: or
Tested-by: for this patch?
Cheers,
Andre
>
> Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
> ---
> v2: Added further details in the commit message regarding the
> UART address change required in kernel cmdline parameter.
>
> v1: https://www.spinics.net/lists/kvm/msg301835.html
>
> hw/rtc.c | 3 +++
> hw/serial.c | 4 ++++
> riscv/include/kvm/kvm-arch.h | 10 ++++++++--
> 3 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/hw/rtc.c b/hw/rtc.c
> index 9b8785a..da696e1 100644
> --- a/hw/rtc.c
> +++ b/hw/rtc.c
> @@ -9,6 +9,9 @@
> #if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
> #define RTC_BUS_TYPE DEVICE_BUS_MMIO
> #define RTC_BASE_ADDRESS ARM_RTC_MMIO_BASE
> +#elif defined(CONFIG_RISCV)
> +#define RTC_BUS_TYPE DEVICE_BUS_MMIO
> +#define RTC_BASE_ADDRESS RISCV_RTC_MMIO_BASE
> #else
> /* PORT 0070-007F - CMOS RAM/RTC (REAL TIME CLOCK) */
> #define RTC_BUS_TYPE DEVICE_BUS_IOPORT
> diff --git a/hw/serial.c b/hw/serial.c
> index 3d53362..b6263a0 100644
> --- a/hw/serial.c
> +++ b/hw/serial.c
> @@ -17,6 +17,10 @@
> #define serial_iobase(nr) (ARM_UART_MMIO_BASE + (nr) * 0x1000)
> #define serial_irq(nr) (32 + (nr))
> #define SERIAL8250_BUS_TYPE DEVICE_BUS_MMIO
> +#elif defined(CONFIG_RISCV)
> +#define serial_iobase(nr) (RISCV_UART_MMIO_BASE + (nr) * 0x1000)
> +#define serial_irq(nr) (1 + (nr))
> +#define SERIAL8250_BUS_TYPE DEVICE_BUS_MMIO
> #else
> #define serial_iobase_0 (KVM_IOPORT_AREA + 0x3f8)
> #define serial_iobase_1 (KVM_IOPORT_AREA + 0x2f8)
> diff --git a/riscv/include/kvm/kvm-arch.h b/riscv/include/kvm/kvm-arch.h
> index 3f96d00..620c796 100644
> --- a/riscv/include/kvm/kvm-arch.h
> +++ b/riscv/include/kvm/kvm-arch.h
> @@ -11,7 +11,7 @@
> #define RISCV_IOPORT 0x00000000ULL
> #define RISCV_IOPORT_SIZE SZ_64K
> #define RISCV_IRQCHIP 0x08000000ULL
> -#define RISCV_IRQCHIP_SIZE SZ_128M
> +#define RISCV_IRQCHIP_SIZE SZ_128M
> #define RISCV_MMIO 0x10000000ULL
> #define RISCV_MMIO_SIZE SZ_512M
> #define RISCV_PCI 0x30000000ULL
> @@ -35,10 +35,16 @@
> #define RISCV_MAX_MEMORY(kvm) RISCV_LOMAP_MAX_MEMORY
> #endif
>
> +#define RISCV_UART_MMIO_BASE RISCV_MMIO
> +#define RISCV_UART_MMIO_SIZE 0x10000
> +
> +#define RISCV_RTC_MMIO_BASE (RISCV_UART_MMIO_BASE + RISCV_UART_MMIO_SIZE)
> +#define RISCV_RTC_MMIO_SIZE 0x10000
> +
> #define KVM_IOPORT_AREA RISCV_IOPORT
> #define KVM_PCI_CFG_AREA RISCV_PCI
> #define KVM_PCI_MMIO_AREA (KVM_PCI_CFG_AREA + RISCV_PCI_CFG_SIZE)
> -#define KVM_VIRTIO_MMIO_AREA RISCV_MMIO
> +#define KVM_VIRTIO_MMIO_AREA (RISCV_RTC_MMIO_BASE + RISCV_UART_MMIO_SIZE)
>
> #define KVM_IOEVENTFD_HAS_PIO 0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 kvmtool] riscv: Move serial and rtc from IO port space to MMIO area.
2023-02-01 16:35 ` Andre Przywara
@ 2023-02-01 17:04 ` Rajnesh Kanwal
2023-02-01 17:18 ` Alexandru Elisei
0 siblings, 1 reply; 6+ messages in thread
From: Rajnesh Kanwal @ 2023-02-01 17:04 UTC (permalink / raw)
To: Andre Przywara; +Cc: apatel, atishp, alexandru.elisei, kvm, Will Deacon
On Wed, Feb 1, 2023 at 4:35 PM
Andre Przywara <andre.przywara@foss.arm.com> wrote:
>
> On Wed, 1 Feb 2023 16:01:37 +0000
> Rajnesh Kanwal <rkanwal@rivosinc.com> wrote:
>
> Hi,
>
> > The default serial and rtc IO region overlaps with PCI IO bar
> > region leading bar 0 activation to fail. Moving these devices
> > to MMIO region similar to ARM.
> >
> > Given serial has been moved from 0x3f8 to 0x10000000, this
> > requires us to now pass earlycon=uart8250,mmio,0x10000000
> > from cmdline rather than earlycon=uart8250,mmio,0x3f8.
>
> Doesn't it work either way with just "earlycon"? At least on the ARM side
> it then finds the UART type and base address by following the DT's
> stdout-path property. This would not only make this more robust, but also
> more VMM agnostic.
>
Sorry I didn't know that. Thanks for pointing this out. Just tested this and it
works fine with just "earlycon".
$ ./lkvm-static run -c1 --console virtio -p "console=hvc1 earlycon
root=/dev/vda " -k ./Image -d rootfs.ext4
[ 0.000000] earlycon: ns16550a0 at MMIO 0x0000000010000000 (options '')
[ 0.000000] printk: bootconsole [ns16550a0] enabled
I will update the commit message in the next version.
Thanks,
Rajnesh
> Also, Atish, Anup: can one of you please provide a Reviewed-by: or
> Tested-by: for this patch?
>
> Cheers,
> Andre
>
> >
> > Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
> > ---
> > v2: Added further details in the commit message regarding the
> > UART address change required in kernel cmdline parameter.
> >
> > v1: https://www.spinics.net/lists/kvm/msg301835.html
> >
> > hw/rtc.c | 3 +++
> > hw/serial.c | 4 ++++
> > riscv/include/kvm/kvm-arch.h | 10 ++++++++--
> > 3 files changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/rtc.c b/hw/rtc.c
> > index 9b8785a..da696e1 100644
> > --- a/hw/rtc.c
> > +++ b/hw/rtc.c
> > @@ -9,6 +9,9 @@
> > #if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
> > #define RTC_BUS_TYPE DEVICE_BUS_MMIO
> > #define RTC_BASE_ADDRESS ARM_RTC_MMIO_BASE
> > +#elif defined(CONFIG_RISCV)
> > +#define RTC_BUS_TYPE DEVICE_BUS_MMIO
> > +#define RTC_BASE_ADDRESS RISCV_RTC_MMIO_BASE
> > #else
> > /* PORT 0070-007F - CMOS RAM/RTC (REAL TIME CLOCK) */
> > #define RTC_BUS_TYPE DEVICE_BUS_IOPORT
> > diff --git a/hw/serial.c b/hw/serial.c
> > index 3d53362..b6263a0 100644
> > --- a/hw/serial.c
> > +++ b/hw/serial.c
> > @@ -17,6 +17,10 @@
> > #define serial_iobase(nr) (ARM_UART_MMIO_BASE + (nr) * 0x1000)
> > #define serial_irq(nr) (32 + (nr))
> > #define SERIAL8250_BUS_TYPE DEVICE_BUS_MMIO
> > +#elif defined(CONFIG_RISCV)
> > +#define serial_iobase(nr) (RISCV_UART_MMIO_BASE + (nr) * 0x1000)
> > +#define serial_irq(nr) (1 + (nr))
> > +#define SERIAL8250_BUS_TYPE DEVICE_BUS_MMIO
> > #else
> > #define serial_iobase_0 (KVM_IOPORT_AREA + 0x3f8)
> > #define serial_iobase_1 (KVM_IOPORT_AREA + 0x2f8)
> > diff --git a/riscv/include/kvm/kvm-arch.h b/riscv/include/kvm/kvm-arch.h
> > index 3f96d00..620c796 100644
> > --- a/riscv/include/kvm/kvm-arch.h
> > +++ b/riscv/include/kvm/kvm-arch.h
> > @@ -11,7 +11,7 @@
> > #define RISCV_IOPORT 0x00000000ULL
> > #define RISCV_IOPORT_SIZE SZ_64K
> > #define RISCV_IRQCHIP 0x08000000ULL
> > -#define RISCV_IRQCHIP_SIZE SZ_128M
> > +#define RISCV_IRQCHIP_SIZE SZ_128M
> > #define RISCV_MMIO 0x10000000ULL
> > #define RISCV_MMIO_SIZE SZ_512M
> > #define RISCV_PCI 0x30000000ULL
> > @@ -35,10 +35,16 @@
> > #define RISCV_MAX_MEMORY(kvm) RISCV_LOMAP_MAX_MEMORY
> > #endif
> >
> > +#define RISCV_UART_MMIO_BASE RISCV_MMIO
> > +#define RISCV_UART_MMIO_SIZE 0x10000
> > +
> > +#define RISCV_RTC_MMIO_BASE (RISCV_UART_MMIO_BASE + RISCV_UART_MMIO_SIZE)
> > +#define RISCV_RTC_MMIO_SIZE 0x10000
> > +
> > #define KVM_IOPORT_AREA RISCV_IOPORT
> > #define KVM_PCI_CFG_AREA RISCV_PCI
> > #define KVM_PCI_MMIO_AREA (KVM_PCI_CFG_AREA + RISCV_PCI_CFG_SIZE)
> > -#define KVM_VIRTIO_MMIO_AREA RISCV_MMIO
> > +#define KVM_VIRTIO_MMIO_AREA (RISCV_RTC_MMIO_BASE + RISCV_UART_MMIO_SIZE)
> >
> > #define KVM_IOEVENTFD_HAS_PIO 0
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 kvmtool] riscv: Move serial and rtc from IO port space to MMIO area.
2023-02-01 17:04 ` Rajnesh Kanwal
@ 2023-02-01 17:18 ` Alexandru Elisei
2023-02-01 17:28 ` Rajnesh Kanwal
0 siblings, 1 reply; 6+ messages in thread
From: Alexandru Elisei @ 2023-02-01 17:18 UTC (permalink / raw)
To: Rajnesh Kanwal; +Cc: Andre Przywara, apatel, atishp, kvm, Will Deacon
Hi,
On Wed, Feb 01, 2023 at 05:04:36PM +0000, Rajnesh Kanwal wrote:
> On Wed, Feb 1, 2023 at 4:35 PM
> Andre Przywara <andre.przywara@foss.arm.com> wrote:
> >
> > On Wed, 1 Feb 2023 16:01:37 +0000
> > Rajnesh Kanwal <rkanwal@rivosinc.com> wrote:
> >
> > Hi,
> >
> > > The default serial and rtc IO region overlaps with PCI IO bar
> > > region leading bar 0 activation to fail. Moving these devices
> > > to MMIO region similar to ARM.
> > >
> > > Given serial has been moved from 0x3f8 to 0x10000000, this
> > > requires us to now pass earlycon=uart8250,mmio,0x10000000
> > > from cmdline rather than earlycon=uart8250,mmio,0x3f8.
> >
> > Doesn't it work either way with just "earlycon"? At least on the ARM side
> > it then finds the UART type and base address by following the DT's
> > stdout-path property. This would not only make this more robust, but also
> > more VMM agnostic.
It might actually be better to have both ways of specifying the UART using
earlycon in the commit message. Some might find it easier to do git log
hw/serial.c to find the exact parameters than to follow the code and do the
math.
Spearking for myself, the commit message for the coresponding arm change
contains the exact parameters (earlycon=uart,mmio,0x1000000) and that has
been helpful when trying to figure out the address (for example, for
kvm-unit-tests, you can configure the uart address at compile time, and
that provides an earlycon which is usable even before the DTB is parsed).
I think Andre was just trying to be helpful and point out that you don't
need to full parameters to get earlycon working for a Linux guest.
Thanks,
Alex
> >
>
> Sorry I didn't know that. Thanks for pointing this out. Just tested this and it
> works fine with just "earlycon".
>
> $ ./lkvm-static run -c1 --console virtio -p "console=hvc1 earlycon
> root=/dev/vda " -k ./Image -d rootfs.ext4
> [ 0.000000] earlycon: ns16550a0 at MMIO 0x0000000010000000 (options '')
> [ 0.000000] printk: bootconsole [ns16550a0] enabled
>
> I will update the commit message in the next version.
>
> Thanks,
> Rajnesh
>
> > Also, Atish, Anup: can one of you please provide a Reviewed-by: or
> > Tested-by: for this patch?
> >
> > Cheers,
> > Andre
> >
> > >
> > > Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
> > > ---
> > > v2: Added further details in the commit message regarding the
> > > UART address change required in kernel cmdline parameter.
> > >
> > > v1: https://www.spinics.net/lists/kvm/msg301835.html
> > >
> > > hw/rtc.c | 3 +++
> > > hw/serial.c | 4 ++++
> > > riscv/include/kvm/kvm-arch.h | 10 ++++++++--
> > > 3 files changed, 15 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/hw/rtc.c b/hw/rtc.c
> > > index 9b8785a..da696e1 100644
> > > --- a/hw/rtc.c
> > > +++ b/hw/rtc.c
> > > @@ -9,6 +9,9 @@
> > > #if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
> > > #define RTC_BUS_TYPE DEVICE_BUS_MMIO
> > > #define RTC_BASE_ADDRESS ARM_RTC_MMIO_BASE
> > > +#elif defined(CONFIG_RISCV)
> > > +#define RTC_BUS_TYPE DEVICE_BUS_MMIO
> > > +#define RTC_BASE_ADDRESS RISCV_RTC_MMIO_BASE
> > > #else
> > > /* PORT 0070-007F - CMOS RAM/RTC (REAL TIME CLOCK) */
> > > #define RTC_BUS_TYPE DEVICE_BUS_IOPORT
> > > diff --git a/hw/serial.c b/hw/serial.c
> > > index 3d53362..b6263a0 100644
> > > --- a/hw/serial.c
> > > +++ b/hw/serial.c
> > > @@ -17,6 +17,10 @@
> > > #define serial_iobase(nr) (ARM_UART_MMIO_BASE + (nr) * 0x1000)
> > > #define serial_irq(nr) (32 + (nr))
> > > #define SERIAL8250_BUS_TYPE DEVICE_BUS_MMIO
> > > +#elif defined(CONFIG_RISCV)
> > > +#define serial_iobase(nr) (RISCV_UART_MMIO_BASE + (nr) * 0x1000)
> > > +#define serial_irq(nr) (1 + (nr))
> > > +#define SERIAL8250_BUS_TYPE DEVICE_BUS_MMIO
> > > #else
> > > #define serial_iobase_0 (KVM_IOPORT_AREA + 0x3f8)
> > > #define serial_iobase_1 (KVM_IOPORT_AREA + 0x2f8)
> > > diff --git a/riscv/include/kvm/kvm-arch.h b/riscv/include/kvm/kvm-arch.h
> > > index 3f96d00..620c796 100644
> > > --- a/riscv/include/kvm/kvm-arch.h
> > > +++ b/riscv/include/kvm/kvm-arch.h
> > > @@ -11,7 +11,7 @@
> > > #define RISCV_IOPORT 0x00000000ULL
> > > #define RISCV_IOPORT_SIZE SZ_64K
> > > #define RISCV_IRQCHIP 0x08000000ULL
> > > -#define RISCV_IRQCHIP_SIZE SZ_128M
> > > +#define RISCV_IRQCHIP_SIZE SZ_128M
> > > #define RISCV_MMIO 0x10000000ULL
> > > #define RISCV_MMIO_SIZE SZ_512M
> > > #define RISCV_PCI 0x30000000ULL
> > > @@ -35,10 +35,16 @@
> > > #define RISCV_MAX_MEMORY(kvm) RISCV_LOMAP_MAX_MEMORY
> > > #endif
> > >
> > > +#define RISCV_UART_MMIO_BASE RISCV_MMIO
> > > +#define RISCV_UART_MMIO_SIZE 0x10000
> > > +
> > > +#define RISCV_RTC_MMIO_BASE (RISCV_UART_MMIO_BASE + RISCV_UART_MMIO_SIZE)
> > > +#define RISCV_RTC_MMIO_SIZE 0x10000
> > > +
> > > #define KVM_IOPORT_AREA RISCV_IOPORT
> > > #define KVM_PCI_CFG_AREA RISCV_PCI
> > > #define KVM_PCI_MMIO_AREA (KVM_PCI_CFG_AREA + RISCV_PCI_CFG_SIZE)
> > > -#define KVM_VIRTIO_MMIO_AREA RISCV_MMIO
> > > +#define KVM_VIRTIO_MMIO_AREA (RISCV_RTC_MMIO_BASE + RISCV_UART_MMIO_SIZE)
> > >
> > > #define KVM_IOEVENTFD_HAS_PIO 0
> > >
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 kvmtool] riscv: Move serial and rtc from IO port space to MMIO area.
2023-02-01 17:18 ` Alexandru Elisei
@ 2023-02-01 17:28 ` Rajnesh Kanwal
2023-02-02 18:29 ` Atish Kumar Patra
0 siblings, 1 reply; 6+ messages in thread
From: Rajnesh Kanwal @ 2023-02-01 17:28 UTC (permalink / raw)
To: Alexandru Elisei; +Cc: Andre Przywara, apatel, atishp, kvm, Will Deacon
On Wed, Feb 1, 2023 at 5:18 PM Alexandru Elisei
<alexandru.elisei@arm.com> wrote:
>
> Hi,
>
> On Wed, Feb 01, 2023 at 05:04:36PM +0000, Rajnesh Kanwal wrote:
> > On Wed, Feb 1, 2023 at 4:35 PM
> > Andre Przywara <andre.przywara@foss.arm.com> wrote:
> > >
> > > On Wed, 1 Feb 2023 16:01:37 +0000
> > > Rajnesh Kanwal <rkanwal@rivosinc.com> wrote:
> > >
> > > Hi,
> > >
> > > > The default serial and rtc IO region overlaps with PCI IO bar
> > > > region leading bar 0 activation to fail. Moving these devices
> > > > to MMIO region similar to ARM.
> > > >
> > > > Given serial has been moved from 0x3f8 to 0x10000000, this
> > > > requires us to now pass earlycon=uart8250,mmio,0x10000000
> > > > from cmdline rather than earlycon=uart8250,mmio,0x3f8.
> > >
> > > Doesn't it work either way with just "earlycon"? At least on the ARM side
> > > it then finds the UART type and base address by following the DT's
> > > stdout-path property. This would not only make this more robust, but also
> > > more VMM agnostic.
>
> It might actually be better to have both ways of specifying the UART using
> earlycon in the commit message. Some might find it easier to do git log
> hw/serial.c to find the exact parameters than to follow the code and do the
> math.
>
> Spearking for myself, the commit message for the coresponding arm change
> contains the exact parameters (earlycon=uart,mmio,0x1000000) and that has
> been helpful when trying to figure out the address (for example, for
> kvm-unit-tests, you can configure the uart address at compile time, and
> that provides an earlycon which is usable even before the DTB is parsed).
>
> I think Andre was just trying to be helpful and point out that you don't
> need to full parameters to get earlycon working for a Linux guest.
>
Thanks Alex. I actually didn't know that we can just specify "earlycon" only.
Just in case I will mention both ways in the commit message to keep it clear.
Cheers,
Rajnesh
> Thanks,
> Alex
>
> > >
> >
> > Sorry I didn't know that. Thanks for pointing this out. Just tested this and it
> > works fine with just "earlycon".
> >
> > $ ./lkvm-static run -c1 --console virtio -p "console=hvc1 earlycon
> > root=/dev/vda " -k ./Image -d rootfs.ext4
> > [ 0.000000] earlycon: ns16550a0 at MMIO 0x0000000010000000 (options '')
> > [ 0.000000] printk: bootconsole [ns16550a0] enabled
> >
> > I will update the commit message in the next version.
> >
> > Thanks,
> > Rajnesh
> >
> > > Also, Atish, Anup: can one of you please provide a Reviewed-by: or
> > > Tested-by: for this patch?
> > >
> > > Cheers,
> > > Andre
> > >
> > > >
> > > > Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
> > > > ---
> > > > v2: Added further details in the commit message regarding the
> > > > UART address change required in kernel cmdline parameter.
> > > >
> > > > v1: https://www.spinics.net/lists/kvm/msg301835.html
> > > >
> > > > hw/rtc.c | 3 +++
> > > > hw/serial.c | 4 ++++
> > > > riscv/include/kvm/kvm-arch.h | 10 ++++++++--
> > > > 3 files changed, 15 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/hw/rtc.c b/hw/rtc.c
> > > > index 9b8785a..da696e1 100644
> > > > --- a/hw/rtc.c
> > > > +++ b/hw/rtc.c
> > > > @@ -9,6 +9,9 @@
> > > > #if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
> > > > #define RTC_BUS_TYPE DEVICE_BUS_MMIO
> > > > #define RTC_BASE_ADDRESS ARM_RTC_MMIO_BASE
> > > > +#elif defined(CONFIG_RISCV)
> > > > +#define RTC_BUS_TYPE DEVICE_BUS_MMIO
> > > > +#define RTC_BASE_ADDRESS RISCV_RTC_MMIO_BASE
> > > > #else
> > > > /* PORT 0070-007F - CMOS RAM/RTC (REAL TIME CLOCK) */
> > > > #define RTC_BUS_TYPE DEVICE_BUS_IOPORT
> > > > diff --git a/hw/serial.c b/hw/serial.c
> > > > index 3d53362..b6263a0 100644
> > > > --- a/hw/serial.c
> > > > +++ b/hw/serial.c
> > > > @@ -17,6 +17,10 @@
> > > > #define serial_iobase(nr) (ARM_UART_MMIO_BASE + (nr) * 0x1000)
> > > > #define serial_irq(nr) (32 + (nr))
> > > > #define SERIAL8250_BUS_TYPE DEVICE_BUS_MMIO
> > > > +#elif defined(CONFIG_RISCV)
> > > > +#define serial_iobase(nr) (RISCV_UART_MMIO_BASE + (nr) * 0x1000)
> > > > +#define serial_irq(nr) (1 + (nr))
> > > > +#define SERIAL8250_BUS_TYPE DEVICE_BUS_MMIO
> > > > #else
> > > > #define serial_iobase_0 (KVM_IOPORT_AREA + 0x3f8)
> > > > #define serial_iobase_1 (KVM_IOPORT_AREA + 0x2f8)
> > > > diff --git a/riscv/include/kvm/kvm-arch.h b/riscv/include/kvm/kvm-arch.h
> > > > index 3f96d00..620c796 100644
> > > > --- a/riscv/include/kvm/kvm-arch.h
> > > > +++ b/riscv/include/kvm/kvm-arch.h
> > > > @@ -11,7 +11,7 @@
> > > > #define RISCV_IOPORT 0x00000000ULL
> > > > #define RISCV_IOPORT_SIZE SZ_64K
> > > > #define RISCV_IRQCHIP 0x08000000ULL
> > > > -#define RISCV_IRQCHIP_SIZE SZ_128M
> > > > +#define RISCV_IRQCHIP_SIZE SZ_128M
> > > > #define RISCV_MMIO 0x10000000ULL
> > > > #define RISCV_MMIO_SIZE SZ_512M
> > > > #define RISCV_PCI 0x30000000ULL
> > > > @@ -35,10 +35,16 @@
> > > > #define RISCV_MAX_MEMORY(kvm) RISCV_LOMAP_MAX_MEMORY
> > > > #endif
> > > >
> > > > +#define RISCV_UART_MMIO_BASE RISCV_MMIO
> > > > +#define RISCV_UART_MMIO_SIZE 0x10000
> > > > +
> > > > +#define RISCV_RTC_MMIO_BASE (RISCV_UART_MMIO_BASE + RISCV_UART_MMIO_SIZE)
> > > > +#define RISCV_RTC_MMIO_SIZE 0x10000
> > > > +
> > > > #define KVM_IOPORT_AREA RISCV_IOPORT
> > > > #define KVM_PCI_CFG_AREA RISCV_PCI
> > > > #define KVM_PCI_MMIO_AREA (KVM_PCI_CFG_AREA + RISCV_PCI_CFG_SIZE)
> > > > -#define KVM_VIRTIO_MMIO_AREA RISCV_MMIO
> > > > +#define KVM_VIRTIO_MMIO_AREA (RISCV_RTC_MMIO_BASE + RISCV_UART_MMIO_SIZE)
> > > >
> > > > #define KVM_IOEVENTFD_HAS_PIO 0
> > > >
> > >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 kvmtool] riscv: Move serial and rtc from IO port space to MMIO area.
2023-02-01 17:28 ` Rajnesh Kanwal
@ 2023-02-02 18:29 ` Atish Kumar Patra
0 siblings, 0 replies; 6+ messages in thread
From: Atish Kumar Patra @ 2023-02-02 18:29 UTC (permalink / raw)
To: Rajnesh Kanwal; +Cc: Alexandru Elisei, Andre Przywara, apatel, kvm, Will Deacon
On Wed, Feb 1, 2023 at 9:29 AM Rajnesh Kanwal <rkanwal@rivosinc.com> wrote:
>
> On Wed, Feb 1, 2023 at 5:18 PM Alexandru Elisei
> <alexandru.elisei@arm.com> wrote:
> >
> > Hi,
> >
> > On Wed, Feb 01, 2023 at 05:04:36PM +0000, Rajnesh Kanwal wrote:
> > > On Wed, Feb 1, 2023 at 4:35 PM
> > > Andre Przywara <andre.przywara@foss.arm.com> wrote:
> > > >
> > > > On Wed, 1 Feb 2023 16:01:37 +0000
> > > > Rajnesh Kanwal <rkanwal@rivosinc.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > > The default serial and rtc IO region overlaps with PCI IO bar
> > > > > region leading bar 0 activation to fail. Moving these devices
> > > > > to MMIO region similar to ARM.
> > > > >
> > > > > Given serial has been moved from 0x3f8 to 0x10000000, this
> > > > > requires us to now pass earlycon=uart8250,mmio,0x10000000
> > > > > from cmdline rather than earlycon=uart8250,mmio,0x3f8.
> > > >
> > > > Doesn't it work either way with just "earlycon"? At least on the ARM side
> > > > it then finds the UART type and base address by following the DT's
> > > > stdout-path property. This would not only make this more robust, but also
> > > > more VMM agnostic.
> >
> > It might actually be better to have both ways of specifying the UART using
> > earlycon in the commit message. Some might find it easier to do git log
> > hw/serial.c to find the exact parameters than to follow the code and do the
> > math.
> >
> > Spearking for myself, the commit message for the coresponding arm change
> > contains the exact parameters (earlycon=uart,mmio,0x1000000) and that has
> > been helpful when trying to figure out the address (for example, for
> > kvm-unit-tests, you can configure the uart address at compile time, and
> > that provides an earlycon which is usable even before the DTB is parsed).
> >
I agree. The kvm-riscv repo documentation[1] also mentions x3f8. I am
assuming some folks may have this in their script
which will fail once this is merged. Thus, it would be good to find a
clear descriptive commit message.
Additionally, we should update the documentation once this patch is
merged upstream.
https://github.com/kvm-riscv/howto/wiki/KVM-RISCV64-on-QEMU#7-run-risc-v-kvm-on-qemu
> > I think Andre was just trying to be helpful and point out that you don't
> > need to full parameters to get earlycon working for a Linux guest.
> >
>
> Thanks Alex. I actually didn't know that we can just specify "earlycon" only.
> Just in case I will mention both ways in the commit message to keep it clear.
>
> Cheers,
> Rajnesh
>
> > Thanks,
> > Alex
> >
> > > >
> > >
> > > Sorry I didn't know that. Thanks for pointing this out. Just tested this and it
> > > works fine with just "earlycon".
> > >
> > > $ ./lkvm-static run -c1 --console virtio -p "console=hvc1 earlycon
> > > root=/dev/vda " -k ./Image -d rootfs.ext4
> > > [ 0.000000] earlycon: ns16550a0 at MMIO 0x0000000010000000 (options '')
> > > [ 0.000000] printk: bootconsole [ns16550a0] enabled
> > >
> > > I will update the commit message in the next version.
> > >
> > > Thanks,
> > > Rajnesh
> > >
> > > > Also, Atish, Anup: can one of you please provide a Reviewed-by: or
> > > > Tested-by: for this patch?
> > > >
> > > > Cheers,
> > > > Andre
> > > >
> > > > >
> > > > > Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
> > > > > ---
> > > > > v2: Added further details in the commit message regarding the
> > > > > UART address change required in kernel cmdline parameter.
> > > > >
> > > > > v1: https://www.spinics.net/lists/kvm/msg301835.html
> > > > >
> > > > > hw/rtc.c | 3 +++
> > > > > hw/serial.c | 4 ++++
> > > > > riscv/include/kvm/kvm-arch.h | 10 ++++++++--
> > > > > 3 files changed, 15 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/hw/rtc.c b/hw/rtc.c
> > > > > index 9b8785a..da696e1 100644
> > > > > --- a/hw/rtc.c
> > > > > +++ b/hw/rtc.c
> > > > > @@ -9,6 +9,9 @@
> > > > > #if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
> > > > > #define RTC_BUS_TYPE DEVICE_BUS_MMIO
> > > > > #define RTC_BASE_ADDRESS ARM_RTC_MMIO_BASE
> > > > > +#elif defined(CONFIG_RISCV)
> > > > > +#define RTC_BUS_TYPE DEVICE_BUS_MMIO
> > > > > +#define RTC_BASE_ADDRESS RISCV_RTC_MMIO_BASE
> > > > > #else
> > > > > /* PORT 0070-007F - CMOS RAM/RTC (REAL TIME CLOCK) */
> > > > > #define RTC_BUS_TYPE DEVICE_BUS_IOPORT
> > > > > diff --git a/hw/serial.c b/hw/serial.c
> > > > > index 3d53362..b6263a0 100644
> > > > > --- a/hw/serial.c
> > > > > +++ b/hw/serial.c
> > > > > @@ -17,6 +17,10 @@
> > > > > #define serial_iobase(nr) (ARM_UART_MMIO_BASE + (nr) * 0x1000)
> > > > > #define serial_irq(nr) (32 + (nr))
> > > > > #define SERIAL8250_BUS_TYPE DEVICE_BUS_MMIO
> > > > > +#elif defined(CONFIG_RISCV)
> > > > > +#define serial_iobase(nr) (RISCV_UART_MMIO_BASE + (nr) * 0x1000)
> > > > > +#define serial_irq(nr) (1 + (nr))
> > > > > +#define SERIAL8250_BUS_TYPE DEVICE_BUS_MMIO
> > > > > #else
> > > > > #define serial_iobase_0 (KVM_IOPORT_AREA + 0x3f8)
> > > > > #define serial_iobase_1 (KVM_IOPORT_AREA + 0x2f8)
> > > > > diff --git a/riscv/include/kvm/kvm-arch.h b/riscv/include/kvm/kvm-arch.h
> > > > > index 3f96d00..620c796 100644
> > > > > --- a/riscv/include/kvm/kvm-arch.h
> > > > > +++ b/riscv/include/kvm/kvm-arch.h
> > > > > @@ -11,7 +11,7 @@
> > > > > #define RISCV_IOPORT 0x00000000ULL
> > > > > #define RISCV_IOPORT_SIZE SZ_64K
> > > > > #define RISCV_IRQCHIP 0x08000000ULL
> > > > > -#define RISCV_IRQCHIP_SIZE SZ_128M
> > > > > +#define RISCV_IRQCHIP_SIZE SZ_128M
> > > > > #define RISCV_MMIO 0x10000000ULL
> > > > > #define RISCV_MMIO_SIZE SZ_512M
> > > > > #define RISCV_PCI 0x30000000ULL
> > > > > @@ -35,10 +35,16 @@
> > > > > #define RISCV_MAX_MEMORY(kvm) RISCV_LOMAP_MAX_MEMORY
> > > > > #endif
> > > > >
> > > > > +#define RISCV_UART_MMIO_BASE RISCV_MMIO
> > > > > +#define RISCV_UART_MMIO_SIZE 0x10000
> > > > > +
> > > > > +#define RISCV_RTC_MMIO_BASE (RISCV_UART_MMIO_BASE + RISCV_UART_MMIO_SIZE)
> > > > > +#define RISCV_RTC_MMIO_SIZE 0x10000
> > > > > +
> > > > > #define KVM_IOPORT_AREA RISCV_IOPORT
> > > > > #define KVM_PCI_CFG_AREA RISCV_PCI
> > > > > #define KVM_PCI_MMIO_AREA (KVM_PCI_CFG_AREA + RISCV_PCI_CFG_SIZE)
> > > > > -#define KVM_VIRTIO_MMIO_AREA RISCV_MMIO
> > > > > +#define KVM_VIRTIO_MMIO_AREA (RISCV_RTC_MMIO_BASE + RISCV_UART_MMIO_SIZE)
> > > > >
> > > > > #define KVM_IOEVENTFD_HAS_PIO 0
> > > > >
> > > >
FWIW,
Tested-by: Atish Patra <atishp@rivosinc.com>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-02-02 18:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-01 16:01 [PATCH v2 kvmtool] riscv: Move serial and rtc from IO port space to MMIO area Rajnesh Kanwal
2023-02-01 16:35 ` Andre Przywara
2023-02-01 17:04 ` Rajnesh Kanwal
2023-02-01 17:18 ` Alexandru Elisei
2023-02-01 17:28 ` Rajnesh Kanwal
2023-02-02 18:29 ` Atish Kumar Patra
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.