Re: [PATCH 3/4] KVM: Emulate MSI-X table in kernel
On Wednesday 02 March 2011 04:18:58 Marcelo Tosatti wrote:
> On Fri, Feb 25, 2011 at 10:29:38AM +0200, Michael S. Tsirkin wrote:
> > On Fri, Feb 25, 2011 at 02:28:02PM +0800, Sheng Yang wrote:
> > > On Thursday 24 February 2011 18:45:08 Michael S. Tsirkin wrote:
> > > > On Thu, Feb 24, 2011 at 05:51:04PM +0800, Sheng Yang wrote:
> > > > > Then we can support mask bit operation of assigned devices now.
> > > > >
> > > > > Signed-off-by: Sheng Yang
> > > >
> > > > Doesn't look like all comments got addressed.
> > > > E.g. gpa_t entry_base is still there and in reality
> > > > you said it's a host virtual address so
> > > > should be void __user *;
> > >
> > > Would update it.
> > >
> > > > And ENOTSYNC meaning 'MSIX' is pretty hacky.
> > >
> > > I'd like to discuss it later. We may need some work on all MMIO
> > > handling side to make it more straightforward. But I don't want to
> > > bundle it with this one...
> >
> > It's not PCI related so I'll defer to Avi/Marcelo on this.
> > Are you guys happy with the ENOTSYNC meaning 'MSIX'
>
> What would be a better alternative to ENOTSYNC? Can't see any.
>
> > and userspace_exit_needed hacks in this code?
>
> I thought this was handled by mmio_needed in a previous patch?
>
> Since x86_emulate_instruction does
>
> } else if (vcpu->mmio_needed) {
> if (vcpu->mmio_is_write)
> vcpu->mmio_needed = 0;
> r = EMULATE_DO_MMIO;
>
> It should be fine. Sheng why did you introduce userspace_exit_needed?
Because strictly speaking it's not MMIO exit, I don't know if Avi would object
the
confusing concept here, so I introduced another type of exit.
But if it's OK, I still would use mmio_needed in the next version, which is
also
more simple.
--
regards
Yang, Sheng
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] KVM: Emulate MSI-X table in kernel
On Tue, Mar 01, 2011 at 05:18:58PM -0300, Marcelo Tosatti wrote:
> On Fri, Feb 25, 2011 at 10:29:38AM +0200, Michael S. Tsirkin wrote:
> > On Fri, Feb 25, 2011 at 02:28:02PM +0800, Sheng Yang wrote:
> > > On Thursday 24 February 2011 18:45:08 Michael S. Tsirkin wrote:
> > > > On Thu, Feb 24, 2011 at 05:51:04PM +0800, Sheng Yang wrote:
> > > > > Then we can support mask bit operation of assigned devices now.
> > > > >
> > > > > Signed-off-by: Sheng Yang
> > > >
> > > > Doesn't look like all comments got addressed.
> > > > E.g. gpa_t entry_base is still there and in reality
> > > > you said it's a host virtual address so
> > > > should be void __user *;
> > >
> > > Would update it.
> > >
> > > > And ENOTSYNC meaning 'MSIX' is pretty hacky.
> > >
> > > I'd like to discuss it later. We may need some work on all MMIO handling
> > > side to
> > > make it more straightforward. But I don't want to bundle it with this
> > > one...
> >
> > It's not PCI related so I'll defer to Avi/Marcelo on this.
> > Are you guys happy with the ENOTSYNC meaning 'MSIX'
>
> What would be a better alternative to ENOTSYNC? Can't see any.
Return a negative value on error, positive exit code
if we want to exit to userspace.
As a bonus MSIX knowledge is localized in one file.
> > and userspace_exit_needed hacks in this code?
>
> I thought this was handled by mmio_needed in a previous patch?
>
> Since x86_emulate_instruction does
>
> } else if (vcpu->mmio_needed) {
> if (vcpu->mmio_is_write)
> vcpu->mmio_needed = 0;
> r = EMULATE_DO_MMIO;
>
> It should be fine. Sheng why did you introduce userspace_exit_needed?
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] KVM: Emulate MSI-X table in kernel
On Fri, Feb 25, 2011 at 10:29:38AM +0200, Michael S. Tsirkin wrote:
> On Fri, Feb 25, 2011 at 02:28:02PM +0800, Sheng Yang wrote:
> > On Thursday 24 February 2011 18:45:08 Michael S. Tsirkin wrote:
> > > On Thu, Feb 24, 2011 at 05:51:04PM +0800, Sheng Yang wrote:
> > > > Then we can support mask bit operation of assigned devices now.
> > > >
> > > > Signed-off-by: Sheng Yang
> > >
> > > Doesn't look like all comments got addressed.
> > > E.g. gpa_t entry_base is still there and in reality
> > > you said it's a host virtual address so
> > > should be void __user *;
> >
> > Would update it.
> >
> > > And ENOTSYNC meaning 'MSIX' is pretty hacky.
> >
> > I'd like to discuss it later. We may need some work on all MMIO handling
> > side to
> > make it more straightforward. But I don't want to bundle it with this
> > one...
>
> It's not PCI related so I'll defer to Avi/Marcelo on this.
> Are you guys happy with the ENOTSYNC meaning 'MSIX'
What would be a better alternative to ENOTSYNC? Can't see any.
> and userspace_exit_needed hacks in this code?
I thought this was handled by mmio_needed in a previous patch?
Since x86_emulate_instruction does
} else if (vcpu->mmio_needed) {
if (vcpu->mmio_is_write)
vcpu->mmio_needed = 0;
r = EMULATE_DO_MMIO;
It should be fine. Sheng why did you introduce userspace_exit_needed?
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] KVM: Emulate MSI-X table in kernel
On Tue, Mar 01, 2011 at 02:20:02PM +0200, Michael S. Tsirkin wrote:
> On Tue, Mar 01, 2011 at 02:10:37PM +0800, Sheng Yang wrote:
> > On Monday 28 February 2011 19:27:29 Michael S. Tsirkin wrote:
> > > On Mon, Feb 28, 2011 at 03:20:04PM +0800, Sheng Yang wrote:
> > > > @@ -1877,6 +1879,24 @@ static long kvm_vm_ioctl(struct file *filp,
> > > >
> > > > mutex_unlock(&kvm->lock);
> > > > break;
> > > >
> > > > #endif
> > > >
> > > > + case KVM_REGISTER_MSIX_MMIO: {
> > > > + struct kvm_msix_mmio_user mmio_user;
> > > > +
> > > > + r = -EFAULT;
> > > > + if (copy_from_user(&mmio_user, argp, sizeof mmio_user))
> > > > + goto out;
> > > > + r = kvm_vm_ioctl_register_msix_mmio(kvm, &mmio_user);
> > > > + break;
> > > > + }
> > > > + case KVM_UNREGISTER_MSIX_MMIO: {
> > > > + struct kvm_msix_mmio_user mmio_user;
> > > > +
> > > > + r = -EFAULT;
> > > > + if (copy_from_user(&mmio_user, argp, sizeof mmio_user))
> > > > + goto out;
> > > > + r = kvm_vm_ioctl_unregister_msix_mmio(kvm, &mmio_user);
> > > > + break;
> > > > + }
> > > >
> > > > default:
> > > > r = kvm_arch_vm_ioctl(filp, ioctl, arg);
> > > > if (r == -ENOTTY)
> > > >
> > > > @@ -1988,6 +2008,12 @@ static int kvm_dev_ioctl_create_vm(void)
> > > >
> > > > return r;
> > > >
> > > > }
> > > >
> > > > #endif
> > > >
> > > > + r = kvm_register_msix_mmio_dev(kvm);
> > > > + if (r < 0) {
> > > > + kvm_put_kvm(kvm);
> > > > + return r;
> > > > + }
> > > > +
> > >
> > > Need to fix error handling below as well?
> > > Better do it with chained gotos if yes.
> >
> > Let's make it another separate patch.
>
> Well you add a new failure mode, you need to cleanup
> properly ...
Oh, I think kvm_put_kvm() should take care of this?
--
regards
Yang, Sheng
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] KVM: Emulate MSI-X table in kernel
On Tue, Mar 01, 2011 at 02:20:02PM +0200, Michael S. Tsirkin wrote:
> On Tue, Mar 01, 2011 at 02:10:37PM +0800, Sheng Yang wrote:
> > On Monday 28 February 2011 19:27:29 Michael S. Tsirkin wrote:
> > > On Mon, Feb 28, 2011 at 03:20:04PM +0800, Sheng Yang wrote:
> > > > Then we can support mask bit operation of assigned devices now.
> > > >
> > > > Signed-off-by: Sheng Yang
> > >
> > > A general question: we implement mmio read and write
> > > operation here, but seem to do nothing
> > > about ordering. In particular, pci mmio reads are normally
> > > assumed to flush all writes out to the device
> > > and all device writes in to the CPU.
> > >
> > > How exactly does it work here?
> >
> > I don't understand your problem... We emulate all operation here, where is
> > "ordering" issue?
> >
> > And Michael, thanks for you detail comments, but could you give comments
> > all at
> > once? Now I've tried my best to address comments, but still feeling endless
> > comments are coming.
>
> Hmm, sorry about that. At least some of the comments are in
> the new code, so I could not have commented on it earlier ...
> E.g. the ext_data thing only appeared in the latest version
> or the one before that. In some cases such as the non-standard
> error codes used, I don't always udnerstand the logic, as
> the error handling is switched to standard conventions
> so comments appear as the logic becomes apparent. This
> applies to EFAULT handling below.
>
> > And I would leave Intel this Friday, I want to get it done
> > before I leave.
>
> Permanently?
> I think it would be helpful, in that case, if you publish some
> testing data detailing how best to test the patch,
> which hardware etc. We will then do my best to carry on, fix up
> the remaining nits if any, test and apply the patch.
Yes. I would relocate to California, and work for another company start next
month.
Since the patch is already v11 now, if there is not big logic issue, I really
want to get it done before I leave. So I would still try my best to address
all comments and get it checked in ASAP.
>
> > >
> > > > ---
> > > >
> > > > arch/x86/include/asm/kvm_host.h |1 +
> > > > arch/x86/kvm/Makefile |2 +-
> > > > arch/x86/kvm/mmu.c |2 +
> > > > arch/x86/kvm/x86.c | 40 -
> > > > include/linux/kvm.h | 28
> > > > include/linux/kvm_host.h| 36 +
> > > > virt/kvm/assigned-dev.c | 44 ++
> > > > virt/kvm/kvm_main.c | 38 +-
> > > > virt/kvm/msix_mmio.c| 301
> > > > +++ virt/kvm/msix_mmio.h
> > > > | 25
> > > > 10 files changed, 504 insertions(+), 13 deletions(-)
> > > > create mode 100644 virt/kvm/msix_mmio.c
> > > > create mode 100644 virt/kvm/msix_mmio.h
> > > >
> > > > diff --git a/arch/x86/include/asm/kvm_host.h
> > > > b/arch/x86/include/asm/kvm_host.h index aa75f21..4a390a4 100644
> > > > --- a/arch/x86/include/asm/kvm_host.h
> > > > +++ b/arch/x86/include/asm/kvm_host.h
> > > > @@ -635,6 +635,7 @@ enum emulation_result {
> > > >
> > > > EMULATE_DONE, /* no further processing */
> > > > EMULATE_DO_MMIO, /* kvm_run filled with mmio request */
> > > > EMULATE_FAIL, /* can't emulate this instruction */
> > > >
> > > > + EMULATE_USERSPACE_EXIT, /* we need exit to userspace */
> > > >
> > > > };
> > > >
> > > > #define EMULTYPE_NO_DECODE (1 << 0)
> > > >
> > > > diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> > > > index f15501f..3a0d851 100644
> > > > --- a/arch/x86/kvm/Makefile
> > > > +++ b/arch/x86/kvm/Makefile
> > > > @@ -7,7 +7,7 @@ CFLAGS_vmx.o := -I.
> > > >
> > > > kvm-y += $(addprefix ../../../virt/kvm/, kvm_main.o
> > > > ioapic.o \
> > > >
> > > > coalesced_mmio.o irq_comm.o eventfd.o \
> > > >
> > > > - assigned-dev.o)
> > > > + assigned-dev.o msix_mmio.o)
> > > >
> > > > kvm-$(CONFIG_IOMMU_API)+= $(addprefix ../../../virt/kvm/,
> > > > iommu.o)
> > > > kvm-$(CONFIG_KVM_ASYNC_PF) += $(addprefix ../../../virt/kvm/,
> > > > async_pf.o)
> > > >
> > > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > > > index 9cafbb4..912dca4 100644
> > > > --- a/arch/x86/kvm/mmu.c
> > > > +++ b/arch/x86/kvm/mmu.c
> > > > @@ -3358,6 +3358,8 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu,
> > > > gva_t
> > > > cr2, u32 error_code,
> > > >
> > > > case EMULATE_DO_MMIO:
> > > > ++vcpu->stat.mmio_exits;
> > > > /* fall through */
> > > >
> > > > + case EMULATE_USERSPACE_EXIT:
> > > > + /* fall through */
> > > >
> > > > case EMULATE_FAIL:
> > > > return 0;
> > > >
> > > > default:
> > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > >
Re: [PATCH 3/4] KVM: Emulate MSI-X table in kernel
On Tue, Mar 01, 2011 at 02:10:37PM +0800, Sheng Yang wrote:
> On Monday 28 February 2011 19:27:29 Michael S. Tsirkin wrote:
> > On Mon, Feb 28, 2011 at 03:20:04PM +0800, Sheng Yang wrote:
> > > Then we can support mask bit operation of assigned devices now.
> > >
> > > Signed-off-by: Sheng Yang
> >
> > A general question: we implement mmio read and write
> > operation here, but seem to do nothing
> > about ordering. In particular, pci mmio reads are normally
> > assumed to flush all writes out to the device
> > and all device writes in to the CPU.
> >
> > How exactly does it work here?
>
> I don't understand your problem... We emulate all operation here, where is
> "ordering" issue?
>
> And Michael, thanks for you detail comments, but could you give comments all
> at
> once? Now I've tried my best to address comments, but still feeling endless
> comments are coming.
Hmm, sorry about that. At least some of the comments are in
the new code, so I could not have commented on it earlier ...
E.g. the ext_data thing only appeared in the latest version
or the one before that. In some cases such as the non-standard
error codes used, I don't always udnerstand the logic, as
the error handling is switched to standard conventions
so comments appear as the logic becomes apparent. This
applies to EFAULT handling below.
> And I would leave Intel this Friday, I want to get it done
> before I leave.
Permanently?
I think it would be helpful, in that case, if you publish some
testing data detailing how best to test the patch,
which hardware etc. We will then do my best to carry on, fix up
the remaining nits if any, test and apply the patch.
> >
> > > ---
> > >
> > > arch/x86/include/asm/kvm_host.h |1 +
> > > arch/x86/kvm/Makefile |2 +-
> > > arch/x86/kvm/mmu.c |2 +
> > > arch/x86/kvm/x86.c | 40 -
> > > include/linux/kvm.h | 28
> > > include/linux/kvm_host.h| 36 +
> > > virt/kvm/assigned-dev.c | 44 ++
> > > virt/kvm/kvm_main.c | 38 +-
> > > virt/kvm/msix_mmio.c| 301
> > > +++ virt/kvm/msix_mmio.h
> > > | 25
> > > 10 files changed, 504 insertions(+), 13 deletions(-)
> > > create mode 100644 virt/kvm/msix_mmio.c
> > > create mode 100644 virt/kvm/msix_mmio.h
> > >
> > > diff --git a/arch/x86/include/asm/kvm_host.h
> > > b/arch/x86/include/asm/kvm_host.h index aa75f21..4a390a4 100644
> > > --- a/arch/x86/include/asm/kvm_host.h
> > > +++ b/arch/x86/include/asm/kvm_host.h
> > > @@ -635,6 +635,7 @@ enum emulation_result {
> > >
> > > EMULATE_DONE, /* no further processing */
> > > EMULATE_DO_MMIO, /* kvm_run filled with mmio request */
> > > EMULATE_FAIL, /* can't emulate this instruction */
> > >
> > > + EMULATE_USERSPACE_EXIT, /* we need exit to userspace */
> > >
> > > };
> > >
> > > #define EMULTYPE_NO_DECODE (1 << 0)
> > >
> > > diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> > > index f15501f..3a0d851 100644
> > > --- a/arch/x86/kvm/Makefile
> > > +++ b/arch/x86/kvm/Makefile
> > > @@ -7,7 +7,7 @@ CFLAGS_vmx.o := -I.
> > >
> > > kvm-y+= $(addprefix ../../../virt/kvm/, kvm_main.o
> > > ioapic.o \
> > >
> > > coalesced_mmio.o irq_comm.o eventfd.o \
> > >
> > > - assigned-dev.o)
> > > + assigned-dev.o msix_mmio.o)
> > >
> > > kvm-$(CONFIG_IOMMU_API) += $(addprefix ../../../virt/kvm/, iommu.o)
> > > kvm-$(CONFIG_KVM_ASYNC_PF) += $(addprefix ../../../virt/kvm/,
> > > async_pf.o)
> > >
> > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > > index 9cafbb4..912dca4 100644
> > > --- a/arch/x86/kvm/mmu.c
> > > +++ b/arch/x86/kvm/mmu.c
> > > @@ -3358,6 +3358,8 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t
> > > cr2, u32 error_code,
> > >
> > > case EMULATE_DO_MMIO:
> > > ++vcpu->stat.mmio_exits;
> > > /* fall through */
> > >
> > > + case EMULATE_USERSPACE_EXIT:
> > > + /* fall through */
> > >
> > > case EMULATE_FAIL:
> > > return 0;
> > >
> > > default:
> > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > index 21b84e2..87308eb 100644
> > > --- a/arch/x86/kvm/x86.c
> > > +++ b/arch/x86/kvm/x86.c
> > > @@ -1966,6 +1966,7 @@ int kvm_dev_ioctl_check_extension(long ext)
> > >
> > > case KVM_CAP_X86_ROBUST_SINGLESTEP:
> > > case KVM_CAP_XSAVE:
> > >
> > > case KVM_CAP_ASYNC_PF:
> > > + case KVM_CAP_MSIX_MMIO:
> > > r = 1;
> > > break;
> > >
> > > case KVM_CAP_COALESCED_MMIO:
> > > @@ -3809,6 +3810,7 @@ static int emulator_write_emulated_onepage(unsigned
> > > long addr,
> > >
> > > {
> > >
> > > gpa_t gpa;
> > > struct kvm_io_ext_data ext_data;
> > >
> > > + int r;
> > >
> > > gpa = kvm_mmu_gva_to_gpa_write(vcpu, addr, excep
Re: [PATCH 3/4] KVM: Emulate MSI-X table in kernel
On Monday 28 February 2011 19:27:29 Michael S. Tsirkin wrote:
> On Mon, Feb 28, 2011 at 03:20:04PM +0800, Sheng Yang wrote:
> > Then we can support mask bit operation of assigned devices now.
> >
> > Signed-off-by: Sheng Yang
>
> A general question: we implement mmio read and write
> operation here, but seem to do nothing
> about ordering. In particular, pci mmio reads are normally
> assumed to flush all writes out to the device
> and all device writes in to the CPU.
>
> How exactly does it work here?
I don't understand your problem... We emulate all operation here, where is
"ordering" issue?
And Michael, thanks for you detail comments, but could you give comments all at
once? Now I've tried my best to address comments, but still feeling endless
comments are coming. And I would leave Intel this Friday, I want to get it done
before I leave.
>
> > ---
> >
> > arch/x86/include/asm/kvm_host.h |1 +
> > arch/x86/kvm/Makefile |2 +-
> > arch/x86/kvm/mmu.c |2 +
> > arch/x86/kvm/x86.c | 40 -
> > include/linux/kvm.h | 28
> > include/linux/kvm_host.h| 36 +
> > virt/kvm/assigned-dev.c | 44 ++
> > virt/kvm/kvm_main.c | 38 +-
> > virt/kvm/msix_mmio.c| 301
> > +++ virt/kvm/msix_mmio.h
> > | 25
> > 10 files changed, 504 insertions(+), 13 deletions(-)
> > create mode 100644 virt/kvm/msix_mmio.c
> > create mode 100644 virt/kvm/msix_mmio.h
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h
> > b/arch/x86/include/asm/kvm_host.h index aa75f21..4a390a4 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -635,6 +635,7 @@ enum emulation_result {
> >
> > EMULATE_DONE, /* no further processing */
> > EMULATE_DO_MMIO, /* kvm_run filled with mmio request */
> > EMULATE_FAIL, /* can't emulate this instruction */
> >
> > + EMULATE_USERSPACE_EXIT, /* we need exit to userspace */
> >
> > };
> >
> > #define EMULTYPE_NO_DECODE (1 << 0)
> >
> > diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> > index f15501f..3a0d851 100644
> > --- a/arch/x86/kvm/Makefile
> > +++ b/arch/x86/kvm/Makefile
> > @@ -7,7 +7,7 @@ CFLAGS_vmx.o := -I.
> >
> > kvm-y += $(addprefix ../../../virt/kvm/, kvm_main.o
> > ioapic.o \
> >
> > coalesced_mmio.o irq_comm.o eventfd.o \
> >
> > - assigned-dev.o)
> > + assigned-dev.o msix_mmio.o)
> >
> > kvm-$(CONFIG_IOMMU_API)+= $(addprefix ../../../virt/kvm/, iommu.o)
> > kvm-$(CONFIG_KVM_ASYNC_PF) += $(addprefix ../../../virt/kvm/,
> > async_pf.o)
> >
> > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > index 9cafbb4..912dca4 100644
> > --- a/arch/x86/kvm/mmu.c
> > +++ b/arch/x86/kvm/mmu.c
> > @@ -3358,6 +3358,8 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t
> > cr2, u32 error_code,
> >
> > case EMULATE_DO_MMIO:
> > ++vcpu->stat.mmio_exits;
> > /* fall through */
> >
> > + case EMULATE_USERSPACE_EXIT:
> > + /* fall through */
> >
> > case EMULATE_FAIL:
> > return 0;
> >
> > default:
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 21b84e2..87308eb 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -1966,6 +1966,7 @@ int kvm_dev_ioctl_check_extension(long ext)
> >
> > case KVM_CAP_X86_ROBUST_SINGLESTEP:
> > case KVM_CAP_XSAVE:
> >
> > case KVM_CAP_ASYNC_PF:
> > + case KVM_CAP_MSIX_MMIO:
> > r = 1;
> > break;
> >
> > case KVM_CAP_COALESCED_MMIO:
> > @@ -3809,6 +3810,7 @@ static int emulator_write_emulated_onepage(unsigned
> > long addr,
> >
> > {
> >
> > gpa_t gpa;
> > struct kvm_io_ext_data ext_data;
> >
> > + int r;
> >
> > gpa = kvm_mmu_gva_to_gpa_write(vcpu, addr, exception);
> >
> > @@ -3824,18 +3826,32 @@ static int
> > emulator_write_emulated_onepage(unsigned long addr,
> >
> > mmio:
> > trace_kvm_mmio(KVM_TRACE_MMIO_WRITE, bytes, gpa, *(u64 *)val);
> >
> > + r = vcpu_mmio_write(vcpu, gpa, bytes, val, &ext_data);
> >
> > /*
> >
> > * Is this MMIO handled locally?
> > */
> >
> > - if (!vcpu_mmio_write(vcpu, gpa, bytes, val, &ext_data))
> > + if (!r)
> >
> > return X86EMUL_CONTINUE;
> >
> > - vcpu->mmio_needed = 1;
> > - vcpu->run->exit_reason = KVM_EXIT_MMIO;
> > - vcpu->run->mmio.phys_addr = vcpu->mmio_phys_addr = gpa;
> > - vcpu->run->mmio.len = vcpu->mmio_size = bytes;
> > - vcpu->run->mmio.is_write = vcpu->mmio_is_write = 1;
> > - memcpy(vcpu->run->mmio.data, val, bytes);
> > + if (r == -ENOTSYNC) {
>
> Can you replace -ENOTSYNC with KVM_EXIT_MSIX_ROUTING_UPDATE all over
> please?
How about let Avi/Marcelo decide?
>
> > +
Re: [PATCH 3/4] KVM: Emulate MSI-X table in kernel
On Mon, Feb 28, 2011 at 03:20:04PM +0800, Sheng Yang wrote:
> Then we can support mask bit operation of assigned devices now.
>
> Signed-off-by: Sheng Yang
A general question: we implement mmio read and write
operation here, but seem to do nothing
about ordering. In particular, pci mmio reads are normally
assumed to flush all writes out to the device
and all device writes in to the CPU.
How exactly does it work here?
> ---
> arch/x86/include/asm/kvm_host.h |1 +
> arch/x86/kvm/Makefile |2 +-
> arch/x86/kvm/mmu.c |2 +
> arch/x86/kvm/x86.c | 40 -
> include/linux/kvm.h | 28
> include/linux/kvm_host.h| 36 +
> virt/kvm/assigned-dev.c | 44 ++
> virt/kvm/kvm_main.c | 38 +-
> virt/kvm/msix_mmio.c| 301
> +++
> virt/kvm/msix_mmio.h| 25
> 10 files changed, 504 insertions(+), 13 deletions(-)
> create mode 100644 virt/kvm/msix_mmio.c
> create mode 100644 virt/kvm/msix_mmio.h
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index aa75f21..4a390a4 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -635,6 +635,7 @@ enum emulation_result {
> EMULATE_DONE, /* no further processing */
> EMULATE_DO_MMIO, /* kvm_run filled with mmio request */
> EMULATE_FAIL, /* can't emulate this instruction */
> + EMULATE_USERSPACE_EXIT, /* we need exit to userspace */
> };
>
> #define EMULTYPE_NO_DECODE (1 << 0)
> diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> index f15501f..3a0d851 100644
> --- a/arch/x86/kvm/Makefile
> +++ b/arch/x86/kvm/Makefile
> @@ -7,7 +7,7 @@ CFLAGS_vmx.o := -I.
>
> kvm-y+= $(addprefix ../../../virt/kvm/, kvm_main.o
> ioapic.o \
> coalesced_mmio.o irq_comm.o eventfd.o \
> - assigned-dev.o)
> + assigned-dev.o msix_mmio.o)
> kvm-$(CONFIG_IOMMU_API) += $(addprefix ../../../virt/kvm/, iommu.o)
> kvm-$(CONFIG_KVM_ASYNC_PF) += $(addprefix ../../../virt/kvm/, async_pf.o)
>
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 9cafbb4..912dca4 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -3358,6 +3358,8 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t
> cr2, u32 error_code,
> case EMULATE_DO_MMIO:
> ++vcpu->stat.mmio_exits;
> /* fall through */
> + case EMULATE_USERSPACE_EXIT:
> + /* fall through */
> case EMULATE_FAIL:
> return 0;
> default:
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 21b84e2..87308eb 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1966,6 +1966,7 @@ int kvm_dev_ioctl_check_extension(long ext)
> case KVM_CAP_X86_ROBUST_SINGLESTEP:
> case KVM_CAP_XSAVE:
> case KVM_CAP_ASYNC_PF:
> + case KVM_CAP_MSIX_MMIO:
> r = 1;
> break;
> case KVM_CAP_COALESCED_MMIO:
> @@ -3809,6 +3810,7 @@ static int emulator_write_emulated_onepage(unsigned
> long addr,
> {
> gpa_t gpa;
> struct kvm_io_ext_data ext_data;
> + int r;
>
> gpa = kvm_mmu_gva_to_gpa_write(vcpu, addr, exception);
>
> @@ -3824,18 +3826,32 @@ static int emulator_write_emulated_onepage(unsigned
> long addr,
>
> mmio:
> trace_kvm_mmio(KVM_TRACE_MMIO_WRITE, bytes, gpa, *(u64 *)val);
> + r = vcpu_mmio_write(vcpu, gpa, bytes, val, &ext_data);
> /*
>* Is this MMIO handled locally?
>*/
> - if (!vcpu_mmio_write(vcpu, gpa, bytes, val, &ext_data))
> + if (!r)
> return X86EMUL_CONTINUE;
>
> - vcpu->mmio_needed = 1;
> - vcpu->run->exit_reason = KVM_EXIT_MMIO;
> - vcpu->run->mmio.phys_addr = vcpu->mmio_phys_addr = gpa;
> - vcpu->run->mmio.len = vcpu->mmio_size = bytes;
> - vcpu->run->mmio.is_write = vcpu->mmio_is_write = 1;
> - memcpy(vcpu->run->mmio.data, val, bytes);
> + if (r == -ENOTSYNC) {
Can you replace -ENOTSYNC with KVM_EXIT_MSIX_ROUTING_UPDATE all over please?
> + vcpu->userspace_exit_needed = 1;
> + vcpu->run->exit_reason = KVM_EXIT_MSIX_ROUTING_UPDATE;
> + vcpu->run->msix_routing.dev_id =
> + ext_data.msix_routing.dev_id;
> + vcpu->run->msix_routing.type =
> + ext_data.msix_routing.type;
> + vcpu->run->msix_routing.entry_idx =
> + ext_data.msix_routing.entry_idx;
> + vcpu->run->msix_routing.flags =
> + ext_data.msix_routing.flags;
> + } else {
> + vcpu->mmio_needed = 1;
> + vcpu->run->exit_reason = KVM_EXIT_MMIO;
> + vcpu->run->mmio.phys_addr = vcpu->mmio_phys_addr = gpa;
> +
Re: [PATCH 3/4] KVM: Emulate MSI-X table in kernel
On Friday 25 February 2011 16:29:38 Michael S. Tsirkin wrote:
> On Fri, Feb 25, 2011 at 02:28:02PM +0800, Sheng Yang wrote:
> > On Thursday 24 February 2011 18:45:08 Michael S. Tsirkin wrote:
> > > On Thu, Feb 24, 2011 at 05:51:04PM +0800, Sheng Yang wrote:
> > > > Then we can support mask bit operation of assigned devices now.
> > > >
> > > > Signed-off-by: Sheng Yang
> > >
> > > Doesn't look like all comments got addressed.
> > > E.g. gpa_t entry_base is still there and in reality
> > > you said it's a host virtual address so
> > > should be void __user *;
> >
> > Would update it.
> >
> > > And ENOTSYNC meaning 'MSIX' is pretty hacky.
> >
> > I'd like to discuss it later. We may need some work on all MMIO handling
> > side to make it more straightforward. But I don't want to bundle it with
> > this one...
>
> It's not PCI related so I'll defer to Avi/Marcelo on this.
> Are you guys happy with the ENOTSYNC meaning 'MSIX'
> and userspace_exit_needed hacks in this code?
>
> > > > ---
> > > >
> > > > arch/x86/include/asm/kvm_host.h |1 +
> > > > arch/x86/kvm/Makefile |2 +-
> > > > arch/x86/kvm/mmu.c |2 +
> > > > arch/x86/kvm/x86.c | 40 -
> > > > include/linux/kvm.h | 28
> > > > include/linux/kvm_host.h| 34 +
> > > > virt/kvm/assigned-dev.c | 44 ++
> > > > virt/kvm/kvm_main.c | 38 +-
> > > > virt/kvm/msix_mmio.c| 296
> > > > +++ virt/kvm/msix_mmio.h
> > > >
> > > > | 25
> > > >
> > > > 10 files changed, 497 insertions(+), 13 deletions(-)
> > > > create mode 100644 virt/kvm/msix_mmio.c
> > > > create mode 100644 virt/kvm/msix_mmio.h
> > > >
> > > > diff --git a/arch/x86/include/asm/kvm_host.h
> > > > b/arch/x86/include/asm/kvm_host.h index aa75f21..4a390a4 100644
> > > > --- a/arch/x86/include/asm/kvm_host.h
> > > > +++ b/arch/x86/include/asm/kvm_host.h
> > > > @@ -635,6 +635,7 @@ enum emulation_result {
> > > >
> > > > EMULATE_DONE, /* no further processing */
> > > > EMULATE_DO_MMIO, /* kvm_run filled with mmio request */
> > > > EMULATE_FAIL, /* can't emulate this instruction */
> > > >
> > > > + EMULATE_USERSPACE_EXIT, /* we need exit to userspace */
> > > >
> > > > };
> > > >
> > > > #define EMULTYPE_NO_DECODE (1 << 0)
> > > >
> > > > diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> > > > index f15501f..3a0d851 100644
> > > > --- a/arch/x86/kvm/Makefile
> > > > +++ b/arch/x86/kvm/Makefile
> > > > @@ -7,7 +7,7 @@ CFLAGS_vmx.o := -I.
> > > >
> > > > kvm-y += $(addprefix ../../../virt/kvm/, kvm_main.o
> > > > ioapic.o
\
> > > >
> > > > coalesced_mmio.o irq_comm.o eventfd.o \
> > > >
> > > > - assigned-dev.o)
> > > > + assigned-dev.o msix_mmio.o)
> > > >
> > > > kvm-$(CONFIG_IOMMU_API)+= $(addprefix ../../../virt/kvm/,
> > > > iommu.o)
> > > > kvm-$(CONFIG_KVM_ASYNC_PF) += $(addprefix ../../../virt/kvm/,
> > > > async_pf.o)
> > > >
> > > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > > > index 9cafbb4..912dca4 100644
> > > > --- a/arch/x86/kvm/mmu.c
> > > > +++ b/arch/x86/kvm/mmu.c
> > > > @@ -3358,6 +3358,8 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu,
> > > > gva_t cr2, u32 error_code,
> > > >
> > > > case EMULATE_DO_MMIO:
> > > > ++vcpu->stat.mmio_exits;
> > > > /* fall through */
> > > >
> > > > + case EMULATE_USERSPACE_EXIT:
> > > > + /* fall through */
> > > >
> > > > case EMULATE_FAIL:
> > > > return 0;
> > > >
> > > > default:
> > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > > index 21b84e2..87308eb 100644
> > > > --- a/arch/x86/kvm/x86.c
> > > > +++ b/arch/x86/kvm/x86.c
> > > > @@ -1966,6 +1966,7 @@ int kvm_dev_ioctl_check_extension(long ext)
> > > >
> > > > case KVM_CAP_X86_ROBUST_SINGLESTEP:
> > > > case KVM_CAP_XSAVE:
> > > >
> > > > case KVM_CAP_ASYNC_PF:
> > > > + case KVM_CAP_MSIX_MMIO:
> > > > r = 1;
> > > > break;
> > > >
> > > > case KVM_CAP_COALESCED_MMIO:
> > > > @@ -3809,6 +3810,7 @@ static int
> > > > emulator_write_emulated_onepage(unsigned long addr,
> > > >
> > > > {
> > > >
> > > > gpa_t gpa;
> > > > struct kvm_io_ext_data ext_data;
> > > >
> > > > + int r;
> > > >
> > > > gpa = kvm_mmu_gva_to_gpa_write(vcpu, addr, exception);
> > > >
> > > > @@ -3824,18 +3826,32 @@ static int
> > > > emulator_write_emulated_onepage(unsigned long addr,
> > > >
> > > > mmio:
> > > > trace_kvm_mmio(KVM_TRACE_MMIO_WRITE, bytes, gpa, *(u64 *)val);
> > > >
> > > > + r = vcpu_mmio_write(vcpu, gpa, bytes, val, &ext_data
Re: [PATCH 3/4] KVM: Emulate MSI-X table in kernel
On Fri, Feb 25, 2011 at 02:28:02PM +0800, Sheng Yang wrote:
> On Thursday 24 February 2011 18:45:08 Michael S. Tsirkin wrote:
> > On Thu, Feb 24, 2011 at 05:51:04PM +0800, Sheng Yang wrote:
> > > Then we can support mask bit operation of assigned devices now.
> > >
> > > Signed-off-by: Sheng Yang
> >
> > Doesn't look like all comments got addressed.
> > E.g. gpa_t entry_base is still there and in reality
> > you said it's a host virtual address so
> > should be void __user *;
>
> Would update it.
>
> > And ENOTSYNC meaning 'MSIX' is pretty hacky.
>
> I'd like to discuss it later. We may need some work on all MMIO handling side
> to
> make it more straightforward. But I don't want to bundle it with this one...
It's not PCI related so I'll defer to Avi/Marcelo on this.
Are you guys happy with the ENOTSYNC meaning 'MSIX'
and userspace_exit_needed hacks in this code?
> >
> > > ---
> > >
> > > arch/x86/include/asm/kvm_host.h |1 +
> > > arch/x86/kvm/Makefile |2 +-
> > > arch/x86/kvm/mmu.c |2 +
> > > arch/x86/kvm/x86.c | 40 -
> > > include/linux/kvm.h | 28
> > > include/linux/kvm_host.h| 34 +
> > > virt/kvm/assigned-dev.c | 44 ++
> > > virt/kvm/kvm_main.c | 38 +-
> > > virt/kvm/msix_mmio.c| 296
> > > +++ virt/kvm/msix_mmio.h
> > > | 25
> > > 10 files changed, 497 insertions(+), 13 deletions(-)
> > > create mode 100644 virt/kvm/msix_mmio.c
> > > create mode 100644 virt/kvm/msix_mmio.h
> > >
> > > diff --git a/arch/x86/include/asm/kvm_host.h
> > > b/arch/x86/include/asm/kvm_host.h index aa75f21..4a390a4 100644
> > > --- a/arch/x86/include/asm/kvm_host.h
> > > +++ b/arch/x86/include/asm/kvm_host.h
> > > @@ -635,6 +635,7 @@ enum emulation_result {
> > >
> > > EMULATE_DONE, /* no further processing */
> > > EMULATE_DO_MMIO, /* kvm_run filled with mmio request */
> > > EMULATE_FAIL, /* can't emulate this instruction */
> > >
> > > + EMULATE_USERSPACE_EXIT, /* we need exit to userspace */
> > >
> > > };
> > >
> > > #define EMULTYPE_NO_DECODE (1 << 0)
> > >
> > > diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> > > index f15501f..3a0d851 100644
> > > --- a/arch/x86/kvm/Makefile
> > > +++ b/arch/x86/kvm/Makefile
> > > @@ -7,7 +7,7 @@ CFLAGS_vmx.o := -I.
> > >
> > > kvm-y+= $(addprefix ../../../virt/kvm/, kvm_main.o
> > > ioapic.o \
> > >
> > > coalesced_mmio.o irq_comm.o eventfd.o \
> > >
> > > - assigned-dev.o)
> > > + assigned-dev.o msix_mmio.o)
> > >
> > > kvm-$(CONFIG_IOMMU_API) += $(addprefix ../../../virt/kvm/, iommu.o)
> > > kvm-$(CONFIG_KVM_ASYNC_PF) += $(addprefix ../../../virt/kvm/,
> > > async_pf.o)
> > >
> > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > > index 9cafbb4..912dca4 100644
> > > --- a/arch/x86/kvm/mmu.c
> > > +++ b/arch/x86/kvm/mmu.c
> > > @@ -3358,6 +3358,8 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t
> > > cr2, u32 error_code,
> > >
> > > case EMULATE_DO_MMIO:
> > > ++vcpu->stat.mmio_exits;
> > > /* fall through */
> > >
> > > + case EMULATE_USERSPACE_EXIT:
> > > + /* fall through */
> > >
> > > case EMULATE_FAIL:
> > > return 0;
> > >
> > > default:
> > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > index 21b84e2..87308eb 100644
> > > --- a/arch/x86/kvm/x86.c
> > > +++ b/arch/x86/kvm/x86.c
> > > @@ -1966,6 +1966,7 @@ int kvm_dev_ioctl_check_extension(long ext)
> > >
> > > case KVM_CAP_X86_ROBUST_SINGLESTEP:
> > > case KVM_CAP_XSAVE:
> > >
> > > case KVM_CAP_ASYNC_PF:
> > > + case KVM_CAP_MSIX_MMIO:
> > > r = 1;
> > > break;
> > >
> > > case KVM_CAP_COALESCED_MMIO:
> > > @@ -3809,6 +3810,7 @@ static int emulator_write_emulated_onepage(unsigned
> > > long addr,
> > >
> > > {
> > >
> > > gpa_t gpa;
> > > struct kvm_io_ext_data ext_data;
> > >
> > > + int r;
> > >
> > > gpa = kvm_mmu_gva_to_gpa_write(vcpu, addr, exception);
> > >
> > > @@ -3824,18 +3826,32 @@ static int
> > > emulator_write_emulated_onepage(unsigned long addr,
> > >
> > > mmio:
> > > trace_kvm_mmio(KVM_TRACE_MMIO_WRITE, bytes, gpa, *(u64 *)val);
> > >
> > > + r = vcpu_mmio_write(vcpu, gpa, bytes, val, &ext_data);
> > >
> > > /*
> > >
> > >* Is this MMIO handled locally?
> > >*/
> > >
> > > - if (!vcpu_mmio_write(vcpu, gpa, bytes, val, &ext_data))
> > > + if (!r)
> > >
> > > return X86EMUL_CONTINUE;
> > >
> > > - vcpu->mmio_needed = 1;
> > > - vcpu->run->exit_reason = KVM_EXIT_MMIO;
> > > - vcpu->run->mmio.phys_addr = vcpu->mmio_phys_addr = gpa;
> > > - vcpu->run->mmio.len = vcpu->mmio_size = bytes;
> > > - vcpu->run->mmio.is_write = vcpu->mmio_is_write = 1;
> > > - memcpy(vcpu->ru
Re: [PATCH 3/4] KVM: Emulate MSI-X table in kernel
On Thursday 24 February 2011 18:45:08 Michael S. Tsirkin wrote:
> On Thu, Feb 24, 2011 at 05:51:04PM +0800, Sheng Yang wrote:
> > Then we can support mask bit operation of assigned devices now.
> >
> > Signed-off-by: Sheng Yang
>
> Doesn't look like all comments got addressed.
> E.g. gpa_t entry_base is still there and in reality
> you said it's a host virtual address so
> should be void __user *;
Would update it.
> And ENOTSYNC meaning 'MSIX' is pretty hacky.
I'd like to discuss it later. We may need some work on all MMIO handling side
to
make it more straightforward. But I don't want to bundle it with this one...
>
> > ---
> >
> > arch/x86/include/asm/kvm_host.h |1 +
> > arch/x86/kvm/Makefile |2 +-
> > arch/x86/kvm/mmu.c |2 +
> > arch/x86/kvm/x86.c | 40 -
> > include/linux/kvm.h | 28
> > include/linux/kvm_host.h| 34 +
> > virt/kvm/assigned-dev.c | 44 ++
> > virt/kvm/kvm_main.c | 38 +-
> > virt/kvm/msix_mmio.c| 296
> > +++ virt/kvm/msix_mmio.h
> > | 25
> > 10 files changed, 497 insertions(+), 13 deletions(-)
> > create mode 100644 virt/kvm/msix_mmio.c
> > create mode 100644 virt/kvm/msix_mmio.h
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h
> > b/arch/x86/include/asm/kvm_host.h index aa75f21..4a390a4 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -635,6 +635,7 @@ enum emulation_result {
> >
> > EMULATE_DONE, /* no further processing */
> > EMULATE_DO_MMIO, /* kvm_run filled with mmio request */
> > EMULATE_FAIL, /* can't emulate this instruction */
> >
> > + EMULATE_USERSPACE_EXIT, /* we need exit to userspace */
> >
> > };
> >
> > #define EMULTYPE_NO_DECODE (1 << 0)
> >
> > diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> > index f15501f..3a0d851 100644
> > --- a/arch/x86/kvm/Makefile
> > +++ b/arch/x86/kvm/Makefile
> > @@ -7,7 +7,7 @@ CFLAGS_vmx.o := -I.
> >
> > kvm-y += $(addprefix ../../../virt/kvm/, kvm_main.o
> > ioapic.o \
> >
> > coalesced_mmio.o irq_comm.o eventfd.o \
> >
> > - assigned-dev.o)
> > + assigned-dev.o msix_mmio.o)
> >
> > kvm-$(CONFIG_IOMMU_API)+= $(addprefix ../../../virt/kvm/, iommu.o)
> > kvm-$(CONFIG_KVM_ASYNC_PF) += $(addprefix ../../../virt/kvm/,
> > async_pf.o)
> >
> > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > index 9cafbb4..912dca4 100644
> > --- a/arch/x86/kvm/mmu.c
> > +++ b/arch/x86/kvm/mmu.c
> > @@ -3358,6 +3358,8 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t
> > cr2, u32 error_code,
> >
> > case EMULATE_DO_MMIO:
> > ++vcpu->stat.mmio_exits;
> > /* fall through */
> >
> > + case EMULATE_USERSPACE_EXIT:
> > + /* fall through */
> >
> > case EMULATE_FAIL:
> > return 0;
> >
> > default:
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 21b84e2..87308eb 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -1966,6 +1966,7 @@ int kvm_dev_ioctl_check_extension(long ext)
> >
> > case KVM_CAP_X86_ROBUST_SINGLESTEP:
> > case KVM_CAP_XSAVE:
> >
> > case KVM_CAP_ASYNC_PF:
> > + case KVM_CAP_MSIX_MMIO:
> > r = 1;
> > break;
> >
> > case KVM_CAP_COALESCED_MMIO:
> > @@ -3809,6 +3810,7 @@ static int emulator_write_emulated_onepage(unsigned
> > long addr,
> >
> > {
> >
> > gpa_t gpa;
> > struct kvm_io_ext_data ext_data;
> >
> > + int r;
> >
> > gpa = kvm_mmu_gva_to_gpa_write(vcpu, addr, exception);
> >
> > @@ -3824,18 +3826,32 @@ static int
> > emulator_write_emulated_onepage(unsigned long addr,
> >
> > mmio:
> > trace_kvm_mmio(KVM_TRACE_MMIO_WRITE, bytes, gpa, *(u64 *)val);
> >
> > + r = vcpu_mmio_write(vcpu, gpa, bytes, val, &ext_data);
> >
> > /*
> >
> > * Is this MMIO handled locally?
> > */
> >
> > - if (!vcpu_mmio_write(vcpu, gpa, bytes, val, &ext_data))
> > + if (!r)
> >
> > return X86EMUL_CONTINUE;
> >
> > - vcpu->mmio_needed = 1;
> > - vcpu->run->exit_reason = KVM_EXIT_MMIO;
> > - vcpu->run->mmio.phys_addr = vcpu->mmio_phys_addr = gpa;
> > - vcpu->run->mmio.len = vcpu->mmio_size = bytes;
> > - vcpu->run->mmio.is_write = vcpu->mmio_is_write = 1;
> > - memcpy(vcpu->run->mmio.data, val, bytes);
> > + if (r == -ENOTSYNC) {
> > + vcpu->userspace_exit_needed = 1;
> > + vcpu->run->exit_reason = KVM_EXIT_MSIX_ROUTING_UPDATE;
> > + vcpu->run->msix_routing.dev_id =
> > + ext_data.msix_routing.dev_id;
> > + vcpu->run->msix_routing.type =
> > + ext_data.msix_routing.type;
> > + vcpu->run->msix_routing.e
Re: [PATCH 3/4] KVM: Emulate MSI-X table in kernel
On Thu, Feb 24, 2011 at 05:51:04PM +0800, Sheng Yang wrote:
> Then we can support mask bit operation of assigned devices now.
>
> Signed-off-by: Sheng Yang
Doesn't look like all comments got addressed.
E.g. gpa_t entry_base is still there and in reality
you said it's a host virtual address so
should be void __user *;
And ENOTSYNC meaning 'MSIX' is pretty hacky.
> ---
> arch/x86/include/asm/kvm_host.h |1 +
> arch/x86/kvm/Makefile |2 +-
> arch/x86/kvm/mmu.c |2 +
> arch/x86/kvm/x86.c | 40 -
> include/linux/kvm.h | 28
> include/linux/kvm_host.h| 34 +
> virt/kvm/assigned-dev.c | 44 ++
> virt/kvm/kvm_main.c | 38 +-
> virt/kvm/msix_mmio.c| 296
> +++
> virt/kvm/msix_mmio.h| 25
> 10 files changed, 497 insertions(+), 13 deletions(-)
> create mode 100644 virt/kvm/msix_mmio.c
> create mode 100644 virt/kvm/msix_mmio.h
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index aa75f21..4a390a4 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -635,6 +635,7 @@ enum emulation_result {
> EMULATE_DONE, /* no further processing */
> EMULATE_DO_MMIO, /* kvm_run filled with mmio request */
> EMULATE_FAIL, /* can't emulate this instruction */
> + EMULATE_USERSPACE_EXIT, /* we need exit to userspace */
> };
>
> #define EMULTYPE_NO_DECODE (1 << 0)
> diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> index f15501f..3a0d851 100644
> --- a/arch/x86/kvm/Makefile
> +++ b/arch/x86/kvm/Makefile
> @@ -7,7 +7,7 @@ CFLAGS_vmx.o := -I.
>
> kvm-y+= $(addprefix ../../../virt/kvm/, kvm_main.o
> ioapic.o \
> coalesced_mmio.o irq_comm.o eventfd.o \
> - assigned-dev.o)
> + assigned-dev.o msix_mmio.o)
> kvm-$(CONFIG_IOMMU_API) += $(addprefix ../../../virt/kvm/, iommu.o)
> kvm-$(CONFIG_KVM_ASYNC_PF) += $(addprefix ../../../virt/kvm/, async_pf.o)
>
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 9cafbb4..912dca4 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -3358,6 +3358,8 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t
> cr2, u32 error_code,
> case EMULATE_DO_MMIO:
> ++vcpu->stat.mmio_exits;
> /* fall through */
> + case EMULATE_USERSPACE_EXIT:
> + /* fall through */
> case EMULATE_FAIL:
> return 0;
> default:
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 21b84e2..87308eb 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1966,6 +1966,7 @@ int kvm_dev_ioctl_check_extension(long ext)
> case KVM_CAP_X86_ROBUST_SINGLESTEP:
> case KVM_CAP_XSAVE:
> case KVM_CAP_ASYNC_PF:
> + case KVM_CAP_MSIX_MMIO:
> r = 1;
> break;
> case KVM_CAP_COALESCED_MMIO:
> @@ -3809,6 +3810,7 @@ static int emulator_write_emulated_onepage(unsigned
> long addr,
> {
> gpa_t gpa;
> struct kvm_io_ext_data ext_data;
> + int r;
>
> gpa = kvm_mmu_gva_to_gpa_write(vcpu, addr, exception);
>
> @@ -3824,18 +3826,32 @@ static int emulator_write_emulated_onepage(unsigned
> long addr,
>
> mmio:
> trace_kvm_mmio(KVM_TRACE_MMIO_WRITE, bytes, gpa, *(u64 *)val);
> + r = vcpu_mmio_write(vcpu, gpa, bytes, val, &ext_data);
> /*
>* Is this MMIO handled locally?
>*/
> - if (!vcpu_mmio_write(vcpu, gpa, bytes, val, &ext_data))
> + if (!r)
> return X86EMUL_CONTINUE;
>
> - vcpu->mmio_needed = 1;
> - vcpu->run->exit_reason = KVM_EXIT_MMIO;
> - vcpu->run->mmio.phys_addr = vcpu->mmio_phys_addr = gpa;
> - vcpu->run->mmio.len = vcpu->mmio_size = bytes;
> - vcpu->run->mmio.is_write = vcpu->mmio_is_write = 1;
> - memcpy(vcpu->run->mmio.data, val, bytes);
> + if (r == -ENOTSYNC) {
> + vcpu->userspace_exit_needed = 1;
> + vcpu->run->exit_reason = KVM_EXIT_MSIX_ROUTING_UPDATE;
> + vcpu->run->msix_routing.dev_id =
> + ext_data.msix_routing.dev_id;
> + vcpu->run->msix_routing.type =
> + ext_data.msix_routing.type;
> + vcpu->run->msix_routing.entry_idx =
> + ext_data.msix_routing.entry_idx;
> + vcpu->run->msix_routing.flags =
> + ext_data.msix_routing.flags;
> + } else {
> + vcpu->mmio_needed = 1;
> + vcpu->run->exit_reason = KVM_EXIT_MMIO;
> + vcpu->run->mmio.phys_addr = vcpu->mmio_phys_addr = gpa;
> + vcpu->run->mmio.len = vcpu->mmio_size = bytes;
> + vcpu->run->mmio.is_write = vcpu->mmio_is_write = 1;
> +
