RE: [E1000-devel] [Bug 56981] New: [SR-IOV] Intel I350 NIC VF cannot work in a Windows 2008 guest.

2013-04-24 Thread Ren, Yongjie
 -Original Message-
 From: Shah, Ashish N
 Sent: Wednesday, April 24, 2013 4:31 AM
 To: Rose, Gregory V; Ren, Yongjie; e1000-de...@lists.sourceforge.net;
 kvm@vger.kernel.org
 Subject: RE: [E1000-devel] [Bug 56981] New: [SR-IOV] Intel I350 NIC VF
 cannot work in a Windows 2008 guest.
 
 Greg is correct, I350 is only supported in Windows 2008 R2 and later.
 
 The only 1G Windows 2008 support currently is for 82576.
 
Understood. Thanks for your clarification. 
I just tried I350 VF in a Windows 2008 R2 guest. It worked fine.

- Jay

 -ashish
 
 -Original Message-
 From: Rose, Gregory V
 Sent: Tuesday, April 23, 2013 1:27 PM
 To: Ren, Yongjie; e1000-de...@lists.sourceforge.net;
 kvm@vger.kernel.org; Shah, Ashish N
 Subject: RE: [E1000-devel] [Bug 56981] New: [SR-IOV] Intel I350 NIC VF
 cannot work in a Windows 2008 guest.
 
 Adding Ashish Shah.  IIRC Windows 2008 isn't supported by the VF driver
 but I may be mistaken.  Ashish should know.
 
 - Greg
 
  -Original Message-
  From: Ren, Yongjie [mailto:yongjie@intel.com]
  Sent: Monday, April 22, 2013 6:16 PM
  To: e1000-de...@lists.sourceforge.net; kvm@vger.kernel.org
  Subject: Re: [E1000-devel] [Bug 56981] New: [SR-IOV] Intel I350 NIC VF
  cannot work in a Windows 2008 guest.
 
  Added e1000-devel list to see whether this's a known issue.
 
  Best Regards,
   Yongjie (Jay)
 
 
   -Original Message-
   From: kvm-ow...@vger.kernel.org
 [mailto:kvm-ow...@vger.kernel.org]
   On Behalf Of bugzilla-dae...@bugzilla.kernel.org
   Sent: Monday, April 22, 2013 10:41 PM
   To: kvm@vger.kernel.org
   Subject: [Bug 56981] New: [SR-IOV] Intel I350 NIC VF cannot work in a
   Windows 2008 guest.
  
   https://bugzilla.kernel.org/show_bug.cgi?id=56981
  
  Summary: [SR-IOV] Intel I350 NIC VF cannot work in a
   Windows
   2008 guest.
  Product: Virtualization
  Version: unspecified
   Kernel Version: 3.9.0-rc3
 Platform: All
   OS/Version: Linux
 Tree: Mainline
   Status: NEW
 Severity: normal
 Priority: P1
Component: kvm
   AssignedTo: virtualization_...@kernel-bugs.osdl.org
   ReportedBy: yongjie@intel.com
   Regression: No
  
  
   Environment:
   
   Host OS (ia32/ia32e/IA64):ia32e
   Guest OS (ia32/ia32e/IA64):ia32e
   Guest OS Type (Linux/Windows):Windows
   kvm.git Commit:79558f112fc0352e057f7b5e158e3d88b8b62c60
   qemu-kvm Commit:8912bdea01e8671e59fe0287314379be9c1f40ec
   Host Kernel Version:3.9.0-rc3
   Hardware: Sandy Bridge - EP
  
  
   Bug detailed description:
   --
   The assigned Intel I350 NIC VF (a igbvf) cannot work in a Windows 2008
   guest.
  
   1. If the guest is Windows 7/8/2012 or RHEL6.x , the Intel I350 NIC VF
   can work fine in the guest.
   2. Intel 82576 NIC VF (also igbvf) can work in a Windows 2008 guest.
  
  
   Reproduce steps:
   
   1. ./pcistub.sh -h 0b:10.0   ( to hide a device)
   2. qemu-system-x86_64 -enable-kvm -m 4096 -smp 4 -device
   pci-assign,host=0b:10.0 -net none -hda win2k8.img
  
   Current result:
   
   Intel I350 NIC VF cannot work in win2k8 guest
  
   Expected result:
   
   Intel I350 NIC VF can work in win2k8 guest
  
   --
   Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
   --- You are receiving this mail because: --- You are watching
   the assignee of the bug.
   --
   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
  --
  
  Precog is a next-generation analytics platform capable of advanced
  analytics on semi-structured data. The platform includes APIs for building
  apps and a phenomenal toolset for data science. Developers can use our
  toolset for easy data analysis  visualization. Get a free account!
  http://www2.precog.com/precogplatform/slashdotnewsletter
  ___
  E1000-devel mailing list
  e1000-de...@lists.sourceforge.net
  https://lists.sourceforge.net/lists/listinfo/e1000-devel
  To learn more about Intel#174; Ethernet, visit
  http://communities.intel.com/community/wired
--
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] kvm tools: virtio-net mergable rx buffers

2013-04-24 Thread Pekka Enberg
Hi,

On 04/23/2013 12:35 PM, Eric Northup wrote:
 Do you care about guests with drivers that don't negotiate
 VIRTIO_NET_F_MRG_RXBUF?

On Wed, Apr 24, 2013 at 5:51 AM, Sasha Levin sasha.le...@oracle.com wrote:
 We usually try to keep backward compatibility, but in this case
 mergable RX buffers are about 5 years old now, so it's safe to
 assume they'll be running in any guest.

 Unless there is a specific reason to allow working without them
 I'd rather keep the code simple in this case.

Are there such guests around? What's the failure scenario for them
after this patch?

Pekka
--
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


[Bug 56981] [SR-IOV] Intel I350 NIC VF cannot work in a Windows 2008 guest.

2013-04-24 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=56981


Jay Ren yongjie@intel.com changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution||INVALID




--- Comment #3 from Jay Ren yongjie@intel.com  2013-04-24 06:54:03 ---
According to the message from Intel igb driver team, Windows 2008 is not
supported by Intel I350 driver. I350 VF can work in Windows 2008 R2 (not
Windows 2008).

So, I'll close this bug.

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are watching the assignee of the bug.
--
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


[Bug 56981] [SR-IOV] Intel I350 NIC VF cannot work in a Windows 2008 guest.

2013-04-24 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=56981


Jay Ren yongjie@intel.com changed:

   What|Removed |Added

 Status|RESOLVED|CLOSED




-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are watching the assignee of the bug.
--
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: [Bug 53611] New: nVMX: Add nested EPT

2013-04-24 Thread Jan Kiszka
On 2013-03-22 17:45, Jan Kiszka wrote:
 On 2013-03-22 07:23, Nakajima, Jun wrote:
 On Mon, Mar 4, 2013 at 8:45 PM, Nakajima, Jun jun.nakaj...@intel.com wrote:
 I have some updates on this. We rebased the patched to the latest KVM
 (L0). It turned out that the version of L1 KVM/Linux matters. At that
 time, actually I used v3.7 kernel for L1, and the L2 didn't work as I
 described above. If I use v3.5 or older for L1, L2 works with the EPT
 patches. So, I guess some changes made to v3.6 might have exposed a
 bug with the nested EPT patches or somewhere. We are looking at the
 changes to root-cause it.


 Finally I've had more time to work on this, and I think I've fixed
 this. The problem was that the exit qualification for EPT violation
 (to L1) was not accurate (enough). And I needed to save the exit
 qualification upon EPT violation somewhere. Today, that information is
 converted to error_code (see below), and we lose the information.  We
 need to use  at least the lower 3 bits when injecting EPT violation to
 the L1 VMM. I tried to use the upper bytes of error_code to pass  part
 of the exit qualification, but it didn't work well. Any suggestion for
 the place to store the value? kvm_vcpu?

...
 /* It is a write fault? */
 error_code = exit_qualification  (1U  1);
 /* ept page table is present? */
 error_code |= (exit_qualification  3)  0x1;

 return kvm_mmu_page_fault(vcpu, gpa, error_code, NULL, 0);
 
 I don't have a full picture (already asked you to post / git-push your
 intermediate state), but nested related states typically go to
 nested_vmx, thus vcpu_vmx.

Ping regarding publication. I'm about to redo your porting work as we
are making no progress.

Jan




signature.asc
Description: OpenPGP digital signature


Re: [PATCH] kvm tools: remove arbitrary minimum RAM limitation

2013-04-24 Thread Pekka Enberg

On 04/23/2013 05:57 PM, Sasha Levin wrote:

We don't really need 64MB of RAM to boot, it's a nice default if we don't
have anything else - but it's not actually required for anything:

sh-4.2# free -h
  total   used   free sharedbuffers cached
Mem:   20M15M   4.2M 0B 0B   4.2M
-/+ buffers/cache:11M   8.3M
Swap:   0B 0B 0B

Signed-off-by: Sasha Levin sasha.le...@oracle.com


Applied, thanks!

Pekka

--
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: [Qemu-devel] KVM call minutes for 2013-04-23

2013-04-24 Thread Stefan Hajnoczi
On Tue, Apr 23, 2013 at 10:06:41AM -0600, Eric Blake wrote:
 On 04/23/2013 08:45 AM, Juan Quintela wrote:
we can change drive_mirror to use a new command to see if there
are the new features.
 
 drive-mirror changed in 1.4 to add optional buf-size parameter; right
 now, libvirt is forced to limit itself to 1.3 interface (no buf-size or
 granularity) because there is no introspection and no query-* command
 that witnesses that the feature is present.  Idea was that we need to
 add a new query-drive-mirror-capabilities (name subject to bikeshedding)
 command into 1.5 that would let libvirt know that buf-size/granularity
 is usable (done right, it would also prevent the situation of buf-size
 being a write-only interface where it is set when starting the mirror
 but can not be queried later to see what size is in use).
 
 Unclear whether anyone was signing up to tackle the addition of a query
 command counterpart for drive-mirror in time for 1.5.

Seems like the trivial solution is a query-command-capabilities QMP
command.

  query-command-capabilities drive-mirror
  = ['buf-size']

It should only be a few lines of code and can be used for other commands
that add optional parameters in the future.  In other words:

typedef struct mon_cmd_t {
...
const char **capabilities; /* drive-mirror uses [buf-size, NULL] */
};

  
if we have a stable c-api we can do test cases that work. 
 
 Having such a testsuite would make a stable C API more important.

Writing tests in Python has been productive, see qemu-iotests 041 and
friends.  The tests spawn QEMU guests and use QMP to interact:

  result = self.vm.qmp('query-block')
  self.assert_qmp(result, 'return[0]/inserted/file', target_img)

Using this XPath-style syntax it's very easy to access the JSON.

QEMU users tend not to use C, except libvirt.  Even libvirt implements
the QMP protocol dynamically and can handle optional arguments well.

I don't think a static C API makes sense when we have an extensible JSON
protocol.  Let's use the extensibility to our advantage.

Stefan
--
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] kvm tools: remove arbitrary minimum RAM limitation

2013-04-24 Thread Michael Ellerman
On Tue, 2013-04-23 at 10:57 -0400, Sasha Levin wrote:
 We don't really need 64MB of RAM to boot, it's a nice default if we don't
 have anything else - but it's not actually required for anything:

Nice, I am carrying something similar locally so I can boot in 4K :)

cheers

--
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 -v2] kvm: Emulate MOVBE

2013-04-24 Thread Borislav Petkov
On Tue, Apr 23, 2013 at 04:50:43PM -0700, H. Peter Anvin wrote:
 On 04/23/2013 04:41 PM, Borislav Petkov wrote:
  
  Btw, in thinking about this more, I'm kinda sceptical we want to use the
  CPUID layout for this new KVM_GET_EMULATED_* ioctl. And the reason why
  I'm sceptical is that not every instruction is behind a CPUID capability
  bit and if we want to tell userspace that we do emulate any insn, even
  one for which there's no CPUID bit, we're going to have to either
  simulate a kvm-specific CPUID leaf or, maybe even better, come up with
  our own format for emulated capabilities. Maybe a bit vector with set
  bits for the respective capability, or something more nifty.
  
  In any case, it doesn't really need to be CPUID-like, IMHO.
  
 Using CPUID has the major advantage that it is well-defined.

Right, and it is actually a tree of bitvectors of height of 1. :-)

However, in the remote case where we want to state an emulated
capability for which there is no CPUID bit, we probably need something.

From looking at do_cpuid_ent(), kvm uses the 0x4000 so we probably
could define such a flag in there. If needed, of course.

So KVM_GET_EMULATED_CPUID it is. :-)

-- 
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
--
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 -v2] kvm: Emulate MOVBE

2013-04-24 Thread Gleb Natapov
On Tue, Apr 23, 2013 at 04:50:43PM -0700, H. Peter Anvin wrote:
 On 04/23/2013 04:41 PM, Borislav Petkov wrote:
  
  Btw, in thinking about this more, I'm kinda sceptical we want to use the
  CPUID layout for this new KVM_GET_EMULATED_* ioctl. And the reason why
  I'm sceptical is that not every instruction is behind a CPUID capability
  bit and if we want to tell userspace that we do emulate any insn, even
  one for which there's no CPUID bit, we're going to have to either
  simulate a kvm-specific CPUID leaf or, maybe even better, come up with
  our own format for emulated capabilities. Maybe a bit vector with set
  bits for the respective capability, or something more nifty.
  
  In any case, it doesn't really need to be CPUID-like, IMHO.
  
 
 Using CPUID has the major advantage that it is well-defined.
 
This. And I really hope vendors will not add instructions without
corespondent CPUID bits nowadays.

--
Gleb.
--
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] kvm tools: virtio-net mergable rx buffers

2013-04-24 Thread Michael S. Tsirkin
On Wed, Apr 24, 2013 at 09:51:57AM +0300, Pekka Enberg wrote:
 Hi,
 
 On 04/23/2013 12:35 PM, Eric Northup wrote:
  Do you care about guests with drivers that don't negotiate
  VIRTIO_NET_F_MRG_RXBUF?
 
 On Wed, Apr 24, 2013 at 5:51 AM, Sasha Levin sasha.le...@oracle.com wrote:
  We usually try to keep backward compatibility, but in this case
  mergable RX buffers are about 5 years old now, so it's safe to
  assume they'll be running in any guest.
 
  Unless there is a specific reason to allow working without them
  I'd rather keep the code simple in this case.
 
 Are there such guests around? What's the failure scenario for them
 after this patch?
 
 Pekka

Warning: have not looked at the patch, just a general comment.

I think it's reasonable to assume embedded guests such as PXE won't
negotiate any features.  And, running old guests is one of the reasons
people use virtualization at all. So 5 years is not a lot.

In any case, stick to the device spec please, if you want it changed
please send a spec patch, don't deviate from it randomly.

-- 
MST

--
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 v3 11/32] arm64: KVM: CPU specific system registers handling

2013-04-24 Thread Marc Zyngier
On 23/04/13 23:59, Christoffer Dall wrote:
 On Fri, Apr 12, 2013 at 05:04:14PM +0100, Marc Zyngier wrote:
 On 10/04/13 18:06, Will Deacon wrote:
 On Mon, Apr 08, 2013 at 05:17:13PM +0100, Marc Zyngier wrote:
 Add the support code for CPU specific system registers. Not much
 here yet.

 Reviewed-by: Christopher Covington c...@codeaurora.org
 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm64/kvm/sys_regs_generic_v8.c | 85 
 
  1 file changed, 85 insertions(+)
  create mode 100644 arch/arm64/kvm/sys_regs_generic_v8.c

 diff --git a/arch/arm64/kvm/sys_regs_generic_v8.c 
 b/arch/arm64/kvm/sys_regs_generic_v8.c
 new file mode 100644
 index 000..d4e8039
 --- /dev/null
 +++ b/arch/arm64/kvm/sys_regs_generic_v8.c
 @@ -0,0 +1,85 @@
 +/*
 + * Copyright (C) 2012,2013 - ARM Ltd
 + * Author: Marc Zyngier marc.zyng...@arm.com
 + *
 + * Based on arch/arm/kvm/coproc_a15.c:
 + * Copyright (C) 2012 - Virtual Open Systems and Columbia University
 + * Authors: Rusty Russell ru...@rustcorp.au
 + *  Christoffer Dall c.d...@virtualopensystems.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License, version 2, as
 + * published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +#include linux/kvm_host.h
 +#include asm/cputype.h
 +#include asm/kvm_arm.h
 +#include asm/kvm_asm.h
 +#include asm/kvm_host.h
 +#include asm/kvm_emulate.h
 +#include asm/kvm_coproc.h
 +#include linux/init.h
 +
 +#include sys_regs.h
 +
 +static bool access_actlr(struct kvm_vcpu *vcpu,
 +   const struct sys_reg_params *p,
 +   const struct sys_reg_desc *r)
 +{
 +  if (p-is_write)
 +  return ignore_write(vcpu, p);
 +
 +  *vcpu_reg(vcpu, p-Rt) = vcpu_sys_reg(vcpu, ACTLR_EL1);
 +  return true;
 +}
 +
 +static void reset_actlr(struct kvm_vcpu *vcpu, const struct sys_reg_desc 
 *r)
 +{
 +  u64 actlr;
 +
 +  asm volatile(mrs %0, actlr_el1\n : =r (actlr));
 +  vcpu_sys_reg(vcpu, ACTLR_EL1) = actlr;
 +}

 Do we actually need this? If so, there are likely other registers (things
 like the ectlr) that should be considered too.

 I'm focussing on the architected registers, and only those. ECTLR is
 implementation dependent, and is not specified as an architected sysreg.

 As this is one of the registers that we trap (TACR set in HCR_EL2), we
 have to emulate it. Now, maybe it is not that useful to trap it (nobody
 uses that code path yet).

 why is this even in a generic_v8 file then? Should it not be able to be
 handled generically in the sys_regs file and only when you have specific
 meanings of the register for a specific core should you add something
 like this file?

That would be a possibility. But I'd rather have a separate file that
describes targets, rather then stuffing this into sys_regs.c (which is
already too big for my taste), and have the implementation defined
registers there.

The AEMv8/Foundation being generic implementations of ARMv8, I'm
giving them their own target file. The A57 mention in this file is just
a temporary hack until I get to boot the stuff on a real A57.

 I think it's preferred to have something that traps and shouts than
 something that may or may not work (ie. just allowing the guest to read
 the register directly), but you guys know better than me what kind of
 things can be exposed through this register in the future.

At least for the models, this handling of ACTLR_EL1 should be correct.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...

--
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 v8 3/3] tcm_vhost: Add ioctl to get and set events missed flag

2013-04-24 Thread Michael S. Tsirkin
On Wed, Apr 24, 2013 at 11:32:24AM +0800, Asias He wrote:
 Signed-off-by: Asias He as...@redhat.com

Looks like a good idea to me. I missed this patch
previously, thanks for addressing this.
Some implementation comments below.

 ---
  drivers/vhost/tcm_vhost.c | 16 
  drivers/vhost/tcm_vhost.h |  3 +++
  2 files changed, 19 insertions(+)
 
 diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
 index def0c57..11fca73 100644
 --- a/drivers/vhost/tcm_vhost.c
 +++ b/drivers/vhost/tcm_vhost.c
 @@ -1204,6 +1204,8 @@ static long vhost_scsi_ioctl(struct file *f, unsigned 
 int ioctl,
   u64 __user *featurep = argp;
   u64 features;
   int r, abi_version = VHOST_SCSI_ABI_VERSION;
 + struct vhost_virtqueue *vq = vs-vqs[VHOST_SCSI_VQ_EVT];
 + bool events_missed;
  
   switch (ioctl) {
   case VHOST_SCSI_SET_ENDPOINT:
 @@ -1224,6 +1226,20 @@ static long vhost_scsi_ioctl(struct file *f, unsigned 
 int ioctl,
   if (copy_to_user(argp, abi_version, sizeof abi_version))
   return -EFAULT;
   return 0;
 + case VHOST_SCSI_SET_EVENTS_MISSED:
 + if (copy_from_user(events_missed, argp, sizeof(events_missed)))
 + return -EFAULT;

This can trigger undefined behaviour because userspace can
make bool have values other than 0 or 1.
So please read an integer here.
This also let you use get_user.

 + mutex_lock(vq-mutex);
 + vs-vs_events_missed = events_missed;
 + mutex_unlock(vq-mutex);
 + return 0;
 + case VHOST_SCSI_GET_EVENTS_MISSED:
 + mutex_lock(vq-mutex);
 + events_missed = vs-vs_events_missed;
 + mutex_unlock(vq-mutex);
 + if (copy_to_user(argp, events_missed, sizeof(events_missed)))
 + return -EFAULT;
 + return 0;
   case VHOST_GET_FEATURES:
   features = VHOST_SCSI_FEATURES;
   if (copy_to_user(featurep, features, sizeof features))
 diff --git a/drivers/vhost/tcm_vhost.h b/drivers/vhost/tcm_vhost.h
 index 94e9ee53..b473e25 100644
 --- a/drivers/vhost/tcm_vhost.h
 +++ b/drivers/vhost/tcm_vhost.h
 @@ -123,3 +123,6 @@ struct vhost_scsi_target {
  #define VHOST_SCSI_CLEAR_ENDPOINT _IOW(VHOST_VIRTIO, 0x41, struct 
 vhost_scsi_target)
  /* Changing this breaks userspace. */
  #define VHOST_SCSI_GET_ABI_VERSION _IOW(VHOST_VIRTIO, 0x42, int)
 +/* Set and get the events missed flag */
 +#define VHOST_SCSI_SET_EVENTS_MISSED _IOW(VHOST_VIRTIO, 0x43, bool)
 +#define VHOST_SCSI_GET_EVENTS_MISSED _IOW(VHOST_VIRTIO, 0x44, bool)

Please don't do bool in user interfaces. No one does. Make it __u32.

 -- 
 1.8.1.4
--
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 v8 2/3] tcm_vhost: Add hotplug/hotunplug support

2013-04-24 Thread Michael S. Tsirkin
On Wed, Apr 24, 2013 at 11:32:23AM +0800, Asias He wrote:
 In commit 365a7150094 ([SCSI] virtio-scsi: hotplug support for
 virtio-scsi), hotplug support is added to virtio-scsi.
 
 This patch adds hotplug and hotunplug support to tcm_vhost.
 
 You can create or delete a LUN in targetcli to hotplug or hotunplug a
 LUN in guest.
 
 Changes in v8:
 - Use vq-mutex for event
 - Drop tcm_vhost: Add helper to check if endpoint is setup
 - Rename vs_events_dropped to vs_events_missed
 - Init lun[] explicitly
 
 Changes in v7:
 - Add vhost_work_flush for vs-vs_event_work to this series
 
 Changes in v6:
 - Pass tcm_vhost_evt to tcm_vhost_do_evt_work
 
 Changes in v5:
 - Switch to int from u64 to vs_events_nr
 - Set s-vs_events_dropped flag in tcm_vhost_allocate_evt
 - Do not nest dev mutex within vq mutex
 - Use vs_events_lock to protect vs_events_dropped and vs_events_nr
 - Rebase to target/master
 
 Changes in v4:
 - Drop tcm_vhost_check_endpoint in tcm_vhost_send_evt
 - Add tcm_vhost_check_endpoint in vhost_scsi_evt_handle_kick
 
 Changes in v3:
 - Separate the bug fix to another thread
 
 Changes in v2:
 - Remove code duplication in tcm_vhost_{hotplug,hotunplug}
 - Fix racing of vs_events_nr
 - Add flush fix patch to this series
 
 Signed-off-by: Asias He as...@redhat.com
 Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
 ---
  drivers/vhost/tcm_vhost.c | 203 
 +-
  drivers/vhost/tcm_vhost.h |  10 +++
  2 files changed, 209 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
 index 88ebb79..def0c57 100644
 --- a/drivers/vhost/tcm_vhost.c
 +++ b/drivers/vhost/tcm_vhost.c
 @@ -66,11 +66,13 @@ enum {
   * TODO: debug and remove the workaround.
   */
  enum {
 - VHOST_SCSI_FEATURES = VHOST_FEATURES  (~VIRTIO_RING_F_EVENT_IDX)
 + VHOST_SCSI_FEATURES = (VHOST_FEATURES  (~VIRTIO_RING_F_EVENT_IDX)) |
 +   (1ULL  VIRTIO_SCSI_F_HOTPLUG)
  };
  
  #define VHOST_SCSI_MAX_TARGET256
  #define VHOST_SCSI_MAX_VQ128
 +#define VHOST_SCSI_MAX_EVENT 128
  
  struct vhost_scsi {
   /* Protected by vhost_scsi-dev.mutex */
 @@ -82,6 +84,12 @@ struct vhost_scsi {
  
   struct vhost_work vs_completion_work; /* cmd completion work item */
   struct llist_head vs_completion_list; /* cmd completion queue */
 +
 + struct vhost_work vs_event_work; /* evt injection work item */
 + struct llist_head vs_event_list; /* evt injection queue */
 +
 + bool vs_events_missed; /* any missed events, protected by vq-mutex */
 + int vs_events_nr; /* num of pending events, protected by vq-mutex */
  };
  
  /* Local pointer to allocated TCM configfs fabric module */
 @@ -361,6 +369,32 @@ static int tcm_vhost_queue_tm_rsp(struct se_cmd *se_cmd)
   return 0;
  }
  
 +static void tcm_vhost_free_evt(struct vhost_scsi *vs, struct tcm_vhost_evt 
 *evt)
 +{
 + vs-vs_events_nr--;
 + kfree(evt);
 +}
 +
 +static struct tcm_vhost_evt *tcm_vhost_allocate_evt(struct vhost_scsi *vs,
 + u32 event, u32 reason)
 +{
 + struct tcm_vhost_evt *evt;
 +
 + if (vs-vs_events_nr  VHOST_SCSI_MAX_EVENT) {
 + vs-vs_events_missed = true;
 + return NULL;
 + }
 +
 + evt = kmalloc(sizeof(*evt), GFP_KERNEL);
 + if (evt) {

!evt should trigger vq_err I think.
Please do
if (!evt) {
vq_err
return NULL
}

this way code below has less nesting.

 + evt-event.event = event;
 + evt-event.reason = reason;
 + vs-vs_events_nr++;
 + }
 +
 + return evt;
 +}
 +
  static void vhost_scsi_free_cmd(struct tcm_vhost_cmd *tv_cmd)
  {
   struct se_cmd *se_cmd = tv_cmd-tvc_se_cmd;
 @@ -379,6 +413,74 @@ static void vhost_scsi_free_cmd(struct tcm_vhost_cmd 
 *tv_cmd)
   kfree(tv_cmd);
  }
  
 +static void tcm_vhost_do_evt_work(struct vhost_scsi *vs,
 + struct tcm_vhost_evt *evt)
 +{
 + struct vhost_virtqueue *vq = vs-vqs[VHOST_SCSI_VQ_EVT];
 + struct virtio_scsi_event *event = evt-event;
 + struct virtio_scsi_event __user *eventp;
 + unsigned out, in;
 + int head, ret;
 +
 +again:
 + vhost_disable_notify(vs-dev, vq);
 + head = vhost_get_vq_desc(vs-dev, vq, vq-iov,
 + ARRAY_SIZE(vq-iov), out, in,
 + NULL, NULL);
 + if (head  0) {
 + vs-vs_events_missed = true;
 + return;
 + }
 + if (head == vq-num) {
 + if (vhost_enable_notify(vs-dev, vq))
 + goto again;
 + vs-vs_events_missed = true;
 + return;
 + }
 +
 + if ((vq-iov[out].iov_len != sizeof(struct virtio_scsi_event))) {
 + vq_err(vq, Expecting virtio_scsi_event, got %zu bytes\n,
 + vq-iov[out].iov_len);
 + vs-vs_events_missed = true;
 + return;
 + }
 +
 + if (vs-vs_events_missed) {
 + 

Re: [PATCH] kvm tools: remove arbitrary minimum RAM limitation

2013-04-24 Thread Asias He
On Wed, Apr 24, 2013 at 06:25:30PM +1000, Michael Ellerman wrote:
 On Tue, 2013-04-23 at 10:57 -0400, Sasha Levin wrote:
  We don't really need 64MB of RAM to boot, it's a nice default if we don't
  have anything else - but it's not actually required for anything:
 
 Nice, I am carrying something similar locally so I can boot in 4K :)

Show me your $free ;-)

 cheers
 

-- 
Asias
--
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] kvm tools: virtio-net mergable rx buffers

