Re: [PATCH v7 01/10] KVM: arm64: Allow userspace to get the writable masks for feature ID registers

From: Cornelia Huck <cohuck@redhat.com>
To: Oliver Upton <oliver.upton@linux.dev>,
	Jing Zhang <jingzhangos@google.com>
Cc: KVM <kvm@vger.kernel.org>, KVMARM <kvmarm@lists.linux.dev>,
	ARMLinux <linux-arm-kernel@lists.infradead.org>,
	Marc Zyngier <maz@kernel.org>, Will Deacon <will@kernel.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	James Morse <james.morse@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Fuad Tabba <tabba@google.com>, Reiji Watanabe <reijiw@google.com>,
	Raghavendra Rao Ananta <rananta@google.com>,
	Suraj Jitindar Singh <surajjs@amazon.com>
Subject: Re: [PATCH v7 01/10] KVM: arm64: Allow userspace to get the writable masks for feature ID registers
Date: Thu, 03 Aug 2023 15:20:41 +0200	[thread overview]
Message-ID: <877cqc8dp2.fsf@redhat.com> (raw)
In-Reply-To: <ZMqMofRCmB14XUZr@linux.dev>

On Wed, Aug 02 2023, Oliver Upton <oliver.upton@linux.dev> wrote:

> On Wed, Aug 02, 2023 at 08:55:43AM -0700, Jing Zhang wrote:
>> > > +#define ARM64_FEATURE_ID_SPACE_SIZE  (3 * 8 * 8)
>> > > +
>> > > +struct feature_id_writable_masks {
>> > > +     __u64 mask[ARM64_FEATURE_ID_SPACE_SIZE];
>> > > +};
>> >
>> > This UAPI is rather difficult to extend in the future. We may need to
>> > support describing the masks of multiple ranges of registers in the
>> > future. I was thinking something along the lines of:
>> >
>> >         enum reg_mask_range_idx {
>> >                 FEATURE_ID,
>> >         };
>> >
>> >         struct reg_mask_range {
>> >                 __u64 idx;
>> >                 __u64 *masks;
>> >                 __u64 rsvd[6];
>> >         };
>> >
>> Since have the way to map sysregs encoding to the index in the mask
>> array, we can extend the UAPI by just adding a size field in struct
>> feature_id_writable_masks like below:
>> struct feature_id_writable_masks {
>>          __u64 size;
>>          __u64 mask[ARM64_FEATURE_ID_SPACE_SIZE];
>> };
>> The 'size' field can be used as input for the size of 'mask' array and
>> output for the number of masks actually read in.
>> This way, we can freely add more ranges without breaking anything in userspace.
>> WDYT?
>
> Sorry, 'index' is a bit overloaded in this context. The point I was
> trying to get across is that we might want to describe a completely
> different range of registers than the feature ID registers in the
> future. Nonetheless, we shouldn't even presume the shape of future
> extensions to the ioctl.
>
> 	struct reg_mask_range {
> 		__u64 addr;	/* pointer to mask array */
> 		__u64 rsvd[7];
> 	};
>
> Then in KVM we should require ::rsvd be zero and fail the ioctl
> otherwise.

[I assume rsvd == reserved? I think I have tried to divine further
meaning into this for far too long...]

Is the idea here for userspace the request a mask array for FEATURE_ID
and future ranges separately instead of getting all id-type regs in one
go?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel