On Thu, 19 Jan 2023 13:39:32 +0000
Alexandru Elisei <alexandru.elisei@xxxxxxx> wrote:
Hi Alex,
> CC'ing the kvmtool maintainers and other people that might be interested in this
> thread. Sorry for hijacking your patch!
>
> On Wed, Jan 18, 2023 at 05:20:07PM +0000, Rajnesh Kanwal wrote:
> > Adding force-pci option to allow forcing virtio
> > devices to use pci as the default transport.
>
> arm is in the same situation, MMIO is the default virtio transport. I was bitten
> by that in the past. It also cought other people unaware, and I remember maz
> complaining about it on the list.
>
> So I was thinking about adding a new command line parameter, --virtio-transport,
> with the possible values mmio-legacy, mmio, pci-legacy and pci. Then each
> architecture can define the default value for the transport. For arm, that would
> be pci.
>
> What do you guys think?
That sounds good to me, definitely much better than proliferating the
force-pci parameter, which is a bit of a misnomer anyway.
I also heard people recently asking explicitly for MMIO transport,
since it's leaner and you can remove PCI support from the guest kernel
entirely. So giving the user explicit choice is probably the way to go.
Cheers,
Andre
> > Signed-off-by: Rajnesh Kanwal <rkanwal@xxxxxxxxxxxx>
> > ---
> > riscv/include/kvm/kvm-arch.h | 6 ++++--
> > riscv/include/kvm/kvm-config-arch.h | 7 ++++++-
> > 2 files changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/riscv/include/kvm/kvm-arch.h b/riscv/include/kvm/kvm-arch.h
> > index 1e130f5..2cf41c5 100644
> > --- a/riscv/include/kvm/kvm-arch.h
> > +++ b/riscv/include/kvm/kvm-arch.h
> > @@ -46,8 +46,10 @@
> >
> > #define KVM_VM_TYPE 0
> >
> > -#define VIRTIO_DEFAULT_TRANS(kvm) \
> > - ((kvm)->cfg.virtio_legacy ? VIRTIO_MMIO_LEGACY : VIRTIO_MMIO)
> > +#define VIRTIO_DEFAULT_TRANS(kvm) \
> > + ((kvm)->cfg.arch.virtio_trans_pci ? \
> > + ((kvm)->cfg.virtio_legacy ? VIRTIO_PCI_LEGACY : VIRTIO_PCI) : \
> > + ((kvm)->cfg.virtio_legacy ? VIRTIO_MMIO_LEGACY : VIRTIO_MMIO))
> >
> > #define VIRTIO_RING_ENDIAN VIRTIO_ENDIAN_LE
> >
> > diff --git a/riscv/include/kvm/kvm-config-arch.h b/riscv/include/kvm/kvm-config-arch.h
> > index 188125c..901a5e0 100644
> > --- a/riscv/include/kvm/kvm-config-arch.h
> > +++ b/riscv/include/kvm/kvm-config-arch.h
> > @@ -6,6 +6,7 @@
> > struct kvm_config_arch {
> > const char *dump_dtb_filename;
> > bool ext_disabled[KVM_RISCV_ISA_EXT_MAX];
> > + bool virtio_trans_pci;
> > };
> >
> > #define OPT_ARCH_RUN(pfx, cfg) \
> > @@ -26,6 +27,10 @@ struct kvm_config_arch {
> > "Disable Zicbom Extension"), \
> > OPT_BOOLEAN('\0', "disable-zihintpause", \
> > &(cfg)->ext_disabled[KVM_RISCV_ISA_EXT_ZIHINTPAUSE],\
> > - "Disable Zihintpause Extension"),
> > + "Disable Zihintpause Extension"), \
> > + OPT_BOOLEAN('\0', "force-pci", \
> > + &(cfg)->virtio_trans_pci, \
> > + "Force virtio devices to use PCI as their " \
> > + "default transport"),
> >
> > #endif /* KVM__KVM_CONFIG_ARCH_H */
> > --
> > 2.25.1
> >
