From: Marc Zyngier <maz@kernel.org>
To: James Morse <james.morse@arm.com>
Cc: stable@vger.kernel.org, kvmarm@lists.cs.columbia.edu,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/3] KVM: arm64: Stop writing aarch32's CSSELR into ACTLR
Date: Thu, 28 May 2020 13:10:23 +0100 [thread overview]
Message-ID: <705687e37c5d5339a6baafa9e31675cb@kernel.org> (raw)
In-Reply-To: <09dca8e9-c548-43fd-a95b-747a77f19e02@arm.com>
On 2020-05-28 12:59, James Morse wrote:
> Hi Marc,
>
> On 28/05/2020 09:57, Marc Zyngier wrote:
>> On 2020-05-26 17:18, James Morse wrote:
>>> access_csselr() uses the 32bit r->reg value to access the 64bit
>>> array,
>>> so reads and write the wrong value. sys_regs[4], is ACTLR_EL1, which
>>> is subsequently save/restored when we enter the guest.
>
>>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>>> index 51db934702b6..2eda539f3281 100644
>>> --- a/arch/arm64/kvm/sys_regs.c
>>> +++ b/arch/arm64/kvm/sys_regs.c
>>> @@ -2060,7 +2060,7 @@ static const struct sys_reg_desc cp15_regs[] =
>>> {
>>>
>>> { Op1(1), CRn( 0), CRm( 0), Op2(0), access_ccsidr },
>>> { Op1(1), CRn( 0), CRm( 0), Op2(1), access_clidr },
>>> - { Op1(2), CRn( 0), CRm( 0), Op2(0), access_csselr, NULL,
>>> c0_CSSELR },
>>> + { Op1(2), CRn( 0), CRm( 0), Op2(0), access_csselr_el1, NULL,
>>> CSSELR_EL1 },
>>> };
>
>> This is a departure from the way we deal with 32bit CP15 registers.
>> We deal with this exact issue in a very different way for other
>> CP15 regs, by adjusting the index in the sys_regs array (see the
>> way we handle the VM regs).
>>
>> How about something like this (untested):
>
> [like access_vm_reg() does]
>
> Sure, I'll give that a test and re-post it.
Thanks!
>
>
>> Ideally, I'd like the core sys_reg code to deal with this sort
>> of funnies, but I'm trying to keep the change minimal...
>
> Roll this '/2' and upper/lower bits stuff into a vcpu_write_cp15_reg()
> that calls
> vcpu_write_sys_reg()? (/me hunts out the todo list)
I was thinking of hiding it differently: in emulate_cp, substitute the
sys_reg_desc structure for a temporary one that represents the 64bit
version, and make it completely transparent.
I'm sure there is a couple of nits around that though...
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
WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: James Morse <james.morse@arm.com>
Cc: stable@vger.kernel.org,
Julien Thierry <julien.thierry.kdev@gmail.com>,
kvmarm@lists.cs.columbia.edu,
linux-arm-kernel@lists.infradead.org,
Suzuki K Poulose <suzuki.poulose@arm.com>
Subject: Re: [PATCH 1/3] KVM: arm64: Stop writing aarch32's CSSELR into ACTLR
Date: Thu, 28 May 2020 13:10:23 +0100 [thread overview]
Message-ID: <705687e37c5d5339a6baafa9e31675cb@kernel.org> (raw)
In-Reply-To: <09dca8e9-c548-43fd-a95b-747a77f19e02@arm.com>
On 2020-05-28 12:59, James Morse wrote:
> Hi Marc,
>
> On 28/05/2020 09:57, Marc Zyngier wrote:
>> On 2020-05-26 17:18, James Morse wrote:
>>> access_csselr() uses the 32bit r->reg value to access the 64bit
>>> array,
>>> so reads and write the wrong value. sys_regs[4], is ACTLR_EL1, which
>>> is subsequently save/restored when we enter the guest.
>
>>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>>> index 51db934702b6..2eda539f3281 100644
>>> --- a/arch/arm64/kvm/sys_regs.c
>>> +++ b/arch/arm64/kvm/sys_regs.c
>>> @@ -2060,7 +2060,7 @@ static const struct sys_reg_desc cp15_regs[] =
>>> {
>>>
>>> { Op1(1), CRn( 0), CRm( 0), Op2(0), access_ccsidr },
>>> { Op1(1), CRn( 0), CRm( 0), Op2(1), access_clidr },
>>> - { Op1(2), CRn( 0), CRm( 0), Op2(0), access_csselr, NULL,
>>> c0_CSSELR },
>>> + { Op1(2), CRn( 0), CRm( 0), Op2(0), access_csselr_el1, NULL,
>>> CSSELR_EL1 },
>>> };
>
>> This is a departure from the way we deal with 32bit CP15 registers.
>> We deal with this exact issue in a very different way for other
>> CP15 regs, by adjusting the index in the sys_regs array (see the
>> way we handle the VM regs).
>>
>> How about something like this (untested):
>
> [like access_vm_reg() does]
>
> Sure, I'll give that a test and re-post it.
Thanks!
>
>
>> Ideally, I'd like the core sys_reg code to deal with this sort
>> of funnies, but I'm trying to keep the change minimal...
>
> Roll this '/2' and upper/lower bits stuff into a vcpu_write_cp15_reg()
> that calls
> vcpu_write_sys_reg()? (/me hunts out the todo list)
I was thinking of hiding it differently: in emulate_cp, substitute the
sys_reg_desc structure for a temporary one that represents the 64bit
version, and make it completely transparent.
I'm sure there is a couple of nits around that though...
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: James Morse <james.morse@arm.com>
Cc: kvmarm@lists.cs.columbia.edu,
linux-arm-kernel@lists.infradead.org,
Julien Thierry <julien.thierry.kdev@gmail.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
stable@vger.kernel.org
Subject: Re: [PATCH 1/3] KVM: arm64: Stop writing aarch32's CSSELR into ACTLR
Date: Thu, 28 May 2020 13:10:23 +0100 [thread overview]
Message-ID: <705687e37c5d5339a6baafa9e31675cb@kernel.org> (raw)
In-Reply-To: <09dca8e9-c548-43fd-a95b-747a77f19e02@arm.com>
On 2020-05-28 12:59, James Morse wrote:
> Hi Marc,
>
> On 28/05/2020 09:57, Marc Zyngier wrote:
>> On 2020-05-26 17:18, James Morse wrote:
>>> access_csselr() uses the 32bit r->reg value to access the 64bit
>>> array,
>>> so reads and write the wrong value. sys_regs[4], is ACTLR_EL1, which
>>> is subsequently save/restored when we enter the guest.
>
>>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>>> index 51db934702b6..2eda539f3281 100644
>>> --- a/arch/arm64/kvm/sys_regs.c
>>> +++ b/arch/arm64/kvm/sys_regs.c
>>> @@ -2060,7 +2060,7 @@ static const struct sys_reg_desc cp15_regs[] =
>>> {
>>>
>>> { Op1(1), CRn( 0), CRm( 0), Op2(0), access_ccsidr },
>>> { Op1(1), CRn( 0), CRm( 0), Op2(1), access_clidr },
>>> - { Op1(2), CRn( 0), CRm( 0), Op2(0), access_csselr, NULL,
>>> c0_CSSELR },
>>> + { Op1(2), CRn( 0), CRm( 0), Op2(0), access_csselr_el1, NULL,
>>> CSSELR_EL1 },
>>> };
>
>> This is a departure from the way we deal with 32bit CP15 registers.
>> We deal with this exact issue in a very different way for other
>> CP15 regs, by adjusting the index in the sys_regs array (see the
>> way we handle the VM regs).
>>
>> How about something like this (untested):
>
> [like access_vm_reg() does]
>
> Sure, I'll give that a test and re-post it.
Thanks!
>
>
>> Ideally, I'd like the core sys_reg code to deal with this sort
>> of funnies, but I'm trying to keep the change minimal...
>
> Roll this '/2' and upper/lower bits stuff into a vcpu_write_cp15_reg()
> that calls
> vcpu_write_sys_reg()? (/me hunts out the todo list)
I was thinking of hiding it differently: in emulate_cp, substitute the
sys_reg_desc structure for a temporary one that represents the 64bit
version, and make it completely transparent.
I'm sure there is a couple of nits around that though...
M.
--
Jazz is not dead. It just smells funny...
next prev parent reply other threads:[~2020-05-28 12:10 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-26 16:18 [PATCH 0/3] KVM: arm64: aarch32 ACTLR accesses James Morse
2020-05-26 16:18 ` James Morse
2020-05-26 16:18 ` [PATCH 1/3] KVM: arm64: Stop writing aarch32's CSSELR into ACTLR James Morse
2020-05-26 16:18 ` James Morse
2020-05-26 16:18 ` James Morse
2020-05-27 16:57 ` Sasha Levin
2020-05-27 16:57 ` Sasha Levin
2020-05-27 16:57 ` Sasha Levin
2020-05-28 8:57 ` Marc Zyngier
2020-05-28 8:57 ` Marc Zyngier
2020-05-28 8:57 ` Marc Zyngier
2020-05-28 11:59 ` James Morse
2020-05-28 11:59 ` James Morse
2020-05-28 11:59 ` James Morse
2020-05-28 12:10 ` Marc Zyngier [this message]
2020-05-28 12:10 ` Marc Zyngier
2020-05-28 12:10 ` Marc Zyngier
2020-05-26 16:18 ` [PATCH 2/3] KVM: arm64: Stop save/restoring ACTLR_EL1 James Morse
2020-05-26 16:18 ` James Morse
2020-05-28 12:36 ` Marc Zyngier
2020-05-28 12:36 ` Marc Zyngier
2020-05-28 12:38 ` Marc Zyngier
2020-05-28 12:38 ` Marc Zyngier
2020-05-28 12:55 ` James Morse
2020-05-28 12:55 ` James Morse
2020-05-26 16:18 ` [PATCH 3/3] KVM: arm64: Add emulation for 32bit guests accessing ACTLR2 James Morse
2020-05-26 16:18 ` James Morse
2020-05-28 12:51 ` Marc Zyngier
2020-05-28 12:51 ` Marc Zyngier
2020-05-31 13:37 ` [PATCH 0/3] KVM: arm64: aarch32 ACTLR accesses Marc Zyngier
2020-05-31 13:37 ` Marc Zyngier
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=705687e37c5d5339a6baafa9e31675cb@kernel.org \
--to=maz@kernel.org \
--cc=james.morse@arm.com \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=stable@vger.kernel.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 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.