Re: [PATCH v14 16/39] arm64/sme: Implement traps and syscall handling for SME
On 2022/12/7 22:16, Mark Brown wrote: On Wed, Dec 07, 2022 at 10:00:17PM +0800, Zenghui Yu wrote: On 2022/4/19 19:22, Mark Brown wrote: + /* +* If SME is active then exit streaming mode. If ZA is active +* then flush the SVE registers but leave userspace access to +* both SVE and SME enabled, otherwise disable SME for the +* task and fall through to disabling SVE too. This means It looks a bit confusing to me that in the current implementation, if ZA is *not* active, we don't actually go to disable SME for the task (which IMHO can be disabled as user-space is not actively using it now). Unlike SVE there's no overhead from leaving SME enabled, the enable bits for SM and ZA tell us if there is extra register state to be saved so we don't have to worry about the costs there like we do with SVE. The only reason for not just unconditionally enabling SME is the potentially large backing storage required for the registers, if a task isn't using SME there's no need to impose that overhead. If we disable SME for userspace after the storage has been allocated then we just require an additional trip through EL1 to reenable it for any future usage which would hurt performance but not really gain us anything otherwise. We don't want to discurage applications from disabling ZA when not in use given that there's likely to be physical overheads from keeping it enabled. Ah, thanks a lot for the explanations. The comment itself can be improved though (I think). + if (svcr & SYS_SVCR_EL0_SM_MASK) + sme_smstop_sm(); As per the SME syscall ABI | On syscall PSTATE.SM will be cleared and the SVE registers will be | handled as per the standard SVE ABI. and the SVE syscall ABI | On syscall, V0..V31 are preserved (as without SVE). Thus, bits | [127:0] of Z0..Z31 are preserved. All other bits of Z0..Z31, and all | of P0..P15 and FFR become zero on return from a syscall. Can we infer from the documentation that V0-V31 should be preserved on return from a syscall? But with sme_smstop_sm(), all implemented bits of Z0-Z31 are set to zero by hardware. Is this intentional? Please fix me up if I've mis-read things here. No, the intention is to say that we exit streaming mode and then handle things as per the non-streaming ABI. Exiting streaming mode has the effect of clearing the values as you say. Thanks, Zenghui ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 3/4] KVM: arm64: selftests: Align VA space allocator with TTBR0
On Thu, Dec 08, 2022, Oliver Upton wrote:
> On Thu, Dec 08, 2022 at 12:18:07AM +, Sean Christopherson wrote:
>
> [...]
>
> > Together, what about? The #ifdef is a bit gross, especially around
> > "hi_start",
> > but it's less duplicate code. And IMO, having things bundled in the same
> > place
> > makes it a lot easier for newbies (to arm64 or kernel coding in general) to
> > understand what's going on and why arm64 is different.
>
> I'd rather we not go this route. We really shouldn't make any attempt to
> de-dupe something that is inherently architecture specific.
>
> For example:
>
> > + /*
> > +* All architectures supports splitting the virtual address space into
> > +* a high and a low half. Populate both halves, except for arm64 which
> > +* currently uses only TTBR0_EL1 (arbitrary selftests "logic"), i.e.
> > +* only has a valid low half.
> > +*/
> > + sparsebit_num_t nr_va_bits = (1ULL << (vm->va_bits - 1)) >>
> > vm->page_shift;
>
> This is still wrong for arm64. When we say the VA space is 48 bits, we
> really do mean that TTBR0 is able to address a full 48 bits. So this
> truncates the MSB for the addressing mode.
Ah, I missed the lack of a "-1" in the arm64 code.
> With the code living in the arm64 side of the shop, I can also tailor
> the comment to directly match the architecture to provide breadcrumbs
> tying it back to the Arm ARM.
The main reason why I don't like splitting the code this way is that it makes it
harder for non-arm64 folks to understand what makes arm64 different. Case in
point, my overlooking of the "-1". I read the changelog and the comment and
still missed that small-but-important detail, largely because I am completely
unfamiliar with how TTBR{0,1}_EL1 works.
Actually, before we do anything, we should get confirmation from the s390 and
RISC-V folks on whether they have a canonical hole like x86, i.e. maybe x86 is
the oddball.
Anyways, assuming one architecture is the oddball (I'm betting it's x86), I have
no objection to bleeding some of the details into the common code, including a
large comment to document the gory details. If every architecture manges to be
different, then yeah, a hook is probably warranted.
That said, I also don't mind shoving a bit of abstraction into arch code if that
avoids some #ifdef ugliness or allows for better documentation, flexibility,
etc.
What I don't like is duplicating the logic of turning "VA bits" into the bitmap.
E.g. something like this would also be an option. Readers would obviously need
to track down has_split_va_space, but that should be fairly easy and can come
with a big arch-specific comment, and meanwhile the core logic of how selftests
populate the va bitmaps is common.
Or if arm64 is the only arch without a split, invert the flag and have arm64 set
the vm->has_combined_va_space or whatever.
static void vm_vaddr_populate_bitmap(struct kvm_vm *vm)
{
unsigned int eff_va_bits = vm->va_bits;
sparsebit_num_t nr_bits;
/* blah blah blah */
if (vm->has_split_va_space)
eff_va_bits--;
nr_bits = (1ULL << eff_va_bits) >> vm->page_shift;
sparsebit_set_num(vm->vpages_valid, 0, nr_va_bits);
if (vm->has_split_va_space)
sparsebit_set_num(vm->vpages_valid,
(~((1ULL << eff_va_bits) - 1)) >>
vm->page_shift,
nr_bits);
}
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 2/4] KVM: selftests: Setup ucall after loading program into guest memory
On Thu, Dec 08, 2022 at 12:24:20AM +, Sean Christopherson wrote:
> On Thu, Dec 08, 2022, Oliver Upton wrote:
> > On Wed, Dec 07, 2022 at 11:57:27PM +, Sean Christopherson wrote:
> > > > diff --git a/tools/testing/selftests/kvm/aarch64/page_fault_test.c
> > > > b/tools/testing/selftests/kvm/aarch64/page_fault_test.c
> > > > index 92d3a91153b6..95d22cfb7b41 100644
> > > > --- a/tools/testing/selftests/kvm/aarch64/page_fault_test.c
> > > > +++ b/tools/testing/selftests/kvm/aarch64/page_fault_test.c
> > > > @@ -609,8 +609,13 @@ static void setup_memslots(struct kvm_vm *vm,
> > > > struct test_params *p)
> > > > data_size / guest_page_size,
> > > > p->test_desc->data_memslot_flags);
> > > > vm->memslots[MEM_REGION_TEST_DATA] = TEST_DATA_MEMSLOT;
> > > > +}
> > > > +
> > > > +static void setup_ucall(struct kvm_vm *vm)
> > > > +{
> > > > + struct userspace_mem_region *region = vm_get_mem_region(vm,
> > > > MEM_REGION_TEST_DATA);
> > > >
> > > > - ucall_init(vm, data_gpa + data_size);
> > > > + ucall_init(vm, region->region.guest_phys_addr +
> > > > region->region.memory_size);
> > >
> > > Isn't there a hole after CODE_AND_DATA_MEMSLOT? I.e. after memslot 0?
> >
> > Sure, but that's only guaranteed in the PA space.
> >
> > > The reason
> > > I ask is because if so, then we can do the temporarily heinous, but
> > > hopefully forward
> > > looking thing of adding a helper to wrap kvm_vm_elf_load() + ucall_init().
> > >
> > > E.g. I think we can do this immediately, and then at some point in the
> > > 6.2 cycle
> > > add a dedicated region+memslot for the ucall MMIO page.
> >
> > Even still, that's just a kludge to make ucalls work. We have other
> > MMIO devices (GIC distributor, for example) that work by chance since
> > nothing conflicts with the constant GPAs we've selected in the tests.
> >
> > I'd rather we go down the route of having an address allocator for the
> > for both the VA and PA spaces to provide carveouts at runtime.
>
> Aren't those two separate issues? The PA, a.k.a. memslots space, can be
> solved
> by allocating a dedicated memslot, i.e. doesn't need a carve. At worst,
> collisions
> will yield very explicit asserts, which IMO is better than whatever might go
> wrong
> with a carve out.
Perhaps the use of the term 'carveout' wasn't right here.
What I'm suggesting is we cannot rely on KVM memslots alone to act as an
allocator for the PA space. KVM can provide devices to the guest that
aren't represented as memslots. If we're trying to fix PA allocations
anyway, why not make it generic enough to suit the needs of things
beyond ucalls?
--
Thanks,
Oliver
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 3/4] KVM: arm64: selftests: Align VA space allocator with TTBR0
On Thu, Dec 08, 2022 at 12:18:07AM +, Sean Christopherson wrote: [...] > Together, what about? The #ifdef is a bit gross, especially around > "hi_start", > but it's less duplicate code. And IMO, having things bundled in the same > place > makes it a lot easier for newbies (to arm64 or kernel coding in general) to > understand what's going on and why arm64 is different. I'd rather we not go this route. We really shouldn't make any attempt to de-dupe something that is inherently architecture specific. For example: > + /* > + * All architectures supports splitting the virtual address space into > + * a high and a low half. Populate both halves, except for arm64 which > + * currently uses only TTBR0_EL1 (arbitrary selftests "logic"), i.e. > + * only has a valid low half. > + */ > + sparsebit_num_t nr_va_bits = (1ULL << (vm->va_bits - 1)) >> > vm->page_shift; This is still wrong for arm64. When we say the VA space is 48 bits, we really do mean that TTBR0 is able to address a full 48 bits. So this truncates the MSB for the addressing mode. With the code living in the arm64 side of the shop, I can also tailor the comment to directly match the architecture to provide breadcrumbs tying it back to the Arm ARM. -- Thanks, Oliver ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 2/4] KVM: selftests: Setup ucall after loading program into guest memory
On Thu, Dec 08, 2022, Oliver Upton wrote:
> On Wed, Dec 07, 2022 at 11:57:27PM +, Sean Christopherson wrote:
> > > diff --git a/tools/testing/selftests/kvm/aarch64/page_fault_test.c
> > > b/tools/testing/selftests/kvm/aarch64/page_fault_test.c
> > > index 92d3a91153b6..95d22cfb7b41 100644
> > > --- a/tools/testing/selftests/kvm/aarch64/page_fault_test.c
> > > +++ b/tools/testing/selftests/kvm/aarch64/page_fault_test.c
> > > @@ -609,8 +609,13 @@ static void setup_memslots(struct kvm_vm *vm, struct
> > > test_params *p)
> > > data_size / guest_page_size,
> > > p->test_desc->data_memslot_flags);
> > > vm->memslots[MEM_REGION_TEST_DATA] = TEST_DATA_MEMSLOT;
> > > +}
> > > +
> > > +static void setup_ucall(struct kvm_vm *vm)
> > > +{
> > > + struct userspace_mem_region *region = vm_get_mem_region(vm,
> > > MEM_REGION_TEST_DATA);
> > >
> > > - ucall_init(vm, data_gpa + data_size);
> > > + ucall_init(vm, region->region.guest_phys_addr +
> > > region->region.memory_size);
> >
> > Isn't there a hole after CODE_AND_DATA_MEMSLOT? I.e. after memslot 0?
>
> Sure, but that's only guaranteed in the PA space.
>
> > The reason
> > I ask is because if so, then we can do the temporarily heinous, but
> > hopefully forward
> > looking thing of adding a helper to wrap kvm_vm_elf_load() + ucall_init().
> >
> > E.g. I think we can do this immediately, and then at some point in the 6.2
> > cycle
> > add a dedicated region+memslot for the ucall MMIO page.
>
> Even still, that's just a kludge to make ucalls work. We have other
> MMIO devices (GIC distributor, for example) that work by chance since
> nothing conflicts with the constant GPAs we've selected in the tests.
>
> I'd rather we go down the route of having an address allocator for the
> for both the VA and PA spaces to provide carveouts at runtime.
Aren't those two separate issues? The PA, a.k.a. memslots space, can be solved
by allocating a dedicated memslot, i.e. doesn't need a carve. At worst,
collisions
will yield very explicit asserts, which IMO is better than whatever might go
wrong
with a carve out.
> There's another issue with the new ucall implementation where identity
> mapping could stomp on a program segment that I'm fighting with right now
> which only further highlights the problems with our (mis)management of
> address spaces in selftests.
Oooh, this crud:
virt_pg_map(vm, mmio_gpa, mmio_gpa);
Yeah, that needs to be fixed. But again, that's a separate issue, e.g.
selftests
can allocate a virtual address and map the read-only memslot.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 3/4] KVM: arm64: selftests: Align VA space allocator with TTBR0
On Wed, Dec 07, 2022, Oliver Upton wrote:
> diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c
> b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> index 316de70db91d..5972a23b2765 100644
> --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
> +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> @@ -541,3 +541,13 @@ void kvm_selftest_arch_init(void)
>*/
> guest_modes_append_default();
> }
> +
> +void vm_vaddr_populate_bitmap(struct kvm_vm *vm)
Add "arch" so that it's obvious this can be overidden? The "__weak" conveys
that
for the implementation, but not for the call site. E.g.
vm_arch_vaddr_populate_bitmap().
Actually, IIUC, the issue is that the high half isn't mapped (probably the wrong
terminology). I.e. the calculation for the low half stays the same, and the
high
half just goes away.
> +{
> + /*
> + * arm64 selftests use only TTBR0_EL1, meaning that the valid VA space
> + * is [0, 2^(64 - TCR_EL1.T0SZ)).
> + */
> + sparsebit_set_num(vm->vpages_valid, 0,
> + (1ULL << vm->va_bits) >> vm->page_shift);
> +}
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c
> b/tools/testing/selftests/kvm/lib/kvm_util.c
> index e9607eb089be..c88c3ace16d2 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -186,6 +186,15 @@ const struct vm_guest_mode_params vm_guest_mode_params[]
> = {
> _Static_assert(sizeof(vm_guest_mode_params)/sizeof(struct
> vm_guest_mode_params) == NUM_VM_MODES,
> "Missing new mode params?");
>
> +__weak void vm_vaddr_populate_bitmap(struct kvm_vm *vm)
> +{
> + sparsebit_set_num(vm->vpages_valid,
> + 0, (1ULL << (vm->va_bits - 1)) >> vm->page_shift);
> + sparsebit_set_num(vm->vpages_valid,
> + (~((1ULL << (vm->va_bits - 1)) - 1)) >> vm->page_shift,
> + (1ULL << (vm->va_bits - 1)) >> vm->page_shift);
Any objection to fixing up the formatting? Actually, we can do more than just
fix the indentation, e.g. the number of bits is identical, and documenting that
this does a high/low split would be helpful.
Together, what about? The #ifdef is a bit gross, especially around "hi_start",
but it's less duplicate code. And IMO, having things bundled in the same place
makes it a lot easier for newbies (to arm64 or kernel coding in general) to
understand what's going on and why arm64 is different.
---
tools/testing/selftests/kvm/lib/kvm_util.c | 23 +-
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c
b/tools/testing/selftests/kvm/lib/kvm_util.c
index e9607eb089be..d6f2c17e3d40 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -186,6 +186,23 @@ const struct vm_guest_mode_params vm_guest_mode_params[] =
{
_Static_assert(sizeof(vm_guest_mode_params)/sizeof(struct
vm_guest_mode_params) == NUM_VM_MODES,
"Missing new mode params?");
+static void vm_vaddr_populate_bitmap(struct kvm_vm *vm)
+{
+ /*
+* All architectures supports splitting the virtual address space into
+* a high and a low half. Populate both halves, except for arm64 which
+* currently uses only TTBR0_EL1 (arbitrary selftests "logic"), i.e.
+* only has a valid low half.
+*/
+ sparsebit_num_t nr_va_bits = (1ULL << (vm->va_bits - 1)) >>
vm->page_shift;
+#ifndef __aarch64__
+ sparsebit_num_t hi_start = (~((1ULL << (vm->va_bits - 1)) - 1)) >>
vm->page_shift
+
+ sparsebit_set_num(vm->vpages_valid, hi_start, nr_bits);
+#endif
+ sparsebit_set_num(vm->vpages_valid, 0, nr_va_bits);
+}
+
struct kvm_vm *vm_create(enum vm_guest_mode mode)
{
struct kvm_vm *vm;
@@ -274,11 +291,7 @@ struct kvm_vm *vm_create(enum vm_guest_mode mode)
/* Limit to VA-bit canonical virtual addresses. */
vm->vpages_valid = sparsebit_alloc();
- sparsebit_set_num(vm->vpages_valid,
- 0, (1ULL << (vm->va_bits - 1)) >> vm->page_shift);
- sparsebit_set_num(vm->vpages_valid,
- (~((1ULL << (vm->va_bits - 1)) - 1)) >> vm->page_shift,
- (1ULL << (vm->va_bits - 1)) >> vm->page_shift);
+ vm_vaddr_populate_bitmap(vm);
/* Limit physical addresses to PA-bits. */
vm->max_gfn = vm_compute_max_gfn(vm);
base-commit: 35aecc3289eebf193fd70a067ea448ae2f0bb9b9
--
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 2/4] KVM: selftests: Setup ucall after loading program into guest memory
On Wed, Dec 07, 2022 at 11:57:27PM +, Sean Christopherson wrote:
> On Wed, Dec 07, 2022, Oliver Upton wrote:
> > The new ucall infrastructure needs to update a couple of guest globals
> > to pass through the ucall MMIO addr and pool of ucall structs. A
> > precondition of this actually working is to have the program image
> > already loaded into guest memory.
>
> Ouch. Might be worth explicitly stating what goes wrong. Even though it's
> super
> obvious in hindsight, it still took me a few seconds to understand what
> precondition you were referring to, e.g. I was trying to figure out how
> selecting
> the MMIO address depended on the guest code being loaded...
>
> >
> > Call ucall_init() after kvm_vm_elf_load(). Continue to park the ucall
> > MMIO addr after MEM_REGION_TEST_DATA.
> >
> > Signed-off-by: Oliver Upton
> > ---
> > tools/testing/selftests/kvm/aarch64/page_fault_test.c | 8 +++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/kvm/aarch64/page_fault_test.c
> > b/tools/testing/selftests/kvm/aarch64/page_fault_test.c
> > index 92d3a91153b6..95d22cfb7b41 100644
> > --- a/tools/testing/selftests/kvm/aarch64/page_fault_test.c
> > +++ b/tools/testing/selftests/kvm/aarch64/page_fault_test.c
> > @@ -609,8 +609,13 @@ static void setup_memslots(struct kvm_vm *vm, struct
> > test_params *p)
> > data_size / guest_page_size,
> > p->test_desc->data_memslot_flags);
> > vm->memslots[MEM_REGION_TEST_DATA] = TEST_DATA_MEMSLOT;
> > +}
> > +
> > +static void setup_ucall(struct kvm_vm *vm)
> > +{
> > + struct userspace_mem_region *region = vm_get_mem_region(vm,
> > MEM_REGION_TEST_DATA);
> >
> > - ucall_init(vm, data_gpa + data_size);
> > + ucall_init(vm, region->region.guest_phys_addr +
> > region->region.memory_size);
>
> Isn't there a hole after CODE_AND_DATA_MEMSLOT? I.e. after memslot 0?
Sure, but that's only guaranteed in the PA space.
> The reason
> I ask is because if so, then we can do the temporarily heinous, but hopefully
> forward
> looking thing of adding a helper to wrap kvm_vm_elf_load() + ucall_init().
>
> E.g. I think we can do this immediately, and then at some point in the 6.2
> cycle
> add a dedicated region+memslot for the ucall MMIO page.
Even still, that's just a kludge to make ucalls work. We have other
MMIO devices (GIC distributor, for example) that work by chance since
nothing conflicts with the constant GPAs we've selected in the tests.
I'd rather we go down the route of having an address allocator for the
for both the VA and PA spaces to provide carveouts at runtime. There's
another issue with the new ucall implementation where identity mapping
could stomp on a program segment that I'm fighting with right now which
only further highlights the problems with our (mis)management of address
spaces in selftests.
--
Thanks,
Oliver
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 2/4] KVM: selftests: Setup ucall after loading program into guest memory
On Wed, Dec 07, 2022, Oliver Upton wrote:
> The new ucall infrastructure needs to update a couple of guest globals
> to pass through the ucall MMIO addr and pool of ucall structs. A
> precondition of this actually working is to have the program image
> already loaded into guest memory.
Ouch. Might be worth explicitly stating what goes wrong. Even though it's
super
obvious in hindsight, it still took me a few seconds to understand what
precondition you were referring to, e.g. I was trying to figure out how
selecting
the MMIO address depended on the guest code being loaded...
>
> Call ucall_init() after kvm_vm_elf_load(). Continue to park the ucall
> MMIO addr after MEM_REGION_TEST_DATA.
>
> Signed-off-by: Oliver Upton
> ---
> tools/testing/selftests/kvm/aarch64/page_fault_test.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/kvm/aarch64/page_fault_test.c
> b/tools/testing/selftests/kvm/aarch64/page_fault_test.c
> index 92d3a91153b6..95d22cfb7b41 100644
> --- a/tools/testing/selftests/kvm/aarch64/page_fault_test.c
> +++ b/tools/testing/selftests/kvm/aarch64/page_fault_test.c
> @@ -609,8 +609,13 @@ static void setup_memslots(struct kvm_vm *vm, struct
> test_params *p)
> data_size / guest_page_size,
> p->test_desc->data_memslot_flags);
> vm->memslots[MEM_REGION_TEST_DATA] = TEST_DATA_MEMSLOT;
> +}
> +
> +static void setup_ucall(struct kvm_vm *vm)
> +{
> + struct userspace_mem_region *region = vm_get_mem_region(vm,
> MEM_REGION_TEST_DATA);
>
> - ucall_init(vm, data_gpa + data_size);
> + ucall_init(vm, region->region.guest_phys_addr +
> region->region.memory_size);
Isn't there a hole after CODE_AND_DATA_MEMSLOT? I.e. after memslot 0? The
reason
I ask is because if so, then we can do the temporarily heinous, but hopefully
forward
looking thing of adding a helper to wrap kvm_vm_elf_load() + ucall_init().
E.g. I think we can do this immediately, and then at some point in the 6.2 cycle
add a dedicated region+memslot for the ucall MMIO page.
---
.../selftests/kvm/aarch64/page_fault_test.c | 10 +--
.../selftests/kvm/include/kvm_util_base.h | 1 +
tools/testing/selftests/kvm/lib/kvm_util.c| 28 +++
3 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/tools/testing/selftests/kvm/aarch64/page_fault_test.c
b/tools/testing/selftests/kvm/aarch64/page_fault_test.c
index 95d22cfb7b41..68c47db2eb2e 100644
--- a/tools/testing/selftests/kvm/aarch64/page_fault_test.c
+++ b/tools/testing/selftests/kvm/aarch64/page_fault_test.c
@@ -611,13 +611,6 @@ static void setup_memslots(struct kvm_vm *vm, struct
test_params *p)
vm->memslots[MEM_REGION_TEST_DATA] = TEST_DATA_MEMSLOT;
}
-static void setup_ucall(struct kvm_vm *vm)
-{
- struct userspace_mem_region *region = vm_get_mem_region(vm,
MEM_REGION_TEST_DATA);
-
- ucall_init(vm, region->region.guest_phys_addr +
region->region.memory_size);
-}
-
static void setup_default_handlers(struct test_desc *test)
{
if (!test->mmio_handler)
@@ -706,8 +699,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
vm = vm_create(mode);
setup_memslots(vm, p);
- kvm_vm_elf_load(vm, program_invocation_name);
- setup_ucall(vm);
+ vm_init_guest_code_and_data(vm);
vcpu = vm_vcpu_add(vm, 0, guest_code);
setup_gva_maps(vm);
diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h
b/tools/testing/selftests/kvm/include/kvm_util_base.h
index fbc2a79369b8..175b5ca0c061 100644
--- a/tools/testing/selftests/kvm/include/kvm_util_base.h
+++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
@@ -682,6 +682,7 @@ vm_paddr_t vm_phy_page_alloc(struct kvm_vm *vm, vm_paddr_t
paddr_min,
vm_paddr_t vm_phy_pages_alloc(struct kvm_vm *vm, size_t num,
vm_paddr_t paddr_min, uint32_t memslot);
vm_paddr_t vm_alloc_page_table(struct kvm_vm *vm);
+void vm_init_guest_code_and_data(struct kvm_vm *vm);
/*
* vm_create() does KVM_CREATE_VM and little else. __vm_create() also
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c
b/tools/testing/selftests/kvm/lib/kvm_util.c
index c88c3ace16d2..0eab6b11a6e9 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -329,12 +329,27 @@ static uint64_t vm_nr_pages_required(enum vm_guest_mode
mode,
return vm_adjust_num_guest_pages(mode, nr_pages);
}
+void vm_init_guest_code_and_data(struct kvm_vm *vm)
+{
+ struct userspace_mem_region *slot;
+
+ kvm_vm_elf_load(vm, program_invocation_name);
+
+ /*
+* TODO: Add a dedicated memory region to carve out MMIO. KVM treats
+* writes to read-only memslots as MMIO, and creating a read-only
+* memslot for the MMIO region would prevent silently clobbering the
+* MMIO region.
+*/
+
Re: [PATCH 4/4] KVM: selftests: Allocate ucall pool from MEM_REGION_DATA
On Wed, Dec 07, 2022 at 11:44:50PM +, Sean Christopherson wrote:
> On Wed, Dec 07, 2022, Oliver Upton wrote:
> > MEM_REGION_TEST_DATA is meant to hold data explicitly used by a
> > selftest, not implicit allocations due to the selftests infrastructure.
> > Allocate the ucall pool from MEM_REGION_DATA much like the rest of the
> > selftests library allocations.
> >
> > Fixes: 426729b2cf2e ("KVM: selftests: Add ucall pool based implementation")
>
> Not that it really matters because no one will backport this verbatim, but
> this
> is the wrong commit to blame. As of commit 426729b2cf2e, MEM_REGION_DATA
> does not
> exist. And similarly, the common ucall code didn't exist when Ricardo's
> series
> introduced MEM_REGION_DATA.
>
> $ git show 426729b2cf2e:tools/testing/selftests/kvm/include/kvm_util_base.h
> | grep MEM_REGION_DATA
> $ git show 290c5b54012b7:tools/testing/selftests/kvm/lib/ucall_common.c
> fatal: path 'tools/testing/selftests/kvm/lib/ucall_common.c' exists on
> disk, but not in '290c5b54012b7'
>
> The commit where the two collided is:
>
> Fixes: cc7544101eec ("Merge tag 'kvmarm-6.2' of
> https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm into HEAD")
Yeah, this Fixes is garbage, apologies.
I imagine Paolo is going to squash some things into the kvmarm merge, so
the Fixes tag ought to be dropped altogether.
> > Signed-off-by: Oliver Upton
> > ---
>
> Fixes nit aside,
>
> Reviewed-by: Sean Christopherson
Thanks!
--
Best,
Oliver
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 4/4] KVM: selftests: Allocate ucall pool from MEM_REGION_DATA
On Wed, Dec 07, 2022, Oliver Upton wrote:
> MEM_REGION_TEST_DATA is meant to hold data explicitly used by a
> selftest, not implicit allocations due to the selftests infrastructure.
> Allocate the ucall pool from MEM_REGION_DATA much like the rest of the
> selftests library allocations.
>
> Fixes: 426729b2cf2e ("KVM: selftests: Add ucall pool based implementation")
Not that it really matters because no one will backport this verbatim, but this
is the wrong commit to blame. As of commit 426729b2cf2e, MEM_REGION_DATA does
not
exist. And similarly, the common ucall code didn't exist when Ricardo's series
introduced MEM_REGION_DATA.
$ git show 426729b2cf2e:tools/testing/selftests/kvm/include/kvm_util_base.h |
grep MEM_REGION_DATA
$ git show 290c5b54012b7:tools/testing/selftests/kvm/lib/ucall_common.c
fatal: path 'tools/testing/selftests/kvm/lib/ucall_common.c' exists on disk,
but not in '290c5b54012b7'
The commit where the two collided is:
Fixes: cc7544101eec ("Merge tag 'kvmarm-6.2' of
https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm into HEAD")
> Signed-off-by: Oliver Upton
> ---
Fixes nit aside,
Reviewed-by: Sean Christopherson
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [GIT PULL] KVM/arm64 updates for 6.2
On Tue, Dec 06, 2022 at 06:41:21PM +0100, Paolo Bonzini wrote: > On 12/5/22 16:58, Marc Zyngier wrote: > > - There is a lot of selftest conflicts with your own branch, see: > > > >https://lore.kernel.org/r/20221201112432.4cb9a...@canb.auug.org.au > >https://lore.kernel.org/r/20221201113626.438f1...@canb.auug.org.au > >https://lore.kernel.org/r/20221201115741.7de32...@canb.auug.org.au > >https://lore.kernel.org/r/20221201120939.3c19f...@canb.auug.org.au > >https://lore.kernel.org/r/20221201131623.18ebc...@canb.auug.org.au > > > >for a rather exhaustive collection. > > Yeah, I saw them in Stephen's messages but missed your reply. > > In retrospect, at least Gavin's series for memslot_perf_test should have > been applied by both of us with a topic branch, but there's so many > conflicts all over the place that it's hard to single out one series. > It just happens. > > The only conflict in non-x86 code is the following one, please check > if I got it right. > > diff --git a/tools/testing/selftests/kvm/aarch64/page_fault_test.c > b/tools/testing/selftests/kvm/aarch64/page_fault_test.c > index 05bb6a6369c2..0cda70bef5d5 100644 > --- a/tools/testing/selftests/kvm/aarch64/page_fault_test.c > +++ b/tools/testing/selftests/kvm/aarch64/page_fault_test.c > @@ -609,6 +609,8 @@ static void setup_memslots(struct kvm_vm *vm, struct > test_params *p) > data_size / guest_page_size, > p->test_desc->data_memslot_flags); > vm->memslots[MEM_REGION_TEST_DATA] = TEST_DATA_MEMSLOT; > + > + ucall_init(vm, data_gpa + data_size); > } > static void setup_default_handlers(struct test_desc *test) > @@ -704,8 +706,6 @@ static void run_test(enum vm_guest_mode mode, void *arg) > setup_gva_maps(vm); > - ucall_init(vm, NULL); > - > reset_event_counts(); > /* > > > Special care is needed here because the test uses vm_create(). > > I haven't pushed to kvm/next yet to give you time to check, so the > merge is currently in kvm/queue only. Have a look at this series, which gets things building and actually passing again: https://lore.kernel.org/kvm/20221207214809.489070-1-oliver.up...@linux.dev/ > > - For the 6.3 cycle, we are going to experiment with Oliver taking > >care of most of the patch herding. I'm sure he'll do a great job, > >but if there is the odd mistake, please cut him some slack and blame > >me instead. > > Absolutely - you both have all the slack you need, synchronization > is harder than it seems. Appreciated! -- Thanks, Oliver ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 4/4] KVM: selftests: Allocate ucall pool from MEM_REGION_DATA
MEM_REGION_TEST_DATA is meant to hold data explicitly used by a
selftest, not implicit allocations due to the selftests infrastructure.
Allocate the ucall pool from MEM_REGION_DATA much like the rest of the
selftests library allocations.
Fixes: 426729b2cf2e ("KVM: selftests: Add ucall pool based implementation")
Signed-off-by: Oliver Upton
---
tools/testing/selftests/kvm/lib/ucall_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/lib/ucall_common.c
b/tools/testing/selftests/kvm/lib/ucall_common.c
index 820ce6c82829..0cc0971ce60e 100644
--- a/tools/testing/selftests/kvm/lib/ucall_common.c
+++ b/tools/testing/selftests/kvm/lib/ucall_common.c
@@ -22,7 +22,7 @@ void ucall_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa)
vm_vaddr_t vaddr;
int i;
- vaddr = vm_vaddr_alloc(vm, sizeof(*hdr), KVM_UTIL_MIN_VADDR);
+ vaddr = __vm_vaddr_alloc(vm, sizeof(*hdr), KVM_UTIL_MIN_VADDR,
MEM_REGION_DATA);
hdr = (struct ucall_header *)addr_gva2hva(vm, vaddr);
memset(hdr, 0, sizeof(*hdr));
--
2.39.0.rc0.267.gcb52ba06e7-goog
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 3/4] KVM: arm64: selftests: Align VA space allocator with TTBR0
An interesting feature of the Arm architecture is that the stage-1 MMU
supports two distinct VA regions, controlled by TTBR{0,1}_EL1. As KVM
selftests on arm64 only uses TTBR0_EL1, the VA space is constrained to
[0, 2^(va_bits-1)). This is different from other architectures that
allow for addressing low and high regions of the VA space from a single
page table.
KVM selftests' VA space allocator presumes the valid address range is
split between low and high memory based the MSB, which of course is a
poor match for arm64's TTBR0 region.
Allow architectures to override the default VA space layout. Make use of
the override to align vpages_valid with the behavior of TTBR0 on arm64.
Signed-off-by: Oliver Upton
---
.../testing/selftests/kvm/include/kvm_util_base.h | 1 +
.../testing/selftests/kvm/lib/aarch64/processor.c | 10 ++
tools/testing/selftests/kvm/lib/kvm_util.c| 15 ++-
3 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h
b/tools/testing/selftests/kvm/include/kvm_util_base.h
index 6cd86da698b3..fbc2a79369b8 100644
--- a/tools/testing/selftests/kvm/include/kvm_util_base.h
+++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
@@ -420,6 +420,7 @@ void vm_mem_region_set_flags(struct kvm_vm *vm, uint32_t
slot, uint32_t flags);
void vm_mem_region_move(struct kvm_vm *vm, uint32_t slot, uint64_t new_gpa);
void vm_mem_region_delete(struct kvm_vm *vm, uint32_t slot);
struct kvm_vcpu *__vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id);
+void vm_populate_vaddr_bitmap(struct kvm_vm *vm);
vm_vaddr_t vm_vaddr_unused_gap(struct kvm_vm *vm, size_t sz, vm_vaddr_t
vaddr_min);
vm_vaddr_t vm_vaddr_alloc(struct kvm_vm *vm, size_t sz, vm_vaddr_t vaddr_min);
vm_vaddr_t __vm_vaddr_alloc(struct kvm_vm *vm, size_t sz, vm_vaddr_t vaddr_min,
diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c
b/tools/testing/selftests/kvm/lib/aarch64/processor.c
index 316de70db91d..5972a23b2765 100644
--- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
+++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
@@ -541,3 +541,13 @@ void kvm_selftest_arch_init(void)
*/
guest_modes_append_default();
}
+
+void vm_vaddr_populate_bitmap(struct kvm_vm *vm)
+{
+ /*
+* arm64 selftests use only TTBR0_EL1, meaning that the valid VA space
+* is [0, 2^(64 - TCR_EL1.T0SZ)).
+*/
+ sparsebit_set_num(vm->vpages_valid, 0,
+ (1ULL << vm->va_bits) >> vm->page_shift);
+}
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c
b/tools/testing/selftests/kvm/lib/kvm_util.c
index e9607eb089be..c88c3ace16d2 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -186,6 +186,15 @@ const struct vm_guest_mode_params vm_guest_mode_params[] =
{
_Static_assert(sizeof(vm_guest_mode_params)/sizeof(struct
vm_guest_mode_params) == NUM_VM_MODES,
"Missing new mode params?");
+__weak void vm_vaddr_populate_bitmap(struct kvm_vm *vm)
+{
+ sparsebit_set_num(vm->vpages_valid,
+ 0, (1ULL << (vm->va_bits - 1)) >> vm->page_shift);
+ sparsebit_set_num(vm->vpages_valid,
+ (~((1ULL << (vm->va_bits - 1)) - 1)) >> vm->page_shift,
+ (1ULL << (vm->va_bits - 1)) >> vm->page_shift);
+}
+
struct kvm_vm *vm_create(enum vm_guest_mode mode)
{
struct kvm_vm *vm;
@@ -274,11 +283,7 @@ struct kvm_vm *vm_create(enum vm_guest_mode mode)
/* Limit to VA-bit canonical virtual addresses. */
vm->vpages_valid = sparsebit_alloc();
- sparsebit_set_num(vm->vpages_valid,
- 0, (1ULL << (vm->va_bits - 1)) >> vm->page_shift);
- sparsebit_set_num(vm->vpages_valid,
- (~((1ULL << (vm->va_bits - 1)) - 1)) >> vm->page_shift,
- (1ULL << (vm->va_bits - 1)) >> vm->page_shift);
+ vm_vaddr_populate_bitmap(vm);
/* Limit physical addresses to PA-bits. */
vm->max_gfn = vm_compute_max_gfn(vm);
--
2.39.0.rc0.267.gcb52ba06e7-goog
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 2/4] KVM: selftests: Setup ucall after loading program into guest memory
The new ucall infrastructure needs to update a couple of guest globals
to pass through the ucall MMIO addr and pool of ucall structs. A
precondition of this actually working is to have the program image
already loaded into guest memory.
Call ucall_init() after kvm_vm_elf_load(). Continue to park the ucall
MMIO addr after MEM_REGION_TEST_DATA.
Signed-off-by: Oliver Upton
---
tools/testing/selftests/kvm/aarch64/page_fault_test.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/aarch64/page_fault_test.c
b/tools/testing/selftests/kvm/aarch64/page_fault_test.c
index 92d3a91153b6..95d22cfb7b41 100644
--- a/tools/testing/selftests/kvm/aarch64/page_fault_test.c
+++ b/tools/testing/selftests/kvm/aarch64/page_fault_test.c
@@ -609,8 +609,13 @@ static void setup_memslots(struct kvm_vm *vm, struct
test_params *p)
data_size / guest_page_size,
p->test_desc->data_memslot_flags);
vm->memslots[MEM_REGION_TEST_DATA] = TEST_DATA_MEMSLOT;
+}
+
+static void setup_ucall(struct kvm_vm *vm)
+{
+ struct userspace_mem_region *region = vm_get_mem_region(vm,
MEM_REGION_TEST_DATA);
- ucall_init(vm, data_gpa + data_size);
+ ucall_init(vm, region->region.guest_phys_addr +
region->region.memory_size);
}
static void setup_default_handlers(struct test_desc *test)
@@ -702,6 +707,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
vm = vm_create(mode);
setup_memslots(vm, p);
kvm_vm_elf_load(vm, program_invocation_name);
+ setup_ucall(vm);
vcpu = vm_vcpu_add(vm, 0, guest_code);
setup_gva_maps(vm);
--
2.39.0.rc0.267.gcb52ba06e7-goog
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 1/4] KVM: selftests: Fix build due to ucall_uninit() removal
From: Mark Brown
Today's -next fails to build on arm64 due to:
In file included from include/kvm_util.h:11,
from aarch64/page_fault_test.c:15:
include/ucall_common.h:36:47: note: expected ‘vm_paddr_t’ {aka ‘long unsigned
int’} but argument is of type ‘void *’
36 | void ucall_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa);
|~~~^~~~
aarch64/page_fault_test.c:725:2: warning: implicit declaration of function
‘ucall_uninit’; did you mean ‘ucall_init’? [-Wimplicit-function-declaration]
725 | ucall_uninit(vm);
| ^~~~
| ucall_init
which is caused by commit
interacting poorly with commit
28a65567acb5 ("KVM: selftests: Drop now-unnecessary ucall_uninit()")
As is done for other ucall_uninit() users remove the call in the newly added
page_fault_test.c.
Fixes: 28a65567acb5 ("KVM: selftests: Drop now-unnecessary ucall_uninit()")
Fixes: 35c581015712 ("KVM: selftests: aarch64: Add aarch64/page_fault_test")
Signed-off-by: Mark Brown
Cc: Sean Christopherson
Cc: Ricardo Koller
Cc: Marc Zyngier
Signed-off-by: Oliver Upton
---
tools/testing/selftests/kvm/aarch64/page_fault_test.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/aarch64/page_fault_test.c
b/tools/testing/selftests/kvm/aarch64/page_fault_test.c
index 0cda70bef5d5..92d3a91153b6 100644
--- a/tools/testing/selftests/kvm/aarch64/page_fault_test.c
+++ b/tools/testing/selftests/kvm/aarch64/page_fault_test.c
@@ -722,7 +722,6 @@ static void run_test(enum vm_guest_mode mode, void *arg)
vcpu_run_loop(vm, vcpu, test);
- ucall_uninit(vm);
kvm_vm_free(vm);
free_uffd(test, pt_uffd, data_uffd);
--
2.39.0.rc0.267.gcb52ba06e7-goog
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 0/4] KVM: selftests: Fixes for ucall pool + page_fault_test
The combination of the pool-based ucall implementation + page_fault_test resulted in some 'fun' bugs, not the least of which is that our handling of the VA space on arm64 is completely wrong. Small series to fix up the issues on kvm/queue. Patches 1-2 can probably be squashed into Paolo's merge resolution, if desired. Mark Brown (1): KVM: selftests: Fix build due to ucall_uninit() removal Oliver Upton (3): KVM: selftests: Setup ucall after loading program into guest memory KVM: arm64: selftests: Align VA space allocator with TTBR0 KVM: selftests: Allocate ucall pool from MEM_REGION_DATA .../selftests/kvm/aarch64/page_fault_test.c | 9 +++-- .../testing/selftests/kvm/include/kvm_util_base.h | 1 + .../testing/selftests/kvm/lib/aarch64/processor.c | 10 ++ tools/testing/selftests/kvm/lib/kvm_util.c| 15 ++- tools/testing/selftests/kvm/lib/ucall_common.c| 2 +- 5 files changed, 29 insertions(+), 8 deletions(-) -- 2.39.0.rc0.267.gcb52ba06e7-goog ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v3 7/8] perf: Add perf_event_attr::config3
On Tue, Dec 6, 2022 at 10:28 AM Mark Rutland wrote: > > Peter, it looks like this series is blocked on the below now; what would you > prefer out of: > > (a) Take this as is, and look add adding additional validation on top. > > (b) Add some flag to indicate a PMU driver supports config3, and have the core > code check that, but leave the existing fields as-is for now (and > hopefully > follow up with further validation later for the existing fields). That looks something like this: diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index 853f64b6c8c2..845162b152ea 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -286,6 +286,7 @@ struct perf_event; #define PERF_PMU_CAP_NO_EXCLUDE0x0080 #define PERF_PMU_CAP_AUX_OUTPUT0x0100 #define PERF_PMU_CAP_EXTENDED_HW_TYPE 0x0200 +#define PERF_PMU_CAP_CONFIG3 0x0400 struct perf_output_handle; diff --git a/kernel/events/core.c b/kernel/events/core.c index aefc1e08e015..4414ae64432a 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -11314,6 +11314,9 @@ static int perf_try_init_event(struct pmu *pmu, struct perf_event *event) event_has_any_exclude_flag(event)) ret = -EINVAL; + if (!(pmu->capabilities & PERF_PMU_CAP_CONFIG3) && event->attr.config3) + ret = -EINVAL; + if (ret && event->destroy) event->destroy(event); } ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v14 16/39] arm64/sme: Implement traps and syscall handling for SME
On Wed, Dec 07, 2022 at 10:00:17PM +0800, Zenghui Yu wrote: > On 2022/4/19 19:22, Mark Brown wrote: > > + /* > > +* If SME is active then exit streaming mode. If ZA is active > > +* then flush the SVE registers but leave userspace access to > > +* both SVE and SME enabled, otherwise disable SME for the > > +* task and fall through to disabling SVE too. This means > It looks a bit confusing to me that in the current implementation, if > ZA is *not* active, we don't actually go to disable SME for the task > (which IMHO can be disabled as user-space is not actively using it now). Unlike SVE there's no overhead from leaving SME enabled, the enable bits for SM and ZA tell us if there is extra register state to be saved so we don't have to worry about the costs there like we do with SVE. The only reason for not just unconditionally enabling SME is the potentially large backing storage required for the registers, if a task isn't using SME there's no need to impose that overhead. If we disable SME for userspace after the storage has been allocated then we just require an additional trip through EL1 to reenable it for any future usage which would hurt performance but not really gain us anything otherwise. We don't want to discurage applications from disabling ZA when not in use given that there's likely to be physical overheads from keeping it enabled. > > + if (svcr & SYS_SVCR_EL0_SM_MASK) > > + sme_smstop_sm(); > As per the SME syscall ABI > | On syscall PSTATE.SM will be cleared and the SVE registers will be > | handled as per the standard SVE ABI. > and the SVE syscall ABI > | On syscall, V0..V31 are preserved (as without SVE). Thus, bits > | [127:0] of Z0..Z31 are preserved. All other bits of Z0..Z31, and all > | of P0..P15 and FFR become zero on return from a syscall. > Can we infer from the documentation that V0-V31 should be preserved on > return from a syscall? But with sme_smstop_sm(), all implemented bits of > Z0-Z31 are set to zero by hardware. Is this intentional? > Please fix me up if I've mis-read things here. No, the intention is to say that we exit streaming mode and then handle things as per the non-streaming ABI. Exiting streaming mode has the effect of clearing the values as you say. signature.asc Description: PGP signature ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v14 16/39] arm64/sme: Implement traps and syscall handling for SME
On 2022/4/19 19:22, Mark Brown wrote:
diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
index c938603b3ba0..92c69e5ac269 100644
--- a/arch/arm64/kernel/syscall.c
+++ b/arch/arm64/kernel/syscall.c
@@ -158,11 +158,36 @@ static void el0_svc_common(struct pt_regs *regs, int
scno, int sc_nr,
syscall_trace_exit(regs);
}
-static inline void sve_user_discard(void)
+/*
+ * As per the ABI exit SME streaming mode and clear the SVE state not
+ * shared with FPSIMD on syscall entry.
+ */
+static inline void fp_user_discard(void)
{
+ /*
+* If SME is active then exit streaming mode. If ZA is active
+* then flush the SVE registers but leave userspace access to
+* both SVE and SME enabled, otherwise disable SME for the
+* task and fall through to disabling SVE too. This means
It looks a bit confusing to me that in the current implementation, if
ZA is *not* active, we don't actually go to disable SME for the task
(which IMHO can be disabled as user-space is not actively using it now).
+* that after a syscall we never have any streaming mode
+* register state to track, if this changes the KVM code will
+* need updating.
+*/
+ if (system_supports_sme() && test_thread_flag(TIF_SME)) {
+ u64 svcr = read_sysreg_s(SYS_SVCR_EL0);
+
+ if (svcr & SYS_SVCR_EL0_SM_MASK)
+ sme_smstop_sm();
As per the SME syscall ABI
| On syscall PSTATE.SM will be cleared and the SVE registers will be
| handled as per the standard SVE ABI.
and the SVE syscall ABI
| On syscall, V0..V31 are preserved (as without SVE). Thus, bits
| [127:0] of Z0..Z31 are preserved. All other bits of Z0..Z31, and all
| of P0..P15 and FFR become zero on return from a syscall.
Can we infer from the documentation that V0-V31 should be preserved on
return from a syscall? But with sme_smstop_sm(), all implemented bits of
Z0-Z31 are set to zero by hardware. Is this intentional?
Please fix me up if I've mis-read things here.
+ }
+
if (!system_supports_sve())
return;
+ /*
+* If SME is not active then disable SVE, the registers will
+* be cleared when userspace next attempts to access them and
+* we do not need to track the SVE register state until then.
+*/
clear_thread_flag(TIF_SVE);
/*
Thanks,
Zenghui
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 1/2] ACPI/AEST: Initial AEST driver
Hi, Tyler.
I am very interested in your work about AEST.
When do you plan to update the v2 patch series?
Best regards.
在 2022/5/9 21:37, Tyler Baicar 写道:
Hi Shuuichirou,
I should be able to get a v2 patch series out by the end of the month.
Thanks,
Tyler
On 4/20/2022 3:54 AM, ishii.shuuic...@fujitsu.com wrote:
Hi, Tyler.
When do you plan to post the v2 patch series?
Please let me know if you don't mind.
Best regards.
-Original Message-
From: Tyler Baicar
Sent: Friday, December 17, 2021 8:33 AM
To: Ishii, Shuuichirou/石井 周一郎 ; 'Tyler
Baicar'
; patc...@amperecomputing.com;
abdulha...@os.amperecomputing.com; dar...@os.amperecomputing.com;
catalin.mari...@arm.com; w...@kernel.org; m...@kernel.org;
james.mo...@arm.com; alexandru.eli...@arm.com; suzuki.poul...@arm.com;
lorenzo.pieral...@arm.com; guohan...@huawei.com; sudeep.ho...@arm.com;
raf...@kernel.org; l...@kernel.org; tony.l...@intel.com; b...@alien8.de;
mark.rutl...@arm.com; anshuman.khand...@arm.com;
vincenzo.frasc...@arm.com; ta...@google.com; mar...@marcan.st;
keesc...@chromium.org; jthie...@redhat.com; masahi...@kernel.org;
samitolva...@google.com; john.ga...@huawei.com;
daniel.lezc...@linaro.org;
g...@linux.ibm.com; zhangshao...@hisilicon.com; tmri...@linux.ibm.com;
dchin...@redhat.com; t...@linutronix.de; linux-ker...@vger.kernel.org;
linux-arm-ker...@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
linux-a...@vger.kernel.org; linux-e...@vger.kernel.org;
vineeth.pil...@microsoft.com
Subject: Re: [PATCH 1/2] ACPI/AEST: Initial AEST driver
Hi Shuuichirou,
Thank you for your feedback!
On 12/9/2021 3:10 AM, ishii.shuuic...@fujitsu.com wrote:
Hi, Tyler.
We would like to make a few comments.
-Original Message-
From: Tyler Baicar
Sent: Thursday, November 25, 2021 2:07 AM
To: patc...@amperecomputing.com; abdulha...@os.amperecomputing.com;
dar...@os.amperecomputing.com; catalin.mari...@arm.com;
w...@kernel.org; m...@kernel.org; james.mo...@arm.com;
alexandru.eli...@arm.com; suzuki.poul...@arm.com;
lorenzo.pieral...@arm.com; guohan...@huawei.com;
sudeep.ho...@arm.com; raf...@kernel.org; l...@kernel.org;
tony.l...@intel.com; b...@alien8.de; mark.rutl...@arm.com;
anshuman.khand...@arm.com; vincenzo.frasc...@arm.com;
ta...@google.com; mar...@marcan.st; keesc...@chromium.org;
jthie...@redhat.com; masahi...@kernel.org; samitolva...@google.com;
john.ga...@huawei.com; daniel.lezc...@linaro.org; g...@linux.ibm.com;
zhangshao...@hisilicon.com; tmri...@linux.ibm.com;
dchin...@redhat.com; t...@linutronix.de;
linux-ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
kvmarm@lists.cs.columbia.edu; linux-a...@vger.kernel.org;
linux-e...@vger.kernel.org; Ishii, Shuuichirou/石井
周一郎 ; vineeth.pil...@microsoft.com
Cc: Tyler Baicar
Subject: [PATCH 1/2] ACPI/AEST: Initial AEST driver
Add support for parsing the ARM Error Source Table and basic handling
of errors reported through both memory mapped and system register
interfaces.
Assume system register interfaces are only registered with private
peripheral interrupts (PPIs); otherwise there is no guarantee the
core handling the error is the core which took the error and has the
syndrome info in its system registers.
Add logging for all detected errors and trigger a kernel panic if
there is any uncorrected error present.
Signed-off-by: Tyler Baicar
---
[...]
+static int __init aest_init_node(struct acpi_aest_hdr *node) {
+ union acpi_aest_processor_data *proc_data;
+ union aest_node_spec *node_spec;
+ struct aest_node_data *data;
+ int ret;
+
+ data = kzalloc(sizeof(struct aest_node_data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->node_type = node->type;
+
+ node_spec = ACPI_ADD_PTR(union aest_node_spec, node,
node->node_specific_offset);
+
+ switch (node->type) {
+ case ACPI_AEST_PROCESSOR_ERROR_NODE:
+ memcpy(&data->data, node_spec, sizeof(struct
acpi_aest_processor));
+ break;
+ case ACPI_AEST_MEMORY_ERROR_NODE:
+ memcpy(&data->data, node_spec, sizeof(struct
acpi_aest_memory));
+ break;
+ case ACPI_AEST_SMMU_ERROR_NODE:
+ memcpy(&data->data, node_spec, sizeof(struct
acpi_aest_smmu));
+ break;
+ case ACPI_AEST_VENDOR_ERROR_NODE:
+ memcpy(&data->data, node_spec, sizeof(struct
acpi_aest_vendor));
+ break;
+ case ACPI_AEST_GIC_ERROR_NODE:
+ memcpy(&data->data, node_spec, sizeof(struct
acpi_aest_gic));
+ break;
+ default:
+ kfree(data);
+ return -EINVAL;
+ }
+
+ if (node->type == ACPI_AEST_PROCESSOR_ERROR_NODE) {
+ proc_data = ACPI_ADD_PTR(union acpi_aest_processor_data,
node_spec,
+ sizeof(acpi_aest_processor));
+
+ switch (data->data.processor.resource_type) {
+ case ACPI_AEST_CACHE_RESOURCE:
+ memcpy(&data->proc_data, proc_data,
+ sizeof(struct acpi_aest_processor_cache));
+ break;
+ case ACPI_AEST_TLB_RESOURCE:
+ memcpy
