On Mon, 2020-12-14 at 13:19 +0000, Joao Martins wrote:
> But I think there's a flaw here. That is handling the case where you don't have a
> vcpu_info registered, and only shared info. The vcpu_info is then placed elsewhere, i.e.
> another offset out of shared_info -- which is *I think* the case for PVHVM Windows guests.
There is no such case any more. In my v3 patch set I *stopped* the
kernel from attempting to use the vcpu_info embedded in the shinfo, and
went to *requiring* that the VMM explicitly tell the kernel where it
is.
$ git diff xenpv-post-2..xenpv-post-3 -- Documentation
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index d98c2ff90880..d1c30105e6fd 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -4834,7 +4834,7 @@ experience inconsistent filtering behavior on MSR accesses.
__u64 gfn;
} shared_info;
struct {
- __u32 vcpu;
+ __u32 vcpu_id;
__u64 gpa;
} vcpu_attr;
__u64 pad[4];
@@ -4849,9 +4849,13 @@ KVM_XEN_ATTR_TYPE_LONG_MODE
KVM_XEN_ATTR_TYPE_SHARED_INFO
Sets the guest physical frame number at which the Xen "shared info"
- page resides. It is the default location for the vcpu_info for the
- first 32 vCPUs, and contains other VM-wide data structures shared
- between the VM and the host.
+ page resides. Note that although Xen places vcpu_info for the first
+ 32 vCPUs in the shared_info page, KVM does not automatically do so
+ and requires that KVM_XEN_ATTR_TYPE_VCPU_INFO be used explicitly
+ even when the vcpu_info for a given vCPU resides at the "default"
+ location in the shared_info page. This is because KVM is not aware
+ of the Xen CPU id which is used as the index into the vcpu_info[]
+ array, so cannot know the correct default location.
KVM_XEN_ATTR_TYPE_VCPU_INFO
Sets the guest physical address of the vcpu_info for a given vCPU.
> Perhaps introducing a helper which adds xen_vcpu_info() and returns you the hva (picking
> the right cache) similar to the RFC patch. Albeit that was with page pinning, but
> borrowing an older version I had with hva_to_gfn_cache incantation would probably look like:
>
>
> if (v->arch.xen.vcpu_info_set) {
> ghc = &v->arch.xen.vcpu_info_cache;
> } else {
> ghc = &v->arch.xen.vcpu_info_cache;
> offset += offsetof(struct shared_info, vcpu_info);
> offset += (v - kvm_get_vcpu_by_id(0)) * sizeof(struct vcpu_info);
> }
The problem is that we *can't* just use shared_info->vcpu_info[N]
because we don't know what N is.
This is what I spent half of last week chasing down, because whatever I
tried, the original version just ended up delivering interrupts to the
wrong CPUs.
The N we want there is actually the *XEN* vcpu_id, which Xen puts into
CPUID leaf 0x40000004.ebx and which is also the ACPI ID. (Those two had
better match up since Linux guests use CPUID 0x40000004 for the BSP and
ACPI for the APs).
The kernel knows nothing of those numbers. In particular they do *not*
match up to the indices in kvm->vcpus[M] (which is vcpu->vcpu_idx and
means nothing except the chronological order in which each vCPU's
userspace thread happened to create it during startup), and they do not
match up to vcpu->vcpu_id which is the APIC (not ACPI) ID.
The kernel doesn't know. Let userspace tell it, since we already needed
that API for the separate vcpu_info case anyway.
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
