On Fri, 2022-12-16 at 16:34 +0000, Sean Christopherson wrote:
> On Fri, Dec 16, 2022, Yu Zhang wrote:
> > Currently, KVM xen and its shared info selftest code uses
> > 'GPA_INVALID' for GFN values, but actually it is more accurate
> > to use the name 'INVALID_GFN'. So just add a new definition
> > and use it.
> >
> > No functional changes intended.
> >
> > Suggested-by: David Woodhouse <dwmw2@xxxxxxxxxxxxx>
> > Signed-off-by: Yu Zhang <yu.c.zhang@xxxxxxxxxxxxxxx>
> > ---
> > arch/x86/kvm/xen.c | 4 ++--
> > include/linux/kvm_types.h | 1 +
> > tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c | 4 ++--
> > 3 files changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> > index d7af40240248..6908a74ab303 100644
> > --- a/arch/x86/kvm/xen.c
> > +++ b/arch/x86/kvm/xen.c
> > @@ -41,7 +41,7 @@ static int kvm_xen_shared_info_init(struct kvm *kvm, gfn_t gfn)
> > int ret = 0;
> > int idx = srcu_read_lock(&kvm->srcu);
> >
> > - if (gfn == GPA_INVALID) {
> > + if (gfn == INVALID_GFN) {
>
> Grrr! This magic value is ABI, as "gfn == -1" yields different behavior than a
> random, garbage gfn.
>
> So, sadly, we can't simply introduce INVALID_GFN here, and instead need to do
> something like:
>
Well... you can still use INVALID_GFN as long as its value remains the
same ((uint64_t)-1).
But yes, the KVM API differs here from Xen because Xen only allows a
guest to *set* these (and later they invented SHUTDOWN_soft_reset).
While KVM lets the userspace VMM handle soft reset, and needs to allow
them to be *unset*. And since zero is a valid GPA/GFN, -1 is a
reasonable value for 'INVALID'. But I do think that detail escaped the
documentation and the uapi headers, so your suggestion below is a good
one, although strictly we need a GPA one too.
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 20522d4ba1e0..2d31caaf812c 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1766,6 +1766,7 @@ struct kvm_xen_hvm_attr {
> __u8 vector;
> __u8 runstate_update_flag;
> struct {
> +#define KVM_XEN_INVALID_GFN (~0ull)
> __u64 gfn;
> } shared_info;
> struct {
>
> > kvm_gpc_deactivate(gpc);
> > goto out;
> > }
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
