On Wed, Feb 1, 2023 at 9:29 AM Rajnesh Kanwal <rkanwal@xxxxxxxxxxxx> wrote: > > On Wed, Feb 1, 2023 at 5:18 PM Alexandru Elisei > <alexandru.elisei@xxxxxxx> 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@xxxxxxxxxxxx> wrote: > > > > > > > > On Wed, 1 Feb 2023 16:01:37 +0000 > > > > Rajnesh Kanwal <rkanwal@xxxxxxxxxxxx> 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@xxxxxxxxxxxx> > > > > > --- > > > > > 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@xxxxxxxxxxxx> Reviewed-by: Atish Patra <atishp@xxxxxxxxxxxx>
