Hi Paolo,
On Thu, Apr 21, 2022 at 02:04:39PM -0400, Paolo Bonzini wrote:
> The KVM_SYSTEM_EVENT_NDATA_VALID mechanism that was introduced
> contextually with KVM_SYSTEM_EVENT_SEV_TERM is not a good match
> for ARM and RISC-V, which want to communicate information even
> for existing KVM_SYSTEM_EVENT_* constants. Userspace is not ready
> to filter out bit 31 of type, and fails to process the
> KVM_EXIT_SYSTEM_EVENT exit.
>
> Therefore, tie the availability of ndata to a system capability;
> if the capability is present, ndata is always valid, so patch 1
> makes x86 always initialize it. Then patches 2 and 3 fix
> ARM and RISC-V compilation and patch 4 enables the capability.
>
> Only compiled on x86, waiting for acks.
>
> Paolo
>
> Paolo Bonzini (4):
> KVM: x86: always initialize system_event.ndata
> KVM: ARM: replace system_event.flags with ndata and data[0]
> KVM: RISC-V: replace system_event.flags with ndata and data[0]
> KVM: tell userspace that system_event.ndata is valid
Is there any way we could clean this up in 5.18 and leave the whole
ndata/data pattern for 5.19?
IOW, for 5.18 go back and fix the padding:
struct {
__u32 type;
__u32 pad;
__u64 flags;
} system_event;
Then for 5.19 circle back on the data business, except use a flag bit
for it:
struct {
__u32 type;
__u32 pad;
#define KVM_SYSTEM_EVENT_NDATA_VALID (1u << 63)
__u64 flags;
__u64 ndata;
__u64 data[16];
} system_event;
Where we apply that bit to system_event::flags this time instead of
::type. Could also go the CAP route.
Wouldn't this be enough to preserve ABI with whatever userspace has been
spun up for system_event::flags already and also keep the SEV stuff
happy in 5.19?
It is a bit weird to churn existing UAPI usage in the very next kernel
cycle, but could be convinced otherwise :)
--
Thanks,
Oliver
On 4/22/22 09:58, Oliver Upton wrote:
> Is there any way we could clean this up in 5.18 and leave the whole
> ndata/data pattern for 5.19?
>
> IOW, for 5.18 go back and fix the padding:
>
> struct {
> __u32 type;
> __u32 pad;
> __u64 flags;
> } system_event;
>
> Then for 5.19 circle back on the data business, except use a flag bit
> for it:
>
> struct {
> __u32 type;
> __u32 pad;
> #define KVM_SYSTEM_EVENT_NDATA_VALID (1u << 63)
> __u64 flags;
> __u64 ndata;
> __u64 data[16];
> } system_event;
>
> Where we apply that bit to system_event::flags this time instead of
> ::type. Could also go the CAP route.
These patches are against kvm/next, so that is already what I did. :)
On the other hand right now the ARM and RISC-V flags are unusable with
32-bit userspace, so we need to fix _something_ in 5.18 as well. For
your proposal, all that's missing is a 5.18 patch to add the padding.
But since the flags UAPI was completely unused before 5.18 and there's
no reason to inflict the different naming of fields to userspace. So I
think we want to apply this UAPI change in 5.18 too.
Paolo