| Message ID | 20231108111806.92604-10-nsaenz@amazon.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: hyperv: Introduce VSM support | expand |
On 08.11.23 12:17, Nicolas Saenz Julienne wrote: > Introduce two helper functions. The first one queries a vCPU's VTL > level, the second one, given a struct kvm_vcpu and VTL pair, returns the > corresponding 'sibling' struct kvm_vcpu at the right VTL. > > We keep track of each VTL's state by having a distinct struct kvm_vpcu > for each level. VTL-vCPUs that belong to the same guest CPU share the > same physical APIC id, but belong to different APIC groups where the > apic group represents the vCPU's VTL. > > Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com> > --- > arch/x86/kvm/hyperv.h | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h > index 2bfed69ba0db..5433107e7cc8 100644 > --- a/arch/x86/kvm/hyperv.h > +++ b/arch/x86/kvm/hyperv.h > @@ -23,6 +23,7 @@ > > #include <linux/kvm_host.h> > #include "x86.h" > +#include "lapic.h" > > /* "Hv#1" signature */ > #define HYPERV_CPUID_SIGNATURE_EAX 0x31237648 > @@ -83,6 +84,23 @@ static inline struct kvm_hv_syndbg *to_hv_syndbg(struct kvm_vcpu *vcpu) > return &vcpu->kvm->arch.hyperv.hv_syndbg; > } > > +static inline struct kvm_vcpu *kvm_hv_get_vtl_vcpu(struct kvm_vcpu *vcpu, int vtl) > +{ > + struct kvm *kvm = vcpu->kvm; > + u32 target_id = kvm_apic_id(vcpu); > + > + kvm_apic_id_set_group(kvm, vtl, &target_id); > + if (vcpu->vcpu_id == target_id) > + return vcpu; > + > + return kvm_get_vcpu_by_id(kvm, target_id); > +} > + > +static inline u8 kvm_hv_get_active_vtl(struct kvm_vcpu *vcpu) > +{ > + return kvm_apic_group(vcpu); Shouldn't this check whether VTL is active? If someone wants to use APIC groups for a different purpose in the future, they'd suddenly find themselves in VTL code paths in other code (such as memory protections), no? Alex Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879
On Wed Nov 8, 2023 at 12:21 PM UTC, Alexander Graf wrote: > > On 08.11.23 12:17, Nicolas Saenz Julienne wrote: > > Introduce two helper functions. The first one queries a vCPU's VTL > > level, the second one, given a struct kvm_vcpu and VTL pair, returns the > > corresponding 'sibling' struct kvm_vcpu at the right VTL. > > > > We keep track of each VTL's state by having a distinct struct kvm_vpcu > > for each level. VTL-vCPUs that belong to the same guest CPU share the > > same physical APIC id, but belong to different APIC groups where the > > apic group represents the vCPU's VTL. > > > > Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com> > > --- > > arch/x86/kvm/hyperv.h | 18 ++++++++++++++++++ > > 1 file changed, 18 insertions(+) > > > > diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h > > index 2bfed69ba0db..5433107e7cc8 100644 > > --- a/arch/x86/kvm/hyperv.h > > +++ b/arch/x86/kvm/hyperv.h > > @@ -23,6 +23,7 @@ > > > > #include <linux/kvm_host.h> > > #include "x86.h" > > +#include "lapic.h" > > > > /* "Hv#1" signature */ > > #define HYPERV_CPUID_SIGNATURE_EAX 0x31237648 > > @@ -83,6 +84,23 @@ static inline struct kvm_hv_syndbg *to_hv_syndbg(struct kvm_vcpu *vcpu) > > return &vcpu->kvm->arch.hyperv.hv_syndbg; > > } > > > > +static inline struct kvm_vcpu *kvm_hv_get_vtl_vcpu(struct kvm_vcpu *vcpu, int vtl) > > +{ > > + struct kvm *kvm = vcpu->kvm; > > + u32 target_id = kvm_apic_id(vcpu); > > + > > + kvm_apic_id_set_group(kvm, vtl, &target_id); > > + if (vcpu->vcpu_id == target_id) > > + return vcpu; > > + > > + return kvm_get_vcpu_by_id(kvm, target_id); > > +} > > + > > +static inline u8 kvm_hv_get_active_vtl(struct kvm_vcpu *vcpu) > > +{ > > + return kvm_apic_group(vcpu); > > Shouldn't this check whether VTL is active? If someone wants to use APIC > groups for a different purpose in the future, they'd suddenly find > themselves in VTL code paths in other code (such as memory protections), no? Yes, indeed. This is solved by adding a couple of checks vs kvm_hv_vsm_enabled(). I don't have another use-case in mind for APIC ID groups so it's hard to picture if I'm just over engineering things, but I wonder it we need to introduce some sort of protection vs concurrent usages. For example we could introduce masks within the group bits and have consumers explicitly request what they want. Something like: vtl = kvm_apic_group(vcpu, HV_VTL); If user-space didn't reserve bits within the APIC ID group area and marked them with HV_VTL you'd get an error as opposed to 0 which is otherwise a valid group. Nicolas
On Wed, 2023-11-08 at 11:17 +0000, Nicolas Saenz Julienne wrote: > Introduce two helper functions. The first one queries a vCPU's VTL > level, the second one, given a struct kvm_vcpu and VTL pair, returns the > corresponding 'sibling' struct kvm_vcpu at the right VTL. > > We keep track of each VTL's state by having a distinct struct kvm_vpcu > for each level. VTL-vCPUs that belong to the same guest CPU share the > same physical APIC id, but belong to different APIC groups where the > apic group represents the vCPU's VTL. > > Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com> > --- > arch/x86/kvm/hyperv.h | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h > index 2bfed69ba0db..5433107e7cc8 100644 > --- a/arch/x86/kvm/hyperv.h > +++ b/arch/x86/kvm/hyperv.h > @@ -23,6 +23,7 @@ > > #include <linux/kvm_host.h> > #include "x86.h" > +#include "lapic.h" > > /* "Hv#1" signature */ > #define HYPERV_CPUID_SIGNATURE_EAX 0x31237648 > @@ -83,6 +84,23 @@ static inline struct kvm_hv_syndbg *to_hv_syndbg(struct kvm_vcpu *vcpu) > return &vcpu->kvm->arch.hyperv.hv_syndbg; > } > > +static inline struct kvm_vcpu *kvm_hv_get_vtl_vcpu(struct kvm_vcpu *vcpu, int vtl) > +{ > + struct kvm *kvm = vcpu->kvm; > + u32 target_id = kvm_apic_id(vcpu); > + > + kvm_apic_id_set_group(kvm, vtl, &target_id); > + if (vcpu->vcpu_id == target_id) > + return vcpu; > + > + return kvm_get_vcpu_by_id(kvm, target_id); > +} > + > +static inline u8 kvm_hv_get_active_vtl(struct kvm_vcpu *vcpu) > +{ > + return kvm_apic_group(vcpu); > +} > + > static inline u32 kvm_hv_get_vpindex(struct kvm_vcpu *vcpu) > { > struct kvm_vcpu_hv *hv_vcpu = to_hv_vcpu(vcpu); Ideally I'll prefer the kernel to not know the VTL mapping at all but rather, that each vCPU be assigned to an apic group / namespace and has its assigned VTL. Then the kernel works in this way: * Regular APIC IPI -> send it to the apic namespace to which the sender belongs or if we go with the idea of using multiple VMs, then this will work unmodified. * Hardware interrupt -> send it to the vCPU/VM which userspace configured it to send via GSI mappings. * HyperV IPI -> if same VTL as the vCPU assigned VTL -> deal with it the same as with regular IPI -> otherwise exit to the userspace. * Page fault -> if related to violation of current VTL protection, exit to userspace, and the userspace can then queue the SynIC message, and wakeup the higher VTL. Best regards, Maxim Levitsky
diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h index 2bfed69ba0db..5433107e7cc8 100644 --- a/arch/x86/kvm/hyperv.h +++ b/arch/x86/kvm/hyperv.h @@ -23,6 +23,7 @@ #include <linux/kvm_host.h> #include "x86.h" +#include "lapic.h" /* "Hv#1" signature */ #define HYPERV_CPUID_SIGNATURE_EAX 0x31237648 @@ -83,6 +84,23 @@ static inline struct kvm_hv_syndbg *to_hv_syndbg(struct kvm_vcpu *vcpu) return &vcpu->kvm->arch.hyperv.hv_syndbg; } +static inline struct kvm_vcpu *kvm_hv_get_vtl_vcpu(struct kvm_vcpu *vcpu, int vtl) +{ + struct kvm *kvm = vcpu->kvm; + u32 target_id = kvm_apic_id(vcpu); + + kvm_apic_id_set_group(kvm, vtl, &target_id); + if (vcpu->vcpu_id == target_id) + return vcpu; + + return kvm_get_vcpu_by_id(kvm, target_id); +} + +static inline u8 kvm_hv_get_active_vtl(struct kvm_vcpu *vcpu) +{ + return kvm_apic_group(vcpu); +} + static inline u32 kvm_hv_get_vpindex(struct kvm_vcpu *vcpu) { struct kvm_vcpu_hv *hv_vcpu = to_hv_vcpu(vcpu);
Introduce two helper functions. The first one queries a vCPU's VTL level, the second one, given a struct kvm_vcpu and VTL pair, returns the corresponding 'sibling' struct kvm_vcpu at the right VTL. We keep track of each VTL's state by having a distinct struct kvm_vpcu for each level. VTL-vCPUs that belong to the same guest CPU share the same physical APIC id, but belong to different APIC groups where the apic group represents the vCPU's VTL. Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com> --- arch/x86/kvm/hyperv.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)