From: Miguel Luis <miguel.luis@oracle.com>
To: Eric Auger <eauger@redhat.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will@kernel.org>, Marc Zyngier <maz@kernel.org>,
Oliver Upton <oliver.upton@linux.dev>,
James Morse <james.morse@arm.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
Zenghui Yu <yuzenghui@huawei.com>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, kvmarm@lists.linux.dev
Subject: Re: [PATCH v2 2/2] arm64/kvm: Fine grain _EL2 system registers list that affect nested virtualization
Date: Mon, 2 Oct 2023 13:36:50 +0000 [thread overview]
Message-ID: <52f36393-c764-4b9a-b153-1d34126739b2@oracle.com> (raw)
In-Reply-To: <c01525e8-0906-6990-19b9-df374fdb087b@redhat.com>
Hi Eric,
On 29/09/2023 15:08, Eric Auger wrote:
> Hi Miguel,
> On 9/25/23 18:20, Miguel Luis wrote:
>> Some _EL1 registers got included in the _EL2 ranges, which are not
> if they aren't too many, you may list them as it eases the review
Thanks for bringing it up.
Initially I thought those _EL1 registers would be ESR_EL1, TFSR_EL1 and FAR_EL1,
but as I re-run through the process I cannot confirm the statement anymore.
So that statement is a mistake now?
I took as reference Table D18-2 on page D18-6307 where are listed instruction
encodings for non-debug system register accesses. Having to deal with the
document format is surely not an easy task, so I converted it to text using
pdftotext -layout.
After scraping, the end result is a table of encodings which we're allowed to
sort/grep which may be handy to this when you consider the statement that all
accesses (but the exceptions) to system registers ending in _EL2 should trap.
>> affected by NV. Remove them, fine grain the ranges to exclusively
>> include the _EL2 ones and fold SPSR/ELR _EL2 registers into the
>> existing range.
>>
>> Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
> Fixes: d0fc0a2519a6 (" KVM: arm64: nv: Add trap forwarding for HCR_EL2") ?
OK.
>> ---
>> arch/arm64/kvm/emulate-nested.c | 44 ++++++++++++++++++++++++++++-----
>> 1 file changed, 38 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
>> index 9ced1bf0c2b7..f6d0c87803f4 100644
>> --- a/arch/arm64/kvm/emulate-nested.c
>> +++ b/arch/arm64/kvm/emulate-nested.c
>> @@ -649,14 +649,46 @@ static const struct encoding_to_trap_config encoding_to_cgt[] __initconst = {
>> SR_TRAP(SYS_APGAKEYHI_EL1, CGT_HCR_APK),
>> /* All _EL2 registers */
>> SR_RANGE_TRAP(sys_reg(3, 4, 0, 0, 0),
>> - sys_reg(3, 4, 3, 15, 7), CGT_HCR_NV),
>> + sys_reg(3, 4, 4, 0, 1), CGT_HCR_NV),
>> /* Skip the SP_EL1 encoding... */
>> - SR_TRAP(SYS_SPSR_EL2, CGT_HCR_NV),
>> - SR_TRAP(SYS_ELR_EL2, CGT_HCR_NV),
>> - SR_RANGE_TRAP(sys_reg(3, 4, 4, 1, 1),
>> - sys_reg(3, 4, 10, 15, 7), CGT_HCR_NV),
> I am not sure I fully understand the sysreg encoding but globally there
> are not so many _EL2 regs trapped with .NV. And I can see holes within
> somes ranges defined above (I searched all "if EL2Enabled() &&
> HCR_EL2.NV == '1' then" in the ARM ARM). Maybe I don't know how to use
> the ARM ARM doc but I feel difficult to understand if the "holes"
> within the encoding of some ranges are unused or are allocated to some
> other sysregs, which wouldn't be trapped by /NV. I fear range encoding
> may be quite risky.
That's definitely fair and I share the same concerns too.
Having table D18-2 sorted helped defining those ranges although I did not
find the answer to those questions. Perhaps we could query for assumptions
on the desired approach in which such implementation would rely.
>> + SR_RANGE_TRAP(sys_reg(3, 4, 4, 3, 0),
>> + sys_reg(3, 4, 10, 6, 7), CGT_HCR_NV),
>> + /*
>> + * Note that the spec. describes a group of MEC registers
>> + * whose access should not trap, therefore skip the following:
>> + * MECID_A0_EL2, MECID_A1_EL2, MECID_P0_EL2,
>> + * MECID_P1_EL2, MECIDR_EL2, VMECID_A_EL2,
>> + * VMECID_P_EL2.
>> + */
>> SR_RANGE_TRAP(sys_reg(3, 4, 12, 0, 0),
>> - sys_reg(3, 4, 14, 15, 7), CGT_HCR_NV),
>> + sys_reg(3, 4, 12, 1, 1), CGT_HCR_NV),
>> + /* ICH_AP0R<m>_EL2 */
>> + SR_RANGE_TRAP(SYS_ICH_AP0R0_EL2,
>> + SYS_ICH_AP0R3_EL2, CGT_HCR_NV),
>> + /* ICH_AP1R<m>_EL2 */
>> + SR_RANGE_TRAP(SYS_ICH_AP1R0_EL2,
>> + SYS_ICH_AP1R3_EL2, CGT_HCR_NV),
>> + SR_RANGE_TRAP(sys_reg(3, 4, 12, 9, 5),
>> + sys_reg(3, 4, 12, 11, 7), CGT_HCR_NV),
>> + /* ICH_LR<m>_EL2 */
>> + SR_RANGE_TRAP(SYS_ICH_LR0_EL2,
>> + SYS_ICH_LR7_EL2, CGT_HCR_NV),
>> + SR_RANGE_TRAP(SYS_ICH_LR8_EL2,
>> + SYS_ICH_LR15_EL2, CGT_HCR_NV),
>> + SR_RANGE_TRAP(sys_reg(3, 4, 13, 0, 1),
>> + sys_reg(3, 4, 13, 0, 7), CGT_HCR_NV),
>> + /* AMEVCNTVOFF0<n>_EL2 */
>> + SR_RANGE_TRAP(sys_reg(3, 4, 13, 8, 0),
>> + sys_reg(3, 4, 13, 8, 7), CGT_HCR_NV),
>> + SR_RANGE_TRAP(sys_reg(3, 4, 13, 9, 0),
>> + sys_reg(3, 4, 13, 9, 7), CGT_HCR_NV),
> I think those 2 above ranges can be merged
Oh, indeed. For both AMEVCNTVOFF0<n>_EL2 and AMEVCNTVOFF1<n>_EL2.
>> + /* AMEVCNTVOFF1<n>_EL2 */
>> + SR_RANGE_TRAP(sys_reg(3, 4, 13, 10, 0),
>> + sys_reg(3, 4, 13, 10, 7), CGT_HCR_NV),
>> + SR_RANGE_TRAP(sys_reg(3, 4, 13, 11, 0),
>> + sys_reg(3, 4, 13, 11, 7), CGT_HCR_NV),
> /* CNT*_EL2 */
OK.
>> + SR_RANGE_TRAP(sys_reg(3, 4, 14, 0, 3),
>> + sys_reg(3, 4, 14, 5, 2), CGT_HCR_NV),
>> /* All _EL02, _EL12 registers */
>> SR_RANGE_TRAP(sys_reg(3, 5, 0, 0, 0),
>> sys_reg(3, 5, 10, 15, 7), CGT_HCR_NV),
> not related to your patch but wrt the EL02 the only ones that I
> idenftied beeing trapped by NV using above search are
>
> CNTP_TVAL_EL02 3 5 14 2 0
> CNTP_CTL_EL02 3 5 14 2 1
> CNTP_CVAL_EL02 3 5 14 2 2
> CNTV_TVAL_EL02 3 5 14 3 0
> CNTV_CTL_EL02 3 5 14 3 1
> CNTV_CVAL_EL02 3 5 14 3 2
>
That matches my search too. FWIW, below are the _EL12 from my search:
AFSR0_EL12 3 5 5 1 0
AFSR1_EL12 3 5 5 1 1
AMAIR_EL12 3 5 5 3 0
CONTEXTIDR_EL12 3 5 13 0 1
CPACR_EL12 3 5 1 0 2
ESR_EL12 3 5 5 2 0
FAR_EL12 3 5 6 0 0
MAIR_EL12 3 5 10 2 0
SCTLR2_EL12 3 5 1 0 3
SCTLR_EL12 3 5 1 0 0
SMCR_EL12 3 5 1 2 6
TCR2_EL12 3 5 2 0 3
TCR_EL12 3 5 2 0 2
TFSR_EL12 3 5 5 6 0
TTBR0_EL12 3 5 2 0 0
TTBR1_EL12 3 5 2 0 1
VBAR_EL12 3 5 12 0 0
ZCR_EL12 3 5 1 2 0
TRFCR_EL12 3 5 1 2 1
PMSCR_EL12 3 5 9 9 0
CNTKCTL_EL12 3 5 14 1 0
Thanks
Miguel
> Thanks
>
> Eric
>
prev parent reply other threads:[~2023-10-02 13:37 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-25 16:20 [PATCH v2 0/2] Fine grain sysregs allowed to trap for nested virtualization Miguel Luis
2023-09-25 16:20 ` [PATCH v2 1/2] arm64: Add missing _EL12 encodings Miguel Luis
2023-09-28 9:39 ` Eric Auger
2023-09-28 9:59 ` Miguel Luis
2023-09-25 16:20 ` [PATCH v2 2/2] arm64/kvm: Fine grain _EL2 system registers list that affect nested virtualization Miguel Luis
2023-09-29 15:08 ` Eric Auger
2023-10-02 13:36 ` Miguel Luis [this message]
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=52f36393-c764-4b9a-b153-1d34126739b2@oracle.com \
--to=miguel.luis@oracle.com \
--cc=catalin.marinas@arm.com \
--cc=eauger@redhat.com \
--cc=james.morse@arm.com \
--cc=kvmarm@lists.linux.dev \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maz@kernel.org \
--cc=oliver.upton@linux.dev \
--cc=suzuki.poulose@arm.com \
--cc=will@kernel.org \
--cc=yuzenghui@huawei.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 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).