* [PATCH kvmtool 1/1] riscv: pci: Add --force-pci option for riscv VMs.
@ 2023-01-18 17:20 Rajnesh Kanwal
2023-01-19 13:39 ` Alexandru Elisei
0 siblings, 1 reply; 6+ messages in thread
From: Rajnesh Kanwal @ 2023-01-18 17:20 UTC (permalink / raw)
To: apatel, atishp, kvm; +Cc: Rajnesh Kanwal
Adding force-pci option to allow forcing virtio
devices to use pci as the default transport.
Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH kvmtool 1/1] riscv: pci: Add --force-pci option for riscv VMs.
2023-01-18 17:20 [PATCH kvmtool 1/1] riscv: pci: Add --force-pci option for riscv VMs Rajnesh Kanwal
@ 2023-01-19 13:39 ` Alexandru Elisei
2023-01-19 14:24 ` Andre Przywara
2023-01-19 18:16 ` Will Deacon
0 siblings, 2 replies; 6+ messages in thread
From: Alexandru Elisei @ 2023-01-19 13:39 UTC (permalink / raw)
To: Rajnesh Kanwal
Cc: apatel, atishp, kvm, will, julien.thierry.kdev, maz, andre.przywara
Hi,
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?
Thanks,
Alex
>
> Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH kvmtool 1/1] riscv: pci: Add --force-pci option for riscv VMs.
2023-01-19 13:39 ` Alexandru Elisei
@ 2023-01-19 14:24 ` Andre Przywara
2023-01-19 18:16 ` Will Deacon
1 sibling, 0 replies; 6+ messages in thread
From: Andre Przywara @ 2023-01-19 14:24 UTC (permalink / raw)
To: Alexandru Elisei
Cc: Rajnesh Kanwal, apatel, atishp, kvm, will, julien.thierry.kdev, maz
On Thu, 19 Jan 2023 13:39:32 +0000
Alexandru Elisei <alexandru.elisei@arm.com> 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@rivosinc.com>
> > ---
> > 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
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH kvmtool 1/1] riscv: pci: Add --force-pci option for riscv VMs.
2023-01-19 13:39 ` Alexandru Elisei
2023-01-19 14:24 ` Andre Przywara
@ 2023-01-19 18:16 ` Will Deacon
2023-01-21 12:18 ` Marc Zyngier
1 sibling, 1 reply; 6+ messages in thread
From: Will Deacon @ 2023-01-19 18:16 UTC (permalink / raw)
To: Alexandru Elisei
Cc: Rajnesh Kanwal, apatel, atishp, kvm, julien.thierry.kdev, maz,
andre.przywara
On Thu, Jan 19, 2023 at 01:39:32PM +0000, Alexandru Elisei wrote:
> Hi,
>
> 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. Then we can remove --force-pci altogether and maybe
have PCI as the default for everybody? Should make the tool a little easier
to use across architectures.
Will
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH kvmtool 1/1] riscv: pci: Add --force-pci option for riscv VMs.
2023-01-19 18:16 ` Will Deacon
@ 2023-01-21 12:18 ` Marc Zyngier
2023-01-21 15:41 ` Alexandru Elisei
0 siblings, 1 reply; 6+ messages in thread
From: Marc Zyngier @ 2023-01-21 12:18 UTC (permalink / raw)
To: Will Deacon
Cc: Alexandru Elisei, Rajnesh Kanwal, apatel, atishp, kvm,
julien.thierry.kdev, andre.przywara
On Thu, 19 Jan 2023 18:16:15 +0000,
Will Deacon <will@kernel.org> wrote:
>
> On Thu, Jan 19, 2023 at 01:39:32PM +0000, Alexandru Elisei wrote:
> > Hi,
> >
> > 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. Then we can remove --force-pci altogether and maybe
> have PCI as the default for everybody? Should make the tool a little easier
> to use across architectures.
I'd rather keep --force-pci in order no to break existing scripting.
It just won't do anything once we make PCI the default (which we
definitely should).
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH kvmtool 1/1] riscv: pci: Add --force-pci option for riscv VMs.
2023-01-21 12:18 ` Marc Zyngier
@ 2023-01-21 15:41 ` Alexandru Elisei
0 siblings, 0 replies; 6+ messages in thread
From: Alexandru Elisei @ 2023-01-21 15:41 UTC (permalink / raw)
To: Marc Zyngier
Cc: Will Deacon, Rajnesh Kanwal, apatel, atishp, kvm,
julien.thierry.kdev, andre.przywara
Hi,
On Sat, Jan 21, 2023 at 12:18:51PM +0000, Marc Zyngier wrote:
> On Thu, 19 Jan 2023 18:16:15 +0000,
> Will Deacon <will@kernel.org> wrote:
> >
> > On Thu, Jan 19, 2023 at 01:39:32PM +0000, Alexandru Elisei wrote:
> > > Hi,
> > >
> > > 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. Then we can remove --force-pci altogether and maybe
> > have PCI as the default for everybody? Should make the tool a little easier
> > to use across architectures.
>
> I'd rather keep --force-pci in order no to break existing scripting.
> It just won't do anything once we make PCI the default (which we
> definitely should).
My plan was to mark --force-pci and --virtio-legacy as deprecated in the
help message and to parse them to set the correct value for
--virtio-transport (so if --force-pci and --virtio-legacy is specified,
the value for --virtio-transport pci-legacy). So as not to break CIs and
existing scripts.
Since I would be already parsing them, I was wondering if it would be
useful to also print a warning on the command line for users to switch to
--virtio-transport. Any opinions about this?
Thanks,
Alex
>
> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-21 15:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-18 17:20 [PATCH kvmtool 1/1] riscv: pci: Add --force-pci option for riscv VMs Rajnesh Kanwal
2023-01-19 13:39 ` Alexandru Elisei
2023-01-19 14:24 ` Andre Przywara
2023-01-19 18:16 ` Will Deacon
2023-01-21 12:18 ` Marc Zyngier
2023-01-21 15:41 ` Alexandru Elisei
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.