All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: Andrew Jones <drjones@redhat.com>
Cc: kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu,
	pbonzini@redhat.com, steven.price@arm.com
Subject: Re: [PATCH 1/5] KVM: arm64: pvtime: steal-time is only supported when configured
Date: Mon, 27 Jul 2020 18:25:50 +0100	[thread overview]
Message-ID: <e2f23a105af0d7cf2daa6f3f8b596177@kernel.org> (raw)
In-Reply-To: <20200711100434.46660-2-drjones@redhat.com>

Hi Andrew,

On 2020-07-11 11:04, Andrew Jones wrote:
> Don't confuse the guest by saying steal-time is supported when
> it hasn't been configured by userspace and won't work.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  arch/arm64/kvm/pvtime.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/pvtime.c b/arch/arm64/kvm/pvtime.c
> index f7b52ce1557e..2b22214909be 100644
> --- a/arch/arm64/kvm/pvtime.c
> +++ b/arch/arm64/kvm/pvtime.c
> @@ -42,9 +42,12 @@ long kvm_hypercall_pv_features(struct kvm_vcpu 
> *vcpu)
> 
>  	switch (feature) {
>  	case ARM_SMCCC_HV_PV_TIME_FEATURES:
> -	case ARM_SMCCC_HV_PV_TIME_ST:
>  		val = SMCCC_RET_SUCCESS;
>  		break;
> +	case ARM_SMCCC_HV_PV_TIME_ST:
> +		if (vcpu->arch.steal.base != GPA_INVALID)
> +			val = SMCCC_RET_SUCCESS;
> +		break;
>  	}
> 
>  	return val;

I'm not so sure about this. I have always considered the
discovery interface to be "do you know about this SMCCC
function". And if you look at the spec, it says (4.2,
PV_TIME_FEATURES):

<quote>
If PV_call_id identifies PV_TIME_FEATURES, this call returns:
• NOT_SUPPORTED (-1) to indicate that all
paravirtualized time functions in this specification are not
supported.
• SUCCESS (0) to indicate that all the paravirtualized time
functions in this specification are supported.
</quote>

So the way I understand it, you cannot return "supported"
for PV_TIME_FEATURES, and yet return NOT_SUPPORTED for
PV_TIME_ST. It applies to *all* features.

Yes, this is very bizarre. But I don't think we can deviate
from it.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: Andrew Jones <drjones@redhat.com>
Cc: pbonzini@redhat.com, kvmarm@lists.cs.columbia.edu,
	kvm@vger.kernel.org, steven.price@arm.com
Subject: Re: [PATCH 1/5] KVM: arm64: pvtime: steal-time is only supported when configured
Date: Mon, 27 Jul 2020 18:25:50 +0100	[thread overview]
Message-ID: <e2f23a105af0d7cf2daa6f3f8b596177@kernel.org> (raw)
In-Reply-To: <20200711100434.46660-2-drjones@redhat.com>

Hi Andrew,

On 2020-07-11 11:04, Andrew Jones wrote:
> Don't confuse the guest by saying steal-time is supported when
> it hasn't been configured by userspace and won't work.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  arch/arm64/kvm/pvtime.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/pvtime.c b/arch/arm64/kvm/pvtime.c
> index f7b52ce1557e..2b22214909be 100644
> --- a/arch/arm64/kvm/pvtime.c
> +++ b/arch/arm64/kvm/pvtime.c
> @@ -42,9 +42,12 @@ long kvm_hypercall_pv_features(struct kvm_vcpu 
> *vcpu)
> 
>  	switch (feature) {
>  	case ARM_SMCCC_HV_PV_TIME_FEATURES:
> -	case ARM_SMCCC_HV_PV_TIME_ST:
>  		val = SMCCC_RET_SUCCESS;
>  		break;
> +	case ARM_SMCCC_HV_PV_TIME_ST:
> +		if (vcpu->arch.steal.base != GPA_INVALID)
> +			val = SMCCC_RET_SUCCESS;
> +		break;
>  	}
> 
>  	return val;

I'm not so sure about this. I have always considered the
discovery interface to be "do you know about this SMCCC
function". And if you look at the spec, it says (4.2,
PV_TIME_FEATURES):

<quote>
If PV_call_id identifies PV_TIME_FEATURES, this call returns:
• NOT_SUPPORTED (-1) to indicate that all
paravirtualized time functions in this specification are not
supported.
• SUCCESS (0) to indicate that all the paravirtualized time
functions in this specification are supported.
</quote>

So the way I understand it, you cannot return "supported"
for PV_TIME_FEATURES, and yet return NOT_SUPPORTED for
PV_TIME_ST. It applies to *all* features.

Yes, this is very bizarre. But I don't think we can deviate
from it.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

  reply	other threads:[~2020-07-27 17:25 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-11 10:04 [PATCH 0/5] KVM: arm64: pvtime: Fixes and a new cap Andrew Jones
2020-07-11 10:04 ` Andrew Jones
2020-07-11 10:04 ` [PATCH 1/5] KVM: arm64: pvtime: steal-time is only supported when configured Andrew Jones
2020-07-11 10:04   ` Andrew Jones
2020-07-27 17:25   ` Marc Zyngier [this message]
2020-07-27 17:25     ` Marc Zyngier
2020-07-28 12:55     ` Andrew Jones
2020-07-28 12:55       ` Andrew Jones
2020-07-28 13:13       ` Marc Zyngier
2020-07-28 13:13         ` Marc Zyngier
2020-07-28 13:29         ` Andrew Jones
2020-07-28 13:29           ` Andrew Jones
2020-07-11 10:04 ` [PATCH 2/5] KVM: arm64: pvtime: Fix potential loss of stolen time Andrew Jones
2020-07-11 10:04   ` Andrew Jones
2020-07-27 17:29   ` Marc Zyngier
2020-07-27 17:29     ` Marc Zyngier
2020-07-11 10:04 ` [PATCH 3/5] KVM: arm64: pvtime: Fix stolen time accounting across migration Andrew Jones
2020-07-11 10:04   ` Andrew Jones
2020-07-27 17:54   ` Marc Zyngier
2020-07-27 17:54     ` Marc Zyngier
2020-07-11 10:04 ` [PATCH 4/5] KVM: Documentation minor fixups Andrew Jones
2020-07-11 10:04   ` Andrew Jones
2020-07-27 17:55   ` Marc Zyngier
2020-07-27 17:55     ` Marc Zyngier
2020-07-11 10:04 ` [PATCH 5/5] arm64/x86: KVM: Introduce steal-time cap Andrew Jones
2020-07-11 10:04   ` Andrew Jones
2020-07-27 17:58   ` Marc Zyngier
2020-07-27 17:58     ` Marc Zyngier
2020-07-11 10:20 ` [PATCH 0/5] KVM: arm64: pvtime: Fixes and a new cap Andrew Jones
2020-07-11 10:20   ` Andrew Jones
2020-07-13  8:25 ` Steven Price
2020-07-13  8:25   ` Steven Price
2020-07-27 11:02 ` Andrew Jones
2020-07-27 11:02   ` Andrew Jones
2020-07-27 11:15   ` Marc Zyngier
2020-07-27 11:15     ` Marc Zyngier
2020-07-27 18:01 ` Marc Zyngier
2020-07-27 18:01   ` Marc Zyngier
2020-07-28 12:59   ` Andrew Jones
2020-07-28 12:59     ` Andrew Jones

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e2f23a105af0d7cf2daa6f3f8b596177@kernel.org \
    --to=maz@kernel.org \
    --cc=drjones@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=pbonzini@redhat.com \
    --cc=steven.price@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.