2013-04-24 Thread Michael S. Tsirkin
On Wed, Apr 24, 2013 at 01:32:56PM +0800, Asias He wrote:
 On Mon, Apr 22, 2013 at 08:32:27PM -0400, Sasha Levin wrote:
  Support mergable rx buffers for virtio-net. This helps reduce the amount
  of memory the guest kernel has to allocate per rx vq.
 
 With this, we always do an extra copy of the rx data into a linear buffer.
 I am thinking about whether we should keep the non MRG_RXBUF code. For
 tap use case, it is fine, user can use vhost. But for uip use case, we
 can not use vhost.

You have to keep it but it might be ok not to optimize it.

  Signed-off-by: Sasha Levin sasha.le...@oracle.com
  ---
   tools/kvm/include/kvm/uip.h  |  4 ++--
   tools/kvm/include/kvm/util.h |  3 +++
   tools/kvm/net/uip/core.c | 54 
  +---
   tools/kvm/net/uip/tcp.c  |  2 +-
   tools/kvm/net/uip/udp.c  |  2 +-
   tools/kvm/util/util.c| 15 
   tools/kvm/virtio/net.c   | 37 ++
   7 files changed, 55 insertions(+), 62 deletions(-)
  
  diff --git a/tools/kvm/include/kvm/uip.h b/tools/kvm/include/kvm/uip.h
  index ac248d2..fa82f10 100644
  --- a/tools/kvm/include/kvm/uip.h
  +++ b/tools/kvm/include/kvm/uip.h
  @@ -252,7 +252,7 @@ struct uip_tcp_socket {
   };
   
   struct uip_tx_arg {
  -   struct virtio_net_hdr *vnet;
  +   struct virtio_net_hdr_mrg_rxbuf *vnet;
  struct uip_info *info;
  struct uip_eth *eth;
  int vnet_len;
  @@ -332,7 +332,7 @@ static inline u16 uip_eth_hdrlen(struct uip_eth *eth)
   }
   
   int uip_tx(struct iovec *iov, u16 out, struct uip_info *info);
  -int uip_rx(struct iovec *iov, u16 in, struct uip_info *info);
  +int uip_rx(unsigned char *buffer, u32 length, struct uip_info *info);
   int uip_init(struct uip_info *info);
   
   int uip_tx_do_ipv4_udp_dhcp(struct uip_tx_arg *arg);
  diff --git a/tools/kvm/include/kvm/util.h b/tools/kvm/include/kvm/util.h
  index 0df9f0d..6f8ac83 100644
  --- a/tools/kvm/include/kvm/util.h
  +++ b/tools/kvm/include/kvm/util.h
  @@ -22,6 +22,7 @@
   #include sys/param.h
   #include sys/types.h
   #include linux/types.h
  +#include sys/uio.h
   
   #ifdef __GNUC__
   #define NORETURN __attribute__((__noreturn__))
  @@ -94,4 +95,6 @@ struct kvm;
   void *mmap_hugetlbfs(struct kvm *kvm, const char *htlbfs_path, u64 size);
   void *mmap_anon_or_hugetlbfs(struct kvm *kvm, const char *hugetlbfs_path, 
  u64 size);
   
  +int memcpy_toiovecend(const struct iovec *iov, int iovlen, unsigned char 
  *kdata, size_t len);
  +
   #endif /* KVM__UTIL_H */
  diff --git a/tools/kvm/net/uip/core.c b/tools/kvm/net/uip/core.c
  index 4e5bb82..d9e9993 100644
  --- a/tools/kvm/net/uip/core.c
  +++ b/tools/kvm/net/uip/core.c
  @@ -7,7 +7,7 @@
   
   int uip_tx(struct iovec *iov, u16 out, struct uip_info *info)
   {
  -   struct virtio_net_hdr *vnet;
  +   struct virtio_net_hdr_mrg_rxbuf *vnet;
  struct uip_tx_arg arg;
  int eth_len, vnet_len;
  struct uip_eth *eth;
  @@ -74,63 +74,21 @@ int uip_tx(struct iovec *iov, u16 out, struct uip_info 
  *info)
  return vnet_len + eth_len;
   }
   
  -int uip_rx(struct iovec *iov, u16 in, struct uip_info *info)
  +int uip_rx(unsigned char *buffer, u32 length, struct uip_info *info)
   {
  -   struct virtio_net_hdr *vnet;
  -   struct uip_eth *eth;
  struct uip_buf *buf;
  -   int vnet_len;
  -   int eth_len;
  -   char *p;
  int len;
  -   int cnt;
  -   int i;
   
  /*
   * Sleep until there is a buffer for guest
   */
  buf = uip_buf_get_used(info);
   
  -   /*
  -* Fill device to guest buffer, vnet hdr fisrt
  -*/
  -   vnet_len = iov[0].iov_len;
  -   vnet = iov[0].iov_base;
  -   if (buf-vnet_len  vnet_len) {
  -   len = -1;
  -   goto out;
  -   }
  -   memcpy(vnet, buf-vnet, buf-vnet_len);
  -
  -   /*
  -* Then, the real eth data
  -* Note: Be sure buf-eth_len is not bigger than the buffer len that 
  guest provides
  -*/
  -   cnt = buf-eth_len;
  -   p = buf-eth;
  -   for (i = 1; i  in; i++) {
  -   eth_len = iov[i].iov_len;
  -   eth = iov[i].iov_base;
  -   if (cnt  eth_len) {
  -   memcpy(eth, p, eth_len);
  -   cnt -= eth_len;
  -   p += eth_len;
  -   } else {
  -   memcpy(eth, p, cnt);
  -   cnt -= cnt;
  -   break;
  -   }
  -   }
  -
  -   if (cnt) {
  -   pr_warning(uip_rx error);
  -   len = -1;
  -   goto out;
  -   }
  +   memcpy(buffer, buf-vnet, buf-vnet_len);
  +   memcpy(buffer + buf-vnet_len, buf-eth, buf-eth_len);
   
  len = buf-vnet_len + buf-eth_len;
   
  -out:
  uip_buf_set_free(info, buf);
  return len;
   }
  @@ -172,8 +130,8 @@ int uip_init(struct uip_info *info)
  }
   
  list_for_each_entry(buf, buf_head, list) {
  -   buf-vnet   = malloc(sizeof(struct virtio_net_hdr));
  -   buf-vnet_len   = sizeof(struct virtio_net_hdr);

Re: [PATCH v3 03/32] arm64: KVM: HYP mode idmap support

2013-04-24 Thread Marc Zyngier
On 23/04/13 23:57, Christoffer Dall wrote:
 
 
 On Mon, Apr 8, 2013 at 9:17 AM, Marc Zyngier marc.zyng...@arm.com wrote:
 Add the necessary infrastructure for identity-mapped HYP page
 tables. Idmap-ed code must be in the .hyp.idmap.text linker
 section.

 The rest of the HYP ends up in .hyp.text.

 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm64/kernel/vmlinux.lds.S | 16 
  1 file changed, 16 insertions(+)

 diff --git a/arch/arm64/kernel/vmlinux.lds.S 
 b/arch/arm64/kernel/vmlinux.lds.S
 index 3fae2be..855d43d 100644
 --- a/arch/arm64/kernel/vmlinux.lds.S
 +++ b/arch/arm64/kernel/vmlinux.lds.S
 @@ -17,6 +17,15 @@ ENTRY(stext)

  jiffies = jiffies_64;

 +#define HYPERVISOR_TEXT\
 +   . = ALIGN(2048);\
 
 why align to 2048?  Comment?

Vectors require a 2kB alignment. I'll add a comment to that effect.

 are you changing this to the same idea as for the 32-bit code to reduce the 
 kernel size?

The mechanism is exactly the same indeed. Only the alignment
requirements are (massively) different.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...

--
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 2/7] KVM: Introduce __KVM_HAVE_IRQCHIP

2013-04-24 Thread Gleb Natapov
On Wed, Apr 17, 2013 at 02:16:59PM +0200, Alexander Graf wrote:
 
 On 17.04.2013, at 14:15, Paolo Bonzini wrote:
 
  Il 17/04/2013 14:10, Alexander Graf ha scritto:
  Still, __KVM_HAVE_IRQCHIP is clearly a subset of
  __KVM_HAVE_IOAPIC; defining one without the other makes no sense
  and will cause compilation or link errors for trace_kvm_ack_irq.
  
  Either we drop it altogether, or we should not break compilation 
  consciously---especially if the problem is so trivial and obvious
  that you had to think of leaving it out.
  I disagree. I actually _want_ to break it on purpose, so we have even
  more reason to remove all that useless code if nobody complains.
  
  Then remove it in patch 1, and move all the remaining IOAPIC code back
  to arch/x86 at the end.  As things are, you're just leaving someone else
  to do the work (no matter if it is to fix it, or to zap it).
 
 Gleb, Marcelo, any objections to me removing ia64?
 
It was marked BROKEN less then year ago. I would give it one more year
as a last chance. We can reconsider this during 3.11 development cycle,
but if you want this code to make 3.10 can you please fix ia64
compilation. The patch on top of the series is OK.

--
Gleb.
--
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 v3 07/32] arm64: KVM: fault injection into a guest

2013-04-24 Thread Marc Zyngier
On 23/04/13 23:57, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:09PM +0100, Marc Zyngier wrote:
 Implement the injection of a fault (undefined, data abort or
 prefetch abort) into a 64bit guest.

 Reviewed-by: Christopher Covington c...@codeaurora.org
 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm64/kvm/inject_fault.c | 118 
 ++
  1 file changed, 118 insertions(+)
  create mode 100644 arch/arm64/kvm/inject_fault.c

 diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
 new file mode 100644
 index 000..2ff3b78
 --- /dev/null
 +++ b/arch/arm64/kvm/inject_fault.c
 @@ -0,0 +1,118 @@
 +/*
 + * Fault injection for 64bit guests.
 + *
 + * Copyright (C) 2012,2013 - ARM Ltd
 + * Author: Marc Zyngier marc.zyng...@arm.com
 + *
 + * Based on arch/arm/kvm/emulate.c
 + * Copyright (C) 2012 - Virtual Open Systems and Columbia University
 + * Author: Christoffer Dall c.d...@virtualopensystems.com
 + *
 + * This program is free software: you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#include linux/kvm_host.h
 +#include asm/kvm_emulate.h
 +#include asm/esr.h
 +
 +static void inject_abt64(struct kvm_vcpu *vcpu, bool is_iabt, unsigned long 
 addr)
 +{
 +unsigned long cpsr = *vcpu_cpsr(vcpu);
 +int is_aarch32;
 +u32 esr = 0;
 +
 +is_aarch32 = vcpu_mode_is_32bit(vcpu);
 +
 +*vcpu_spsr(vcpu) = cpsr;
 +*vcpu_elr_el1(vcpu) = *vcpu_pc(vcpu);
 +
 +*vcpu_cpsr(vcpu) = PSR_MODE_EL1h | PSR_A_BIT | PSR_F_BIT | PSR_I_BIT;
 +*vcpu_pc(vcpu) = vcpu_sys_reg(vcpu, VBAR_EL1) + 0x200;
 
 consider a define for the 0x200?

Yeah, I think Will already mentioned this.

 +
 +vcpu_sys_reg(vcpu, FAR_EL1) = addr;
 +
 +/*
 + * Build an {i,d}abort, depending on the level and the
 + * instruction set. Report an external synchronous abort.
 + */
 +if (kvm_vcpu_trap_il_is32bit(vcpu))
 +esr |= ESR_EL1_IL;
 +
 +if (is_aarch32 || (cpsr  PSR_MODE_MASK) == PSR_MODE_EL0t)
 +esr |= (ESR_EL1_EC_IABT_EL0  ESR_EL1_EC_SHIFT);
 +else
 +esr |= (ESR_EL1_EC_IABT_EL1  ESR_EL1_EC_SHIFT);
 
 why is it assumed that the abort must come from EL0 if the vcpu is in
 aarch32 mode?

We're injecting a fault into a VM that runs with a 64bit EL1. So if we
end-up faulting in 32bit code, it must come from EL0.

 +
 +if (!is_iabt)
 +esr |= ESR_EL1_EC_DABT_EL0;
 
 this is slightly confusing unless you actually look at the definitions
 and realize that or'ing on that extra bit works both in i- and d-abt
 cases.

Blame Christopher for that, he came up with the idea... ;-)

 +
 +vcpu_sys_reg(vcpu, ESR_EL1) = esr | 0x10; /* External abort */
 +}
 +
 +static void inject_undef64(struct kvm_vcpu *vcpu)
 +{
 +unsigned long cpsr = *vcpu_cpsr(vcpu);
 +u32 esr = (ESR_EL1_EC_UNKNOWN  ESR_EL1_EC_SHIFT);
 +
 +*vcpu_spsr(vcpu) = cpsr;
 +*vcpu_elr_el1(vcpu) = *vcpu_pc(vcpu);
 +
 +*vcpu_cpsr(vcpu) = PSR_MODE_EL1h | PSR_F_BIT | PSR_I_BIT;
 
 would it make sense to have a define for a common set of these bits or
 is this explicitly defined per exception type in the specs?

Maybe. Not sure that would be any clearer though...

M.
-- 
Jazz is not dead. It just smells funny...

--
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 v8 2/3] tcm_vhost: Add hotplug/hotunplug support

