kvm_para: add mmio word store hypercall
Subsystems (4)
RFC PATCH 1/1 kvm_para: add mmio word store hypercall
We face a dilemma: IO mapped addresses are legacy, so, for example, PCI express bridges waste 4K of this space for each link, in effect limiting us to 16 devices using this space.
Memory is supposed to replace them, but memory exits are much slower than PIO because of the need for emulation and page walks.
As a solution, this patch adds an MMIO hypercall with the guest physical address + data.
I did test that this works but didn't benchmark yet.
TODOs:
This only implements a 2 bytes write since this is the minimum required for virtio, but we'll probably need at least 1 byte reads (for ISR read).
We can support up to 8 byte reads/writes for 64 bit guests and up to 4 bytes for 32 ones - better limit to 4 bytes for everyone for consistency, or support the maximum that we can?
Further, a feature bit will need to be exposed to guests so they know they can use the feature.
Need to test performance impact.
Finally the patch was on an ancient kvm version and will need to be rebased.
Posting here for early flames/feedback.
--- arch/x86/kvm/svm.c | 3 +-- arch/x86/kvm/vmx.c | 3 +-- arch/x86/kvm/x86.c | 14 ++++++++++++++ include/linux/kvm_para.h | 1 + 4 files changed, 17 insertions(+), 4 deletions(-)
arch/x86/kvm/svm.c +1 -2
@@ -1833,8 +1833,7 @@ static int vmmcall_interception(struct vcpu_svm *svm) { svm->next_rip =kvm_rip_read (&svm->vcpu) +3 ;skip_emulated_instruction (&svm->vcpu); -kvm_emulate_hypercall (&svm->vcpu); -return 1 ; +return kvm_emulate_hypercall (&svm->vcpu); }static unsigned long nested_svm_get_tdp_cr3 (struct kvm_vcpu *vcpu)
arch/x86/kvm/vmx.c +1 -2
@@ -4597,8 +4597,7 @@ static int handle_halt(struct kvm_vcpu *vcpu)static int handle_vmcall (struct kvm_vcpu *vcpu) {skip_emulated_instruction (vcpu); -kvm_emulate_hypercall (vcpu); -return 1 ; +return kvm_emulate_hypercall (vcpu); }static int handle_invd (struct kvm_vcpu *vcpu)
arch/x86/kvm/x86.c +14 -0
@@ -4915,7 +4915,9 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)int kvm_emulate_hypercall (struct kvm_vcpu *vcpu) { +struct kvm_run *run = vcpu->run;unsigned long nr, a0, a1, a2, a3, ret; +gpa_t gpa;int r =1 ;if (kvm_hv_hypercall_enabled (vcpu->kvm))
@@ -4946,12 +4948,24 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)case KVM_HC_VAPIC_POLL_IRQ : ret =0 ;break ; +case KVM_HC_MMIO_STORE_WORD : + gpa =hc_gpa (vcpu, a1, a2); +if (!write_mmio (vcpu, gpa,2 , &a0) && run) { + run->exit_reason =KVM_EXIT_MMIO ; + run->mmio.phys_addr = gpa; +memcpy (run->mmio.data, &a0,2 ); + run->mmio.len =2 ; + run->mmio.is_write =1 ; + r =0 ; + } + goto noret;default : ret = -KVM_ENOSYS ;break ; } out:kvm_register_write (vcpu,VCPU_REGS_RAX , ret); +noret: ++vcpu->stat.hypercalls;return r; }
include/linux/kvm_para.h +1 -0
@@ -19,6 +19,7 @@#define KVM_HC_MMU_OP 2#define KVM_HC_FEATURES 3#define KVM_HC_PPC_MAP_MAGIC_PAGE 4 +#define KVM_HC_MMIO_STORE_WORD 5/* * hypercalls use architecture specific
Signature · 1 line
1.7.9.111.gf3fb0
H. Peter Anvin
reviewerQuoted 4 lines · Michael S. Tsirkin
We face a dilemma: IO mapped addresses are legacy, so, for example, PCI express bridges waste 4K of this space for each link, in effect limiting us to 16 devices using this space.
That is *only* if they are physical devices on PCIe links. For virtual devices you have no such limitation. What is the problem again?
-hpa
Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf.
Quoted 8 lines · H. Peter Anvin
Quoted 4 lines · Michael S. Tsirkin
We face a dilemma: IO mapped addresses are legacy, so, for example, PCI express bridges waste 4K of this space for each link, in effect limiting us to 16 devices using this space.
That is *only* if they are physical devices on PCIe links. For virtual devices you have no such limitation.
Why don't we? We emulate pci express with exact same limitations for virtual and physical devices.
Quoted 7 lines
What is the problem again?
-hpa
Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf.
Quoted 14 lines · Michael S. Tsirkin
We face a dilemma: IO mapped addresses are legacy, so, for example, PCI express bridges waste 4K of this space for each link, in effect limiting us to 16 devices using this space.
Memory is supposed to replace them, but memory exits are much slower than PIO because of the need for emulation and page walks.
As a solution, this patch adds an MMIO hypercall with the guest physical address + data.
I did test that this works but didn't benchmark yet.
Quoted 8 lines
TODOs:
This only implements a 2 bytes write since this is the minimum required for virtio, but we'll probably need at least 1 byte reads (for ISR read).
We can support up to 8 byte reads/writes for 64 bit guests and up to 4 bytes for 32 ones - better limit to 4 bytes for everyone for consistency, or support the maximum that we can?
Let's support the maximum we can.
Quoted 17 lines
static int
handle_invd(struct kvm_vcpu *vcpu)arch/x86/kvm/x86.c +2
-0@@ -4915,7 +4915,9 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)int kvm_emulate_hypercall (struct kvm_vcpu *vcpu) { +struct kvm_run *run = vcpu->run;unsigned long nr, a0, a1, a2, a3, ret; +gpa_t gpa;int r =1 ;if (kvm_hv_hypercall_enabled (vcpu->kvm))
arch/x86/kvm/x86.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) · line 4951
Quoted 4 lines
arch/x86/kvm/x86.c +1
-0@@ -4946,12 +4948,24 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)case KVM_HC_VAPIC_POLL_IRQ : ret =0 ;break ; +case KVM_HC_MMIO_STORE_WORD :
HC_MEMORY_WRITE
arch/x86/kvm/x86.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) · line 4953
Quoted 2 lines
arch/x86/kvm/x86.c +2
-0+ gpa =hc_gpa (vcpu, a1, a2); +if (!write_mmio (vcpu, gpa,2 , &a0) && run) {
What's this && run thing?
arch/x86/kvm/x86.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) · line 4961
Quoted 8 lines
arch/x86/kvm/x86.c +8
-0+ run->exit_reason =KVM_EXIT_MMIO ; + run->mmio.phys_addr = gpa; +memcpy (run->mmio.data, &a0,2 ); + run->mmio.len =2 ; + run->mmio.is_write =1 ; + r =0 ; + } + goto noret;
What if the address is in RAM?
Note the guest can't tell if a piece of memory is direct mapped or implemented as mmio.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 41 lines · Avi Kivity
Quoted 14 lines · Michael S. Tsirkin
We face a dilemma: IO mapped addresses are legacy, so, for example, PCI express bridges waste 4K of this space for each link, in effect limiting us to 16 devices using this space.
Memory is supposed to replace them, but memory exits are much slower than PIO because of the need for emulation and page walks.
As a solution, this patch adds an MMIO hypercall with the guest physical address + data.
I did test that this works but didn't benchmark yet.
Quoted 8 lines
TODOs:
This only implements a 2 bytes write since this is the minimum required for virtio, but we'll probably need at least 1 byte reads (for ISR read).
We can support up to 8 byte reads/writes for 64 bit guests and up to 4 bytes for 32 ones - better limit to 4 bytes for everyone for consistency, or support the maximum that we can?Let's support the maximum we can.
Quoted 15 lines
static int
handle_invd(struct kvm_vcpu *vcpu)arch/x86/kvm/x86.c +2
-0@@ -4915,7 +4915,9 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)int kvm_emulate_hypercall (struct kvm_vcpu *vcpu) { +struct kvm_run *run = vcpu->run;unsigned long nr, a0, a1, a2, a3, ret; +gpa_t gpa;int r =1 ;
Quoted 8 lines
arch/x86/kvm/x86.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) · line 4951
Quoted 6 lines
arch/x86/kvm/x86.c +1
-0if (kvm_hv_hypercall_enabled (vcpu->kvm))@@ -4946,12 +4948,24 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)case KVM_HC_VAPIC_POLL_IRQ : ret =0 ;break ; +case KVM_HC_MMIO_STORE_WORD :
HC_MEMORY_WRITE
Do we really want guests to access random memory this way though? Even though it can, how about HC_PCI_MEMORY_WRITE to stress the intended usage?
See also discussion below.
Quoted 4 lines
arch/x86/kvm/x86.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) · line 4953
Quoted 2 lines
arch/x86/kvm/x86.c +2
-0+ gpa =hc_gpa (vcpu, a1, a2); +if (!write_mmio (vcpu, gpa,2 , &a0) && run) {What's this && run thing?
I'm not sure - copied this from another other place in emulation:arch/x86/kvm/x86.c:4953: if (!write_mmio(vcpu, gpa, 2, &a0) && run)
I assumed there's some way to trigger emulation while VCPU does not run. No?
Quoted 13 lines
arch/x86/kvm/x86.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) · line 4961
Quoted 9 lines
arch/x86/kvm/x86.c +8
-0+ run->exit_reason =KVM_EXIT_MMIO ; + run->mmio.phys_addr = gpa; +memcpy (run->mmio.data, &a0,2 ); + run->mmio.len =2 ; + run->mmio.is_write =1 ; + r =0 ; + } + goto noret;What if the address is in RAM?
Note the guest can't tell if a piece of memory is direct mapped or implemented as mmio.
True but doing hypercalls for memory which can be
mapped directly is bad for performance - it's
the reverse of what we are trying to do here.
The intent is to use this for virtio where we can explicitly let the guest know whether using a hypercall is safe.
Acceptable? What do you suggest?
Quoted 3 lines
Signature · 2 lines
-- error compiling committee.c: too many arguments to function
Quoted 11 lines · Michael S. Tsirkin
Quoted 5 lines
arch/x86/kvm/x86.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) · line 4953
Quoted 3 lines
arch/x86/kvm/x86.c +2
-0+ gpa =hc_gpa (vcpu, a1, a2); +if (!write_mmio (vcpu, gpa,2 , &a0) && run) {What's this && run thing?
I'm not sure - copied this from another other place in emulation:
arch/x86/kvm/x86.c:4953: if (!write_mmio(vcpu, gpa, 2, &a0) && run)I assumed there's some way to trigger emulation while VCPU does not run. No?
Not the way you initialize run above.
Quoted 15 lines
Quoted 14 lines
arch/x86/kvm/x86.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) · line 4961
Quoted 10 lines
arch/x86/kvm/x86.c +8
-0+ run->exit_reason =KVM_EXIT_MMIO ; + run->mmio.phys_addr = gpa; +memcpy (run->mmio.data, &a0,2 ); + run->mmio.len =2 ; + run->mmio.is_write =1 ; + r =0 ; + } + goto noret;What if the address is in RAM?
Note the guest can't tell if a piece of memory is direct mapped or implemented as mmio.
Quoted 3 lines
True but doing hypercalls for memory which can be
mapped directly is bad for performance - it's
the reverse of what we are trying to do here.
It's bad, but the guest can't tell.
Suppose someone implements virtio in hardware and we pass it through to a guest. It should continue working, no?
Quoted 4 lines
The intent is to use this for virtio where we can explicitly let the guest know whether using a hypercall is safe.
Acceptable? What do you suggest?
It's iffy.
What's the performance gain from this thing?
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 7 lines · Avi Kivity
Quoted 6 lines · Michael S. Tsirkin
Quoted 5 lines
arch/x86/kvm/x86.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) · line 4953
Quoted 3 lines
arch/x86/kvm/x86.c +2
-0+ gpa =hc_gpa (vcpu, a1, a2); +if (!write_mmio (vcpu, gpa,2 , &a0) && run) {What's this && run thing?
Quoted 7 lines
Quoted 5 lines
I'm not sure - copied this from another other place in emulation:
arch/x86/kvm/x86.c:4953: if (!write_mmio(vcpu, gpa, 2, &a0) && run)I assumed there's some way to trigger emulation while VCPU does not run. No?
Not the way you initialize run above.
Thanks for pointing this out, I'll drop the test.
Quoted 15 lines
Quoted 15 lines
Quoted 14 lines
arch/x86/kvm/x86.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) · line 4961
Quoted 10 lines
arch/x86/kvm/x86.c +8
-0+ run->exit_reason =KVM_EXIT_MMIO ; + run->mmio.phys_addr = gpa; +memcpy (run->mmio.data, &a0,2 ); + run->mmio.len =2 ; + run->mmio.is_write =1 ; + r =0 ; + } + goto noret;What if the address is in RAM?
Note the guest can't tell if a piece of memory is direct mapped or implemented as mmio.
Quoted 8 lines
Quoted 3 lines
True but doing hypercalls for memory which can be
mapped directly is bad for performance - it's
the reverse of what we are trying to do here.It's bad, but the guest can't tell.
Suppose someone implements virtio in hardware and we pass it through to a guest. It should continue working, no?
Why would we want hypercalls then?
As I see it, virtio device would have a capability that tells the guest to use hypercalls for access. An actual PCI device won't expose this capability, as would a device on a host which lacks the hypercall.
Quoted 6 lines
Quoted 4 lines
The intent is to use this for virtio where we can explicitly let the guest know whether using a hypercall is safe.
Acceptable? What do you suggest?
It's iffy.
Question is, do we want a bunch of dead code sitting there just in case? And what are the chances it'll work correctly when we need it to?
Quoted 1 line
What's the performance gain from this thing?
I'll test and post separately.
Quoted 3 lines
Signature · 2 lines
-- error compiling committee.c: too many arguments to function
Quoted 24 lines · Michael S. Tsirkin
Quoted 15 lines
Quoted 15 lines
Quoted 14 lines
arch/x86/kvm/x86.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) · line 4961
Quoted 10 lines
arch/x86/kvm/x86.c +8
-0+ run->exit_reason =KVM_EXIT_MMIO ; + run->mmio.phys_addr = gpa; +memcpy (run->mmio.data, &a0,2 ); + run->mmio.len =2 ; + run->mmio.is_write =1 ; + r =0 ; + } + goto noret;What if the address is in RAM?
Note the guest can't tell if a piece of memory is direct mapped or implemented as mmio.Quoted 8 lines
Quoted 3 lines
True but doing hypercalls for memory which can be
mapped directly is bad for performance - it's
the reverse of what we are trying to do here.It's bad, but the guest can't tell.
Suppose someone implements virtio in hardware and we pass it through to a guest. It should continue working, no?
Quoted 6 lines
Why would we want hypercalls then?
As I see it, virtio device would have a capability that tells the guest to use hypercalls for access. An actual PCI device won't expose this capability, as would a device on a host which lacks the hypercall.
Ok, makes sense.
Quoted 10 lines
Quoted 6 lines
Quoted 4 lines
The intent is to use this for virtio where we can explicitly let the guest know whether using a hypercall is safe.
Acceptable? What do you suggest?
It's iffy.
Question is, do we want a bunch of dead code sitting there just in case? And what are the chances it'll work correctly when we need it to?
If we make it device specific, I guess not.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 56 lines · Michael S. Tsirkin
Quoted 41 lines · Avi Kivity
Quoted 14 lines · Michael S. Tsirkin
We face a dilemma: IO mapped addresses are legacy, so, for example, PCI express bridges waste 4K of this space for each link, in effect limiting us to 16 devices using this space.
Memory is supposed to replace them, but memory exits are much slower than PIO because of the need for emulation and page walks.
As a solution, this patch adds an MMIO hypercall with the guest physical address + data.
I did test that this works but didn't benchmark yet.
Quoted 8 lines
TODOs:
This only implements a 2 bytes write since this is the minimum required for virtio, but we'll probably need at least 1 byte reads (for ISR read).
We can support up to 8 byte reads/writes for 64 bit guests and up to 4 bytes for 32 ones - better limit to 4 bytes for everyone for consistency, or support the maximum that we can?Let's support the maximum we can.
Quoted 15 lines
static int
handle_invd(struct kvm_vcpu *vcpu)arch/x86/kvm/x86.c +2
-0@@ -4915,7 +4915,9 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)int kvm_emulate_hypercall (struct kvm_vcpu *vcpu) { +struct kvm_run *run = vcpu->run;unsigned long nr, a0, a1, a2, a3, ret; +gpa_t gpa;int r =1 ;Quoted 8 lines
arch/x86/kvm/x86.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) · line 4951
Quoted 6 lines
arch/x86/kvm/x86.c +1
-0if (kvm_hv_hypercall_enabled (vcpu->kvm))@@ -4946,12 +4948,24 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)case KVM_HC_VAPIC_POLL_IRQ : ret =0 ;break ; +case KVM_HC_MMIO_STORE_WORD :
HC_MEMORY_WRITEDo we really want guests to access random memory this way though? Even though it can, how about
HC_PCI_MEMORY_WRITEto stress the intended usage?
See also discussion below.
Quoted 8 lines
Quoted 4 lines
arch/x86/kvm/x86.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) · line 4953
Quoted 2 lines
arch/x86/kvm/x86.c +2
-0+ gpa =hc_gpa (vcpu, a1, a2); +if (!write_mmio (vcpu, gpa,2 , &a0) && run) {What's this && run thing?
I'm not sure - copied this from another other place in emulation:
arch/x86/kvm/x86.c:4953: if (!write_mmio(vcpu, gpa, 2, &a0) && run)
What git tree is this from? I think that's the one you added.
Quoted 27 lines
I assumed there's some way to trigger emulation while VCPU does not run. No?
Quoted 13 lines
arch/x86/kvm/x86.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) · line 4961
Quoted 9 lines
arch/x86/kvm/x86.c +8
-0+ run->exit_reason =KVM_EXIT_MMIO ; + run->mmio.phys_addr = gpa; +memcpy (run->mmio.data, &a0,2 ); + run->mmio.len =2 ; + run->mmio.is_write =1 ; + r =0 ; + } + goto noret;What if the address is in RAM?
Note the guest can't tell if a piece of memory is direct mapped or implemented as mmio.True but doing hypercalls for memory which can be
mapped directly is bad for performance - it's
the reverse of what we are trying to do here.The intent is to use this for virtio where we can explicitly let the guest know whether using a hypercall is safe.
Acceptable? What do you suggest?
Quoted 1 line
Quoted 7 lines
Quoted 2 lines
Signature · 1 line
error compiling committee.c: too many arguments to function--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
Gleb.
Quoted 9 lines · Gleb Natapov
Quoted 8 lines
Quoted 4 lines
arch/x86/kvm/x86.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) · line 4953
Quoted 2 lines
arch/x86/kvm/x86.c +2
-0+ gpa =hc_gpa (vcpu, a1, a2); +if (!write_mmio (vcpu, gpa,2 , &a0) && run) {What's this && run thing?
I'm not sure - copied this from another other place in emulation:
arch/x86/kvm/x86.c:4953: if (!write_mmio(vcpu, gpa, 2, &a0) && run)What git tree is this from? I think that's the one you added.
Ugh. My mistake. So the answer is this, as I said, this is on an ancient tree that did not have 112592da0dc2460c95e8a89d0c5657c6a30286aa
I'll remove this check, thanks for pointing this out.