linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: marc.zyngier@arm.com (Marc Zyngier)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 30/32] arm64: KVM: enable initialization of a 32bit vcpu
Date: Tue, 07 May 2013 17:36:52 +0100	[thread overview]
Message-ID: <51892DA4.4050401@arm.com> (raw)
In-Reply-To: <CAEDV+g+OL+37w9jA8=0WGea5FgW5y3mazqFzv9GLxDgjCe6dtg@mail.gmail.com>

On 24/04/13 18:17, Christoffer Dall wrote:
> On Wed, Apr 24, 2013 at 6:49 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On 24/04/13 00:02, Christoffer Dall wrote:
>>> On Mon, Apr 08, 2013 at 05:17:32PM +0100, Marc Zyngier wrote:
>>>> Wire the init of a 32bit vcpu by allowing 32bit modes in pstate,
>>>> and providing sensible defaults out of reset state.
>>>>
>>>> This feature is of course conditioned by the presence of 32bit
>>>> capability on the physical CPU, and is checked by the KVM_CAP_ARM_EL1_32BIT
>>>> capability.
>>>>
>>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>>> ---
>>>>  arch/arm64/include/asm/kvm_host.h |  2 +-
>>>>  arch/arm64/include/uapi/asm/kvm.h |  1 +
>>>>  arch/arm64/kvm/guest.c            |  6 ++++++
>>>>  arch/arm64/kvm/reset.c            | 25 ++++++++++++++++++++++++-
>>>>  include/uapi/linux/kvm.h          |  1 +
>>>>  5 files changed, 33 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>>>> index d44064d..c3ec107 100644
>>>> --- a/arch/arm64/include/asm/kvm_host.h
>>>> +++ b/arch/arm64/include/asm/kvm_host.h
>>>> @@ -34,7 +34,7 @@
>>>>  #include <asm/kvm_vgic.h>
>>>>  #include <asm/kvm_arch_timer.h>
>>>>
>>>> -#define KVM_VCPU_MAX_FEATURES 1
>>>> +#define KVM_VCPU_MAX_FEATURES 2
>>>>
>>>>  /* We don't currently support large pages. */
>>>>  #define KVM_HPAGE_GFN_SHIFT(x)      0
>>>> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
>>>> index 5b1110c..5031f42 100644
>>>> --- a/arch/arm64/include/uapi/asm/kvm.h
>>>> +++ b/arch/arm64/include/uapi/asm/kvm.h
>>>> @@ -75,6 +75,7 @@ struct kvm_regs {
>>>>  #define KVM_VGIC_V2_CPU_SIZE                0x2000
>>>>
>>>>  #define KVM_ARM_VCPU_POWER_OFF              0 /* CPU is started in OFF state */
>>>> +#define KVM_ARM_VCPU_EL1_32BIT              1 /* CPU running a 32bit VM */
>>>>
>>>>  struct kvm_vcpu_init {
>>>>      __u32 target;
>>>> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
>>>> index 47d3729..74ef7d5 100644
>>>> --- a/arch/arm64/kvm/guest.c
>>>> +++ b/arch/arm64/kvm/guest.c
>>>> @@ -93,6 +93,12 @@ static int set_core_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
>>>>      if (off == KVM_REG_ARM_CORE_REG(regs.pstate)) {
>>>>              unsigned long mode = (*(unsigned long *)valp) & COMPAT_PSR_MODE_MASK;
>>>>              switch (mode) {
>>>> +            case COMPAT_PSR_MODE_USR:
>>>> +            case COMPAT_PSR_MODE_FIQ:
>>>> +            case COMPAT_PSR_MODE_IRQ:
>>>> +            case COMPAT_PSR_MODE_SVC:
>>>> +            case COMPAT_PSR_MODE_ABT:
>>>> +            case COMPAT_PSR_MODE_UND:
>>>>              case PSR_MODE_EL0t:
>>>>              case PSR_MODE_EL1t:
>>>>              case PSR_MODE_EL1h:
>>>> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
>>>> index bc33e76..a282d35 100644
>>>> --- a/arch/arm64/kvm/reset.c
>>>> +++ b/arch/arm64/kvm/reset.c
>>>> @@ -35,11 +35,27 @@ static struct kvm_regs default_regs_reset = {
>>>>      .regs.pstate = PSR_MODE_EL1h | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT,
>>>>  };
>>>>
>>>> +static struct kvm_regs default_regs_reset32 = {
>>>> +    .regs.pstate = (COMPAT_PSR_MODE_SVC | COMPAT_PSR_A_BIT |
>>>> +                    COMPAT_PSR_I_BIT | COMPAT_PSR_F_BIT),
>>>> +};
>>>> +
>>>> +static bool cpu_has_32bit_el1(void)
>>>> +{
>>>> +    u64 pfr0;
>>>> +
>>>> +    pfr0 = read_cpuid(ID_AA64PFR0_EL1);
>>>> +    return !!(pfr0 & 0x20);
>>>
>>> again we don't need the double negation
>>
>> I still hold that it makes things more readable.
>>
>>>> +}
>>>> +
>>>>  int kvm_arch_dev_ioctl_check_extention(long ext)
>>>>  {
>>>>      int r;
>>>>
>>>>      switch (ext) {
>>>> +    case KVM_CAP_ARM_EL1_32BIT:
>>>> +            r = cpu_has_32bit_el1();
>>>> +            break;
>>>>      default:
>>>>              r = 0;
>>>>      }
>>>> @@ -62,7 +78,14 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>>>>
>>>>      switch (vcpu->arch.target) {
>>>>      default:
>>>> -            cpu_reset = &default_regs_reset;
>>>> +            if (test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu->arch.features)) {
>>>> +                    if (!cpu_has_32bit_el1())
>>>> +                            return -EINVAL;
>>>
>>> I'm not sure EINVAL is appropriate here, the value specified was not
>>> incorrect, it's that the hardware doesn't support it. ENXIO, ENODEV, and
>>> add that in Documentation/virtual/kvm/api.txt ?
>>
>> Not sure. If you ended up here, it means you tried to start a 32bit
>> guest on a 64bit-only CPU, despite KVM_CAP_ARM_EL1_32BIT telling you
>> that your CPU is not 32bit capable.
>>
>> This is clearly an invalid input, isn't it?
>>
> check the API documentation for this ioctl, I don't think that's the
> type of invalid input meant when describing the meaning of EINVAL. If
> you feel strongly about it of course it's no big deal, but I think
> EINVAL is so overloaded anyway that telling the user something more
> specific would be great, but I'll leave it up to you.

[bit late on this one...]

Here's what the documentation says:
<quote>
4.77 KVM_ARM_VCPU_INIT

Capability: basic
Architectures: arm, arm64
Type: vcpu ioctl
Parameters: struct struct kvm_vcpu_init (in)
Returns: 0 on success; -1 on error
Errors:
  EINVAL:    the target is unknown, or the combination of features is invalid.
  ENOENT:    a features bit specified is unknown.
</quote>

When this call fails, it is because you've requested a feature
that is invalid for this CPU. To me, that exactly fits the
EINVAL entry copied above.

Or am I completely misunderstanding it?

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

  reply	other threads:[~2013-05-07 16:36 UTC|newest]

Thread overview: 116+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-08 16:17 [PATCH v3 00/32] Port of KVM to arm64 Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 01/32] arm64: add explicit symbols to ESR_EL1 decoding Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 02/32] arm64: KVM: define HYP and Stage-2 translation page flags Marc Zyngier
2013-04-10 14:07   ` Will Deacon
2013-04-12 15:22     ` Marc Zyngier
2013-04-26 17:01   ` Catalin Marinas
2013-04-26 17:11     ` Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 03/32] arm64: KVM: HYP mode idmap support Marc Zyngier
2013-04-23 22:57   ` Christoffer Dall
2013-04-24  9:36     ` Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 04/32] arm64: KVM: EL2 register definitions Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 05/32] arm64: KVM: system register definitions for 64bit guests Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 06/32] arm64: KVM: Basic ESR_EL2 helpers and vcpu register access Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 07/32] arm64: KVM: fault injection into a guest Marc Zyngier
2013-04-10 16:40   ` Will Deacon
2013-04-12 15:29     ` Marc Zyngier
2013-04-23 22:57   ` Christoffer Dall
2013-04-24 10:04     ` Marc Zyngier
2013-04-24 16:46       ` Christoffer Dall
2013-04-29 16:26   ` Catalin Marinas
2013-05-07 16:29     ` Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 08/32] arm64: KVM: architecture specific MMU backend Marc Zyngier
2013-04-23 22:58   ` Christoffer Dall
2013-04-24 11:03     ` Marc Zyngier
2013-04-24 11:10       ` Will Deacon
2013-04-24 16:50         ` Christoffer Dall
2013-04-24 16:55       ` Christoffer Dall
2013-04-25 12:59         ` Marc Zyngier
2013-04-25 15:13           ` Christoffer Dall
2013-04-29 17:35   ` Catalin Marinas
2013-04-30 10:23     ` Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 09/32] arm64: KVM: user space interface Marc Zyngier
2013-04-10 16:45   ` Will Deacon
2013-04-12 15:31     ` Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 10/32] arm64: KVM: system register handling Marc Zyngier
2013-04-10 17:04   ` Will Deacon
2013-04-12 15:48     ` Marc Zyngier
2013-04-23 23:01   ` Christoffer Dall
2013-04-24 13:37     ` Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 11/32] arm64: KVM: CPU specific system registers handling Marc Zyngier
2013-04-10 17:06   ` Will Deacon
2013-04-12 16:04     ` Marc Zyngier
2013-04-23 22:59       ` Christoffer Dall
2013-04-24  9:33         ` Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 12/32] arm64: KVM: virtual CPU reset Marc Zyngier
2013-04-10 17:07   ` Will Deacon
2013-04-12 16:04     ` Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 13/32] arm64: KVM: kvm_arch and kvm_vcpu_arch definitions Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 14/32] arm64: KVM: MMIO access backend Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 15/32] arm64: KVM: guest one-reg interface Marc Zyngier
2013-04-10 17:13   ` Will Deacon
2013-04-12 16:35     ` Marc Zyngier
2013-04-23 22:59   ` Christoffer Dall
2013-04-24 11:27     ` Marc Zyngier
2013-04-24 17:05       ` Christoffer Dall
2013-04-08 16:17 ` [PATCH v3 16/32] arm64: KVM: hypervisor initialization code Marc Zyngier
2013-05-02 11:03   ` Catalin Marinas
2013-05-02 13:28     ` Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 17/32] arm64: KVM: HYP mode world switch implementation Marc Zyngier
2013-04-23 22:59   ` Christoffer Dall
2013-04-24 11:39     ` Marc Zyngier
2013-04-24 17:08       ` Christoffer Dall
2013-04-08 16:17 ` [PATCH v3 18/32] arm64: KVM: Exit handling Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 19/32] arm64: KVM: Plug the VGIC Marc Zyngier
2013-04-23 23:00   ` Christoffer Dall
2013-04-24 11:43     ` Marc Zyngier
2013-05-02 14:38       ` Catalin Marinas
2013-04-08 16:17 ` [PATCH v3 20/32] arm64: KVM: Plug the arch timer Marc Zyngier
2013-04-23 23:00   ` Christoffer Dall
2013-04-24 11:43     ` Marc Zyngier
2013-05-02 15:31       ` Catalin Marinas
2013-04-08 16:17 ` [PATCH v3 21/32] arm64: KVM: PSCI implementation Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 22/32] arm64: KVM: Build system integration Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 23/32] arm64: KVM: define 32bit specific registers Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 24/32] arm64: KVM: 32bit GP register access Marc Zyngier
2013-04-23 23:00   ` Christoffer Dall
2013-04-24 13:06     ` Marc Zyngier
2013-04-24 17:09       ` Christoffer Dall
2013-05-02 16:09   ` Catalin Marinas
2013-05-07 16:28     ` Marc Zyngier
2013-05-07 16:33       ` Catalin Marinas
2013-05-11  0:36         ` Christoffer Dall
2013-05-11  7:51           ` Peter Maydell
2013-05-11  9:43           ` Catalin Marinas
2013-05-12 18:51             ` Christoffer Dall
2013-04-08 16:17 ` [PATCH v3 25/32] arm64: KVM: 32bit conditional execution emulation Marc Zyngier
2013-04-23 23:00   ` Christoffer Dall
2013-04-24 13:13     ` Marc Zyngier
2013-04-24 17:11       ` Christoffer Dall
2013-04-08 16:17 ` [PATCH v3 26/32] arm64: KVM: 32bit handling of coprocessor traps Marc Zyngier
2013-04-23 23:01   ` Christoffer Dall
2013-04-24 13:42     ` Marc Zyngier
2013-04-24 17:14       ` Christoffer Dall
2013-04-08 16:17 ` [PATCH v3 27/32] arm64: KVM: CPU specific 32bit coprocessor access Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 28/32] arm64: KVM: 32bit specific register world switch Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 29/32] arm64: KVM: 32bit guest fault injection Marc Zyngier
2013-04-23 23:02   ` Christoffer Dall
2013-04-24 13:46     ` Marc Zyngier
2013-04-24 17:15       ` Christoffer Dall
2013-04-08 16:17 ` [PATCH v3 30/32] arm64: KVM: enable initialization of a 32bit vcpu Marc Zyngier
2013-04-23 23:02   ` Christoffer Dall
2013-04-24 13:49     ` Marc Zyngier
2013-04-24 17:17       ` Christoffer Dall
2013-05-07 16:36         ` Marc Zyngier [this message]
2013-05-11  0:38           ` Christoffer Dall
2013-05-11  8:04             ` Peter Maydell
2013-05-11 16:26               ` Christoffer Dall
2013-05-11 16:31                 ` Peter Maydell
2013-05-13  9:01             ` Marc Zyngier
2013-05-13 15:46               ` Christoffer Dall
2013-04-08 16:17 ` [PATCH v3 31/32] arm64: KVM: userspace API documentation Marc Zyngier
2013-04-23 23:02   ` Christoffer Dall
2013-04-24 13:52     ` Marc Zyngier
2013-04-08 16:17 ` [PATCH v3 32/32] arm64: KVM: MAINTAINERS update Marc Zyngier
2013-04-23 23:04 ` [PATCH v3 00/32] Port of KVM to arm64 Christoffer Dall
2013-05-03 13:17 ` Catalin Marinas

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=51892DA4.4050401@arm.com \
    --to=marc.zyngier@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).