2013-04-24 Thread Asias He
On Wed, Apr 24, 2013 at 12:18:33PM +0300, Michael S. Tsirkin wrote:
 On Wed, Apr 24, 2013 at 11:32:23AM +0800, Asias He wrote:
  In commit 365a7150094 ([SCSI] virtio-scsi: hotplug support for
  virtio-scsi), hotplug support is added to virtio-scsi.
  
  This patch adds hotplug and hotunplug support to tcm_vhost.
  
  You can create or delete a LUN in targetcli to hotplug or hotunplug a
  LUN in guest.
  
  Changes in v8:
  - Use vq-mutex for event
  - Drop tcm_vhost: Add helper to check if endpoint is setup
  - Rename vs_events_dropped to vs_events_missed
  - Init lun[] explicitly
  
  Changes in v7:
  - Add vhost_work_flush for vs-vs_event_work to this series
  
  Changes in v6:
  - Pass tcm_vhost_evt to tcm_vhost_do_evt_work
  
  Changes in v5:
  - Switch to int from u64 to vs_events_nr
  - Set s-vs_events_dropped flag in tcm_vhost_allocate_evt
  - Do not nest dev mutex within vq mutex
  - Use vs_events_lock to protect vs_events_dropped and vs_events_nr
  - Rebase to target/master
  
  Changes in v4:
  - Drop tcm_vhost_check_endpoint in tcm_vhost_send_evt
  - Add tcm_vhost_check_endpoint in vhost_scsi_evt_handle_kick
  
  Changes in v3:
  - Separate the bug fix to another thread
  
  Changes in v2:
  - Remove code duplication in tcm_vhost_{hotplug,hotunplug}
  - Fix racing of vs_events_nr
  - Add flush fix patch to this series
  
  Signed-off-by: Asias He as...@redhat.com
  Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
  ---
   drivers/vhost/tcm_vhost.c | 203 
  +-
   drivers/vhost/tcm_vhost.h |  10 +++
   2 files changed, 209 insertions(+), 4 deletions(-)
  
  diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
  index 88ebb79..def0c57 100644
  --- a/drivers/vhost/tcm_vhost.c
  +++ b/drivers/vhost/tcm_vhost.c
  @@ -66,11 +66,13 @@ enum {
* TODO: debug and remove the workaround.
*/
   enum {
  -   VHOST_SCSI_FEATURES = VHOST_FEATURES  (~VIRTIO_RING_F_EVENT_IDX)
  +   VHOST_SCSI_FEATURES = (VHOST_FEATURES  (~VIRTIO_RING_F_EVENT_IDX)) |
  + (1ULL  VIRTIO_SCSI_F_HOTPLUG)
   };
   
   #define VHOST_SCSI_MAX_TARGET  256
   #define VHOST_SCSI_MAX_VQ  128
  +#define VHOST_SCSI_MAX_EVENT   128
   
   struct vhost_scsi {
  /* Protected by vhost_scsi-dev.mutex */
  @@ -82,6 +84,12 @@ struct vhost_scsi {
   
  struct vhost_work vs_completion_work; /* cmd completion work item */
  struct llist_head vs_completion_list; /* cmd completion queue */
  +
  +   struct vhost_work vs_event_work; /* evt injection work item */
  +   struct llist_head vs_event_list; /* evt injection queue */
  +
  +   bool vs_events_missed; /* any missed events, protected by vq-mutex */
  +   int vs_events_nr; /* num of pending events, protected by vq-mutex */
   };
   
   /* Local pointer to allocated TCM configfs fabric module */
  @@ -361,6 +369,32 @@ static int tcm_vhost_queue_tm_rsp(struct se_cmd 
  *se_cmd)
  return 0;
   }
   
  +static void tcm_vhost_free_evt(struct vhost_scsi *vs, struct tcm_vhost_evt 
  *evt)
  +{
  +   vs-vs_events_nr--;
  +   kfree(evt);
  +}
  +
  +static struct tcm_vhost_evt *tcm_vhost_allocate_evt(struct vhost_scsi *vs,
  +   u32 event, u32 reason)
  +{
  +   struct tcm_vhost_evt *evt;
  +
  +   if (vs-vs_events_nr  VHOST_SCSI_MAX_EVENT) {
  +   vs-vs_events_missed = true;
  +   return NULL;
  +   }
  +
  +   evt = kmalloc(sizeof(*evt), GFP_KERNEL);
  +   if (evt) {
 
 !evt should trigger vq_err I think.
 Please do
   if (!evt) {
   vq_err
   return NULL
   }
 
 this way code below has less nesting.

okay.

  +   evt-event.event = event;
  +   evt-event.reason = reason;
  +   vs-vs_events_nr++;
  +   }
  +
  +   return evt;
  +}
  +
   static void vhost_scsi_free_cmd(struct tcm_vhost_cmd *tv_cmd)
   {
  struct se_cmd *se_cmd = tv_cmd-tvc_se_cmd;
  @@ -379,6 +413,74 @@ static void vhost_scsi_free_cmd(struct tcm_vhost_cmd 
  *tv_cmd)
  kfree(tv_cmd);
   }
   
  +static void tcm_vhost_do_evt_work(struct vhost_scsi *vs,
  +   struct tcm_vhost_evt *evt)
  +{
  +   struct vhost_virtqueue *vq = vs-vqs[VHOST_SCSI_VQ_EVT];
  +   struct virtio_scsi_event *event = evt-event;
  +   struct virtio_scsi_event __user *eventp;
  +   unsigned out, in;
  +   int head, ret;
  +
  +again:
  +   vhost_disable_notify(vs-dev, vq);
  +   head = vhost_get_vq_desc(vs-dev, vq, vq-iov,
  +   ARRAY_SIZE(vq-iov), out, in,
  +   NULL, NULL);
  +   if (head  0) {
  +   vs-vs_events_missed = true;
  +   return;
  +   }
  +   if (head == vq-num) {
  +   if (vhost_enable_notify(vs-dev, vq))
  +   goto again;
  +   vs-vs_events_missed = true;
  +   return;
  +   }
  +
  +   if ((vq-iov[out].iov_len != sizeof(struct virtio_scsi_event))) {
  +   vq_err(vq, Expecting virtio_scsi_event, got %zu bytes\n,
  +   vq-iov[out].iov_len);
  +   

Re: [PATCH 0/7] KVM: irqfd generalization prepare patch set

2013-04-24 Thread Gleb Natapov
On Tue, Apr 16, 2013 at 07:26:08PM +0200, Alexander Graf wrote:
 The concept of an irqfd and interrupt routing are nothing particularly tied
 into the IOAPIC implementation. In fact, most of the code already is perfectly
 generic.
 
 This patch set decouples most bits of the existing irqchip and irqfd
 implementation to make it reusable for non-IOAPIC platforms, like the PPC 
 MPIC.
 
 I also have a patch that implements working irqfd support on top of these,
 but that requires the in-kernel MPIC implementation to go upstream first, so
 I'm holding off on it until we settled everything there, so the concept
 certainly does work.
 
 Alex
 
Nice cleanup, thanks! Should expect a new series with ifdef
kvm_irqchip and ia64 compilation fixed. The fixes are minor enough for
me to fix them while applying.

 Alexander Graf (7):
   KVM: Add KVM_IRQCHIP_NUM_PINS in addition to KVM_IOAPIC_NUM_PINS
   KVM: Introduce __KVM_HAVE_IRQCHIP
   KVM: Remove kvm_get_intr_delivery_bitmask
   KVM: Move irq routing to generic code
   KVM: Extract generic irqchip logic into irqchip.c
   KVM: Move irq routing setup to irqchip.c
   KVM: Move irqfd resample cap handling to generic code
 
  arch/x86/include/asm/kvm_host.h |2 +
  arch/x86/include/uapi/asm/kvm.h |1 +
  arch/x86/kvm/Makefile   |2 +-
  arch/x86/kvm/x86.c  |1 -
  include/linux/kvm_host.h|   14 +--
  include/trace/events/kvm.h  |   12 ++-
  include/uapi/linux/kvm.h|2 +-
  virt/kvm/assigned-dev.c |   30 -
  virt/kvm/eventfd.c  |6 +-
  virt/kvm/irq_comm.c |  193 +---
  virt/kvm/irqchip.c  |  237 
 +++
  virt/kvm/kvm_main.c |   33 ++
  12 files changed, 297 insertions(+), 236 deletions(-)
  create mode 100644 virt/kvm/irqchip.c

--
Gleb.
--
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


[PATCH] fix source operand decoding for 8bit mov[zs]x instructions

2013-04-24 Thread Gleb Natapov
Source operand for one byte mov[zs]x is decoded incorrectly if it is in high 
byte
register. Fix that.

Cc: sta...@vger.kernel.org
Signed-off-by: Gleb Natapov g...@redhat.com

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 46f63b8..8e517bb 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -4172,6 +4172,10 @@ static int decode_operand(struct x86_emulate_ctxt *ctxt, 
struct operand *op,
break;
case OpMem8:
ctxt-memop.bytes = 1;
+   if (ctxt-memop.type == OP_REG) {
+   ctxt-memop.addr.reg = decode_register(ctxt, 
ctxt-modrm_rm, 1);
+   fetch_register_operand(ctxt-memop);
+   }
goto mem_common;
case OpMem16:
ctxt-memop.bytes = 2;
--
Gleb.
--
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 v8 2/3] tcm_vhost: Add hotplug/hotunplug support

2013-04-24 Thread Michael S. Tsirkin
On Wed, Apr 24, 2013 at 06:06:25PM +0800, Asias He wrote:
 On Wed, Apr 24, 2013 at 12:18:33PM +0300, Michael S. Tsirkin wrote:
  On Wed, Apr 24, 2013 at 11:32:23AM +0800, Asias He wrote:
   In commit 365a7150094 ([SCSI] virtio-scsi: hotplug support for
   virtio-scsi), hotplug support is added to virtio-scsi.
   
   This patch adds hotplug and hotunplug support to tcm_vhost.
   
   You can create or delete a LUN in targetcli to hotplug or hotunplug a
   LUN in guest.
   
   Changes in v8:
   - Use vq-mutex for event
   - Drop tcm_vhost: Add helper to check if endpoint is setup
   - Rename vs_events_dropped to vs_events_missed
   - Init lun[] explicitly
   
   Changes in v7:
   - Add vhost_work_flush for vs-vs_event_work to this series
   
   Changes in v6:
   - Pass tcm_vhost_evt to tcm_vhost_do_evt_work
   
   Changes in v5:
   - Switch to int from u64 to vs_events_nr
   - Set s-vs_events_dropped flag in tcm_vhost_allocate_evt
   - Do not nest dev mutex within vq mutex
   - Use vs_events_lock to protect vs_events_dropped and vs_events_nr
   - Rebase to target/master
   
   Changes in v4:
   - Drop tcm_vhost_check_endpoint in tcm_vhost_send_evt
   - Add tcm_vhost_check_endpoint in vhost_scsi_evt_handle_kick
   
   Changes in v3:
   - Separate the bug fix to another thread
   
   Changes in v2:
   - Remove code duplication in tcm_vhost_{hotplug,hotunplug}
   - Fix racing of vs_events_nr
   - Add flush fix patch to this series
   
   Signed-off-by: Asias He as...@redhat.com
   Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
   ---
drivers/vhost/tcm_vhost.c | 203 
   +-
drivers/vhost/tcm_vhost.h |  10 +++
2 files changed, 209 insertions(+), 4 deletions(-)
   
   diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
   index 88ebb79..def0c57 100644
   --- a/drivers/vhost/tcm_vhost.c
   +++ b/drivers/vhost/tcm_vhost.c
   @@ -66,11 +66,13 @@ enum {
 * TODO: debug and remove the workaround.
 */
enum {
   - VHOST_SCSI_FEATURES = VHOST_FEATURES  (~VIRTIO_RING_F_EVENT_IDX)
   + VHOST_SCSI_FEATURES = (VHOST_FEATURES  (~VIRTIO_RING_F_EVENT_IDX)) |
   +   (1ULL  VIRTIO_SCSI_F_HOTPLUG)
};

#define VHOST_SCSI_MAX_TARGET256
#define VHOST_SCSI_MAX_VQ128
   +#define VHOST_SCSI_MAX_EVENT 128

struct vhost_scsi {
 /* Protected by vhost_scsi-dev.mutex */
   @@ -82,6 +84,12 @@ struct vhost_scsi {

 struct vhost_work vs_completion_work; /* cmd completion work item */
 struct llist_head vs_completion_list; /* cmd completion queue */
   +
   + struct vhost_work vs_event_work; /* evt injection work item */
   + struct llist_head vs_event_list; /* evt injection queue */
   +
   + bool vs_events_missed; /* any missed events, protected by vq-mutex */
   + int vs_events_nr; /* num of pending events, protected by vq-mutex */
};

/* Local pointer to allocated TCM configfs fabric module */
   @@ -361,6 +369,32 @@ static int tcm_vhost_queue_tm_rsp(struct se_cmd 
   *se_cmd)
 return 0;
}

   +static void tcm_vhost_free_evt(struct vhost_scsi *vs, struct 
   tcm_vhost_evt *evt)
   +{
   + vs-vs_events_nr--;
   + kfree(evt);
   +}
   +
   +static struct tcm_vhost_evt *tcm_vhost_allocate_evt(struct vhost_scsi 
   *vs,
   + u32 event, u32 reason)
   +{
   + struct tcm_vhost_evt *evt;
   +
   + if (vs-vs_events_nr  VHOST_SCSI_MAX_EVENT) {
   + vs-vs_events_missed = true;
   + return NULL;
   + }
   +
   + evt = kmalloc(sizeof(*evt), GFP_KERNEL);
   + if (evt) {
  
  !evt should trigger vq_err I think.
  Please do
  if (!evt) {
  vq_err
  return NULL
  }
  
  this way code below has less nesting.
 
 okay.
 
   + evt-event.event = event;
   + evt-event.reason = reason;
   + vs-vs_events_nr++;
   + }
   +
   + return evt;
   +}
   +
static void vhost_scsi_free_cmd(struct tcm_vhost_cmd *tv_cmd)
{
 struct se_cmd *se_cmd = tv_cmd-tvc_se_cmd;
   @@ -379,6 +413,74 @@ static void vhost_scsi_free_cmd(struct tcm_vhost_cmd 
   *tv_cmd)
 kfree(tv_cmd);
}

   +static void tcm_vhost_do_evt_work(struct vhost_scsi *vs,
   + struct tcm_vhost_evt *evt)
   +{
   + struct vhost_virtqueue *vq = vs-vqs[VHOST_SCSI_VQ_EVT];
   + struct virtio_scsi_event *event = evt-event;
   + struct virtio_scsi_event __user *eventp;
   + unsigned out, in;
   + int head, ret;
   +
   +again:
   + vhost_disable_notify(vs-dev, vq);
   + head = vhost_get_vq_desc(vs-dev, vq, vq-iov,
   + ARRAY_SIZE(vq-iov), out, in,
   + NULL, NULL);
   + if (head  0) {
   + vs-vs_events_missed = true;
   + return;
   + }
   + if (head == vq-num) {
   + if (vhost_enable_notify(vs-dev, vq))
   + goto again;
   + vs-vs_events_missed = true;
   + return;
   + }
   +
   + if ((vq-iov[out].iov_len != sizeof(struct virtio_scsi_event))) {
   + 

[PATCH unit test] add test for mov[zs]x %ah, ... instruction

2013-04-24 Thread Gleb Natapov

Signed-off-by: Gleb Natapov g...@redhat.com
diff --git a/x86/realmode.c b/x86/realmode.c
index f5426e0..91c93a9 100644
--- a/x86/realmode.c
+++ b/x86/realmode.c
@@ -1358,12 +1358,19 @@ static void test_movzx_movsx(void)
 {
 MK_INSN(movsx, movsx %al, %ebx);
 MK_INSN(movzx, movzx %al, %ebx);
+MK_INSN(movzsah, movsx %ah, %ebx);
+MK_INSN(movzxah, movzx %ah, %ebx);
 
 inregs.eax = 0x1234569c;
+inregs.esp = 0x;
 exec_in_big_real_mode(insn_movsx);
 report(movsx, R_BX, outregs.ebx == (signed char)inregs.eax);
 exec_in_big_real_mode(insn_movzx);
 report(movzx, R_BX, outregs.ebx == (unsigned char)inregs.eax);
+exec_in_big_real_mode(insn_movzsah);
+report(movsx ah, R_BX, outregs.ebx == (signed char)(inregs.eax8));
+exec_in_big_real_mode(insn_movzxah);
+report(movzx ah, R_BX, outregs.ebx == (unsigned char)(inregs.eax  8));
 }
 
 static void test_bswap(void)
--
Gleb.
--
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 v8 2/3] tcm_vhost: Add hotplug/hotunplug support

2013-04-24 Thread Michael S. Tsirkin
On Wed, Apr 24, 2013 at 06:06:25PM +0800, Asias He wrote:
   +{
   +
   + struct vhost_scsi *vs = tpg-vhost_scsi;
   + struct vhost_virtqueue *vq;
   + u32 reason;
   + int ret;
   +
   + mutex_lock(tpg-tv_tpg_mutex);
   + vs = tpg-vhost_scsi;
   + mutex_unlock(tpg-tv_tpg_mutex);
   + if (!vs)
   + return -ENODEV;
   +
  
  What if clear_endpoint and close happen at this
  point? Can vhost_scsi go away so tcm_vhost_check_feature
  references freed memory?
 
 There is no easy way to handle this.

Take a reference somewhere so this can't happen?

--
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 v3 08/32] arm64: KVM: architecture specific MMU backend

2013-04-24 Thread Marc Zyngier
On 23/04/13 23:58, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:10PM +0100, Marc Zyngier wrote:
 Define the arm64 specific MMU backend:
 - HYP/kernel VA offset
 - S2 4/64kB definitions
 - S2 page table populating and flushing
 - icache cleaning

 Reviewed-by: Christopher Covington c...@codeaurora.org
 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm64/include/asm/kvm_mmu.h | 136 
 +++
  1 file changed, 136 insertions(+)
  create mode 100644 arch/arm64/include/asm/kvm_mmu.h

 diff --git a/arch/arm64/include/asm/kvm_mmu.h 
 b/arch/arm64/include/asm/kvm_mmu.h
 new file mode 100644
 index 000..2eb2230
 --- /dev/null
 +++ b/arch/arm64/include/asm/kvm_mmu.h
 @@ -0,0 +1,136 @@
 +/*
 + * Copyright (C) 2012,2013 - ARM Ltd
 + * Author: Marc Zyngier marc.zyng...@arm.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#ifndef __ARM64_KVM_MMU_H__
 +#define __ARM64_KVM_MMU_H__
 +
 +#include asm/page.h
 +#include asm/memory.h
 +
 +/*
 + * As we only have the TTBR0_EL2 register, we cannot express
 + * negative addresses. This makes it impossible to directly share
 + * mappings with the kernel.
 + *
 + * Instead, give the HYP mode its own VA region at a fixed offset from
 + * the kernel by just masking the top bits (which are all ones for a
 + * kernel address).
 
 For some reason I keep choking on this, despite it being very simple.
 We're just defining a different PAGE_OFFSET, right? Why not do a hard
 define as:
 
 #define HYP_PAGE_OFFSET_MASK  0x
 #define HYP_PAGE_OFFSET   0xffc0
 
 ...or change the second paragraph of the comment to say
 that we definethe HYP_PAGE_OFFSET to be 0x ffc0 .

One of these days, VA_BITS will change to accommodate for more virtual
space. When that day comes, I don't want to touch any of this because it
did hurt enough when writing it. As such, I'll refrain from hardcoding
anything.

I don't mind a comment, though.

 + */
 +#define HYP_PAGE_OFFSET_SHIFT   VA_BITS
 +#define HYP_PAGE_OFFSET_MASK((UL(1)  HYP_PAGE_OFFSET_SHIFT) - 1)
 
 In any case, is there a reason for the HYP_PAGE_OFFSET_SHIFT
 indirection? It may be simpler without...

It is common practice to have XXX_SHIFT and XXX_MASK together.

 +#define HYP_PAGE_OFFSET (PAGE_OFFSET  HYP_PAGE_OFFSET_MASK)
 +
 +/*
 + * Our virtual mapping for the idmap-ed MMU-enable code. Must be
 + * shared across all the page-tables. Conveniently, we use the last
 + * possible page, where no kernel mapping will ever exist.
 + */
 +#define TRAMPOLINE_VA   (HYP_PAGE_OFFSET_MASK  PAGE_MASK)
 
 hmmm, ok, here it's kind of nice to have that define correlation, so
 maybe it's not cleaner.  Something should be improved here in the define
 or the comment to make it more clear.  Perhaps just adding the real
 constants in the comment or in Documentation/arm64/memory.txt would
 help.

Yes, I plan to write something there.

 +
 +#ifdef __ASSEMBLY__
 +
 +/*
 + * Convert a kernel VA into a HYP VA.
 + * reg: VA to be converted.
 + */
 +.macro kern_hyp_va  reg
 +and \reg, \reg, #HYP_PAGE_OFFSET_MASK
 +.endm
 +
 +#else
 +
 +#include asm/cacheflush.h
 +
 +#define KERN_TO_HYP(kva)((unsigned long)kva - PAGE_OFFSET + 
 HYP_PAGE_OFFSET)
 +
 +/*
 + * Align KVM with the kernel's view of physical memory. Should be
 + * 40bit IPA, with PGD being 8kB aligned.
 + */
 +#define KVM_PHYS_SHIFT  PHYS_MASK_SHIFT
 +#define KVM_PHYS_SIZE   (1UL  KVM_PHYS_SHIFT)
 +#define KVM_PHYS_MASK   (KVM_PHYS_SIZE - 1UL)
 +
 +#ifdef CONFIG_ARM64_64K_PAGES
 +#define PAGE_LEVELS 2
 +#define BITS_PER_LEVEL  13
 +#else  /* 4kB pages */
 +#define PAGE_LEVELS 3
 +#define BITS_PER_LEVEL  9
 +#endif
 
 What are the semantics of these defines exactly? They should be
 S2_PAGE_LEVELS and make some assumptions of the VTCR_EL2.SL0 field
 right?

Indeed, we assume SL0 is always 1, just like for the kernel. As for the
semantics, I though they were pretty obvious...

PAGE_LEVELS is just that, the number of page levels. BITS_PER_LEVEL is
the number of bits you need to index a particular level.

 +
 +/* Make sure we get the right size, and thus the right alignment */
 
 this comment is not particularly helpful, something explaining why the
 max() thingy below makes sense would be more so :)  I really can't
 follow the BITS_PER_S2_PGD and PTRS_PER_S2_PGD defines.

Admittedly, 

Re: [PATCH v3 08/32] arm64: KVM: architecture specific MMU backend

2013-04-24 Thread Will Deacon
On Wed, Apr 24, 2013 at 12:03:10PM +0100, Marc Zyngier wrote:
 On 23/04/13 23:58, Christoffer Dall wrote:
  I noticed that this doesn't do any cache cleaning.  Are the MMU page
  table walks guaranteed to be coherent with the MMU on arm64?
 
 I suppose you meant the cache. In this case, yes. The hardware page
 table walker must snoop the caches.

Also, for ARMv7, SMP implies that the hardware walker snoops the cache. I
recently upstreamed some patches for this (see ARM: 7691/1: mm: kill unused
TLB_CAN_READ_FROM_L1_CACHE and use ALT_SMP instead in -next), so you might
want to check if there are any remaining, redundant flushes in kvm for ARMv7.

Will
--
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 v3 15/32] arm64: KVM: guest one-reg interface

2013-04-24 Thread Marc Zyngier
On 23/04/13 23:59, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:17PM +0100, Marc Zyngier wrote:
 Let userspace play with the guest registers.

 Reviewed-by: Christopher Covington c...@codeaurora.org
 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm64/kvm/guest.c | 254 
 +
  1 file changed, 254 insertions(+)
  create mode 100644 arch/arm64/kvm/guest.c

 diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
 new file mode 100644
 index 000..47d3729
 --- /dev/null
 +++ b/arch/arm64/kvm/guest.c
 @@ -0,0 +1,254 @@
 +/*
 + * Copyright (C) 2012,2013 - ARM Ltd
 + * Author: Marc Zyngier marc.zyng...@arm.com
 + *
 + * Derived from arch/arm/kvm/guest.c:
 + * Copyright (C) 2012 - Virtual Open Systems and Columbia University
 + * Author: Christoffer Dall c.d...@virtualopensystems.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#include linux/errno.h
 +#include linux/err.h
 +#include linux/kvm_host.h
 +#include linux/module.h
 +#include linux/vmalloc.h
 +#include linux/fs.h
 +#include asm/cputype.h
 +#include asm/uaccess.h
 +#include asm/kvm.h
 +#include asm/kvm_asm.h
 +#include asm/kvm_emulate.h
 +#include asm/kvm_coproc.h
 +
 +struct kvm_stats_debugfs_item debugfs_entries[] = {
 +{ NULL }
 +};
 +
 +int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
 +{
 +vcpu-arch.hcr_el2 = HCR_GUEST_FLAGS;
 +return 0;
 +}
 +
 +static u64 core_reg_offset_from_id(u64 id)
 +{
 +return id  ~(KVM_REG_ARCH_MASK | KVM_REG_SIZE_MASK | KVM_REG_ARM_CORE);
 +}
 +
 +static int get_core_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg 
 *reg)
 +{
 +__u32 __user *uaddr = (__u32 __user *)(unsigned long)reg-addr;
 +struct kvm_regs *regs = vcpu_gp_regs(vcpu);
 +int nr_regs = sizeof(*regs) / sizeof(__u32);
 
 eh, arent' your regs 64 bit?  Or are you 32-bit indexing into a 64-bit
 structure?  If so, this needs to be described in a big fat comment
 somewhere, which I couldn't even see looking forward in the patch series
 for the documentation part.

As you noticed below, we have a mix of 32/64/128bit fields there. The
index is indeed on 32bit boundary.

 Seems to me you'd want to remove the fp_regs from the core regs and just
 use a 64-bit indexing and have a separate category for the fp stuff...

Hell no! ;-)

FP is mandatory on arm64, and I'm not going down the road of having
separate structures for that. 32bit has historical baggage to deal with,
but not arm64.

This is the register set, and if the ONE_REG API is too cumbersome to
deal with it, then lets change ONE_REG instead (yes, I can run faster
than you think... ;-).

 +u32 off;
 +
 +/* Our ID is an index into the kvm_regs struct. */
 +off = core_reg_offset_from_id(reg-id);
 +if (off = nr_regs ||
 +(off + (KVM_REG_SIZE(reg-id) / sizeof(__u32))) = nr_regs)
 
 According to your documentation you will always save/restore 32-bit
 registers here, so the desijunction shouldn't be necessary, nor will it
 be if you just base everything on 64-bit here.

No. Your *offset* is a 32bit index. The size can be anything, and you
want to make sure you don't read/write past the kvm_regs structure.

 +return -ENOENT;
 +
 +if (copy_to_user(uaddr, ((u32 *)regs) + off, KVM_REG_SIZE(reg-id)))
 +return -EFAULT;
 +
 +return 0;
 +}
 +
 +static int set_core_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg 
 *reg)
 +{
 +__u32 __user *uaddr = (__u32 __user *)(unsigned long)reg-addr;
 +struct kvm_regs *regs = vcpu_gp_regs(vcpu);
 +int nr_regs = sizeof(*regs) / sizeof(__u32);
 
 same concern here

Same answer.

 
 +void *valp;
 +u64 off;
 +int err = 0;
 +
 +/* Our ID is an index into the kvm_regs struct. */
 +off = core_reg_offset_from_id(reg-id);
 +if (off = nr_regs ||
 +(off + (KVM_REG_SIZE(reg-id) / sizeof(__u32))) = nr_regs)
 +return -ENOENT;
 +
 +valp = kmalloc(KVM_REG_SIZE(reg-id), GFP_KERNEL);
 +if (!valp)
 +return -ENOMEM;
 
 Why are you dynamically allocating this?  Do you ever have anything
 larger than a 64 bit core register? Just put that on the stack.

Look at what a ONE_REG access can be: up to 1kB. I'm not allocating that
on the stack.

 +
 +if (copy_from_user(valp, uaddr, KVM_REG_SIZE(reg-id))) {
 +err = -EFAULT;
 +goto out;
 +}
 +
 +if (off == 

Re: [PATCH v3 17/32] arm64: KVM: HYP mode world switch implementation

2013-04-24 Thread Marc Zyngier
On 23/04/13 23:59, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:19PM +0100, Marc Zyngier wrote:
 The HYP mode world switch in all its glory.

 Implements save/restore of host/guest registers, EL2 trapping,
 IPA resolution, and additional services (tlb invalidation).

 Reviewed-by: Christopher Covington c...@codeaurora.org
 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm64/kernel/asm-offsets.c |  34 +++
  arch/arm64/kvm/hyp.S| 602 
 
  2 files changed, 636 insertions(+)
  create mode 100644 arch/arm64/kvm/hyp.S


[...]

 diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
 new file mode 100644
 index 000..c745d20
 --- /dev/null
 +++ b/arch/arm64/kvm/hyp.S
 @@ -0,0 +1,602 @@
 +/*
 + * Copyright (C) 2012,2013 - ARM Ltd
 + * Author: Marc Zyngier marc.zyng...@arm.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#include linux/linkage.h
 +#include linux/irqchip/arm-gic.h
 +
 +#include asm/assembler.h
 +#include asm/memory.h
 +#include asm/asm-offsets.h
 +#include asm/fpsimdmacros.h
 +#include asm/kvm.h
 +#include asm/kvm_asm.h
 +#include asm/kvm_arm.h
 +#include asm/kvm_mmu.h
 +
 +#define CPU_GP_REG_OFFSET(x) (CPU_GP_REGS + x)
 +#define CPU_XREG_OFFSET(x)   CPU_GP_REG_OFFSET(CPU_USER_PT_REGS + 8*x)
 +#define CPU_SPSR_OFFSET(x)   CPU_GP_REG_OFFSET(CPU_SPSR + 8*x)
 +#define CPU_SYSREG_OFFSET(x) (CPU_SYSREGS + 8*x)
 +
 + .text
 + .pushsection.hyp.text, ax
 + .align  PAGE_SHIFT
 +
 +__kvm_hyp_code_start:
 + .globl __kvm_hyp_code_start
 +
 +.macro save_common_regs
 + // x2: base address for cpu context
 + // x3: tmp register
 
 what's with the C99 style comments? Standard for arm64 assembly?

Yes. The toolchain guys got rid of '@' as a single line comment delimiter.

[...]

 +el1_sync:// Guest trapped into EL2
 + pushx0, x1
 + pushx2, x3
 +
 + mrs x1, esr_el2
 + lsr x2, x1, #ESR_EL2_EC_SHIFT
 +
 + cmp x2, #ESR_EL2_EC_HVC64
 + b.neel1_trap
 +
 + mrs x3, vttbr_el2   // If vttbr is valid, the 
 64bit guest
 + cbnzx3, el1_trap// called HVC
 +
 + /* Here, we're pretty sure the host called HVC. */
 + pop x2, x3
 + pop x0, x1
 +
 + pushlr, xzr
 +
 + /*
 +  * Compute the function address in EL2, and shuffle the parameters.
 +  */
 + kern_hyp_va x0
 + mov lr, x0
 + mov x0, x1
 + mov x1, x2
 + mov x2, x3
 + blr lr
 +
 + pop lr, xzr
 + eret
 +
 +el1_trap:
 + /*
 +  * x1: ESR
 +  * x2: ESR_EC
 +  */
 + cmp x2, #ESR_EL2_EC_DABT
 + mov x0, #ESR_EL2_EC_IABT
 + ccmpx2, x0, #4, ne
 + b.ne1f  // Not an abort we care about
 
 why do we get the hpfar_el2 if it's not an abort (or is this for a
 special type of abort) ?

No, we could actually avoid saving HPFAR_EL2 altogether in this case.

 +
 + /* This is an abort. Check for permission fault */
 + and x2, x1, #ESR_EL2_FSC_TYPE
 + cmp x2, #FSC_PERM
 + b.ne1f  // Not a permission fault
 +
 + /*
 +  * Check for Stage-1 page table walk, which is guaranteed
 +  * to give a valid HPFAR_EL2.
 +  */
 + tbnzx1, #7, 1f  // S1PTW is set
 +
 + /*
 +  * Permission fault, HPFAR_EL2 is invalid.
 +  * Resolve the IPA the hard way using the guest VA.
 +  * We always perform an EL1 lookup, as we already
 +  * went through Stage-1.
 +  */
 
 What does the last sentence mean exactly?

It means that the Stage-1 translation already validated the memory
access rights. As such, we can use the EL1 translation regime, and don't
have to distinguish between EL0 and EL1 access.

 + mrs x3, far_el2
 + at  s1e1r, x3
 + isb
 +
 + /* Read result */
 + mrs x3, par_el1
 + tbnzx3, #1, 3f  // Bail out if we failed the 
 translation
 + ubfxx3, x3, #12, #36// Extract IPA
 + lsl x3, x3, #4  // and present it like HPFAR
 + b   2f
 +
 +1:   mrs x3, hpfar_el2
 +
 +2:   mrs x0, tpidr_el2
 + mrs x2, far_el2
 + str x1, [x0, #VCPU_ESR_EL2]
 + str x2, [x0, #VCPU_FAR_EL2]
 + str x3, [x0, #VCPU_HPFAR_EL2]
 +
 + mov x1, #ARM_EXCEPTION_TRAP

Re: [PATCH v3 19/32] arm64: KVM: Plug the VGIC

2013-04-24 Thread Marc Zyngier
On 24/04/13 00:00, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:21PM +0100, Marc Zyngier wrote:
 Add support for the in-kernel GIC emulation. The include file
 is a complete duplicate of the 32bit one - something to fix
 at one point.
 
 seems like something that should be fixed sooner as opposed to later...
 Is it hard?

Hard? No. Disgusting? Yes.

You need to find a common location for the files. Something like
virt/kvm/arm/vgic.h? The x86/ia64 guys already have virt/kvm/ioapic.h as
a (bad) precedent.

While your at it, you could put a lot of the common code there as well.

M.
-- 
Jazz is not dead. It just smells funny...

--
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 v3 20/32] arm64: KVM: Plug the arch timer

2013-04-24 Thread Marc Zyngier
On 24/04/13 00:00, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:22PM +0100, Marc Zyngier wrote:
 Add support for the in-kernel timer emulation. The include file
 is a complete duplicate of the 32bit one - something to fix
 at one point.
 
 again, I'd really like to see this fixed before we merge the code...

Feel free to suggest a solution. None of the one I tried are nice.

M.
-- 
Jazz is not dead. It just smells funny...

--
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 v2 6/6] KVM: MMU: init kvm generation close to mmio wrap-around value

2013-04-24 Thread Gleb Natapov
On Mon, Apr 01, 2013 at 05:56:49PM +0800, Xiao Guangrong wrote:
 Then it has chance to trigger mmio generation number wrap-around
 
 Signed-off-by: Xiao Guangrong xiaoguangr...@linux.vnet.ibm.com
 ---
  arch/x86/include/asm/kvm_host.h |1 +
  arch/x86/kvm/mmu.c  |8 
  virt/kvm/kvm_main.c |6 ++
  3 files changed, 15 insertions(+), 0 deletions(-)
 
 diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
 index 6c1e642..4e1f7cb 100644
 --- a/arch/x86/include/asm/kvm_host.h
 +++ b/arch/x86/include/asm/kvm_host.h
 @@ -767,6 +767,7 @@ void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,
struct kvm_memory_slot *slot,
gfn_t gfn_offset, unsigned long mask);
  void kvm_mmu_zap_all(struct kvm *kvm);
 +void kvm_arch_init_generation(struct kvm *kvm);
  void kvm_mmu_invalid_mmio_sptes(struct kvm *kvm);
  unsigned int kvm_mmu_calculate_mmu_pages(struct kvm *kvm);
  void kvm_mmu_change_mmu_pages(struct kvm *kvm, unsigned int 
 kvm_nr_mmu_pages);
 diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
 index d314e21..dcc059c 100644
 --- a/arch/x86/kvm/mmu.c
 +++ b/arch/x86/kvm/mmu.c
 @@ -4279,6 +4279,14 @@ restart:
   spin_unlock(kvm-mmu_lock);
  }
  
 +void kvm_arch_init_generation(struct kvm *kvm)
 +{
 + mutex_lock(kvm-slots_lock);
 + /* It is easier to trigger mmio generation-number wrap-around. */
 + kvm_memslots(kvm)-generation = MMIO_MAX_GEN - 13;
kvm_memslots(kvm)-generation should never overflow since
(read|write)_cached mechanism does not handle it. Initialising it to
anything but 0 makes overflow more likely.

You can hide mmio overflow trick in kvm_current_mmio_generation():

static unsigned int kvm_current_mmio_generation(struct kvm *kvm)
{
return (kvm_memslots(kvm)-generation + MMIO_MAX_GEN - 13)  
MMIO_GEN_MASK;
}

--
Gleb.
--
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 v3 24/32] arm64: KVM: 32bit GP register access

2013-04-24 Thread Marc Zyngier
On 24/04/13 00:00, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:26PM +0100, Marc Zyngier wrote:
 Allow access to the 32bit register file through the usual API.

 Reviewed-by: Christopher Covington c...@codeaurora.org
 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm64/include/asm/kvm_emulate.h |  17 +++-
  arch/arm64/kvm/Makefile  |   2 +-
  arch/arm64/kvm/regmap.c  | 168 
 +++
  3 files changed, 184 insertions(+), 3 deletions(-)
  create mode 100644 arch/arm64/kvm/regmap.c

 diff --git a/arch/arm64/include/asm/kvm_emulate.h 
 b/arch/arm64/include/asm/kvm_emulate.h
 index 2dcfa74..37a6567 100644
 --- a/arch/arm64/include/asm/kvm_emulate.h
 +++ b/arch/arm64/include/asm/kvm_emulate.h
 @@ -28,6 +28,9 @@
  #include asm/kvm_mmio.h
  #include asm/ptrace.h
  
 +unsigned long *vcpu_reg32(const struct kvm_vcpu *vcpu, u8 reg_num);
 +unsigned long *vcpu_spsr32(const struct kvm_vcpu *vcpu);
 +
  void kvm_inject_undefined(struct kvm_vcpu *vcpu);
  void kvm_inject_dabt(struct kvm_vcpu *vcpu, unsigned long addr);
  void kvm_inject_pabt(struct kvm_vcpu *vcpu, unsigned long addr);
 @@ -49,7 +52,7 @@ static inline unsigned long *vcpu_cpsr(const struct 
 kvm_vcpu *vcpu)
  
  static inline bool vcpu_mode_is_32bit(const struct kvm_vcpu *vcpu)
  {
 -return false;   /* 32bit? Bahhh... */
 +return !!(*vcpu_cpsr(vcpu)  PSR_MODE32_BIT);
 
 nit: you don't need the '!!': it's a bool

No it is not. It is a bitwise and, turned into a bool.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...

--
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 v3 25/32] arm64: KVM: 32bit conditional execution emulation

2013-04-24 Thread Marc Zyngier
On 24/04/13 00:00, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:27PM +0100, Marc Zyngier wrote:
 As conditional instructions can trap on AArch32, add the thinest
 possible emulation layer to keep 32bit guests happy.

 Reviewed-by: Christopher Covington c...@codeaurora.org
 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm64/include/asm/kvm_emulate.h |  13 ++-
  arch/arm64/kvm/Makefile  |   2 +-
  arch/arm64/kvm/emulate.c | 154 
 +++
  3 files changed, 166 insertions(+), 3 deletions(-)
  create mode 100644 arch/arm64/kvm/emulate.c

 diff --git a/arch/arm64/include/asm/kvm_emulate.h 
 b/arch/arm64/include/asm/kvm_emulate.h
 index 37a6567..8d4ab33 100644
 --- a/arch/arm64/include/asm/kvm_emulate.h
 +++ b/arch/arm64/include/asm/kvm_emulate.h
 @@ -31,6 +31,9 @@
  unsigned long *vcpu_reg32(const struct kvm_vcpu *vcpu, u8 reg_num);
  unsigned long *vcpu_spsr32(const struct kvm_vcpu *vcpu);
  
 +bool kvm_condition_valid32(const struct kvm_vcpu *vcpu);
 +void kvm_skip_instr32(struct kvm_vcpu *vcpu, bool is_wide_instr);
 +
  void kvm_inject_undefined(struct kvm_vcpu *vcpu);
  void kvm_inject_dabt(struct kvm_vcpu *vcpu, unsigned long addr);
  void kvm_inject_pabt(struct kvm_vcpu *vcpu, unsigned long addr);
 @@ -57,12 +60,18 @@ static inline bool vcpu_mode_is_32bit(const struct 
 kvm_vcpu *vcpu)
  
  static inline bool kvm_condition_valid(const struct kvm_vcpu *vcpu)
  {
 -return true;/* No conditionals on arm64 */
 +if (vcpu_mode_is_32bit(vcpu))
 +return kvm_condition_valid32(vcpu);
 +
 +return true;
  }
  
  static inline void kvm_skip_instr(struct kvm_vcpu *vcpu, bool is_wide_instr)
  {
 -*vcpu_pc(vcpu) += 4;
 +if (vcpu_mode_is_32bit(vcpu))
 +kvm_skip_instr32(vcpu, is_wide_instr);
 +else
 +*vcpu_pc(vcpu) += 4;
  }
  
  static inline void vcpu_set_thumb(struct kvm_vcpu *vcpu)
 diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
 index 1668448..88c6639 100644
 --- a/arch/arm64/kvm/Makefile
 +++ b/arch/arm64/kvm/Makefile
 @@ -11,7 +11,7 @@ obj-$(CONFIG_KVM_ARM_HOST) += kvm.o
  kvm-$(CONFIG_KVM_ARM_HOST) += $(addprefix ../../../virt/kvm/, kvm_main.o 
 coalesced_mmio.o)
  kvm-$(CONFIG_KVM_ARM_HOST) += $(addprefix ../../../arch/arm/kvm/, arm.o 
 mmu.o mmio.o psci.o perf.o)
  
 -kvm-$(CONFIG_KVM_ARM_HOST) += inject_fault.o regmap.o
 +kvm-$(CONFIG_KVM_ARM_HOST) += emulate.o inject_fault.o regmap.o
  kvm-$(CONFIG_KVM_ARM_HOST) += hyp.o hyp-init.o handle_exit.o
  kvm-$(CONFIG_KVM_ARM_HOST) += guest.o reset.o sys_regs.o 
 sys_regs_generic_v8.o
  
 diff --git a/arch/arm64/kvm/emulate.c b/arch/arm64/kvm/emulate.c
 new file mode 100644
 index 000..01d4713
 --- /dev/null
 +++ b/arch/arm64/kvm/emulate.c
 @@ -0,0 +1,154 @@
 +/*
 + * (not much of an) Emulation layer for 32bit guests.
 + *
 + * Copyright (C) 2012 - Virtual Open Systems and Columbia University
 + * Author: Christoffer Dall c.d...@virtualopensystems.com
 
 don't you want to add yourself here?

There's really nothing I actually wrote here. Most of it is your code,
and the rest comes from other parts of arch/arm.

I guess I could always mention myself for the ESR_EL2_* macros... ;-)

 + *
 + * This program is free software: you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#include linux/kvm_host.h
 +#include asm/kvm_emulate.h
 +
 +/*
 + * stolen from arch/arm/kernel/opcodes.c
 + *
 + * condition code lookup table
 + * index into the table is test code: EQ, NE, ... LT, GT, AL, NV
 + *
 + * bit position in short is condition code: NZCV
 + */
 +static const unsigned short cc_map[16] = {
 +0xF0F0, /* EQ == Z set*/
 +0x0F0F, /* NE */
 +0x, /* CS == C set*/
 +0x, /* CC */
 +0xFF00, /* MI == N set*/
 +0x00FF, /* PL */
 +0x, /* VS == V set*/
 +0x, /* VC */
 +0x0C0C, /* HI == C set  Z clear */
 +0xF3F3, /* LS == C clear || Z set */
 +0xAA55, /* GE == (N==V)   */
 +0x55AA, /* LT == (N!=V)   */
 +0x0A05, /* GT == (!Z  (N==V))   */
 +0xF5FA, /* LE == (Z || (N!=V))*/
 +

Re: [PATCH v2 3/6] KVM: MMU: make return value of mmio page fault handler more readable

2013-04-24 Thread Gleb Natapov
On Mon, Apr 01, 2013 at 05:56:46PM +0800, Xiao Guangrong wrote:
 Define some meaningful names instead of raw code
 
 Signed-off-by: Xiao Guangrong xiaoguangr...@linux.vnet.ibm.com
 ---
  arch/x86/kvm/mmu.c |   15 +--
  arch/x86/kvm/mmu.h |   14 ++
  arch/x86/kvm/vmx.c |4 ++--
  3 files changed, 21 insertions(+), 12 deletions(-)
 
 diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
 index be4f733..31c5586 100644
 --- a/arch/x86/kvm/mmu.c
 +++ b/arch/x86/kvm/mmu.c
 @@ -3182,17 +3182,12 @@ static u64 walk_shadow_page_get_mmio_spte(struct 
 kvm_vcpu *vcpu, u64 addr)
   return spte;
  }
  
 -/*
 - * If it is a real mmio page fault, return 1 and emulat the instruction
 - * directly, return 0 to let CPU fault again on the address, -1 is
 - * returned if bug is detected.
 - */
  int handle_mmio_page_fault_common(struct kvm_vcpu *vcpu, u64 addr, bool 
 direct)
  {
   u64 spte;
  
   if (quickly_check_mmio_pf(vcpu, addr, direct))
 - return 1;
 + return RET_MMIO_PF_EMU;
  
   spte = walk_shadow_page_get_mmio_spte(vcpu, addr);
  
 @@ -3205,7 +3200,7 @@ int handle_mmio_page_fault_common(struct kvm_vcpu 
 *vcpu, u64 addr, bool direct)
  
   trace_handle_mmio_page_fault(addr, gfn, access);
   vcpu_cache_mmio_info(vcpu, addr, gfn, access);
 - return 1;
 + return RET_MMIO_PF_EMU;
   }
  
   /*
 @@ -3213,13 +3208,13 @@ int handle_mmio_page_fault_common(struct kvm_vcpu 
 *vcpu, u64 addr, bool direct)
* it's a BUG if the gfn is not a mmio page.
*/
   if (direct  !check_direct_spte_mmio_pf(spte))
 - return -1;
 + return RET_MMIO_PF_BUG;
  
   /*
* If the page table is zapped by other cpus, let CPU fault again on
* the address.
*/
 - return 0;
 + return RET_MMIO_PF_RETRY;
  }
  EXPORT_SYMBOL_GPL(handle_mmio_page_fault_common);
  
 @@ -3229,7 +3224,7 @@ static int handle_mmio_page_fault(struct kvm_vcpu 
 *vcpu, u64 addr,
   int ret;
  
   ret = handle_mmio_page_fault_common(vcpu, addr, direct);
 - WARN_ON(ret  0);
 + WARN_ON(ret == RET_MMIO_PF_BUG);
   return ret;
  }
  
 diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
 index 2adcbc2..6b4ba1e 100644
 --- a/arch/x86/kvm/mmu.h
 +++ b/arch/x86/kvm/mmu.h
 @@ -52,6 +52,20 @@
  
  int kvm_mmu_get_spte_hierarchy(struct kvm_vcpu *vcpu, u64 addr, u64 
 sptes[4]);
  void kvm_mmu_set_mmio_spte_mask(u64 mmio_mask);
 +
 +/*
 + * Return values of handle_mmio_page_fault_common:
 + * RET_MMIO_PF_EMU: it is a real mmio page fault, emulate the instruction
 + *   directly.
 + * RET_MMIO_PF_RETRY: let CPU fault again on the address.
 + * RET_MMIO_PF_BUG: bug is detected.
 + */
 +enum {
 + RET_MMIO_PF_EMU = 1,
Make it RET_MMIO_PF_EMULATE please.

 + RET_MMIO_PF_RETRY = 0,
 + RET_MMIO_PF_BUG = -1
 +};
 +
  int handle_mmio_page_fault_common(struct kvm_vcpu *vcpu, u64 addr, bool 
 direct);
  int kvm_init_shadow_mmu(struct kvm_vcpu *vcpu, struct kvm_mmu *context);
  
 diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
 index 915ef56..d0f2790 100644
 --- a/arch/x86/kvm/vmx.c
 +++ b/arch/x86/kvm/vmx.c
 @@ -5135,10 +5135,10 @@ static int handle_ept_misconfig(struct kvm_vcpu *vcpu)
   gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS);
  
   ret = handle_mmio_page_fault_common(vcpu, gpa, true);
 - if (likely(ret == 1))
 + if (likely(ret == RET_MMIO_PF_EMU))
   return x86_emulate_instruction(vcpu, gpa, 0, NULL, 0) ==
 EMULATE_DONE;
 - if (unlikely(!ret))
 + if (unlikely(ret == RET_MMIO_PF_RETRY))
   return 1;
  
   /* It is the real ept misconfig */
 -- 
 1.7.7.6

--
Gleb.
--
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 v3 10/32] arm64: KVM: system register handling

2013-04-24 Thread Marc Zyngier
On 24/04/13 00:01, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:12PM +0100, Marc Zyngier wrote:
 Provide 64bit system register handling, modeled after the cp15
 handling for ARM.

 Reviewed-by: Christopher Covington c...@codeaurora.org
 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm64/include/asm/kvm_coproc.h |  51 +++
  arch/arm64/include/uapi/asm/kvm.h   |  29 ++
  arch/arm64/kvm/sys_regs.c   | 871 
 
  arch/arm64/kvm/sys_regs.h   | 138 ++
  include/uapi/linux/kvm.h|   1 +
  5 files changed, 1090 insertions(+)
  create mode 100644 arch/arm64/include/asm/kvm_coproc.h
  create mode 100644 arch/arm64/kvm/sys_regs.c
  create mode 100644 arch/arm64/kvm/sys_regs.h

 diff --git a/arch/arm64/include/asm/kvm_coproc.h 
 b/arch/arm64/include/asm/kvm_coproc.h
 new file mode 100644
 index 000..9b4477a
 --- /dev/null
 +++ b/arch/arm64/include/asm/kvm_coproc.h
 @@ -0,0 +1,51 @@
 +/*
 + * Copyright (C) 2012,2013 - ARM Ltd
 + * Author: Marc Zyngier marc.zyng...@arm.com
 + *
 + * Derived from arch/arm/include/asm/kvm_coproc.h
 + * Copyright (C) 2012 Rusty Russell IBM Corporation
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#ifndef __ARM64_KVM_COPROC_H__
 +#define __ARM64_KVM_COPROC_H__
 +
 +#include linux/kvm_host.h
 +
 +void kvm_reset_sys_regs(struct kvm_vcpu *vcpu);
 +
 +struct kvm_sys_reg_table {
 + const struct sys_reg_desc *table;
 + size_t num;
 +};
 +
 +struct kvm_sys_reg_target_table {
 + struct kvm_sys_reg_table table64;
 +};
 +
 +void kvm_register_target_sys_reg_table(unsigned int target,
 +   struct kvm_sys_reg_target_table *table);
 +
 +int kvm_handle_sys_reg(struct kvm_vcpu *vcpu, struct kvm_run *run);
 +
 +#define kvm_coproc_table_init kvm_sys_reg_table_init
 +void kvm_sys_reg_table_init(void);
 +
 +struct kvm_one_reg;
 +int kvm_arm_copy_sys_reg_indices(struct kvm_vcpu *vcpu, u64 __user 
 *uindices);
 +int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg 
 *);
 +int kvm_arm_sys_reg_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg 
 *);
 +unsigned long kvm_arm_num_sys_reg_descs(struct kvm_vcpu *vcpu);
 +
 +#endif /* __ARM64_KVM_COPROC_H__ */
 diff --git a/arch/arm64/include/uapi/asm/kvm.h 
 b/arch/arm64/include/uapi/asm/kvm.h
 index 4e64570..ebac919 100644
 --- a/arch/arm64/include/uapi/asm/kvm.h
 +++ b/arch/arm64/include/uapi/asm/kvm.h
 @@ -92,6 +92,35 @@ struct kvm_sync_regs {
  struct kvm_arch_memory_slot {
  };

 +/* If you need to interpret the index values, here is the key: */
 +#define KVM_REG_ARM_COPROC_MASK 0x0FFF
 +#define KVM_REG_ARM_COPROC_SHIFT 16
 +
 +/* Normal registers are mapped as coprocessor 16. */
 +#define KVM_REG_ARM_CORE (0x0010  KVM_REG_ARM_COPROC_SHIFT)
 +#define KVM_REG_ARM_CORE_REG(name) (offsetof(struct kvm_regs, name) / 
 sizeof(__u32))
 +
 +/* Some registers need more space to represent values. */
 +#define KVM_REG_ARM_DEMUX (0x0011  KVM_REG_ARM_COPROC_SHIFT)
 +#define KVM_REG_ARM_DEMUX_ID_MASK 0xFF00
 +#define KVM_REG_ARM_DEMUX_ID_SHIFT 8
 +#define KVM_REG_ARM_DEMUX_ID_CCSIDR (0x00  KVM_REG_ARM_DEMUX_ID_SHIFT)
 +#define KVM_REG_ARM_DEMUX_VAL_MASK 0x00FF
 +#define KVM_REG_ARM_DEMUX_VAL_SHIFT 0
 +
 +/* AArch64 system registers */
 +#define KVM_REG_ARM64_SYSREG (0x0013  KVM_REG_ARM_COPROC_SHIFT)
 +#define KVM_REG_ARM64_SYSREG_OP0_MASK 0xc000
 +#define KVM_REG_ARM64_SYSREG_OP0_SHIFT 14
 +#define KVM_REG_ARM64_SYSREG_OP1_MASK 0x3800
 +#define KVM_REG_ARM64_SYSREG_OP1_SHIFT 11
 +#define KVM_REG_ARM64_SYSREG_CRN_MASK 0x0780
 +#define KVM_REG_ARM64_SYSREG_CRN_SHIFT 7
 +#define KVM_REG_ARM64_SYSREG_CRM_MASK 0x0078
 +#define KVM_REG_ARM64_SYSREG_CRM_SHIFT 3
 +#define KVM_REG_ARM64_SYSREG_OP2_MASK 0x0007
 +#define KVM_REG_ARM64_SYSREG_OP2_SHIFT 0
 +
  /* KVM_IRQ_LINE irq field index values */
  #define KVM_ARM_IRQ_TYPE_SHIFT 24
  #define KVM_ARM_IRQ_TYPE_MASK 0xff
 diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
 new file mode 100644
 index 000..9df3b32
 --- /dev/null
 +++ b/arch/arm64/kvm/sys_regs.c
 @@ -0,0 +1,871 @@
 +/*
 + * Copyright (C) 2012,2013 - ARM Ltd
 + * Author: Marc Zyngier marc.zyng...@arm.com
 + *
 + * Derived from arch/arm/kvm/coproc.c:
 + * Copyright (C) 2012 - Virtual Open Systems and Columbia University
 + * Authors: Rusty Russell ru...@rustcorp.com.au
 + *

Re: [PATCH v3 26/32] arm64: KVM: 32bit handling of coprocessor traps

2013-04-24 Thread Marc Zyngier
On 24/04/13 00:01, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:28PM +0100, Marc Zyngier wrote:
 Provide the necessary infrastructure to trap coprocessor accesses that
 occur when running 32bit guests.

 Also wire SMC and HVC trapped in 32bit mode while were at it.

 Reviewed-by: Christopher Covington c...@codeaurora.org
 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm64/include/asm/kvm_coproc.h |   5 +
  arch/arm64/kvm/handle_exit.c|   7 ++
  arch/arm64/kvm/sys_regs.c   | 178 
 ++--
  3 files changed, 183 insertions(+), 7 deletions(-)

 diff --git a/arch/arm64/include/asm/kvm_coproc.h 
 b/arch/arm64/include/asm/kvm_coproc.h
 index 9b4477a..9a59301 100644
 --- a/arch/arm64/include/asm/kvm_coproc.h
 +++ b/arch/arm64/include/asm/kvm_coproc.h
 @@ -32,11 +32,16 @@ struct kvm_sys_reg_table {

  struct kvm_sys_reg_target_table {
   struct kvm_sys_reg_table table64;
 + struct kvm_sys_reg_table table32;
  };

  void kvm_register_target_sys_reg_table(unsigned int target,
  struct kvm_sys_reg_target_table *table);

 +int kvm_handle_cp14_load_store(struct kvm_vcpu *vcpu, struct kvm_run *run);
 +int kvm_handle_cp14_access(struct kvm_vcpu *vcpu, struct kvm_run *run);
 +int kvm_handle_cp15_32(struct kvm_vcpu *vcpu, struct kvm_run *run);
 +int kvm_handle_cp15_64(struct kvm_vcpu *vcpu, struct kvm_run *run);
  int kvm_handle_sys_reg(struct kvm_vcpu *vcpu, struct kvm_run *run);

  #define kvm_coproc_table_init kvm_sys_reg_table_init
 diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
 index 4766b7f..9beaca03 100644
 --- a/arch/arm64/kvm/handle_exit.c
 +++ b/arch/arm64/kvm/handle_exit.c
 @@ -62,6 +62,13 @@ static int kvm_handle_wfi(struct kvm_vcpu *vcpu, struct 
 kvm_run *run)

  static exit_handle_fn arm_exit_handlers[] = {
   [ESR_EL2_EC_WFI]= kvm_handle_wfi,
 + [ESR_EL2_EC_CP15_32]= kvm_handle_cp15_32,
 + [ESR_EL2_EC_CP15_64]= kvm_handle_cp15_64,
 + [ESR_EL2_EC_CP14_MR]= kvm_handle_cp14_access,
 + [ESR_EL2_EC_CP14_LS]= kvm_handle_cp14_load_store,
 + [ESR_EL2_EC_CP14_64]= kvm_handle_cp14_access,
 + [ESR_EL2_EC_HVC32]  = handle_hvc,
 + [ESR_EL2_EC_SMC32]  = handle_smc,
   [ESR_EL2_EC_HVC64]  = handle_hvc,
   [ESR_EL2_EC_SMC64]  = handle_smc,
   [ESR_EL2_EC_SYS64]  = kvm_handle_sys_reg,
 diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
 index 9df3b32..0303218 100644
 --- a/arch/arm64/kvm/sys_regs.c
 +++ b/arch/arm64/kvm/sys_regs.c
 @@ -38,6 +38,10 @@
   * types are different. My gut feeling is that it should be pretty
   * easy to merge, but that would be an ABI breakage -- again. VFP
   * would also need to be abstracted.
 + *
 + * For AArch32, we only take care of what is being trapped. Anything
 + * that has to do with init and userspace access has to go via the
 + * 64bit interface.
   */

  /* 3 bits per cache level, as per CLIDR, but non-existent caches always 0 */
 @@ -163,6 +167,16 @@ static const struct sys_reg_desc sys_reg_descs[] = {
   { Op0(0b01), Op1(0b000), CRn(0b0111), CRm(0b1110), Op2(0b010),
 access_dcsw },

 + /* TEECR32_EL1 */
 + { Op0(0b10), Op1(0b010), CRn(0b), CRm(0b), Op2(0b000),
 +   NULL, reset_val, TEECR32_EL1, 0 },
 + /* TEEHBR32_EL1 */
 + { Op0(0b10), Op1(0b010), CRn(0b0001), CRm(0b), Op2(0b000),
 +   NULL, reset_val, TEEHBR32_EL1, 0 },
 + /* DBGVCR32_EL2 */
 + { Op0(0b10), Op1(0b100), CRn(0b), CRm(0b0111), Op2(0b000),
 +   NULL, reset_val, DBGVCR32_EL2, 0 },
 +
   /* MPIDR_EL1 */
   { Op0(0b11), Op1(0b000), CRn(0b), CRm(0b), Op2(0b101),
 NULL, reset_mpidr, MPIDR_EL1 },
 @@ -273,6 +287,39 @@ static const struct sys_reg_desc sys_reg_descs[] = {
   /* TPIDRRO_EL0 */
   { Op0(0b11), Op1(0b011), CRn(0b1101), CRm(0b), Op2(0b011),
 NULL, reset_unknown, TPIDRRO_EL0 },
 +
 + /* DACR32_EL2 */
 + { Op0(0b11), Op1(0b100), CRn(0b0011), CRm(0b), Op2(0b000),
 +   NULL, reset_unknown, DACR32_EL2 },
 + /* IFSR32_EL2 */
 + { Op0(0b11), Op1(0b100), CRn(0b0101), CRm(0b), Op2(0b001),
 +   NULL, reset_unknown, IFSR32_EL2 },
 + /* FPEXC32_EL2 */
 + { Op0(0b11), Op1(0b100), CRn(0b0101), CRm(0b0011), Op2(0b000),
 +   NULL, reset_val, FPEXC32_EL2, 0x70 },
 +};
 +
 +/* Trapped cp15 registers */
 +static const struct sys_reg_desc cp15_regs[] = {
 + /*
 +  * DC{C,I,CI}SW operations:
 +  */
 + { Op1( 0), CRn( 7), CRm( 6), Op2( 2), access_dcsw },
 + { Op1( 0), CRn( 7), CRm(10), Op2( 2), access_dcsw },
 + { Op1( 0), CRn( 7), CRm(14), Op2( 2), access_dcsw },
 + { Op1( 0), CRn( 9), CRm(12), Op2( 0), pm_fake },
 + { Op1( 0), CRn( 9), CRm(12), Op2( 1), pm_fake },
 + { Op1( 0), CRn( 9), CRm(12), Op2( 2), pm_fake },
 + { Op1( 0), CRn( 9), CRm(12), Op2( 3), pm_fake },
 + { Op1( 0), CRn( 9), 

Re: [PATCH v3 29/32] arm64: KVM: 32bit guest fault injection

2013-04-24 Thread Marc Zyngier
On 24/04/13 00:02, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:31PM +0100, Marc Zyngier wrote:
 Add fault injection capability for 32bit guests.
 
 looks ok, but seems we are duplicating some functionality here, but I
 realize it may not be that easy to share due to different define names
 for things like PSR_X_BIT...  Did you look further into this?

I did, but as you noticed, the namespaces are different. We could
probably define a set of KVM_AARCH32_* symbols, but I'm not sure it
would be any nicer though.

M.
-- 
Jazz is not dead. It just smells funny...

--
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 v3 30/32] arm64: KVM: enable initialization of a 32bit vcpu

2013-04-24 Thread Marc Zyngier
On 24/04/13 00:02, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:32PM +0100, Marc Zyngier wrote:
 Wire the init of a 32bit vcpu by allowing 32bit modes in pstate,
 and providing sensible defaults out of reset state.

 This feature is of course conditioned by the presence of 32bit
 capability on the physical CPU, and is checked by the KVM_CAP_ARM_EL1_32BIT
 capability.

 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm64/include/asm/kvm_host.h |  2 +-
  arch/arm64/include/uapi/asm/kvm.h |  1 +
  arch/arm64/kvm/guest.c|  6 ++
  arch/arm64/kvm/reset.c| 25 -
  include/uapi/linux/kvm.h  |  1 +
  5 files changed, 33 insertions(+), 2 deletions(-)

 diff --git a/arch/arm64/include/asm/kvm_host.h 
 b/arch/arm64/include/asm/kvm_host.h
 index d44064d..c3ec107 100644
 --- a/arch/arm64/include/asm/kvm_host.h
 +++ b/arch/arm64/include/asm/kvm_host.h
 @@ -34,7 +34,7 @@
  #include asm/kvm_vgic.h
  #include asm/kvm_arch_timer.h
  
 -#define KVM_VCPU_MAX_FEATURES 1
 +#define KVM_VCPU_MAX_FEATURES 2
  
  /* We don't currently support large pages. */
  #define KVM_HPAGE_GFN_SHIFT(x)  0
 diff --git a/arch/arm64/include/uapi/asm/kvm.h 
 b/arch/arm64/include/uapi/asm/kvm.h
 index 5b1110c..5031f42 100644
 --- a/arch/arm64/include/uapi/asm/kvm.h
 +++ b/arch/arm64/include/uapi/asm/kvm.h
 @@ -75,6 +75,7 @@ struct kvm_regs {
  #define KVM_VGIC_V2_CPU_SIZE0x2000
  
  #define KVM_ARM_VCPU_POWER_OFF  0 /* CPU is started in OFF 
 state */
 +#define KVM_ARM_VCPU_EL1_32BIT  1 /* CPU running a 32bit VM */
  
  struct kvm_vcpu_init {
  __u32 target;
 diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
 index 47d3729..74ef7d5 100644
 --- a/arch/arm64/kvm/guest.c
 +++ b/arch/arm64/kvm/guest.c
 @@ -93,6 +93,12 @@ static int set_core_reg(struct kvm_vcpu *vcpu, const 
 struct kvm_one_reg *reg)
  if (off == KVM_REG_ARM_CORE_REG(regs.pstate)) {
  unsigned long mode = (*(unsigned long *)valp)  
 COMPAT_PSR_MODE_MASK;
  switch (mode) {
 +case COMPAT_PSR_MODE_USR:
 +case COMPAT_PSR_MODE_FIQ:
 +case COMPAT_PSR_MODE_IRQ:
 +case COMPAT_PSR_MODE_SVC:
 +case COMPAT_PSR_MODE_ABT:
 +case COMPAT_PSR_MODE_UND:
  case PSR_MODE_EL0t:
  case PSR_MODE_EL1t:
  case PSR_MODE_EL1h:
 diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
 index bc33e76..a282d35 100644
 --- a/arch/arm64/kvm/reset.c
 +++ b/arch/arm64/kvm/reset.c
 @@ -35,11 +35,27 @@ static struct kvm_regs default_regs_reset = {
  .regs.pstate = PSR_MODE_EL1h | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT,
  };
  
 +static struct kvm_regs default_regs_reset32 = {
 +.regs.pstate = (COMPAT_PSR_MODE_SVC | COMPAT_PSR_A_BIT |
 +COMPAT_PSR_I_BIT | COMPAT_PSR_F_BIT),
 +};
 +
 +static bool cpu_has_32bit_el1(void)
 +{
 +u64 pfr0;
 +
 +pfr0 = read_cpuid(ID_AA64PFR0_EL1);
 +return !!(pfr0  0x20);
 
 again we don't need the double negation

I still hold that it makes things more readable.

 +}
 +
  int kvm_arch_dev_ioctl_check_extention(long ext)
  {
  int r;
  
  switch (ext) {
 +case KVM_CAP_ARM_EL1_32BIT:
 +r = cpu_has_32bit_el1();
 +break;
  default:
  r = 0;
  }
 @@ -62,7 +78,14 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
  
  switch (vcpu-arch.target) {
  default:
 -cpu_reset = default_regs_reset;
 +if (test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu-arch.features)) {
 +if (!cpu_has_32bit_el1())
 +return -EINVAL;
 
 I'm not sure EINVAL is appropriate here, the value specified was not
 incorrect, it's that the hardware doesn't support it. ENXIO, ENODEV, and
 add that in Documentation/virtual/kvm/api.txt ?

Not sure. If you ended up here, it means you tried to start a 32bit
guest on a 64bit-only CPU, despite KVM_CAP_ARM_EL1_32BIT telling you
that your CPU is not 32bit capable.

This is clearly an invalid input, isn't it?

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...

--
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 v3 31/32] arm64: KVM: userspace API documentation

2013-04-24 Thread Marc Zyngier
On 24/04/13 00:02, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:33PM +0100, Marc Zyngier wrote:
 Unsurprisingly, the arm64 userspace API is extremely similar to
 the 32bit one, the only significant difference being the ONE_REG
 register mapping.

 Signed-off-by: Marc Zyngier marc.zyng...@arm.com

[...]

 +
 +arm64 registers are mapped using the lower 32 bits. The upper 16 of
 +that is the register group type, or coprocessor number:
 +
 +arm64 core/FP-SIMD registers have the following id bit patterns:
 +  0x6002  0010 index into the kvm_regs struct:16
 +
 +arm64 CCSIDR registers are demultiplexed by CSSELR value:
 +  0x6002  0011 00 csselr:8
 +
 +arm64 system registers have the following id bit patterns:
 +  0x6002  0013 op0:2 op1:3 crn:4 crm:4 op2:3
 +
 
 I think these size encodings are 4 bits off, and not accurate for for
 the core registers, which have variable sizes, which should be indicated
 here (unless you decide for a separate category as per my other
 comment).

Indeed you're right, they're completely wrong. I realized that after
your 32bit patch fixing the same thing...

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...

--
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: [Qemu-devel] Virtualbox svga card in KVM

2013-04-24 Thread Veruca Salt

 Date: Mon, 15 Apr 2013 14:26:25 +0200
 From: kra...@redhat.com
 To: peter.mayd...@linaro.org
 CC: srira...@yahoo.com; stefa...@gmail.com; qemu-de...@nongnu.org; 
 kvm@vger.kernel.org
 Subject: Re: [Qemu-devel] Virtualbox svga card in KVM

 On 04/08/13 17:11, Peter Maydell wrote:
  On 6 April 2013 00:52, Sriram Murthy srira...@yahoo.com wrote:
  (actually, the virtualbox SVGA card is based off of the KVM VGA card)
 
  Is it possible to implement it as an extension to the VGA
  card device, or has it diverged incompatibly such that it
  has to be its own separate device model?

 Not needed. One just has to go write a windows driver. The virtual
 hardware can handle any resolution just fine.

 cheers,
 Gerd


Time for a mea culpa; as a user I have extended my research. The KVM vga
 sdl combination is around native speed when tested on a like-for-like 
basis with intel igp.
Which probably explains why evrybody else is happy. :)
What we may have to do is fix up a gpu with drivers. Somewhat non-trivial.
Any suggestions folks?
Regards,
Simon
ps-vbox gives us another 12% flat out.

 --
 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 
   --
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: KVM call minutes for 2013-04-23

2013-04-24 Thread Anthony Liguori
Eric Blake ebl...@redhat.com writes:

 On 04/23/2013 08:45 AM, Juan Quintela wrote:
 
 * 1.5 pending patches (paolo)
   anthony thinks nothing big is outstanding
   rdma: not probably for this release,  too big change on migration
   cpu-hotplug: andreas expect to get it for 1.5
 
 
 * What can libvirt expect in 1.5 for introspection of command-line support?
   command extensions?  libvirt want then
 * What are the rules for adding optional parameters to existing QMP
   commands?  Would it help if we had introspection of QMP commands?
   what are the options that each command support.
 
   command line could work for 1.5
 if we got patches on the next 2 days we can get it.

 Goal is to provide a QMP command that provides JSON representation of
 command line options; I will help review whatever is posted to make sure
 we like the interface.  Anthony agreed the implementation should be
 relatively straightforward and okay to add after soft freeze (but must
 be before hard freeze).  Libvirt has some code that would like to make
 use of the new command-line introspection; Osier will probably be the
 first libvirt developer taking advantage of it - if we can swing it,
 we'd like libvirt 1.0.5 to use the new command (libvirt freezes this
 weekend for a May 2 release).

   rest of introspection need 1.6
 it is challenging
 we are interesting into feature introspection
 and comand extensions?
 one command to return the schema?

 Anthony was okay with the idea of a full JSON introspection of all QMP
 commands, but it is probably too big to squeeze into 1.5 timeframe.
 Furthermore, while the command will be useful, we should always be
 thinking about API - having to parse through JSON to see if a feature is
 present is not always the nicest interface; when adding a new feature,
 consider improving an existing query-* or adding a counterpart new
 query-* command that makes it much easier to tell if a feature is
 available, without having to resort to a QMP introspection.

Ack.

One of the problems with using schema introspection for feature
detection is that there isn't always a 1-1 mapping.  You can imagine
that we have an optional parameter that gets added to a structure and is
initially tied to a specific feature but later gets used by another
feature.

If a distro backports the later and not the former, but a management
tool uses this field to probe for the former feature, it will result in
a false positive.

That's why a more direct feature negotiation mechanism is better IMHO.

Regards,

Anthony Liguori



   if we change a command,  how we change the interface without
   changing the c-api?

 c-api is not yet a strong consideration (but see [1] below).  Also,
 there may be ways to design a C api that is robust to extensions (but
 that means designing it into the QMP up front when adding a new
 command); there has been some list traffic on this thought.

 More importantly, adding an optional parameter to an existing command is
 not okay unless something else is also available to tell whether the
 feature is usable - QMP introspection would solve this, but is not
 necessarily the most elegant way.  For now, while adding QMP
 introspection is a good idea, we still want case-by-case review of any
 command extensions.

 
   we can change drive_mirror to use a new command to see if there
   are the new features.

 drive-mirror changed in 1.4 to add optional buf-size parameter; right
 now, libvirt is forced to limit itself to 1.3 interface (no buf-size or
 granularity) because there is no introspection and no query-* command
 that witnesses that the feature is present.  Idea was that we need to
 add a new query-drive-mirror-capabilities (name subject to bikeshedding)
 command into 1.5 that would let libvirt know that buf-size/granularity
 is usable (done right, it would also prevent the situation of buf-size
 being a write-only interface where it is set when starting the mirror
 but can not be queried later to see what size is in use).

 Unclear whether anyone was signing up to tackle the addition of a query
 command counterpart for drive-mirror in time for 1.5.

 
   if we have a stable c-api we can do test cases that work. 

 Having such a testsuite would make a stable C API more important.

 
 Eric will complete this with his undrestanding from libvirt point of
 view.

 Also under discussion: the existing QMP 'screendump' command is not
 ideal (not extensible, doesn't allow fd passing, hard-coded output
 format).  This was used as an example command that should not be
 extended until we have appropriate feature detection in place; probably
 easier to add a new QMP command than to add parameters to the existing
 one.  At any rate, we're late enough that 'screendump' probably won't be
 improved in 1.5, so we have the full 1.6 cycle to get it right.

 Not on the phone call, but a recent mail that is related to the topic -
 feature detection of whether dump-guest-memory supports paging:
 

Re: [Qemu-devel] KVM call minutes for 2013-04-23

2013-04-24 Thread Luiz Capitulino
On Tue, 23 Apr 2013 10:06:41 -0600
Eric Blake ebl...@redhat.com wrote:

we can change drive_mirror to use a new command to see if there
are the new features.
 
 drive-mirror changed in 1.4 to add optional buf-size parameter; right
 now, libvirt is forced to limit itself to 1.3 interface (no buf-size or
 granularity) because there is no introspection and no query-* command
 that witnesses that the feature is present.  Idea was that we need to
 add a new query-drive-mirror-capabilities (name subject to bikeshedding)
 command into 1.5 that would let libvirt know that buf-size/granularity
 is usable (done right, it would also prevent the situation of buf-size
 being a write-only interface where it is set when starting the mirror
 but can not be queried later to see what size is in use).
 
 Unclear whether anyone was signing up to tackle the addition of a query
 command counterpart for drive-mirror in time for 1.5.

I can do it.

Nice write-up Eric!
--
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: [Qemu-devel] KVM call minutes for 2013-04-23

2013-04-24 Thread Luiz Capitulino
On Wed, 24 Apr 2013 10:03:21 +0200
Stefan Hajnoczi stefa...@gmail.com wrote:

 On Tue, Apr 23, 2013 at 10:06:41AM -0600, Eric Blake wrote:
  On 04/23/2013 08:45 AM, Juan Quintela wrote:
 we can change drive_mirror to use a new command to see if there
 are the new features.
  
  drive-mirror changed in 1.4 to add optional buf-size parameter; right
  now, libvirt is forced to limit itself to 1.3 interface (no buf-size or
  granularity) because there is no introspection and no query-* command
  that witnesses that the feature is present.  Idea was that we need to
  add a new query-drive-mirror-capabilities (name subject to bikeshedding)
  command into 1.5 that would let libvirt know that buf-size/granularity
  is usable (done right, it would also prevent the situation of buf-size
  being a write-only interface where it is set when starting the mirror
  but can not be queried later to see what size is in use).
  
  Unclear whether anyone was signing up to tackle the addition of a query
  command counterpart for drive-mirror in time for 1.5.
 
 Seems like the trivial solution is a query-command-capabilities QMP
 command.
 
   query-command-capabilities drive-mirror
   = ['buf-size']
 
 It should only be a few lines of code and can be used for other commands
 that add optional parameters in the future.  In other words:

IMO, a separate command is better because we'll return CommandNotFound error
if the command doesn't exist. If we add query-command-capabilities we'd need
a new error class, otherwise a client won't be able to tell if the command
passed as argument exists.

Besides, separate commands tend to be simpler; and we already have
query-migrate-capabilities anyway.

The only disadvantage is some duplication and an increase in the number of
commands, but I don't think this is avoidable.

 typedef struct mon_cmd_t {
 ...
 const char **capabilities; /* drive-mirror uses [buf-size, NULL] */
 };
 
   
 if we have a stable c-api we can do test cases that work. 
  
  Having such a testsuite would make a stable C API more important.
 
 Writing tests in Python has been productive, see qemu-iotests 041 and
 friends.  The tests spawn QEMU guests and use QMP to interact:

Good to know.

   result = self.vm.qmp('query-block')
   self.assert_qmp(result, 'return[0]/inserted/file', target_img)
 
 Using this XPath-style syntax it's very easy to access the JSON.
 
 QEMU users tend not to use C, except libvirt.  Even libvirt implements
 the QMP protocol dynamically and can handle optional arguments well.
 
 I don't think a static C API makes sense when we have an extensible JSON
 protocol.  Let's use the extensibility to our advantage.

Agreed.
--
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: [Bug 53611] New: nVMX: Add nested EPT

2013-04-24 Thread Nakajima, Jun
On Wed, Apr 24, 2013 at 12:25 AM, Jan Kiszka jan.kis...@web.de wrote:

 I don't have a full picture (already asked you to post / git-push your
 intermediate state), but nested related states typically go to
 nested_vmx, thus vcpu_vmx.

 Ping regarding publication. I'm about to redo your porting work as we
 are making no progress.


Sorry about the slow progress. We've been distracted by some priority
things. The patches are ready (i.e. working), but we are cleaning them
up. I'll send what we have today.

--
Jun
Intel Open Source Technology Center
--
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: [Bug 53611] New: nVMX: Add nested EPT

2013-04-24 Thread Jan Kiszka
On 2013-04-24 17:55, Nakajima, Jun wrote:
 On Wed, Apr 24, 2013 at 12:25 AM, Jan Kiszka jan.kis...@web.de wrote:

 I don't have a full picture (already asked you to post / git-push your
 intermediate state), but nested related states typically go to
 nested_vmx, thus vcpu_vmx.

 Ping regarding publication. I'm about to redo your porting work as we
 are making no progress.

 
 Sorry about the slow progress. We've been distracted by some priority
 things. The patches are ready (i.e. working), but we are cleaning them
 up. I'll send what we have today.

Great news, thanks a lot!

Jan




signature.asc
Description: OpenPGP digital signature


Re: [PATCH v3 07/32] arm64: KVM: fault injection into a guest

2013-04-24 Thread Christoffer Dall
On Wed, Apr 24, 2013 at 3:04 AM, Marc Zyngier marc.zyng...@arm.com wrote:
 On 23/04/13 23:57, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:09PM +0100, Marc Zyngier wrote:
 Implement the injection of a fault (undefined, data abort or
 prefetch abort) into a 64bit guest.

 Reviewed-by: Christopher Covington c...@codeaurora.org
 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm64/kvm/inject_fault.c | 118 
 ++
  1 file changed, 118 insertions(+)
  create mode 100644 arch/arm64/kvm/inject_fault.c

 diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
 new file mode 100644
 index 000..2ff3b78
 --- /dev/null
 +++ b/arch/arm64/kvm/inject_fault.c
 @@ -0,0 +1,118 @@
 +/*
 + * Fault injection for 64bit guests.
 + *
 + * Copyright (C) 2012,2013 - ARM Ltd
 + * Author: Marc Zyngier marc.zyng...@arm.com
 + *
 + * Based on arch/arm/kvm/emulate.c
 + * Copyright (C) 2012 - Virtual Open Systems and Columbia University
 + * Author: Christoffer Dall c.d...@virtualopensystems.com
 + *
 + * This program is free software: you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#include linux/kvm_host.h
 +#include asm/kvm_emulate.h
 +#include asm/esr.h
 +
 +static void inject_abt64(struct kvm_vcpu *vcpu, bool is_iabt, unsigned 
 long addr)
 +{
 +unsigned long cpsr = *vcpu_cpsr(vcpu);
 +int is_aarch32;
 +u32 esr = 0;
 +
 +is_aarch32 = vcpu_mode_is_32bit(vcpu);
 +
 +*vcpu_spsr(vcpu) = cpsr;
 +*vcpu_elr_el1(vcpu) = *vcpu_pc(vcpu);
 +
 +*vcpu_cpsr(vcpu) = PSR_MODE_EL1h | PSR_A_BIT | PSR_F_BIT | PSR_I_BIT;
 +*vcpu_pc(vcpu) = vcpu_sys_reg(vcpu, VBAR_EL1) + 0x200;

 consider a define for the 0x200?

 Yeah, I think Will already mentioned this.

 +
 +vcpu_sys_reg(vcpu, FAR_EL1) = addr;
 +
 +/*
 + * Build an {i,d}abort, depending on the level and the
 + * instruction set. Report an external synchronous abort.
 + */
 +if (kvm_vcpu_trap_il_is32bit(vcpu))
 +esr |= ESR_EL1_IL;
 +
 +if (is_aarch32 || (cpsr  PSR_MODE_MASK) == PSR_MODE_EL0t)
 +esr |= (ESR_EL1_EC_IABT_EL0  ESR_EL1_EC_SHIFT);
 +else
 +esr |= (ESR_EL1_EC_IABT_EL1  ESR_EL1_EC_SHIFT);

 why is it assumed that the abort must come from EL0 if the vcpu is in
 aarch32 mode?

 We're injecting a fault into a VM that runs with a 64bit EL1. So if we
 end-up faulting in 32bit code, it must come from EL0.


ah ok, it could be mentioned in a comment, but meh...

 +
 +if (!is_iabt)
 +esr |= ESR_EL1_EC_DABT_EL0;

 this is slightly confusing unless you actually look at the definitions
 and realize that or'ing on that extra bit works both in i- and d-abt
 cases.

 Blame Christopher for that, he came up with the idea... ;-)


I think this is one of those cases where the compiler should be this
smart, and C should be a little more verbose, but it's not a cardinal
point.

 +
 +vcpu_sys_reg(vcpu, ESR_EL1) = esr | 0x10; /* External abort */
 +}
 +
 +static void inject_undef64(struct kvm_vcpu *vcpu)
 +{
 +unsigned long cpsr = *vcpu_cpsr(vcpu);
 +u32 esr = (ESR_EL1_EC_UNKNOWN  ESR_EL1_EC_SHIFT);
 +
 +*vcpu_spsr(vcpu) = cpsr;
 +*vcpu_elr_el1(vcpu) = *vcpu_pc(vcpu);
 +
 +*vcpu_cpsr(vcpu) = PSR_MODE_EL1h | PSR_F_BIT | PSR_I_BIT;

 would it make sense to have a define for a common set of these bits or
 is this explicitly defined per exception type in the specs?

 Maybe. Not sure that would be any clearer though...

 M.
 --
 Jazz is not dead. It just smells funny...


 ___
 kvmarm mailing list
 kvm...@lists.cs.columbia.edu
 https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
--
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 v3 08/32] arm64: KVM: architecture specific MMU backend

2013-04-24 Thread Christoffer Dall
On Wed, Apr 24, 2013 at 4:10 AM, Will Deacon will.dea...@arm.com wrote:
 On Wed, Apr 24, 2013 at 12:03:10PM +0100, Marc Zyngier wrote:
 On 23/04/13 23:58, Christoffer Dall wrote:
  I noticed that this doesn't do any cache cleaning.  Are the MMU page
  table walks guaranteed to be coherent with the MMU on arm64?

 I suppose you meant the cache. In this case, yes. The hardware page
 table walker must snoop the caches.

 Also, for ARMv7, SMP implies that the hardware walker snoops the cache. I
 recently upstreamed some patches for this (see ARM: 7691/1: mm: kill unused
 TLB_CAN_READ_FROM_L1_CACHE and use ALT_SMP instead in -next), so you might
 want to check if there are any remaining, redundant flushes in kvm for ARMv7.

always on SMP? I thought this was implementation specific and you
could read this in some feature register?

In any case, I'm quite sure we do some unnecessary cleaning in KVM
then, so I'll look at it.
--
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 v3 08/32] arm64: KVM: architecture specific MMU backend

2013-04-24 Thread Christoffer Dall
On Wed, Apr 24, 2013 at 4:03 AM, Marc Zyngier marc.zyng...@arm.com wrote:
 On 23/04/13 23:58, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:10PM +0100, Marc Zyngier wrote:
 Define the arm64 specific MMU backend:
 - HYP/kernel VA offset
 - S2 4/64kB definitions
 - S2 page table populating and flushing
 - icache cleaning

 Reviewed-by: Christopher Covington c...@codeaurora.org
 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm64/include/asm/kvm_mmu.h | 136 
 +++
  1 file changed, 136 insertions(+)
  create mode 100644 arch/arm64/include/asm/kvm_mmu.h

 diff --git a/arch/arm64/include/asm/kvm_mmu.h 
 b/arch/arm64/include/asm/kvm_mmu.h
 new file mode 100644
 index 000..2eb2230
 --- /dev/null
 +++ b/arch/arm64/include/asm/kvm_mmu.h
 @@ -0,0 +1,136 @@
 +/*
 + * Copyright (C) 2012,2013 - ARM Ltd
 + * Author: Marc Zyngier marc.zyng...@arm.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#ifndef __ARM64_KVM_MMU_H__
 +#define __ARM64_KVM_MMU_H__
 +
 +#include asm/page.h
 +#include asm/memory.h
 +
 +/*
 + * As we only have the TTBR0_EL2 register, we cannot express
 + * negative addresses. This makes it impossible to directly share
 + * mappings with the kernel.
 + *
 + * Instead, give the HYP mode its own VA region at a fixed offset from
 + * the kernel by just masking the top bits (which are all ones for a
 + * kernel address).

 For some reason I keep choking on this, despite it being very simple.
 We're just defining a different PAGE_OFFSET, right? Why not do a hard
 define as:

 #define HYP_PAGE_OFFSET_MASK  0x
 #define HYP_PAGE_OFFSET   0xffc0

 ...or change the second paragraph of the comment to say
 that we definethe HYP_PAGE_OFFSET to be 0x ffc0 .

 One of these days, VA_BITS will change to accommodate for more virtual
 space. When that day comes, I don't want to touch any of this because it
 did hurt enough when writing it. As such, I'll refrain from hardcoding
 anything.

 I don't mind a comment, though.

 + */
 +#define HYP_PAGE_OFFSET_SHIFT   VA_BITS
 +#define HYP_PAGE_OFFSET_MASK((UL(1)  HYP_PAGE_OFFSET_SHIFT) - 1)

 In any case, is there a reason for the HYP_PAGE_OFFSET_SHIFT
 indirection? It may be simpler without...

 It is common practice to have XXX_SHIFT and XXX_MASK together.

 +#define HYP_PAGE_OFFSET (PAGE_OFFSET  HYP_PAGE_OFFSET_MASK)
 +
 +/*
 + * Our virtual mapping for the idmap-ed MMU-enable code. Must be
 + * shared across all the page-tables. Conveniently, we use the last
 + * possible page, where no kernel mapping will ever exist.
 + */
 +#define TRAMPOLINE_VA   (HYP_PAGE_OFFSET_MASK  PAGE_MASK)

 hmmm, ok, here it's kind of nice to have that define correlation, so
 maybe it's not cleaner.  Something should be improved here in the define
 or the comment to make it more clear.  Perhaps just adding the real
 constants in the comment or in Documentation/arm64/memory.txt would
 help.

 Yes, I plan to write something there.

 +
 +#ifdef __ASSEMBLY__
 +
 +/*
 + * Convert a kernel VA into a HYP VA.
 + * reg: VA to be converted.
 + */
 +.macro kern_hyp_va  reg
 +and \reg, \reg, #HYP_PAGE_OFFSET_MASK
 +.endm
 +
 +#else
 +
 +#include asm/cacheflush.h
 +
 +#define KERN_TO_HYP(kva)((unsigned long)kva - PAGE_OFFSET + 
 HYP_PAGE_OFFSET)
 +
 +/*
 + * Align KVM with the kernel's view of physical memory. Should be
 + * 40bit IPA, with PGD being 8kB aligned.
 + */
 +#define KVM_PHYS_SHIFT  PHYS_MASK_SHIFT
 +#define KVM_PHYS_SIZE   (1UL  KVM_PHYS_SHIFT)
 +#define KVM_PHYS_MASK   (KVM_PHYS_SIZE - 1UL)
 +
 +#ifdef CONFIG_ARM64_64K_PAGES
 +#define PAGE_LEVELS 2
 +#define BITS_PER_LEVEL  13
 +#else  /* 4kB pages */
 +#define PAGE_LEVELS 3
 +#define BITS_PER_LEVEL  9
 +#endif

 What are the semantics of these defines exactly? They should be
 S2_PAGE_LEVELS and make some assumptions of the VTCR_EL2.SL0 field
 right?

 Indeed, we assume SL0 is always 1, just like for the kernel. As for the
 semantics, I though they were pretty obvious...


this is all stage2 right? so S2_PAGE_LEVELS may be more clear...

 PAGE_LEVELS is just that, the number of page levels. BITS_PER_LEVEL is
 the number of bits you need to index a particular level.

 +
 +/* Make sure we get the right size, and thus the right alignment */

 this comment is not particularly helpful, something explaining why 

Re: [PATCH v3 15/32] arm64: KVM: guest one-reg interface

2013-04-24 Thread Christoffer Dall
On Wed, Apr 24, 2013 at 4:27 AM, Marc Zyngier marc.zyng...@arm.com wrote:
 On 23/04/13 23:59, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:17PM +0100, Marc Zyngier wrote:
 Let userspace play with the guest registers.

 Reviewed-by: Christopher Covington c...@codeaurora.org
 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm64/kvm/guest.c | 254 
 +
  1 file changed, 254 insertions(+)
  create mode 100644 arch/arm64/kvm/guest.c

 diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
 new file mode 100644
 index 000..47d3729
 --- /dev/null
 +++ b/arch/arm64/kvm/guest.c
 @@ -0,0 +1,254 @@
 +/*
 + * Copyright (C) 2012,2013 - ARM Ltd
 + * Author: Marc Zyngier marc.zyng...@arm.com
 + *
 + * Derived from arch/arm/kvm/guest.c:
 + * Copyright (C) 2012 - Virtual Open Systems and Columbia University
 + * Author: Christoffer Dall c.d...@virtualopensystems.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#include linux/errno.h
 +#include linux/err.h
 +#include linux/kvm_host.h
 +#include linux/module.h
 +#include linux/vmalloc.h
 +#include linux/fs.h
 +#include asm/cputype.h
 +#include asm/uaccess.h
 +#include asm/kvm.h
 +#include asm/kvm_asm.h
 +#include asm/kvm_emulate.h
 +#include asm/kvm_coproc.h
 +
 +struct kvm_stats_debugfs_item debugfs_entries[] = {
 +{ NULL }
 +};
 +
 +int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
 +{
 +vcpu-arch.hcr_el2 = HCR_GUEST_FLAGS;
 +return 0;
 +}
 +
 +static u64 core_reg_offset_from_id(u64 id)
 +{
 +return id  ~(KVM_REG_ARCH_MASK | KVM_REG_SIZE_MASK | 
 KVM_REG_ARM_CORE);
 +}
 +
 +static int get_core_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg 
 *reg)
 +{
 +__u32 __user *uaddr = (__u32 __user *)(unsigned long)reg-addr;
 +struct kvm_regs *regs = vcpu_gp_regs(vcpu);
 +int nr_regs = sizeof(*regs) / sizeof(__u32);

 eh, arent' your regs 64 bit?  Or are you 32-bit indexing into a 64-bit
 structure?  If so, this needs to be described in a big fat comment
 somewhere, which I couldn't even see looking forward in the patch series
 for the documentation part.

 As you noticed below, we have a mix of 32/64/128bit fields there. The
 index is indeed on 32bit boundary.

 Seems to me you'd want to remove the fp_regs from the core regs and just
 use a 64-bit indexing and have a separate category for the fp stuff...

 Hell no! ;-)

easy now


 FP is mandatory on arm64, and I'm not going down the road of having
 separate structures for that. 32bit has historical baggage to deal with,
 but not arm64.

 This is the register set, and if the ONE_REG API is too cumbersome to
 deal with it, then lets change ONE_REG instead (yes, I can run faster
 than you think... ;-).


You chose yourself how to use the fields in ONE_REG, and that's what I
think makes this code a little weird, I don't care that much how the
underlying structure is.  The fact that we (Rusty) used the index into
the 32 bit fields was simply that it was more convenient and quite
unambiguous on the 32-bit side, as opposed to defining in the API
document a specific ID for each register, like the PPC stuff does.

You don't have to follow that on arm64 if it doesn't make sense.
Notice that in the documentation on arm32 (although it had errors in
there) we explicitly specify that all sizes must be 32 bit, so you
need to change that to indicate that it's a variable size and they use
a 32 bit index into variably sized elements, which I think is a super
funky interface, or change it to split it up into more sane categories
and map that onto your internal data structure.

 +u32 off;
 +
 +/* Our ID is an index into the kvm_regs struct. */
 +off = core_reg_offset_from_id(reg-id);
 +if (off = nr_regs ||
 +(off + (KVM_REG_SIZE(reg-id) / sizeof(__u32))) = nr_regs)

 According to your documentation you will always save/restore 32-bit
 registers here, so the desijunction shouldn't be necessary, nor will it
 be if you just base everything on 64-bit here.

 No. Your *offset* is a 32bit index. The size can be anything, and you
 want to make sure you don't read/write past the kvm_regs structure.


I see why you need it as things stand now, my point is that if the
interface definition was different you probably don't need it.

 +return -ENOENT;
 +
 +if (copy_to_user(uaddr, ((u32 *)regs) + off, KVM_REG_SIZE(reg-id)))
 +return -EFAULT;
 

Re: [PATCH v3 17/32] arm64: KVM: HYP mode world switch implementation

2013-04-24 Thread Christoffer Dall
On Wed, Apr 24, 2013 at 4:39 AM, Marc Zyngier marc.zyng...@arm.com wrote:
 On 23/04/13 23:59, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:19PM +0100, Marc Zyngier wrote:
 The HYP mode world switch in all its glory.

 Implements save/restore of host/guest registers, EL2 trapping,
 IPA resolution, and additional services (tlb invalidation).

 Reviewed-by: Christopher Covington c...@codeaurora.org
 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm64/kernel/asm-offsets.c |  34 +++
  arch/arm64/kvm/hyp.S| 602 
 
  2 files changed, 636 insertions(+)
  create mode 100644 arch/arm64/kvm/hyp.S


 [...]

 diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
 new file mode 100644
 index 000..c745d20
 --- /dev/null
 +++ b/arch/arm64/kvm/hyp.S
 @@ -0,0 +1,602 @@
 +/*
 + * Copyright (C) 2012,2013 - ARM Ltd
 + * Author: Marc Zyngier marc.zyng...@arm.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#include linux/linkage.h
 +#include linux/irqchip/arm-gic.h
 +
 +#include asm/assembler.h
 +#include asm/memory.h
 +#include asm/asm-offsets.h
 +#include asm/fpsimdmacros.h
 +#include asm/kvm.h
 +#include asm/kvm_asm.h
 +#include asm/kvm_arm.h
 +#include asm/kvm_mmu.h
 +
 +#define CPU_GP_REG_OFFSET(x) (CPU_GP_REGS + x)
 +#define CPU_XREG_OFFSET(x)   CPU_GP_REG_OFFSET(CPU_USER_PT_REGS + 8*x)
 +#define CPU_SPSR_OFFSET(x)   CPU_GP_REG_OFFSET(CPU_SPSR + 8*x)
 +#define CPU_SYSREG_OFFSET(x) (CPU_SYSREGS + 8*x)
 +
 + .text
 + .pushsection.hyp.text, ax
 + .align  PAGE_SHIFT
 +
 +__kvm_hyp_code_start:
 + .globl __kvm_hyp_code_start
 +
 +.macro save_common_regs
 + // x2: base address for cpu context
 + // x3: tmp register

 what's with the C99 style comments? Standard for arm64 assembly?

 Yes. The toolchain guys got rid of '@' as a single line comment delimiter.

 [...]

 +el1_sync:// Guest trapped into EL2
 + pushx0, x1
 + pushx2, x3
 +
 + mrs x1, esr_el2
 + lsr x2, x1, #ESR_EL2_EC_SHIFT
 +
 + cmp x2, #ESR_EL2_EC_HVC64
 + b.neel1_trap
 +
 + mrs x3, vttbr_el2   // If vttbr is valid, the 
 64bit guest
 + cbnzx3, el1_trap// called HVC
 +
 + /* Here, we're pretty sure the host called HVC. */
 + pop x2, x3
 + pop x0, x1
 +
 + pushlr, xzr
 +
 + /*
 +  * Compute the function address in EL2, and shuffle the parameters.
 +  */
 + kern_hyp_va x0
 + mov lr, x0
 + mov x0, x1
 + mov x1, x2
 + mov x2, x3
 + blr lr
 +
 + pop lr, xzr
 + eret
 +
 +el1_trap:
 + /*
 +  * x1: ESR
 +  * x2: ESR_EC
 +  */
 + cmp x2, #ESR_EL2_EC_DABT
 + mov x0, #ESR_EL2_EC_IABT
 + ccmpx2, x0, #4, ne
 + b.ne1f  // Not an abort we care about

 why do we get the hpfar_el2 if it's not an abort (or is this for a
 special type of abort) ?

 No, we could actually avoid saving HPFAR_EL2 altogether in this case.

 +
 + /* This is an abort. Check for permission fault */
 + and x2, x1, #ESR_EL2_FSC_TYPE
 + cmp x2, #FSC_PERM
 + b.ne1f  // Not a permission fault
 +
 + /*
 +  * Check for Stage-1 page table walk, which is guaranteed
 +  * to give a valid HPFAR_EL2.
 +  */
 + tbnzx1, #7, 1f  // S1PTW is set
 +
 + /*
 +  * Permission fault, HPFAR_EL2 is invalid.
 +  * Resolve the IPA the hard way using the guest VA.
 +  * We always perform an EL1 lookup, as we already
 +  * went through Stage-1.
 +  */

 What does the last sentence mean exactly?

 It means that the Stage-1 translation already validated the memory
 access rights. As such, we can use the EL1 translation regime, and don't
 have to distinguish between EL0 and EL1 access.


ah, right, now I remember this one, I think the comment could say that
more clearly:)

 + mrs x3, far_el2
 + at  s1e1r, x3
 + isb
 +
 + /* Read result */
 + mrs x3, par_el1
 + tbnzx3, #1, 3f  // Bail out if we failed the 
 translation
 + ubfxx3, x3, #12, #36// Extract IPA
 + lsl x3, x3, #4  // and present it like HPFAR
 + b   2f
 +
 +1:   mrs x3, hpfar_el2
 +
 +2:   mrs x0, tpidr_el2
 + mrs x2, 

Re: [PATCH v3 24/32] arm64: KVM: 32bit GP register access

2013-04-24 Thread Christoffer Dall
On Wed, Apr 24, 2013 at 6:06 AM, Marc Zyngier marc.zyng...@arm.com wrote:
 On 24/04/13 00:00, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:26PM +0100, Marc Zyngier wrote:
 Allow access to the 32bit register file through the usual API.

 Reviewed-by: Christopher Covington c...@codeaurora.org
 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm64/include/asm/kvm_emulate.h |  17 +++-
  arch/arm64/kvm/Makefile  |   2 +-
  arch/arm64/kvm/regmap.c  | 168 
 +++
  3 files changed, 184 insertions(+), 3 deletions(-)
  create mode 100644 arch/arm64/kvm/regmap.c

 diff --git a/arch/arm64/include/asm/kvm_emulate.h 
 b/arch/arm64/include/asm/kvm_emulate.h
 index 2dcfa74..37a6567 100644
 --- a/arch/arm64/include/asm/kvm_emulate.h
 +++ b/arch/arm64/include/asm/kvm_emulate.h
 @@ -28,6 +28,9 @@
  #include asm/kvm_mmio.h
  #include asm/ptrace.h

 +unsigned long *vcpu_reg32(const struct kvm_vcpu *vcpu, u8 reg_num);
 +unsigned long *vcpu_spsr32(const struct kvm_vcpu *vcpu);
 +
  void kvm_inject_undefined(struct kvm_vcpu *vcpu);
  void kvm_inject_dabt(struct kvm_vcpu *vcpu, unsigned long addr);
  void kvm_inject_pabt(struct kvm_vcpu *vcpu, unsigned long addr);
 @@ -49,7 +52,7 @@ static inline unsigned long *vcpu_cpsr(const struct 
 kvm_vcpu *vcpu)

  static inline bool vcpu_mode_is_32bit(const struct kvm_vcpu *vcpu)
  {
 -return false;   /* 32bit? Bahhh... */
 +return !!(*vcpu_cpsr(vcpu)  PSR_MODE32_BIT);

 nit: you don't need the '!!': it's a bool

 No it is not. It is a bitwise and, turned into a bool.

yeah and the result of the bitwise and will be cast to a bool as per
the function's return value without the use of '!!' or did I read the
C spec wrong?
--
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 v3 25/32] arm64: KVM: 32bit conditional execution emulation

2013-04-24 Thread Christoffer Dall
On Wed, Apr 24, 2013 at 6:13 AM, Marc Zyngier marc.zyng...@arm.com wrote:
 On 24/04/13 00:00, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:27PM +0100, Marc Zyngier wrote:
 As conditional instructions can trap on AArch32, add the thinest
 possible emulation layer to keep 32bit guests happy.

 Reviewed-by: Christopher Covington c...@codeaurora.org
 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm64/include/asm/kvm_emulate.h |  13 ++-
  arch/arm64/kvm/Makefile  |   2 +-
  arch/arm64/kvm/emulate.c | 154 
 +++
  3 files changed, 166 insertions(+), 3 deletions(-)
  create mode 100644 arch/arm64/kvm/emulate.c

 diff --git a/arch/arm64/include/asm/kvm_emulate.h 
 b/arch/arm64/include/asm/kvm_emulate.h
 index 37a6567..8d4ab33 100644
 --- a/arch/arm64/include/asm/kvm_emulate.h
 +++ b/arch/arm64/include/asm/kvm_emulate.h
 @@ -31,6 +31,9 @@
  unsigned long *vcpu_reg32(const struct kvm_vcpu *vcpu, u8 reg_num);
  unsigned long *vcpu_spsr32(const struct kvm_vcpu *vcpu);

 +bool kvm_condition_valid32(const struct kvm_vcpu *vcpu);
 +void kvm_skip_instr32(struct kvm_vcpu *vcpu, bool is_wide_instr);
 +
  void kvm_inject_undefined(struct kvm_vcpu *vcpu);
  void kvm_inject_dabt(struct kvm_vcpu *vcpu, unsigned long addr);
  void kvm_inject_pabt(struct kvm_vcpu *vcpu, unsigned long addr);
 @@ -57,12 +60,18 @@ static inline bool vcpu_mode_is_32bit(const struct 
 kvm_vcpu *vcpu)

  static inline bool kvm_condition_valid(const struct kvm_vcpu *vcpu)
  {
 -return true;/* No conditionals on arm64 */
 +if (vcpu_mode_is_32bit(vcpu))
 +return kvm_condition_valid32(vcpu);
 +
 +return true;
  }

  static inline void kvm_skip_instr(struct kvm_vcpu *vcpu, bool 
 is_wide_instr)
  {
 -*vcpu_pc(vcpu) += 4;
 +if (vcpu_mode_is_32bit(vcpu))
 +kvm_skip_instr32(vcpu, is_wide_instr);
 +else
 +*vcpu_pc(vcpu) += 4;
  }

  static inline void vcpu_set_thumb(struct kvm_vcpu *vcpu)
 diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
 index 1668448..88c6639 100644
 --- a/arch/arm64/kvm/Makefile
 +++ b/arch/arm64/kvm/Makefile
 @@ -11,7 +11,7 @@ obj-$(CONFIG_KVM_ARM_HOST) += kvm.o
  kvm-$(CONFIG_KVM_ARM_HOST) += $(addprefix ../../../virt/kvm/, kvm_main.o 
 coalesced_mmio.o)
  kvm-$(CONFIG_KVM_ARM_HOST) += $(addprefix ../../../arch/arm/kvm/, arm.o 
 mmu.o mmio.o psci.o perf.o)

 -kvm-$(CONFIG_KVM_ARM_HOST) += inject_fault.o regmap.o
 +kvm-$(CONFIG_KVM_ARM_HOST) += emulate.o inject_fault.o regmap.o
  kvm-$(CONFIG_KVM_ARM_HOST) += hyp.o hyp-init.o handle_exit.o
  kvm-$(CONFIG_KVM_ARM_HOST) += guest.o reset.o sys_regs.o 
 sys_regs_generic_v8.o

 diff --git a/arch/arm64/kvm/emulate.c b/arch/arm64/kvm/emulate.c
 new file mode 100644
 index 000..01d4713
 --- /dev/null
 +++ b/arch/arm64/kvm/emulate.c
 @@ -0,0 +1,154 @@
 +/*
 + * (not much of an) Emulation layer for 32bit guests.
 + *
 + * Copyright (C) 2012 - Virtual Open Systems and Columbia University
 + * Author: Christoffer Dall c.d...@virtualopensystems.com

 don't you want to add yourself here?

 There's really nothing I actually wrote here. Most of it is your code,
 and the rest comes from other parts of arch/arm.

 I guess I could always mention myself for the ESR_EL2_* macros... ;-)

 + *
 + * This program is free software: you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#include linux/kvm_host.h
 +#include asm/kvm_emulate.h
 +
 +/*
 + * stolen from arch/arm/kernel/opcodes.c
 + *
 + * condition code lookup table
 + * index into the table is test code: EQ, NE, ... LT, GT, AL, NV
 + *
 + * bit position in short is condition code: NZCV
 + */
 +static const unsigned short cc_map[16] = {
 +0xF0F0, /* EQ == Z set*/
 +0x0F0F, /* NE */
 +0x, /* CS == C set*/
 +0x, /* CC */
 +0xFF00, /* MI == N set*/
 +0x00FF, /* PL */
 +0x, /* VS == V set*/
 +0x, /* VC */
 +0x0C0C, /* HI == C set  Z clear */
 +0xF3F3, /* LS == C clear || Z set */
 +0xAA55, /* GE == (N==V)   */
 +0x55AA, /* LT == (N!=V)   */
 +0x0A05, /* GT == (!Z  (N==V))   

Re: [PATCH v3 26/32] arm64: KVM: 32bit handling of coprocessor traps

2013-04-24 Thread Christoffer Dall
On Wed, Apr 24, 2013 at 6:42 AM, Marc Zyngier marc.zyng...@arm.com wrote:
 On 24/04/13 00:01, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:28PM +0100, Marc Zyngier wrote:
 Provide the necessary infrastructure to trap coprocessor accesses that
 occur when running 32bit guests.

 Also wire SMC and HVC trapped in 32bit mode while were at it.

 Reviewed-by: Christopher Covington c...@codeaurora.org
 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm64/include/asm/kvm_coproc.h |   5 +
  arch/arm64/kvm/handle_exit.c|   7 ++
  arch/arm64/kvm/sys_regs.c   | 178 
 ++--
  3 files changed, 183 insertions(+), 7 deletions(-)

 diff --git a/arch/arm64/include/asm/kvm_coproc.h 
 b/arch/arm64/include/asm/kvm_coproc.h
 index 9b4477a..9a59301 100644
 --- a/arch/arm64/include/asm/kvm_coproc.h
 +++ b/arch/arm64/include/asm/kvm_coproc.h
 @@ -32,11 +32,16 @@ struct kvm_sys_reg_table {

  struct kvm_sys_reg_target_table {
   struct kvm_sys_reg_table table64;
 + struct kvm_sys_reg_table table32;
  };

  void kvm_register_target_sys_reg_table(unsigned int target,
  struct kvm_sys_reg_target_table 
 *table);

 +int kvm_handle_cp14_load_store(struct kvm_vcpu *vcpu, struct kvm_run *run);
 +int kvm_handle_cp14_access(struct kvm_vcpu *vcpu, struct kvm_run *run);
 +int kvm_handle_cp15_32(struct kvm_vcpu *vcpu, struct kvm_run *run);
 +int kvm_handle_cp15_64(struct kvm_vcpu *vcpu, struct kvm_run *run);
  int kvm_handle_sys_reg(struct kvm_vcpu *vcpu, struct kvm_run *run);

  #define kvm_coproc_table_init kvm_sys_reg_table_init
 diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
 index 4766b7f..9beaca03 100644
 --- a/arch/arm64/kvm/handle_exit.c
 +++ b/arch/arm64/kvm/handle_exit.c
 @@ -62,6 +62,13 @@ static int kvm_handle_wfi(struct kvm_vcpu *vcpu, struct 
 kvm_run *run)

  static exit_handle_fn arm_exit_handlers[] = {
   [ESR_EL2_EC_WFI]= kvm_handle_wfi,
 + [ESR_EL2_EC_CP15_32]= kvm_handle_cp15_32,
 + [ESR_EL2_EC_CP15_64]= kvm_handle_cp15_64,
 + [ESR_EL2_EC_CP14_MR]= kvm_handle_cp14_access,
 + [ESR_EL2_EC_CP14_LS]= kvm_handle_cp14_load_store,
 + [ESR_EL2_EC_CP14_64]= kvm_handle_cp14_access,
 + [ESR_EL2_EC_HVC32]  = handle_hvc,
 + [ESR_EL2_EC_SMC32]  = handle_smc,
   [ESR_EL2_EC_HVC64]  = handle_hvc,
   [ESR_EL2_EC_SMC64]  = handle_smc,
   [ESR_EL2_EC_SYS64]  = kvm_handle_sys_reg,
 diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
 index 9df3b32..0303218 100644
 --- a/arch/arm64/kvm/sys_regs.c
 +++ b/arch/arm64/kvm/sys_regs.c
 @@ -38,6 +38,10 @@
   * types are different. My gut feeling is that it should be pretty
   * easy to merge, but that would be an ABI breakage -- again. VFP
   * would also need to be abstracted.
 + *
 + * For AArch32, we only take care of what is being trapped. Anything
 + * that has to do with init and userspace access has to go via the
 + * 64bit interface.
   */

  /* 3 bits per cache level, as per CLIDR, but non-existent caches always 0 
 */
 @@ -163,6 +167,16 @@ static const struct sys_reg_desc sys_reg_descs[] = {
   { Op0(0b01), Op1(0b000), CRn(0b0111), CRm(0b1110), Op2(0b010),
 access_dcsw },

 + /* TEECR32_EL1 */
 + { Op0(0b10), Op1(0b010), CRn(0b), CRm(0b), Op2(0b000),
 +   NULL, reset_val, TEECR32_EL1, 0 },
 + /* TEEHBR32_EL1 */
 + { Op0(0b10), Op1(0b010), CRn(0b0001), CRm(0b), Op2(0b000),
 +   NULL, reset_val, TEEHBR32_EL1, 0 },
 + /* DBGVCR32_EL2 */
 + { Op0(0b10), Op1(0b100), CRn(0b), CRm(0b0111), Op2(0b000),
 +   NULL, reset_val, DBGVCR32_EL2, 0 },
 +
   /* MPIDR_EL1 */
   { Op0(0b11), Op1(0b000), CRn(0b), CRm(0b), Op2(0b101),
 NULL, reset_mpidr, MPIDR_EL1 },
 @@ -273,6 +287,39 @@ static const struct sys_reg_desc sys_reg_descs[] = {
   /* TPIDRRO_EL0 */
   { Op0(0b11), Op1(0b011), CRn(0b1101), CRm(0b), Op2(0b011),
 NULL, reset_unknown, TPIDRRO_EL0 },
 +
 + /* DACR32_EL2 */
 + { Op0(0b11), Op1(0b100), CRn(0b0011), CRm(0b), Op2(0b000),
 +   NULL, reset_unknown, DACR32_EL2 },
 + /* IFSR32_EL2 */
 + { Op0(0b11), Op1(0b100), CRn(0b0101), CRm(0b), Op2(0b001),
 +   NULL, reset_unknown, IFSR32_EL2 },
 + /* FPEXC32_EL2 */
 + { Op0(0b11), Op1(0b100), CRn(0b0101), CRm(0b0011), Op2(0b000),
 +   NULL, reset_val, FPEXC32_EL2, 0x70 },
 +};
 +
 +/* Trapped cp15 registers */
 +static const struct sys_reg_desc cp15_regs[] = {
 + /*
 +  * DC{C,I,CI}SW operations:
 +  */
 + { Op1( 0), CRn( 7), CRm( 6), Op2( 2), access_dcsw },
 + { Op1( 0), CRn( 7), CRm(10), Op2( 2), access_dcsw },
 + { Op1( 0), CRn( 7), CRm(14), Op2( 2), access_dcsw },
 + { Op1( 0), CRn( 9), CRm(12), Op2( 0), pm_fake },
 + { Op1( 0), CRn( 9), CRm(12), Op2( 1), pm_fake },
 + { Op1( 0), CRn( 9), CRm(12), Op2( 2), pm_fake },
 + 

Re: [PATCH v3 29/32] arm64: KVM: 32bit guest fault injection

2013-04-24 Thread Christoffer Dall
On Wed, Apr 24, 2013 at 6:46 AM, Marc Zyngier marc.zyng...@arm.com wrote:
 On 24/04/13 00:02, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:31PM +0100, Marc Zyngier wrote:
 Add fault injection capability for 32bit guests.

 looks ok, but seems we are duplicating some functionality here, but I
 realize it may not be that easy to share due to different define names
 for things like PSR_X_BIT...  Did you look further into this?

 I did, but as you noticed, the namespaces are different. We could
 probably define a set of KVM_AARCH32_* symbols, but I'm not sure it
 would be any nicer though.

yeah, it's not code that's very likely to change and evolve over time,
so I guess we can live with the duplication...
--
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 v3 30/32] arm64: KVM: enable initialization of a 32bit vcpu

2013-04-24 Thread Christoffer Dall
On Wed, Apr 24, 2013 at 6:49 AM, Marc Zyngier marc.zyng...@arm.com wrote:
 On 24/04/13 00:02, Christoffer Dall wrote:
 On Mon, Apr 08, 2013 at 05:17:32PM +0100, Marc Zyngier wrote:
 Wire the init of a 32bit vcpu by allowing 32bit modes in pstate,
 and providing sensible defaults out of reset state.

 This feature is of course conditioned by the presence of 32bit
 capability on the physical CPU, and is checked by the KVM_CAP_ARM_EL1_32BIT
 capability.

 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm64/include/asm/kvm_host.h |  2 +-
  arch/arm64/include/uapi/asm/kvm.h |  1 +
  arch/arm64/kvm/guest.c|  6 ++
  arch/arm64/kvm/reset.c| 25 -
  include/uapi/linux/kvm.h  |  1 +
  5 files changed, 33 insertions(+), 2 deletions(-)

 diff --git a/arch/arm64/include/asm/kvm_host.h 
 b/arch/arm64/include/asm/kvm_host.h
 index d44064d..c3ec107 100644
 --- a/arch/arm64/include/asm/kvm_host.h
 +++ b/arch/arm64/include/asm/kvm_host.h
 @@ -34,7 +34,7 @@
  #include asm/kvm_vgic.h
  #include asm/kvm_arch_timer.h

 -#define KVM_VCPU_MAX_FEATURES 1
 +#define KVM_VCPU_MAX_FEATURES 2

  /* We don't currently support large pages. */
  #define KVM_HPAGE_GFN_SHIFT(x)  0
 diff --git a/arch/arm64/include/uapi/asm/kvm.h 
 b/arch/arm64/include/uapi/asm/kvm.h
 index 5b1110c..5031f42 100644
 --- a/arch/arm64/include/uapi/asm/kvm.h
 +++ b/arch/arm64/include/uapi/asm/kvm.h
 @@ -75,6 +75,7 @@ struct kvm_regs {
  #define KVM_VGIC_V2_CPU_SIZE0x2000

  #define KVM_ARM_VCPU_POWER_OFF  0 /* CPU is started in OFF 
 state */
 +#define KVM_ARM_VCPU_EL1_32BIT  1 /* CPU running a 32bit VM */

  struct kvm_vcpu_init {
  __u32 target;
 diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
 index 47d3729..74ef7d5 100644
 --- a/arch/arm64/kvm/guest.c
 +++ b/arch/arm64/kvm/guest.c
 @@ -93,6 +93,12 @@ static int set_core_reg(struct kvm_vcpu *vcpu, const 
 struct kvm_one_reg *reg)
  if (off == KVM_REG_ARM_CORE_REG(regs.pstate)) {
  unsigned long mode = (*(unsigned long *)valp)  
 COMPAT_PSR_MODE_MASK;
  switch (mode) {
 +case COMPAT_PSR_MODE_USR:
 +case COMPAT_PSR_MODE_FIQ:
 +case COMPAT_PSR_MODE_IRQ:
 +case COMPAT_PSR_MODE_SVC:
 +case COMPAT_PSR_MODE_ABT:
 +case COMPAT_PSR_MODE_UND:
  case PSR_MODE_EL0t:
  case PSR_MODE_EL1t:
  case PSR_MODE_EL1h:
 diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
 index bc33e76..a282d35 100644
 --- a/arch/arm64/kvm/reset.c
 +++ b/arch/arm64/kvm/reset.c
 @@ -35,11 +35,27 @@ static struct kvm_regs default_regs_reset = {
  .regs.pstate = PSR_MODE_EL1h | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT,
  };

 +static struct kvm_regs default_regs_reset32 = {
 +.regs.pstate = (COMPAT_PSR_MODE_SVC | COMPAT_PSR_A_BIT |
 +COMPAT_PSR_I_BIT | COMPAT_PSR_F_BIT),
 +};
 +
 +static bool cpu_has_32bit_el1(void)
 +{
 +u64 pfr0;
 +
 +pfr0 = read_cpuid(ID_AA64PFR0_EL1);
 +return !!(pfr0  0x20);

 again we don't need the double negation

 I still hold that it makes things more readable.

 +}
 +
  int kvm_arch_dev_ioctl_check_extention(long ext)
  {
  int r;

  switch (ext) {
 +case KVM_CAP_ARM_EL1_32BIT:
 +r = cpu_has_32bit_el1();
 +break;
  default:
  r = 0;
  }
 @@ -62,7 +78,14 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)

  switch (vcpu-arch.target) {
  default:
 -cpu_reset = default_regs_reset;
 +if (test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu-arch.features)) {
 +if (!cpu_has_32bit_el1())
 +return -EINVAL;

 I'm not sure EINVAL is appropriate here, the value specified was not
 incorrect, it's that the hardware doesn't support it. ENXIO, ENODEV, and
 add that in Documentation/virtual/kvm/api.txt ?

 Not sure. If you ended up here, it means you tried to start a 32bit
 guest on a 64bit-only CPU, despite KVM_CAP_ARM_EL1_32BIT telling you
 that your CPU is not 32bit capable.

 This is clearly an invalid input, isn't it?

check the API documentation for this ioctl, I don't think that's the
type of invalid input meant when describing the meaning of EINVAL. If
you feel strongly about it of course it's no big deal, but I think
EINVAL is so overloaded anyway that telling the user something more
specific would be great, but I'll leave it up to you.
--
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] kvm tools: remove arbitrary minimum RAM limitation

2013-04-24 Thread Sasha Levin
On 04/24/2013 04:25 AM, Michael Ellerman wrote:
 On Tue, 2013-04-23 at 10:57 -0400, Sasha Levin wrote:
 We don't really need 64MB of RAM to boot, it's a nice default if we don't
 have anything else - but it's not actually required for anything:
 
 Nice, I am carrying something similar locally so I can boot in 4K :)

Feel free to send stuff you carry locally over! :)


Thanks,
Sasha

--
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: [Qemu-devel] KVM call minutes for 2013-04-23

2013-04-24 Thread Eric Blake
On 04/23/2013 10:06 AM, Eric Blake wrote:

   we can change drive_mirror to use a new command to see if there
   are the new features.
 
 drive-mirror changed in 1.4 to add optional buf-size parameter; right
 now, libvirt is forced to limit itself to 1.3 interface (no buf-size or
 granularity) because there is no introspection and no query-* command
 that witnesses that the feature is present.  Idea was that we need to
 add a new query-drive-mirror-capabilities (name subject to bikeshedding)
 command into 1.5 that would let libvirt know that buf-size/granularity
 is usable (done right, it would also prevent the situation of buf-size
 being a write-only interface where it is set when starting the mirror
 but can not be queried later to see what size is in use).
 
 Unclear whether anyone was signing up to tackle the addition of a query
 command counterpart for drive-mirror in time for 1.5.

Further discussion on this topic:

Luiz proposed such a command:
https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg04937.html

in the meantime, Paolo and I discussed on IRC, and realized that:

1. Current libvirt does not expose buf-size or granularity to the end
user.  Until a future libvirt release actually wants to use these
options, we are in no rush to get it into qemu 1.5; it's okay to leave
things in the same state they were in for 1.4, and have qemu 1.6 be the
first release that coordinates with a new libvirt release actually
wanting to use the options.

2. At least with drive-mirror, the try-and-fail approach generates a
reasonable-enough error message.  Having a capability query may allow
libvirt to save time and/or give a better quality error message, but
this is one case where not knowing whether the parameter exists is not a
fatal flaw to the algorithm, since libvirt can still gracefully recover
from attempting to use the parameter (only when the user requested a
non-default value).  Distros that want to prove their value-added
quality-of-implementation can easily backport whatever solution goes
into qemu 1.6 for nicer detection into the distro stable build, even if
the distro is based on 1.5; libvirt will use the nicer detection when
available but should have no problems using the try-and-fail approach
otherwise.

So at this point, I'm comfortable with not trying to add anything into
1.5 dealing with drive-mirror; it feels like too much of a new feature
past soft freeze with no known current client, where we would be better
off waiting to 1.6 to get the interface right.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] Para-Virtualized Clock Usage

2013-04-24 Thread Marcelo Tosatti
On Tue, Apr 23, 2013 at 08:52:16AM +0300, Gleb Natapov wrote:
 On Mon, Apr 22, 2013 at 04:58:01PM +, Joji Mekkattuparamban (joji) wrote:
  Greetings,
  
  I have a SMP guest application, running on the 2.6.27 Linux kernel. The 
  application, originally written for bare metal, makes extensive use of the 
  TSC, by directly invoking rdtsc from the user space for timestamp purposes. 
  While running on KVM (RHEL version 6.3), we are running into TSC issues on 
  some hardware. As a solution, I am considering migrating to the pvclock. I 
  am wondering if there is an example for migrating from TSC to the pvclock. 
  Any pointers?
  
 Wrong list, you should ask KVM (copied). Recent kernels have pvclock
 vdso support which means that gettimeofday() uses it without entering
 the kernel. Marcelo?
 
 --
   Gleb.

Converting application to make use of gettimeofday() should be the
best way to make use of pvclock, yes.

--
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: [Qemu-devel] Para-Virtualized Clock Usage

2013-04-24 Thread Joji Mekkattuparamban (joji)
Thank you Gleb and Marcelo. I will migrate the API using gettimeofday.

Is there any dependency on the QEMU or the Guest? If the host supports pvclock 
and the guest invokes gettimeofday, would the pvclock be automatically used? Or 
do I require a patch in either the Qemu or the guest kernel?

Thanks!
Joji.

-Original Message-
From: Marcelo Tosatti [mailto:mtosa...@redhat.com] 
Sent: Wednesday, April 24, 2013 1:28 AM
To: Gleb Natapov
Cc: Joji Mekkattuparamban (joji); qemu-de...@nongnu.org; kvm@vger.kernel.org
Subject: Re: [Qemu-devel] Para-Virtualized Clock Usage

On Tue, Apr 23, 2013 at 08:52:16AM +0300, Gleb Natapov wrote:
 On Mon, Apr 22, 2013 at 04:58:01PM +, Joji Mekkattuparamban (joji) wrote:
  Greetings,
  
  I have a SMP guest application, running on the 2.6.27 Linux kernel. The 
  application, originally written for bare metal, makes extensive use of the 
  TSC, by directly invoking rdtsc from the user space for timestamp purposes. 
  While running on KVM (RHEL version 6.3), we are running into TSC issues on 
  some hardware. As a solution, I am considering migrating to the pvclock. I 
  am wondering if there is an example for migrating from TSC to the pvclock. 
  Any pointers?
  
 Wrong list, you should ask KVM (copied). Recent kernels have pvclock 
 vdso support which means that gettimeofday() uses it without entering 
 the kernel. Marcelo?
 
 --
   Gleb.

Converting application to make use of gettimeofday() should be the best way to 
make use of pvclock, yes.

--
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


[PATCH v9 0/3] tcm_vhost hotplug

2013-04-24 Thread Asias He
Asias He (3):
  tcm_vhost: Refactor the lock nesting rule
  tcm_vhost: Add hotplug/hotunplug support
  tcm_vhost: Add ioctl to get and set events missed flag

 drivers/vhost/tcm_vhost.c | 263 +++---
 drivers/vhost/tcm_vhost.h |  13 +++
 2 files changed, 260 insertions(+), 16 deletions(-)

-- 
1.8.1.4

--
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


[PATCH v9 1/3] tcm_vhost: Refactor the lock nesting rule

2013-04-24 Thread Asias He
We want to use tcm_vhost_mutex to make sure hotplug/hotunplug will not
happen when set_endpoint/clear_endpoint is in process.

Signed-off-by: Asias He as...@redhat.com
---
 drivers/vhost/tcm_vhost.c | 32 +++-
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
index 957a0b9..822cd1f 100644
--- a/drivers/vhost/tcm_vhost.c
+++ b/drivers/vhost/tcm_vhost.c
@@ -808,6 +808,9 @@ static void vhost_scsi_flush(struct vhost_scsi *vs)
 /*
  * Called from vhost_scsi_ioctl() context to walk the list of available
  * tcm_vhost_tpg with an active struct tcm_vhost_nexus
+ *
+ *  The lock nesting rule is:
+ *tcm_vhost_mutex - vs-dev.mutex - tpg-tv_tpg_mutex - vq-mutex
  */
 static int vhost_scsi_set_endpoint(
struct vhost_scsi *vs,
@@ -820,26 +823,27 @@ static int vhost_scsi_set_endpoint(
int index, ret, i, len;
bool match = false;
 
+   mutex_lock(tcm_vhost_mutex);
mutex_lock(vs-dev.mutex);
+
/* Verify that ring has been setup correctly. */
for (index = 0; index  vs-dev.nvqs; ++index) {
/* Verify that ring has been setup correctly. */
if (!vhost_vq_access_ok(vs-vqs[index])) {
-   mutex_unlock(vs-dev.mutex);
-   return -EFAULT;
+   ret = -EFAULT;
+   goto out;
}
}
 
len = sizeof(vs_tpg[0]) * VHOST_SCSI_MAX_TARGET;
vs_tpg = kzalloc(len, GFP_KERNEL);
if (!vs_tpg) {
-   mutex_unlock(vs-dev.mutex);
-   return -ENOMEM;
+   ret = -ENOMEM;
+   goto out;
}
if (vs-vs_tpg)
memcpy(vs_tpg, vs-vs_tpg, len);
 
-   mutex_lock(tcm_vhost_mutex);
list_for_each_entry(tv_tpg, tcm_vhost_list, tv_tpg_list) {
mutex_lock(tv_tpg-tv_tpg_mutex);
if (!tv_tpg-tpg_nexus) {
@@ -854,11 +858,10 @@ static int vhost_scsi_set_endpoint(
 
if (!strcmp(tv_tport-tport_name, t-vhost_wwpn)) {
if (vs-vs_tpg  vs-vs_tpg[tv_tpg-tport_tpgt]) {
-   mutex_unlock(tv_tpg-tv_tpg_mutex);
-   mutex_unlock(tcm_vhost_mutex);
-   mutex_unlock(vs-dev.mutex);
kfree(vs_tpg);
-   return -EEXIST;
+   mutex_unlock(tv_tpg-tv_tpg_mutex);
+   ret = -EEXIST;
+   goto out;
}
tv_tpg-tv_tpg_vhost_count++;
vs_tpg[tv_tpg-tport_tpgt] = tv_tpg;
@@ -867,7 +870,6 @@ static int vhost_scsi_set_endpoint(
}
mutex_unlock(tv_tpg-tv_tpg_mutex);
}
-   mutex_unlock(tcm_vhost_mutex);
 
if (match) {
memcpy(vs-vs_vhost_wwpn, t-vhost_wwpn,
@@ -893,7 +895,9 @@ static int vhost_scsi_set_endpoint(
kfree(vs-vs_tpg);
vs-vs_tpg = vs_tpg;
 
+out:
mutex_unlock(vs-dev.mutex);
+   mutex_unlock(tcm_vhost_mutex);
return ret;
 }
 
@@ -908,6 +912,7 @@ static int vhost_scsi_clear_endpoint(
int index, ret, i;
u8 target;
 
+   mutex_lock(tcm_vhost_mutex);
mutex_lock(vs-dev.mutex);
/* Verify that ring has been setup correctly. */
for (index = 0; index  vs-dev.nvqs; ++index) {
@@ -918,8 +923,8 @@ static int vhost_scsi_clear_endpoint(
}
 
if (!vs-vs_tpg) {
-   mutex_unlock(vs-dev.mutex);
-   return 0;
+   ret = 0;
+   goto err_dev;
}
 
for (i = 0; i  VHOST_SCSI_MAX_TARGET; i++) {
@@ -965,13 +970,14 @@ static int vhost_scsi_clear_endpoint(
kfree(vs-vs_tpg);
vs-vs_tpg = NULL;
mutex_unlock(vs-dev.mutex);
-
+   mutex_unlock(tcm_vhost_mutex);
return 0;
 
 err_tpg:
mutex_unlock(tv_tpg-tv_tpg_mutex);
 err_dev:
mutex_unlock(vs-dev.mutex);
+   mutex_unlock(tcm_vhost_mutex);
return ret;
 }
 
-- 
1.8.1.4

--
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


[PATCH v9 2/3] tcm_vhost: Add hotplug/hotunplug support

2013-04-24 Thread Asias He
In commit 365a7150094 ([SCSI] virtio-scsi: hotplug support for
virtio-scsi), hotplug support is added to virtio-scsi.

This patch adds hotplug and hotunplug support to tcm_vhost.

You can create or delete a LUN in targetcli to hotplug or hotunplug a
LUN in guest.

Changes in v8:
- Use vq-mutex for event
- Drop tcm_vhost: Add helper to check if endpoint is setup
- Rename vs_events_dropped to vs_events_missed
- Init lun[] explicitly

Changes in v7:
- Add vhost_work_flush for vs-vs_event_work to this series

Changes in v6:
- Pass tcm_vhost_evt to tcm_vhost_do_evt_work

Changes in v5:
- Switch to int from u64 to vs_events_nr
- Set s-vs_events_dropped flag in tcm_vhost_allocate_evt
- Do not nest dev mutex within vq mutex
- Use vs_events_lock to protect vs_events_dropped and vs_events_nr
- Rebase to target/master

Changes in v4:
- Drop tcm_vhost_check_endpoint in tcm_vhost_send_evt
- Add tcm_vhost_check_endpoint in vhost_scsi_evt_handle_kick

Changes in v3:
- Separate the bug fix to another thread

Changes in v2:
- Remove code duplication in tcm_vhost_{hotplug,hotunplug}
- Fix racing of vs_events_nr
- Add flush fix patch to this series

Signed-off-by: Asias He as...@redhat.com
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
---
 drivers/vhost/tcm_vhost.c | 214 +-
 drivers/vhost/tcm_vhost.h |  10 +++
 2 files changed, 221 insertions(+), 3 deletions(-)

diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
index 822cd1f..137ae34 100644
--- a/drivers/vhost/tcm_vhost.c
+++ b/drivers/vhost/tcm_vhost.c
@@ -66,11 +66,13 @@ enum {
  * TODO: debug and remove the workaround.
  */
 enum {
-   VHOST_SCSI_FEATURES = VHOST_FEATURES  (~VIRTIO_RING_F_EVENT_IDX)
+   VHOST_SCSI_FEATURES = (VHOST_FEATURES  (~VIRTIO_RING_F_EVENT_IDX)) |
+ (1ULL  VIRTIO_SCSI_F_HOTPLUG)
 };
 
 #define VHOST_SCSI_MAX_TARGET  256
 #define VHOST_SCSI_MAX_VQ  128
+#define VHOST_SCSI_MAX_EVENT   128
 
 struct vhost_scsi {
/* Protected by vhost_scsi-dev.mutex */
@@ -82,6 +84,12 @@ struct vhost_scsi {
 
struct vhost_work vs_completion_work; /* cmd completion work item */
struct llist_head vs_completion_list; /* cmd completion queue */
+
+   struct vhost_work vs_event_work; /* evt injection work item */
+   struct llist_head vs_event_list; /* evt injection queue */
+
+   bool vs_events_missed; /* any missed events, protected by vq-mutex */
+   int vs_events_nr; /* num of pending events, protected by vq-mutex */
 };
 
 /* Local pointer to allocated TCM configfs fabric module */
@@ -349,6 +357,37 @@ static int tcm_vhost_queue_tm_rsp(struct se_cmd *se_cmd)
return 0;
 }
 
+static void tcm_vhost_free_evt(struct vhost_scsi *vs, struct tcm_vhost_evt 
*evt)
+{
+   vs-vs_events_nr--;
+   kfree(evt);
+}
+
+static struct tcm_vhost_evt *tcm_vhost_allocate_evt(struct vhost_scsi *vs,
+   u32 event, u32 reason)
+{
+   struct vhost_virtqueue *vq = vs-vqs[VHOST_SCSI_VQ_EVT];
+   struct tcm_vhost_evt *evt;
+
+   if (vs-vs_events_nr  VHOST_SCSI_MAX_EVENT) {
+   vs-vs_events_missed = true;
+   return NULL;
+   }
+
+   evt = kzalloc(sizeof(*evt), GFP_KERNEL);
+   if (!evt) {
+   vq_err(vq, Failed to allocate tcm_vhost_evt\n);
+   vs-vs_events_missed = true;
+   return NULL;
+   }
+
+   evt-event.event = event;
+   evt-event.reason = reason;
+   vs-vs_events_nr++;
+
+   return evt;
+}
+
 static void vhost_scsi_free_cmd(struct tcm_vhost_cmd *tv_cmd)
 {
struct se_cmd *se_cmd = tv_cmd-tvc_se_cmd;
@@ -367,6 +406,75 @@ static void vhost_scsi_free_cmd(struct tcm_vhost_cmd 
*tv_cmd)
kfree(tv_cmd);
 }
 
+static void tcm_vhost_do_evt_work(struct vhost_scsi *vs,
+   struct tcm_vhost_evt *evt)
+{
+   struct vhost_virtqueue *vq = vs-vqs[VHOST_SCSI_VQ_EVT];
+   struct virtio_scsi_event *event = evt-event;
+   struct virtio_scsi_event __user *eventp;
+   unsigned out, in;
+   int head, ret;
+
+   if (!vq-private_data) {
+   vs-vs_events_missed = true;
+   return;
+   }
+
+again:
+   vhost_disable_notify(vs-dev, vq);
+   head = vhost_get_vq_desc(vs-dev, vq, vq-iov,
+   ARRAY_SIZE(vq-iov), out, in,
+   NULL, NULL);
+   if (head  0) {
+   vs-vs_events_missed = true;
+   return;
+   }
+   if (head == vq-num) {
+   if (vhost_enable_notify(vs-dev, vq))
+   goto again;
+   vs-vs_events_missed = true;
+   return;
+   }
+
+   if ((vq-iov[out].iov_len != sizeof(struct virtio_scsi_event))) {
+   vq_err(vq, Expecting virtio_scsi_event, got %zu bytes\n,
+   vq-iov[out].iov_len);
+   vs-vs_events_missed = true;
+   return;
+   }
+
+   if (vs-vs_events_missed) {
+ 

[PATCH v9 3/3] tcm_vhost: Add ioctl to get and set events missed flag

2013-04-24 Thread Asias He
Signed-off-by: Asias He as...@redhat.com
---
 drivers/vhost/tcm_vhost.c | 17 +
 drivers/vhost/tcm_vhost.h |  3 +++
 2 files changed, 20 insertions(+)

diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
index 137ae34..9c01d2a 100644
--- a/drivers/vhost/tcm_vhost.c
+++ b/drivers/vhost/tcm_vhost.c
@@ -1202,8 +1202,11 @@ static long vhost_scsi_ioctl(struct file *f, unsigned 
int ioctl,
struct vhost_scsi_target backend;
void __user *argp = (void __user *)arg;
u64 __user *featurep = argp;
+   u32 __user *eventsp = argp;
+   u32 events_missed;
u64 features;
int r, abi_version = VHOST_SCSI_ABI_VERSION;
+   struct vhost_virtqueue *vq = vs-vqs[VHOST_SCSI_VQ_EVT];
 
switch (ioctl) {
case VHOST_SCSI_SET_ENDPOINT:
@@ -1224,6 +1227,20 @@ static long vhost_scsi_ioctl(struct file *f, unsigned 
int ioctl,
if (copy_to_user(argp, abi_version, sizeof abi_version))
return -EFAULT;
return 0;
+   case VHOST_SCSI_SET_EVENTS_MISSED:
+   if (get_user(events_missed, eventsp))
+   return -EFAULT;
+   mutex_lock(vq-mutex);
+   vs-vs_events_missed = events_missed;
+   mutex_unlock(vq-mutex);
+   return 0;
+   case VHOST_SCSI_GET_EVENTS_MISSED:
+   mutex_lock(vq-mutex);
+   events_missed = vs-vs_events_missed;
+   mutex_unlock(vq-mutex);
+   if (put_user(events_missed, eventsp))
+   return -EFAULT;
+   return 0;
case VHOST_GET_FEATURES:
features = VHOST_SCSI_FEATURES;
if (copy_to_user(featurep, features, sizeof features))
diff --git a/drivers/vhost/tcm_vhost.h b/drivers/vhost/tcm_vhost.h
index a545a5b..514b9fd 100644
--- a/drivers/vhost/tcm_vhost.h
+++ b/drivers/vhost/tcm_vhost.h
@@ -123,3 +123,6 @@ struct vhost_scsi_target {
 #define VHOST_SCSI_CLEAR_ENDPOINT _IOW(VHOST_VIRTIO, 0x41, struct 
vhost_scsi_target)
 /* Changing this breaks userspace. */
 #define VHOST_SCSI_GET_ABI_VERSION _IOW(VHOST_VIRTIO, 0x42, int)
+/* Set and get the events missed flag */
+#define VHOST_SCSI_SET_EVENTS_MISSED _IOW(VHOST_VIRTIO, 0x43, __u32)
+#define VHOST_SCSI_GET_EVENTS_MISSED _IOW(VHOST_VIRTIO, 0x44, __u32)
-- 
1.8.1.4

--
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 v9 1/3] tcm_vhost: Refactor the lock nesting rule

2013-04-24 Thread Michael S. Tsirkin
On Thu, Apr 25, 2013 at 11:10:04AM +0800, Asias He wrote:
 We want to use tcm_vhost_mutex to make sure hotplug/hotunplug will not
 happen when set_endpoint/clear_endpoint is in process.
 
 Signed-off-by: Asias He as...@redhat.com

Acked-by: Michael S. Tsirkin m...@redhat.com

 ---
  drivers/vhost/tcm_vhost.c | 32 +++-
  1 file changed, 19 insertions(+), 13 deletions(-)
 
 diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
 index 957a0b9..822cd1f 100644
 --- a/drivers/vhost/tcm_vhost.c
 +++ b/drivers/vhost/tcm_vhost.c
 @@ -808,6 +808,9 @@ static void vhost_scsi_flush(struct vhost_scsi *vs)
  /*
   * Called from vhost_scsi_ioctl() context to walk the list of available
   * tcm_vhost_tpg with an active struct tcm_vhost_nexus
 + *
 + *  The lock nesting rule is:
 + *tcm_vhost_mutex - vs-dev.mutex - tpg-tv_tpg_mutex - vq-mutex
   */
  static int vhost_scsi_set_endpoint(
   struct vhost_scsi *vs,
 @@ -820,26 +823,27 @@ static int vhost_scsi_set_endpoint(
   int index, ret, i, len;
   bool match = false;
  
 + mutex_lock(tcm_vhost_mutex);
   mutex_lock(vs-dev.mutex);
 +
   /* Verify that ring has been setup correctly. */
   for (index = 0; index  vs-dev.nvqs; ++index) {
   /* Verify that ring has been setup correctly. */
   if (!vhost_vq_access_ok(vs-vqs[index])) {
 - mutex_unlock(vs-dev.mutex);
 - return -EFAULT;
 + ret = -EFAULT;
 + goto out;
   }
   }
  
   len = sizeof(vs_tpg[0]) * VHOST_SCSI_MAX_TARGET;
   vs_tpg = kzalloc(len, GFP_KERNEL);
   if (!vs_tpg) {
 - mutex_unlock(vs-dev.mutex);
 - return -ENOMEM;
 + ret = -ENOMEM;
 + goto out;
   }
   if (vs-vs_tpg)
   memcpy(vs_tpg, vs-vs_tpg, len);
  
 - mutex_lock(tcm_vhost_mutex);
   list_for_each_entry(tv_tpg, tcm_vhost_list, tv_tpg_list) {
   mutex_lock(tv_tpg-tv_tpg_mutex);
   if (!tv_tpg-tpg_nexus) {
 @@ -854,11 +858,10 @@ static int vhost_scsi_set_endpoint(
  
   if (!strcmp(tv_tport-tport_name, t-vhost_wwpn)) {
   if (vs-vs_tpg  vs-vs_tpg[tv_tpg-tport_tpgt]) {
 - mutex_unlock(tv_tpg-tv_tpg_mutex);
 - mutex_unlock(tcm_vhost_mutex);
 - mutex_unlock(vs-dev.mutex);
   kfree(vs_tpg);
 - return -EEXIST;
 + mutex_unlock(tv_tpg-tv_tpg_mutex);
 + ret = -EEXIST;
 + goto out;
   }
   tv_tpg-tv_tpg_vhost_count++;
   vs_tpg[tv_tpg-tport_tpgt] = tv_tpg;
 @@ -867,7 +870,6 @@ static int vhost_scsi_set_endpoint(
   }
   mutex_unlock(tv_tpg-tv_tpg_mutex);
   }
 - mutex_unlock(tcm_vhost_mutex);
  
   if (match) {
   memcpy(vs-vs_vhost_wwpn, t-vhost_wwpn,
 @@ -893,7 +895,9 @@ static int vhost_scsi_set_endpoint(
   kfree(vs-vs_tpg);
   vs-vs_tpg = vs_tpg;
  
 +out:
   mutex_unlock(vs-dev.mutex);
 + mutex_unlock(tcm_vhost_mutex);
   return ret;
  }
  
 @@ -908,6 +912,7 @@ static int vhost_scsi_clear_endpoint(
   int index, ret, i;
   u8 target;
  
 + mutex_lock(tcm_vhost_mutex);
   mutex_lock(vs-dev.mutex);
   /* Verify that ring has been setup correctly. */
   for (index = 0; index  vs-dev.nvqs; ++index) {
 @@ -918,8 +923,8 @@ static int vhost_scsi_clear_endpoint(
   }
  
   if (!vs-vs_tpg) {
 - mutex_unlock(vs-dev.mutex);
 - return 0;
 + ret = 0;
 + goto err_dev;
   }
  
   for (i = 0; i  VHOST_SCSI_MAX_TARGET; i++) {
 @@ -965,13 +970,14 @@ static int vhost_scsi_clear_endpoint(
   kfree(vs-vs_tpg);
   vs-vs_tpg = NULL;
   mutex_unlock(vs-dev.mutex);
 -
 + mutex_unlock(tcm_vhost_mutex);
   return 0;
  
  err_tpg:
   mutex_unlock(tv_tpg-tv_tpg_mutex);
  err_dev:
   mutex_unlock(vs-dev.mutex);
 + mutex_unlock(tcm_vhost_mutex);
   return ret;
  }
  
 -- 
 1.8.1.4
--
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 v9 2/3] tcm_vhost: Add hotplug/hotunplug support

2013-04-24 Thread Michael S. Tsirkin
On Thu, Apr 25, 2013 at 11:10:05AM +0800, Asias He wrote:
 In commit 365a7150094 ([SCSI] virtio-scsi: hotplug support for
 virtio-scsi), hotplug support is added to virtio-scsi.
 
 This patch adds hotplug and hotunplug support to tcm_vhost.
 
 You can create or delete a LUN in targetcli to hotplug or hotunplug a
 LUN in guest.

Patch looks correct, so
Acked-by: Michael S. Tsirkin m...@redhat.com

A couple of trivial formatting comments.
It's up to you whether to address them.

 Changes in v8:
 - Use vq-mutex for event
 - Drop tcm_vhost: Add helper to check if endpoint is setup
 - Rename vs_events_dropped to vs_events_missed
 - Init lun[] explicitly
 
 Changes in v7:
 - Add vhost_work_flush for vs-vs_event_work to this series
 
 Changes in v6:
 - Pass tcm_vhost_evt to tcm_vhost_do_evt_work
 
 Changes in v5:
 - Switch to int from u64 to vs_events_nr
 - Set s-vs_events_dropped flag in tcm_vhost_allocate_evt
 - Do not nest dev mutex within vq mutex
 - Use vs_events_lock to protect vs_events_dropped and vs_events_nr
 - Rebase to target/master
 
 Changes in v4:
 - Drop tcm_vhost_check_endpoint in tcm_vhost_send_evt
 - Add tcm_vhost_check_endpoint in vhost_scsi_evt_handle_kick
 
 Changes in v3:
 - Separate the bug fix to another thread
 
 Changes in v2:
 - Remove code duplication in tcm_vhost_{hotplug,hotunplug}
 - Fix racing of vs_events_nr
 - Add flush fix patch to this series
 
 Signed-off-by: Asias He as...@redhat.com
 Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
 ---
  drivers/vhost/tcm_vhost.c | 214 
 +-
  drivers/vhost/tcm_vhost.h |  10 +++
  2 files changed, 221 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
 index 822cd1f..137ae34 100644
 --- a/drivers/vhost/tcm_vhost.c
 +++ b/drivers/vhost/tcm_vhost.c
 @@ -66,11 +66,13 @@ enum {
   * TODO: debug and remove the workaround.
   */
  enum {
 - VHOST_SCSI_FEATURES = VHOST_FEATURES  (~VIRTIO_RING_F_EVENT_IDX)
 + VHOST_SCSI_FEATURES = (VHOST_FEATURES  (~VIRTIO_RING_F_EVENT_IDX)) |
 +   (1ULL  VIRTIO_SCSI_F_HOTPLUG)
  };
  
  #define VHOST_SCSI_MAX_TARGET256
  #define VHOST_SCSI_MAX_VQ128
 +#define VHOST_SCSI_MAX_EVENT 128
  
  struct vhost_scsi {
   /* Protected by vhost_scsi-dev.mutex */
 @@ -82,6 +84,12 @@ struct vhost_scsi {
  
   struct vhost_work vs_completion_work; /* cmd completion work item */
   struct llist_head vs_completion_list; /* cmd completion queue */
 +
 + struct vhost_work vs_event_work; /* evt injection work item */
 + struct llist_head vs_event_list; /* evt injection queue */
 +
 + bool vs_events_missed; /* any missed events, protected by vq-mutex */
 + int vs_events_nr; /* num of pending events, protected by vq-mutex */
  };
  
  /* Local pointer to allocated TCM configfs fabric module */
 @@ -349,6 +357,37 @@ static int tcm_vhost_queue_tm_rsp(struct se_cmd *se_cmd)
   return 0;
  }
  
 +static void tcm_vhost_free_evt(struct vhost_scsi *vs, struct tcm_vhost_evt 
 *evt)
 +{
 + vs-vs_events_nr--;
 + kfree(evt);
 +}
 +
 +static struct tcm_vhost_evt *tcm_vhost_allocate_evt(struct vhost_scsi *vs,
 + u32 event, u32 reason)
 +{
 + struct vhost_virtqueue *vq = vs-vqs[VHOST_SCSI_VQ_EVT];
 + struct tcm_vhost_evt *evt;
 +
 + if (vs-vs_events_nr  VHOST_SCSI_MAX_EVENT) {
 + vs-vs_events_missed = true;
 + return NULL;
 + }
 +
 + evt = kzalloc(sizeof(*evt), GFP_KERNEL);
 + if (!evt) {
 + vq_err(vq, Failed to allocate tcm_vhost_evt\n);
 + vs-vs_events_missed = true;
 + return NULL;
 + }
 +
 + evt-event.event = event;
 + evt-event.reason = reason;
 + vs-vs_events_nr++;
 +
 + return evt;
 +}
 +
  static void vhost_scsi_free_cmd(struct tcm_vhost_cmd *tv_cmd)
  {
   struct se_cmd *se_cmd = tv_cmd-tvc_se_cmd;
 @@ -367,6 +406,75 @@ static void vhost_scsi_free_cmd(struct tcm_vhost_cmd 
 *tv_cmd)
   kfree(tv_cmd);
  }
  
 +static void tcm_vhost_do_evt_work(struct vhost_scsi *vs,
 + struct tcm_vhost_evt *evt)
 +{
 + struct vhost_virtqueue *vq = vs-vqs[VHOST_SCSI_VQ_EVT];
 + struct virtio_scsi_event *event = evt-event;
 + struct virtio_scsi_event __user *eventp;
 + unsigned out, in;
 + int head, ret;
 +
 + if (!vq-private_data) {
 + vs-vs_events_missed = true;
 + return;
 + }
 +
 +again:
 + vhost_disable_notify(vs-dev, vq);
 + head = vhost_get_vq_desc(vs-dev, vq, vq-iov,
 + ARRAY_SIZE(vq-iov), out, in,
 + NULL, NULL);
 + if (head  0) {
 + vs-vs_events_missed = true;
 + return;
 + }
 + if (head == vq-num) {
 + if (vhost_enable_notify(vs-dev, vq))
 + goto again;
 + vs-vs_events_missed = true;
 + return;
 + }
 +
 + if ((vq-iov[out].iov_len != sizeof(struct 

Re: [PATCH v9 3/3] tcm_vhost: Add ioctl to get and set events missed flag

2013-04-24 Thread Michael S. Tsirkin
On Thu, Apr 25, 2013 at 11:10:06AM +0800, Asias He wrote:
 Signed-off-by: Asias He as...@redhat.com

Please merge with patch 2 so we can use feature flag to
know whether ioctl is supported.

 ---
  drivers/vhost/tcm_vhost.c | 17 +
  drivers/vhost/tcm_vhost.h |  3 +++
  2 files changed, 20 insertions(+)
 
 diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
 index 137ae34..9c01d2a 100644
 --- a/drivers/vhost/tcm_vhost.c
 +++ b/drivers/vhost/tcm_vhost.c
 @@ -1202,8 +1202,11 @@ static long vhost_scsi_ioctl(struct file *f, unsigned 
 int ioctl,
   struct vhost_scsi_target backend;
   void __user *argp = (void __user *)arg;
   u64 __user *featurep = argp;
 + u32 __user *eventsp = argp;
 + u32 events_missed;
   u64 features;
   int r, abi_version = VHOST_SCSI_ABI_VERSION;
 + struct vhost_virtqueue *vq = vs-vqs[VHOST_SCSI_VQ_EVT];
  
   switch (ioctl) {
   case VHOST_SCSI_SET_ENDPOINT:
 @@ -1224,6 +1227,20 @@ static long vhost_scsi_ioctl(struct file *f, unsigned 
 int ioctl,
   if (copy_to_user(argp, abi_version, sizeof abi_version))
   return -EFAULT;
   return 0;
 + case VHOST_SCSI_SET_EVENTS_MISSED:
 + if (get_user(events_missed, eventsp))
 + return -EFAULT;
 + mutex_lock(vq-mutex);
 + vs-vs_events_missed = events_missed;
 + mutex_unlock(vq-mutex);
 + return 0;
 + case VHOST_SCSI_GET_EVENTS_MISSED:
 + mutex_lock(vq-mutex);
 + events_missed = vs-vs_events_missed;
 + mutex_unlock(vq-mutex);
 + if (put_user(events_missed, eventsp))
 + return -EFAULT;
 + return 0;
   case VHOST_GET_FEATURES:
   features = VHOST_SCSI_FEATURES;
   if (copy_to_user(featurep, features, sizeof features))
 diff --git a/drivers/vhost/tcm_vhost.h b/drivers/vhost/tcm_vhost.h
 index a545a5b..514b9fd 100644
 --- a/drivers/vhost/tcm_vhost.h
 +++ b/drivers/vhost/tcm_vhost.h
 @@ -123,3 +123,6 @@ struct vhost_scsi_target {
  #define VHOST_SCSI_CLEAR_ENDPOINT _IOW(VHOST_VIRTIO, 0x41, struct 
 vhost_scsi_target)
  /* Changing this breaks userspace. */
  #define VHOST_SCSI_GET_ABI_VERSION _IOW(VHOST_VIRTIO, 0x42, int)
 +/* Set and get the events missed flag */
 +#define VHOST_SCSI_SET_EVENTS_MISSED _IOW(VHOST_VIRTIO, 0x43, __u32)
 +#define VHOST_SCSI_GET_EVENTS_MISSED _IOW(VHOST_VIRTIO, 0x44, __u32)
 -- 
 1.8.1.4
--
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 v9 0/3] tcm_vhost hotplug

2013-04-24 Thread Michael S. Tsirkin
On Thu, Apr 25, 2013 at 11:10:03AM +0800, Asias He wrote:
 Asias He (3):
   tcm_vhost: Refactor the lock nesting rule
   tcm_vhost: Add hotplug/hotunplug support
   tcm_vhost: Add ioctl to get and set events missed flag

For the series:
Acked-by: Michael S. Tsirkin m...@redhat.com

Reported a minor formatting nit, we can
address these in tree as well.

  drivers/vhost/tcm_vhost.c | 263 
 +++---
  drivers/vhost/tcm_vhost.h |  13 +++
  2 files changed, 260 insertions(+), 16 deletions(-)
 
 -- 
 1.8.1.4
--
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 v9 3/3] tcm_vhost: Add ioctl to get and set events missed flag

2013-04-24 Thread Michael S. Tsirkin
On Thu, Apr 25, 2013 at 11:10:06AM +0800, Asias He wrote:
 Signed-off-by: Asias He as...@redhat.com

Acked-by: Michael S. Tsirkin m...@redhat.com

 ---
  drivers/vhost/tcm_vhost.c | 17 +
  drivers/vhost/tcm_vhost.h |  3 +++
  2 files changed, 20 insertions(+)
 
 diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
 index 137ae34..9c01d2a 100644
 --- a/drivers/vhost/tcm_vhost.c
 +++ b/drivers/vhost/tcm_vhost.c
 @@ -1202,8 +1202,11 @@ static long vhost_scsi_ioctl(struct file *f, unsigned 
 int ioctl,
   struct vhost_scsi_target backend;
   void __user *argp = (void __user *)arg;
   u64 __user *featurep = argp;
 + u32 __user *eventsp = argp;
 + u32 events_missed;
   u64 features;
   int r, abi_version = VHOST_SCSI_ABI_VERSION;
 + struct vhost_virtqueue *vq = vs-vqs[VHOST_SCSI_VQ_EVT];
  
   switch (ioctl) {
   case VHOST_SCSI_SET_ENDPOINT:
 @@ -1224,6 +1227,20 @@ static long vhost_scsi_ioctl(struct file *f, unsigned 
 int ioctl,
   if (copy_to_user(argp, abi_version, sizeof abi_version))
   return -EFAULT;
   return 0;
 + case VHOST_SCSI_SET_EVENTS_MISSED:
 + if (get_user(events_missed, eventsp))
 + return -EFAULT;
 + mutex_lock(vq-mutex);
 + vs-vs_events_missed = events_missed;
 + mutex_unlock(vq-mutex);
 + return 0;
 + case VHOST_SCSI_GET_EVENTS_MISSED:
 + mutex_lock(vq-mutex);
 + events_missed = vs-vs_events_missed;
 + mutex_unlock(vq-mutex);
 + if (put_user(events_missed, eventsp))
 + return -EFAULT;
 + return 0;
   case VHOST_GET_FEATURES:
   features = VHOST_SCSI_FEATURES;
   if (copy_to_user(featurep, features, sizeof features))
 diff --git a/drivers/vhost/tcm_vhost.h b/drivers/vhost/tcm_vhost.h
 index a545a5b..514b9fd 100644
 --- a/drivers/vhost/tcm_vhost.h
 +++ b/drivers/vhost/tcm_vhost.h
 @@ -123,3 +123,6 @@ struct vhost_scsi_target {
  #define VHOST_SCSI_CLEAR_ENDPOINT _IOW(VHOST_VIRTIO, 0x41, struct 
 vhost_scsi_target)
  /* Changing this breaks userspace. */
  #define VHOST_SCSI_GET_ABI_VERSION _IOW(VHOST_VIRTIO, 0x42, int)
 +/* Set and get the events missed flag */
 +#define VHOST_SCSI_SET_EVENTS_MISSED _IOW(VHOST_VIRTIO, 0x43, __u32)
 +#define VHOST_SCSI_GET_EVENTS_MISSED _IOW(VHOST_VIRTIO, 0x44, __u32)
 -- 
 1.8.1.4
--
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 0/7] KVM: irqfd generalization prepare patch set

2013-04-24 Thread Gleb Natapov
On Tue, Apr 16, 2013 at 07:26:08PM +0200, Alexander Graf wrote:
 The concept of an irqfd and interrupt routing are nothing particularly tied
 into the IOAPIC implementation. In fact, most of the code already is perfectly
 generic.
 
 This patch set decouples most bits of the existing irqchip and irqfd
 implementation to make it reusable for non-IOAPIC platforms, like the PPC 
 MPIC.
 
 I also have a patch that implements working irqfd support on top of these,
 but that requires the in-kernel MPIC implementation to go upstream first, so
 I'm holding off on it until we settled everything there, so the concept
 certainly does work.
 
 Alex
 
Nice cleanup, thanks! Should expect a new series with ifdef
kvm_irqchip and ia64 compilation fixed. The fixes are minor enough for
me to fix them while applying.

 Alexander Graf (7):
   KVM: Add KVM_IRQCHIP_NUM_PINS in addition to KVM_IOAPIC_NUM_PINS
   KVM: Introduce __KVM_HAVE_IRQCHIP
   KVM: Remove kvm_get_intr_delivery_bitmask
   KVM: Move irq routing to generic code
   KVM: Extract generic irqchip logic into irqchip.c
   KVM: Move irq routing setup to irqchip.c
   KVM: Move irqfd resample cap handling to generic code
 
  arch/x86/include/asm/kvm_host.h |2 +
  arch/x86/include/uapi/asm/kvm.h |1 +
  arch/x86/kvm/Makefile   |2 +-
  arch/x86/kvm/x86.c  |1 -
  include/linux/kvm_host.h|   14 +--
  include/trace/events/kvm.h  |   12 ++-
  include/uapi/linux/kvm.h|2 +-
  virt/kvm/assigned-dev.c |   30 -
  virt/kvm/eventfd.c  |6 +-
  virt/kvm/irq_comm.c |  193 +---
  virt/kvm/irqchip.c  |  237 
 +++
  virt/kvm/kvm_main.c |   33 ++
  12 files changed, 297 insertions(+), 236 deletions(-)
  create mode 100644 virt/kvm/irqchip.c

--
Gleb.
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html