Re: [PATCH] kvm: x86: fix comment about {mmu,nested_mmu}.gva_to_gpa

[Paolo Bonzini]

On 30/12/2015 17:26, David Matlack wrote:
> The comment had the meaning of mmu.gva_to_gpa and nested_mmu.gva_to_gpa
> swapped. Fix that, and also add some details describing how each translation
> works.
> 
> Signed-off-by: David Matlack 
> ---
>  arch/x86/kvm/mmu.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index e7c2c14..098a9c2 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -4058,10 +4058,12 @@ static void init_kvm_nested_mmu(struct kvm_vcpu *vcpu)
>   g_context->inject_page_fault = kvm_inject_page_fault;
>  
>   /*
> -  * Note that arch.mmu.gva_to_gpa translates l2_gva to l1_gpa. The
> -  * translation of l2_gpa to l1_gpa addresses is done using the
> -  * arch.nested_mmu.gva_to_gpa function. Basically the gva_to_gpa
> -  * functions between mmu and nested_mmu are swapped.
> +  * Note that arch.mmu.gva_to_gpa translates l2_gpa to l1_gpa using
> +  * L1's nested page tables (e.g. EPT12). The nested translation
> +  * of l2_gva to l1_gpa is done by arch.nested_mmu.gva_to_gpa using
> +  * L2's page tables as the first level of translation and L1's
> +  * nested page tables as the second level of translation. Basically
> +  * the gva_to_gpa functions between mmu and nested_mmu are swapped.
>*/
>   if (!is_paging(vcpu)) {
>   g_context->nx = false;
> 

Applied, thanks.

Paolo
--
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 09/20] KVM: ARM64: Add access handler for event counter register

[Marc Zyngier]

On 22/12/15 08:08, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> These kind of registers include PMEVCNTRn, PMCCNTR and PMXEVCNTR which
> is mapped to PMEVCNTRn.
> 
> The access handler translates all aarch32 register offsets to aarch64
> ones and uses vcpu_sys_reg() to access their values to avoid taking care
> of big endian.
> 
> When reading these registers, return the sum of register value and the
> value perf event counts.
> 
> Signed-off-by: Shannon Zhao 
> ---
>  arch/arm64/kvm/sys_regs.c | 138 
> --
>  1 file changed, 134 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index ed2939b..1818947 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -569,6 +569,57 @@ static bool access_pmu_evtyper(struct kvm_vcpu *vcpu, 
> struct sys_reg_params *p,
>   return true;
>  }
>  
> +static bool access_pmu_evcntr(struct kvm_vcpu *vcpu,
> +   struct sys_reg_params *p,
> +   const struct sys_reg_desc *r)
> +{
> + u64 idx, reg, val;
> +
> + if (!p->is_aarch32) {
> + if (r->CRn == 9 && r->CRm == 13 && r->Op2 == 2)
> + /* PMXEVCNTR_EL0 */
> + reg = 0;
> + else
> + /* PMEVCNTRn_EL0 or PMCCNTR_EL0 */
> + reg = r->reg;
> + } else {
> + if (r->CRn == 9 && r->CRm == 13) {
> + reg = (r->Op2 & 2) ? 0 : PMCCNTR_EL0;
> + } else {
> + reg = ((r->CRm & 3) << 3) | (r->Op2 & 7);
> + reg += PMEVCNTR0_EL0;
> + }
> + }

Same remark about the use of 0 instead of PMSELR_EL0.

> +
> + switch (reg) {
> + case PMEVCNTR0_EL0 ... PMEVCNTR30_EL0:
> + idx = reg - PMEVCNTR0_EL0;
> + if (!pmu_counter_idx_valid(vcpu, idx))
> + return true;
> + break;
> + case PMCCNTR_EL0:
> + idx = ARMV8_CYCLE_IDX;
> + break;
> + default:
> + /* PMXEVCNTR_EL0 */
> + idx = vcpu_sys_reg(vcpu, PMSELR_EL0) & ARMV8_COUNTER_MASK;
> + if (!pmu_counter_idx_valid(vcpu, idx))
> + return true;
> +
> + reg = (idx == ARMV8_CYCLE_IDX) ? PMCCNTR_EL0
> +  : PMEVCNTR0_EL0 + idx;
> + break;
> + }
> +
> + val = kvm_pmu_get_counter_value(vcpu, idx);
> + if (p->is_write)
> + vcpu_sys_reg(vcpu, reg) += (s64)p->regval - val;
> + else
> + p->regval = val;
> +
> + return true;
> +}
> +
>  /* Silly macro to expand the DBG{BCR,BVR,WVR,WCR}n_EL1 registers in one go */
>  #define DBG_BCR_BVR_WCR_WVR_EL1(n)   \
>   /* DBGBVRn_EL1 */   \
> @@ -584,6 +635,13 @@ static bool access_pmu_evtyper(struct kvm_vcpu *vcpu, 
> struct sys_reg_params *p,
>   { Op0(0b10), Op1(0b000), CRn(0b), CRm((n)), Op2(0b111), \
> trap_wcr, reset_wcr, n, 0,  get_wcr, set_wcr }
>  
> +/* Macro to expand the PMEVCNTRn_EL0 register */
> +#define PMU_PMEVCNTR_EL0(n)  \
> + /* PMEVCNTRn_EL0 */ \
> + { Op0(0b11), Op1(0b011), CRn(0b1110),   \
> +   CRm((0b1000 | (((n) >> 3) & 0x3))), Op2(((n) & 0x7)), \
> +   access_pmu_evcntr, reset_unknown, (PMEVCNTR0_EL0 + n), }
> +
>  /* Macro to expand the PMEVTYPERn_EL0 register */
>  #define PMU_PMEVTYPER_EL0(n) \
>   /* PMEVTYPERn_EL0 */\
> @@ -784,13 +842,13 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> access_pmceid },
>   /* PMCCNTR_EL0 */
>   { Op0(0b11), Op1(0b011), CRn(0b1001), CRm(0b1101), Op2(0b000),
> -   trap_raz_wi },
> +   access_pmu_evcntr, reset_unknown, PMCCNTR_EL0 },
>   /* PMXEVTYPER_EL0 */
>   { Op0(0b11), Op1(0b011), CRn(0b1001), CRm(0b1101), Op2(0b001),
> access_pmu_evtyper },
>   /* PMXEVCNTR_EL0 */
>   { Op0(0b11), Op1(0b011), CRn(0b1001), CRm(0b1101), Op2(0b010),
> -   trap_raz_wi },
> +   access_pmu_evcntr },
>   /* PMUSERENR_EL0 */
>   { Op0(0b11), Op1(0b011), CRn(0b1001), CRm(0b1110), Op2(0b000),
> trap_raz_wi },
> @@ -805,6 +863,38 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>   { Op0(0b11), Op1(0b011), CRn(0b1101), CRm(0b), Op2(0b011),
> NULL, reset_unknown, TPIDRRO_EL0 },
>  
> + /* PMEVCNTRn_EL0 */
> + PMU_PMEVCNTR_EL0(0),
> + PMU_PMEVCNTR_EL0(1),
> + PMU_PMEVCNTR_EL0(2),
> + PMU_PMEVCNTR_EL0(3),
> + PMU_PMEVCNTR_EL0(4),
> + PMU_PMEVCNTR_EL0(5),
> + PMU_PMEVCNTR_EL0(6),
> + PMU_PMEVCNTR_EL0(7),
> 

[PATCH] arm64: KVM: Fix AArch64 guest userspace exception injection

[Marc Zyngier]

At the moment, our fault injection is pretty limited. We always
generate a SYNC exception into EL1, as if the fault was actually
from EL1h, no matter how it was generated.

This is obviously wrong, as EL0 can generate faults of its own
(not to mention the pretty-much unused EL1t mode).

This patch fixes it by implementing section D1.10.2 of the ARMv8 ARM,
and in particular table D1-7 ("Vector offsets from vector table base
address"), which describes which vector to use depending on the source
exception level and type (synchronous, IRQ, FIQ or SError).

Signed-off-by: Marc Zyngier 
---
 arch/arm64/kvm/inject_fault.c | 38 +++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
index 648112e..4d1ac81 100644
--- a/arch/arm64/kvm/inject_fault.c
+++ b/arch/arm64/kvm/inject_fault.c
@@ -27,7 +27,11 @@
 
 #define PSTATE_FAULT_BITS_64   (PSR_MODE_EL1h | PSR_A_BIT | PSR_F_BIT | \
 PSR_I_BIT | PSR_D_BIT)
-#define EL1_EXCEPT_SYNC_OFFSET 0x200
+
+#define CURRENT_EL_SP_EL0_VECTOR   0x0
+#define CURRENT_EL_SP_ELx_VECTOR   0x200
+#define LOWER_EL_AArch64_VECTOR0x400
+#define LOWER_EL_AArch32_VECTOR0x600
 
 static void prepare_fault32(struct kvm_vcpu *vcpu, u32 mode, u32 vect_offset)
 {
@@ -97,6 +101,34 @@ static void inject_abt32(struct kvm_vcpu *vcpu, bool 
is_pabt,
*fsr = 0x14;
 }
 
+enum exception_type {
+   except_type_sync= 0,
+   except_type_irq = 0x80,
+   except_type_fiq = 0x100,
+   except_type_serror  = 0x180,
+};
+
+static u64 get_except_vector(struct kvm_vcpu *vcpu, enum exception_type type)
+{
+   u64 exc_offset;
+
+   switch (*vcpu_cpsr(vcpu) & (PSR_MODE_MASK | PSR_MODE32_BIT)) {
+   case PSR_MODE_EL1t:
+   exc_offset = CURRENT_EL_SP_EL0_VECTOR;
+   break;
+   case PSR_MODE_EL1h:
+   exc_offset = CURRENT_EL_SP_ELx_VECTOR;
+   break;
+   case PSR_MODE_EL0t:
+   exc_offset = LOWER_EL_AArch64_VECTOR;
+   break;
+   default:
+   exc_offset = LOWER_EL_AArch32_VECTOR;
+   }
+
+   return vcpu_sys_reg(vcpu, VBAR_EL1) + exc_offset + type;
+}
+
 static void inject_abt64(struct kvm_vcpu *vcpu, bool is_iabt, unsigned long 
addr)
 {
unsigned long cpsr = *vcpu_cpsr(vcpu);
@@ -108,8 +140,8 @@ static void inject_abt64(struct kvm_vcpu *vcpu, bool 
is_iabt, unsigned long addr
*vcpu_spsr(vcpu) = cpsr;
*vcpu_elr_el1(vcpu) = *vcpu_pc(vcpu);
 
+   *vcpu_pc(vcpu) = get_except_vector(vcpu, except_type_sync);
*vcpu_cpsr(vcpu) = PSTATE_FAULT_BITS_64;
-   *vcpu_pc(vcpu) = vcpu_sys_reg(vcpu, VBAR_EL1) + EL1_EXCEPT_SYNC_OFFSET;
 
vcpu_sys_reg(vcpu, FAR_EL1) = addr;
 
@@ -143,8 +175,8 @@ static void inject_undef64(struct kvm_vcpu *vcpu)
*vcpu_spsr(vcpu) = cpsr;
*vcpu_elr_el1(vcpu) = *vcpu_pc(vcpu);
 
+   *vcpu_pc(vcpu) = get_except_vector(vcpu, except_type_sync);
*vcpu_cpsr(vcpu) = PSTATE_FAULT_BITS_64;
-   *vcpu_pc(vcpu) = vcpu_sys_reg(vcpu, VBAR_EL1) + EL1_EXCEPT_SYNC_OFFSET;
 
/*
 * Build an unknown exception, depending on the instruction
-- 
2.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: [PULL] KVM/ARM updates for 4.5

[Paolo Bonzini]

On 24/12/2015 12:12, Marc Zyngier wrote:
> Hi Paolo,
> 
> THis is the first pull request for the 4.5 merge window. Not much in
> terms of features, but a rewrite of our 64bit world switch, making it
> a lot nicer, maintainable, and much more likely to cope with things
> like VHE. Also support 16bit VMIDs for systems that need to run that
> many VMs concurrently.
> 
> I was really hoping that the PMU code would make it this time around,
> but it got slightly delayed, and the holiday season didn't help. If
> we're lucky enough (read: if all known issues have been addressed), I
> may send you another pull request early in the new year.
> 
> In the mean time, please pull!

Pulled, thanks.

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

KVM pci-assign - iommu width is not sufficient for mapped address

[Shyam]

Hi All,

We are using Linux Kernel 3.18.19 for running KVM VM's with
pci-assign'ed SRIOV VF interfaces.

We understand that VFIO is the new recommended way, but unfortunately
it reduces performance significantly on our IO workloads (upto the
order of 40-50%) when compared to pci-passthrough. We run trusted VM's
& expose services to the external world. Since we control the VM's,
IOMMU security with VFIO is not exactly mandatory, but performance is
much more important that we get with pci-assign.

We observe a strange behaviour that has already been discussed in this
forum which is upon a VM spawn it causes the following fault resulting
in qemu-kvm crashing

Jan  7 09:41:57 q6-s1 kernel: [90037.228477] intel_iommu_map: iommu
width (48) is not sufficient for the mapped address (fe001000)
Jan  7 09:41:57 q6-s1 kernel: [90037.308229]
kvm_iommu_map_address:iommu failed to map pfn=95000

We observe that this problem happens only if guest linux running 3.5
kernel is spun up & this problem doesnt happen when running guest
linux with 3.6 kernel (i.e. all guest with kernels like 3.2 etc up
till 3.5 causes the above crash whereas any guest kernel >=3.6 doesnt
cause this issue).

So something changed between kernel 3.5 to 3.6 in the guest that
doesnt expose this problem. We have two questions:
1 - we understand that VFIO suffered a similar problem & it was fixed
with 
https://github.com/qemu/qemu/commit/d3a2fd9b29e43e202315d5e99399b99622469c4a.
Alex Williamson suggested that KVM driver needs an equivalent version
of the fix. Can anybody suggest hints on where this fix should be
made?
2 - Any insights on what changes in linux kernel between 3.5 to 3.6 on
the guest that avoids this problem?

Any helps/input greatly appreciated. Thanks!

--Shyam
--
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/s390: drop unpaired smp_mb

[Christian Borntraeger]

On 01/05/2016 05:20 PM, Michael S. Tsirkin wrote:
> smp_mb on vcpu destroy isn't paired with anything, violating pairing
> rules, and seems to be useless.
> 
> Drop it.
> 
> Signed-off-by: Michael S. Tsirkin 

Applied.
(I had to fix this up a little to match kvm/next)

Thanks

> ---
> 
> Untested.
> 
>  arch/s390/kvm/kvm-s390.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 8465892..7305d2c 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -1195,7 +1195,6 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
>   (__u64) vcpu->arch.sie_block)
>   vcpu->kvm->arch.sca->cpu[vcpu->vcpu_id].sda = 0;
>   }
> - smp_mb();
> 
>   if (kvm_is_ucontrol(vcpu->kvm))
>   gmap_free(vcpu->arch.gmap);
> 

--
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 RESEND] kvm:x86:Make sure kvm_write_guest successes for first call in kvm_write_wall_clock

[Paolo Bonzini]

On 30/12/2015 19:08, Nicholas Krause wrote:
> This makes sure that kvm_write_guest successes for the first call
> in order to make sure that the wall clock is successfully written
> to the host system before being calucated as required by the
> guest system.
> 
> Signed-off-by: Nicholas Krause 
> ---
>  arch/x86/kvm/x86.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index eed3228..7551f30 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1167,7 +1167,8 @@ static void kvm_write_wall_clock(struct kvm *kvm, gpa_t 
> wall_clock)
>  
>   ++version;
>  
> - kvm_write_guest(kvm, wall_clock, , sizeof(version));
> + if (kvm_write_guest(kvm, wall_clock, , sizeof(version)))
> + return;
>  
>   /*
>* The guest calculates current wall clock time by adding
> 

Applied, thanks.

Paolo
--
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] How to reserve guest physical region for ACPI

[Igor Mammedov]

On Tue, 5 Jan 2016 18:43:02 +0200
"Michael S. Tsirkin"  wrote:

> On Tue, Jan 05, 2016 at 05:30:25PM +0100, Igor Mammedov wrote:
> > > > bios-linker-loader is a great interface for initializing some
> > > > guest owned data and linking it together but I think it adds
> > > > unnecessary complexity and is misused if it's used to handle
> > > > device owned data/on device memory in this and VMGID cases.
> > > 
> > > I want a generic interface for guest to enumerate these things.  linker
> > > seems quite reasonable but if you see a reason why it won't do, or want
> > > to propose a better interface, fine.
> > > 
> > > PCI would do, too - though windows guys had concerns about
> > > returning PCI BARs from ACPI.  
> > There were potential issues with pSeries bootloader that treated
> > PCI_CLASS_MEMORY_RAM as conventional RAM but it was fixed.
> > Could you point out to discussion about windows issues?
> > 
> > What VMGEN patches that used PCI for mapping purposes were
> > stuck at, was that it was suggested to use PCI_CLASS_MEMORY_RAM
> > class id but we couldn't agree on it.
> > 
> > VMGEN v13 with full discussion is here
> > https://patchwork.ozlabs.org/patch/443554/
> > So to continue with this route we would need to pick some other
> > driver less class id so windows won't prompt for driver or
> > maybe supply our own driver stub to guarantee that no one
> > would touch it. Any suggestions?  
> 
> Pick any device/vendor id pair for which windows specifies no driver.
> There's a small risk that this will conflict with some
> guest but I think it's minimal.
device/vendor id pair was QEMU specific so doesn't conflicts with anything
issue we were trying to solve was to prevent windows asking for driver
even though it does so only once if told not to ask again.

That's why PCI_CLASS_MEMORY_RAM was selected as it's generic driver-less
device descriptor in INF file which matches as the last resort if
there isn't any other diver that's matched device by device/vendor id pair.

> 
> 
> > > 
> > >   
> > > > There was RFC on list to make BIOS boot from NVDIMM already
> > > > doing some ACPI table lookup/parsing. Now if they were forced
> > > > to also parse and execute AML to initialize QEMU with guest
> > > > allocated address that would complicate them quite a bit.
> > > 
> > > If they just need to find a table by name, it won't be
> > > too bad, would it?  
> > that's what they were doing scanning memory for static NVDIMM table.
> > However if it were DataTable, BIOS side would have to execute
> > AML so that the table address could be told to QEMU.  
> 
> Not at all. You can find any table by its signature without
> parsing AML.
yep, and then BIOS would need to tell its address to QEMU
writing to IO port which is allocated statically in QEMU
for this purpose and is described in AML only on guest side.

> 
> 
> > In case of direct mapping or PCI BAR there is no need to initialize
> > QEMU side from AML.
> > That also saves us IO port where this address should be written
> > if bios-linker-loader approach is used.
> >   
> > >   
> > > > While with NVDIMM control memory region mapped directly by QEMU,
> > > > respective patches don't need in any way to initialize QEMU,
> > > > all they would need just read necessary data from control region.
> > > > 
> > > > Also using bios-linker-loader takes away some usable RAM
> > > > from guest and in the end that doesn't scale,
> > > > the more devices I add the less usable RAM is left for guest OS
> > > > while all the device needs is a piece of GPA address space
> > > > that would belong to it.
> > > 
> > > I don't get this comment. I don't think it's MMIO that is wanted.
> > > If it's backed by qemu virtual memory then it's RAM.  
> > Then why don't allocate video card VRAM the same way and try to explain
> > user that a guest started with '-m 128 -device cirrus-vga,vgamem_mb=64Mb'
> > only has 64Mb of available RAM because of we think that on device VRAM
> > is also RAM.
> > 
> > Maybe I've used MMIO term wrongly here but it roughly reflects the idea
> > that on device memory (whether it's VRAM, NVDIMM control block or VMGEN
> > area) is not allocated from guest's usable RAM (as described in E820)
> > but rather directly mapped in guest's GPA and doesn't consume available
> > RAM as guest sees it. That's also the way it's done on real hardware.
> > 
> > What we need in case of VMGEN ID and NVDIMM is on device memory
> > that could be directly accessed by guest.
> > Both direct mapping or PCI BAR do that job and we could use simple
> > static AML without any patching.  
> 
> At least with VMGEN the issue is that there's an AML method
> that returns the physical address.
> Then if guest OS moves the BAR (which is legal), it will break
> since caller has no way to know it's related to the BAR.
I've found a following MS doc "Firmware Allocation of PCI Device Resources in 
Windows". It looks like when MS implemented resource rebalancing in

Re: [PATCH v8 04/20] KVM: ARM64: Add access handler for PMCR register

[Marc Zyngier]

On 22/12/15 08:07, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> Add reset handler which gets host value of PMCR_EL0 and make writable
> bits architecturally UNKNOWN except PMCR.E which is zero. Add an access
> handler for PMCR.
> 
> Signed-off-by: Shannon Zhao 
> ---
>  arch/arm64/kvm/sys_regs.c | 39 +--
>  1 file changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index e8bf374..c60047e 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -34,6 +34,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  
> @@ -439,6 +440,40 @@ static void reset_mpidr(struct kvm_vcpu *vcpu, const 
> struct sys_reg_desc *r)
>   vcpu_sys_reg(vcpu, MPIDR_EL1) = (1ULL << 31) | mpidr;
>  }
>  
> +static void reset_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> +{
> + u64 pmcr, val;
> +
> + asm volatile("mrs %0, pmcr_el0\n" : "=r" (pmcr));
> + /* Writable bits of PMCR_EL0 (ARMV8_PMCR_MASK) is reset to UNKNOWN
> +  * except PMCR.E resetting to zero.
> +  */
> + val = ((pmcr & ~ARMV8_PMCR_MASK) | (ARMV8_PMCR_MASK & 0xdecafbad))
> +   & (~ARMV8_PMCR_E);
> + vcpu_sys_reg(vcpu, r->reg) = val;
> +}
> +
> +static bool access_pmcr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> + const struct sys_reg_desc *r)
> +{
> + u64 val;
> +
> + if (p->is_write) {
> + /* Only update writeable bits of PMCR */
> + val = vcpu_sys_reg(vcpu, r->reg);
> + val &= ~ARMV8_PMCR_MASK;
> + val |= p->regval & ARMV8_PMCR_MASK;
> + vcpu_sys_reg(vcpu, r->reg) = val;
> + } else {
> + /* PMCR.P & PMCR.C are RAZ */
> + val = vcpu_sys_reg(vcpu, r->reg)
> +   & ~(ARMV8_PMCR_P | ARMV8_PMCR_C);
> + p->regval = val;
> + }

How can that work for 32bit, where r->reg is not populated from the trap
table? You *know* that you are accessing PMCR, so just use PMCR_EL0 as
an index into vcpu_sys_reg() in all cases. You can then drop PMCR_EL0
from the 64bit trap table entry.

> +
> + return true;
> +}
> +
>  /* Silly macro to expand the DBG{BCR,BVR,WVR,WCR}n_EL1 registers in one go */
>  #define DBG_BCR_BVR_WCR_WVR_EL1(n)   \
>   /* DBGBVRn_EL1 */   \
> @@ -623,7 +658,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>  
>   /* PMCR_EL0 */
>   { Op0(0b11), Op1(0b011), CRn(0b1001), CRm(0b1100), Op2(0b000),
> -   trap_raz_wi },
> +   access_pmcr, reset_pmcr, PMCR_EL0, },
>   /* PMCNTENSET_EL0 */
>   { Op0(0b11), Op1(0b011), CRn(0b1001), CRm(0b1100), Op2(0b001),
> trap_raz_wi },
> @@ -885,7 +920,7 @@ static const struct sys_reg_desc cp15_regs[] = {
>   { Op1( 0), CRn( 7), CRm(14), Op2( 2), access_dcsw },
>  
>   /* PMU */
> - { Op1( 0), CRn( 9), CRm(12), Op2( 0), trap_raz_wi },
> + { Op1( 0), CRn( 9), CRm(12), Op2( 0), access_pmcr },
>   { Op1( 0), CRn( 9), CRm(12), Op2( 1), trap_raz_wi },
>   { Op1( 0), CRn( 9), CRm(12), Op2( 2), trap_raz_wi },
>   { Op1( 0), CRn( 9), CRm(12), Op2( 3), trap_raz_wi },
> 

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 11/20] KVM: ARM64: Add access handler for PMINTENSET and PMINTENCLR register

[Marc Zyngier]

On 22/12/15 08:08, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> Since the reset value of PMINTENSET and PMINTENCLR is UNKNOWN, use
> reset_unknown for its reset handler. Add a handler to emulate writing
> PMINTENSET or PMINTENCLR register.
> 
> Signed-off-by: Shannon Zhao 
> ---
>  arch/arm64/kvm/sys_regs.c | 27 +++
>  1 file changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 3416881..24ce4fe 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -644,6 +644,25 @@ static bool access_pmcnten(struct kvm_vcpu *vcpu, struct 
> sys_reg_params *p,
>   return true;
>  }
>  
> +static bool access_pminten(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> +const struct sys_reg_desc *r)
> +{
> + u64 mask = kvm_pmu_valid_counter_mask(vcpu);
> +
> + if (p->is_write) {
> + if (r->Op2 & 0x1)
> + /* accessing PMINTENSET_EL1 */
> + vcpu_sys_reg(vcpu, r->reg) |= (p->regval & mask);
> + else
> + /* accessing PMINTENCLR_EL1 */
> + vcpu_sys_reg(vcpu, r->reg) &= ~(p->regval & mask);
> + } else {
> + p->regval = vcpu_sys_reg(vcpu, r->reg) & mask;
> + }
> +
> + return true;
> +}
> +

Same bug again.

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

[PATCH 2/2] KVM: Remove KVM_REQ_MCLOCK_INPROGRESS to save a bit in vcpu->requests

[Takuya Yoshikawa]

Now that we are running out of the bits in vcpu->requests, using one
of them just to call kvm_make_all_cpus_request() with a valid request
number should be avoided.

This patch achieves this by making kvm_make_all_cpus_request() handle
an empty request.

Signed-off-by: Takuya Yoshikawa 
---
 arch/x86/kvm/x86.c   |  2 --
 include/linux/kvm_host.h | 27 +--
 virt/kvm/kvm_main.c  |  8 +---
 3 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index b6102c1..88260d0 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1701,8 +1701,6 @@ static void kvm_gen_update_masterclock(struct kvm *kvm)
kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
 
/* guest entries allowed */
-   kvm_for_each_vcpu(i, vcpu, kvm)
-   clear_bit(KVM_REQ_MCLOCK_INPROGRESS, >requests);
 
spin_unlock(>pvclock_gtod_sync_lock);
 #endif
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index ca9b93e..bb9ae4f 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -131,19 +131,18 @@ static inline bool is_error_page(struct page *page)
 #define KVM_REQ_PMI   15
 #define KVM_REQ_WATCHDOG  16
 #define KVM_REQ_MASTERCLOCK_UPDATE 17
-#define KVM_REQ_MCLOCK_INPROGRESS 18
-#define KVM_REQ_EPR_EXIT  19
-#define KVM_REQ_SCAN_IOAPIC   20
-#define KVM_REQ_GLOBAL_CLOCK_UPDATE 21
-#define KVM_REQ_ENABLE_IBS22
-#define KVM_REQ_DISABLE_IBS   23
-#define KVM_REQ_APIC_PAGE_RELOAD  24
-#define KVM_REQ_SMI   25
-#define KVM_REQ_HV_CRASH  26
-#define KVM_REQ_IOAPIC_EOI_EXIT   27
-#define KVM_REQ_HV_RESET  28
-#define KVM_REQ_HV_EXIT   29
-#define KVM_REQ_HV_STIMER 30
+#define KVM_REQ_EPR_EXIT  18
+#define KVM_REQ_SCAN_IOAPIC   19
+#define KVM_REQ_GLOBAL_CLOCK_UPDATE 20
+#define KVM_REQ_ENABLE_IBS21
+#define KVM_REQ_DISABLE_IBS   22
+#define KVM_REQ_APIC_PAGE_RELOAD  23
+#define KVM_REQ_SMI   24
+#define KVM_REQ_HV_CRASH  25
+#define KVM_REQ_IOAPIC_EOI_EXIT   26
+#define KVM_REQ_HV_RESET  27
+#define KVM_REQ_HV_EXIT   28
+#define KVM_REQ_HV_STIMER 29
 
 #define KVM_USERSPACE_IRQ_SOURCE_ID0
 #define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID   1
@@ -685,7 +684,7 @@ void kvm_flush_remote_tlbs(struct kvm *kvm);
 void kvm_reload_remote_mmus(struct kvm *kvm);
 void kvm_make_mclock_inprogress_request(struct kvm *kvm);
 void kvm_make_scan_ioapic_request(struct kvm *kvm);
-bool kvm_make_all_cpus_request(struct kvm *kvm, unsigned int req);
+bool kvm_make_all_cpus_request(struct kvm *kvm, int req);
 
 long kvm_arch_dev_ioctl(struct file *filp,
unsigned int ioctl, unsigned long arg);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index be3cef1..0100a19 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -156,7 +156,7 @@ static void ack_flush(void *_completed)
 {
 }
 
-bool kvm_make_all_cpus_request(struct kvm *kvm, unsigned int req)
+bool kvm_make_all_cpus_request(struct kvm *kvm, int req)
 {
int i, cpu, me;
cpumask_var_t cpus;
@@ -167,7 +167,9 @@ bool kvm_make_all_cpus_request(struct kvm *kvm, unsigned 
int req)
 
me = get_cpu();
kvm_for_each_vcpu(i, vcpu, kvm) {
-   kvm_make_request(req, vcpu);
+   if (req >= 0)
+   kvm_make_request(req, vcpu);
+
cpu = vcpu->cpu;
 
/* Set ->requests bit before we read ->mode */
@@ -208,7 +210,7 @@ void kvm_reload_remote_mmus(struct kvm *kvm)
 
 void kvm_make_mclock_inprogress_request(struct kvm *kvm)
 {
-   kvm_make_all_cpus_request(kvm, KVM_REQ_MCLOCK_INPROGRESS);
+   kvm_make_all_cpus_request(kvm, -1);
 }
 
 void kvm_make_scan_ioapic_request(struct kvm *kvm)
-- 
2.1.0



--
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 16/20] KVM: ARM64: Add access handler for PMUSERENR register

[Shannon Zhao]

On 2016/1/7 18:14, Marc Zyngier wrote:
> On 22/12/15 08:08, Shannon Zhao wrote:
>> > From: Shannon Zhao 
>> > 
>> > This register resets as unknown in 64bit mode while it resets as zero
>> > in 32bit mode. Here we choose to reset it as zero for consistency.
>> > 
>> > PMUSERENR_EL0 holds some bits which decide whether PMU registers can be
>> > accessed from EL0. Add some check helpers to handle the access from EL0.
>> > 
>> > When these bits are zero, only reading PMUSERENR will trap to EL2 and
>> > writing PMUSERENR or reading/writing other PMU registers will trap to
>> > EL1 other than EL2 when HCR.TGE==0. To current KVM configuration
>> > (HCR.TGE==0) there is no way to get these traps. Here we write 0xf to
>> > physical PMUSERENR register on VM entry, so that it will trap PMU access
>> > from EL0 to EL2. Within the register access handler we check the real
>> > value of guest PMUSERENR register to decide whether this access is
>> > allowed. If not allowed, forward this trap to EL1.
>> > 
>> > Signed-off-by: Shannon Zhao 
>> > ---
>> >  arch/arm64/include/asm/pmu.h |   9 
>> >  arch/arm64/kvm/hyp/switch.c  |   3 ++
>> >  arch/arm64/kvm/sys_regs.c| 122 
>> > +--
>> >  3 files changed, 129 insertions(+), 5 deletions(-)
>> > 
>> > diff --git a/arch/arm64/include/asm/pmu.h b/arch/arm64/include/asm/pmu.h
>> > index 2588f9c..1238ade 100644
>> > --- a/arch/arm64/include/asm/pmu.h
>> > +++ b/arch/arm64/include/asm/pmu.h
>> > @@ -67,4 +67,13 @@
>> >  #define   ARMV8_EXCLUDE_EL0   (1 << 30)
>> >  #define   ARMV8_INCLUDE_EL2   (1 << 27)
>> >  
>> > +/*
>> > + * PMUSERENR: user enable reg
>> > + */
>> > +#define ARMV8_USERENR_MASK0xf /* Mask for writable 
>> > bits */
>> > +#define ARMV8_USERENR_EN  (1 << 0) /* PMU regs can be accessed at EL0 */
>> > +#define ARMV8_USERENR_SW  (1 << 1) /* PMSWINC can be written at EL0 */
>> > +#define ARMV8_USERENR_CR  (1 << 2) /* Cycle counter can be read at EL0 */
>> > +#define ARMV8_USERENR_ER  (1 << 3) /* Event counter can be read at EL0 */
>> > +
>> >  #endif /* __ASM_PMU_H */
>> > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
>> > index ca8f5a5..a85375f 100644
>> > --- a/arch/arm64/kvm/hyp/switch.c
>> > +++ b/arch/arm64/kvm/hyp/switch.c
>> > @@ -37,6 +37,8 @@ static void __hyp_text __activate_traps(struct kvm_vcpu 
>> > *vcpu)
>> >/* Trap on AArch32 cp15 c15 accesses (EL1 or EL0) */
>> >write_sysreg(1 << 15, hstr_el2);
>> >write_sysreg(CPTR_EL2_TTA | CPTR_EL2_TFP, cptr_el2);
>> > +  /* Make sure we trap PMU access from EL0 to EL2 */
>> > +  write_sysreg(15, pmuserenr_el0);
> Please use the ARMV8_USERENR_* constants here instead of a magic number
> (since you went through the hassle of defining them!).
> 
Ok.

>> >write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2);
>> >  }
>> >  
>> > @@ -45,6 +47,7 @@ static void __hyp_text __deactivate_traps(struct 
>> > kvm_vcpu *vcpu)
>> >write_sysreg(HCR_RW, hcr_el2);
>> >write_sysreg(0, hstr_el2);
>> >write_sysreg(read_sysreg(mdcr_el2) & MDCR_EL2_HPMN_MASK, mdcr_el2);
>> > +  write_sysreg(0, pmuserenr_el0);
>> >write_sysreg(0, cptr_el2);
>> >  }
>> >  
>> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>> > index 04281f1..ac0cbf8 100644
>> > --- a/arch/arm64/kvm/sys_regs.c
>> > +++ b/arch/arm64/kvm/sys_regs.c
>> > @@ -453,11 +453,47 @@ static void reset_pmcr(struct kvm_vcpu *vcpu, const 
>> > struct sys_reg_desc *r)
>> >vcpu_sys_reg(vcpu, r->reg) = val;
>> >  }
>> >  
>> > +static inline bool pmu_access_el0_disabled(struct kvm_vcpu *vcpu)
> Please drop all the inline attributes. The compiler knows its stuff well
> enough to do it automagically, and this is hardly a fast path...
> 
>> > +{
>> > +  u64 reg = vcpu_sys_reg(vcpu, PMUSERENR_EL0);
>> > +
>> > +  return !((reg & ARMV8_USERENR_EN) || vcpu_mode_priv(vcpu));
>> > +}
>> > +
>> > +static inline bool pmu_write_swinc_el0_disabled(struct kvm_vcpu *vcpu)
>> > +{
>> > +  u64 reg = vcpu_sys_reg(vcpu, PMUSERENR_EL0);
>> > +
>> > +  return !((reg & (ARMV8_USERENR_SW | ARMV8_USERENR_EN))
>> > +   || vcpu_mode_priv(vcpu));
>> > +}
>> > +
>> > +static inline bool pmu_access_cycle_counter_el0_disabled(struct kvm_vcpu 
>> > *vcpu)
>> > +{
>> > +  u64 reg = vcpu_sys_reg(vcpu, PMUSERENR_EL0);
>> > +
>> > +  return !((reg & (ARMV8_USERENR_CR | ARMV8_USERENR_EN))
>> > +   || vcpu_mode_priv(vcpu));
>> > +}
>> > +
>> > +static inline bool pmu_access_event_counter_el0_disabled(struct kvm_vcpu 
>> > *vcpu)
>> > +{
>> > +  u64 reg = vcpu_sys_reg(vcpu, PMUSERENR_EL0);
>> > +
>> > +  return !((reg & (ARMV8_USERENR_ER | ARMV8_USERENR_EN))
>> > +   || vcpu_mode_priv(vcpu));
>> > +}
>> > +
>> >  static bool access_pmcr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>> >const struct sys_reg_desc *r)
>> >  {
>> >u64 val;
>> >  
>> > +  if 

Re: [PATCH v8 08/20] KVM: ARM64: Add access handler for event typer register

[Shannon Zhao]

On 2015/12/22 16:08, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> These kind of registers include PMEVTYPERn, PMCCFILTR and PMXEVTYPER
> which is mapped to PMEVTYPERn or PMCCFILTR.
> 
> The access handler translates all aarch32 register offsets to aarch64
> ones and uses vcpu_sys_reg() to access their values to avoid taking care
> of big endian.
> 
> When writing to these registers, create a perf_event for the selected
> event type.
> 
> Signed-off-by: Shannon Zhao 
> ---
>  arch/arm64/kvm/sys_regs.c | 156 
> +-
>  1 file changed, 154 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 2552db1..ed2939b 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -505,6 +505,70 @@ static bool access_pmceid(struct kvm_vcpu *vcpu, struct 
> sys_reg_params *p,
>   return true;
>  }
>  
> +static inline bool pmu_counter_idx_valid(struct kvm_vcpu *vcpu, u64 idx)
> +{
> + u64 pmcr, val;
> +
> + pmcr = vcpu_sys_reg(vcpu, PMCR_EL0);
> + val = (pmcr >> ARMV8_PMCR_N_SHIFT) & ARMV8_PMCR_N_MASK;
> + if (idx >= val && idx != ARMV8_CYCLE_IDX)
> + return false;
> +
> + return true;
> +}
> +
> +static bool access_pmu_evtyper(struct kvm_vcpu *vcpu, struct sys_reg_params 
> *p,
> +const struct sys_reg_desc *r)
> +{
> + u64 idx, reg;
> +
> + if (r->CRn == 9) {
> + /* PMXEVTYPER_EL0 */
> + reg = 0;
> + } else {
> + if (!p->is_aarch32) {
> + /* PMEVTYPERn_EL0 or PMCCFILTR_EL0 */
> + reg = r->reg;
> + } else {
> + if (r->CRn == 14 && r->CRm == 15 && r->Op2 == 7) {
> + reg = PMCCFILTR_EL0;
> + } else {
> + reg = ((r->CRm & 3) << 3) | (r->Op2 & 7);
> + reg += PMEVTYPER0_EL0;
> + }
> + }
> + }
> +
> + switch (reg) {
> + case PMEVTYPER0_EL0 ... PMEVTYPER30_EL0:
> + idx = reg - PMEVTYPER0_EL0;
> + if (!pmu_counter_idx_valid(vcpu, idx))
> + return true;
Hi Marc,

Here should we return false to inject an UND since there is no
PMEVTYPER(idx)_EL0? The ARMv8 spec says it should.

Thanks,
-- 
Shannon

--
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 01/20] ARM64: Move PMU register related defines to asm/pmu.h

[Marc Zyngier]

On 22/12/15 08:07, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> To use the ARMv8 PMU related register defines from the KVM code,
> we move the relevant definitions to asm/pmu.h header file.
> 
> Signed-off-by: Anup Patel 
> Signed-off-by: Shannon Zhao 

Acked-by: Marc Zyngier 

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 06/20] KVM: ARM64: Add access handler for PMCEID0 and PMCEID1 register

[Marc Zyngier]

On 22/12/15 08:08, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> Add access handler which gets host value of PMCEID0 or PMCEID1 when
> guest access these registers. Writing action to PMCEID0 or PMCEID1 is
> UNDEFINED.
> 
> Signed-off-by: Shannon Zhao 
> ---
>  arch/arm64/kvm/sys_regs.c | 27 +++
>  1 file changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index f9985fc..2552db1 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -486,6 +486,25 @@ static bool access_pmselr(struct kvm_vcpu *vcpu, struct 
> sys_reg_params *p,
>   return true;
>  }
>  
> +static bool access_pmceid(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> +   const struct sys_reg_desc *r)
> +{
> + u64 pmceid;
> +
> + if (p->is_write) {
> + kvm_inject_undefined(vcpu);

Just "return false", which will do the right thing.

> + } else {
> + if (!(p->Op2 & 1))
> + asm volatile("mrs %0, pmceid0_el0\n" : "=r" (pmceid));
> + else
> + asm volatile("mrs %0, pmceid1_el0\n" : "=r" (pmceid));
> +
> + p->regval = pmceid;
> + }
> +
> + return true;
> +}
> +
>  /* Silly macro to expand the DBG{BCR,BVR,WVR,WCR}n_EL1 registers in one go */
>  #define DBG_BCR_BVR_WCR_WVR_EL1(n)   \
>   /* DBGBVRn_EL1 */   \
> @@ -688,10 +707,10 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> access_pmselr, reset_unknown, PMSELR_EL0 },
>   /* PMCEID0_EL0 */
>   { Op0(0b11), Op1(0b011), CRn(0b1001), CRm(0b1100), Op2(0b110),
> -   trap_raz_wi },
> +   access_pmceid },
>   /* PMCEID1_EL0 */
>   { Op0(0b11), Op1(0b011), CRn(0b1001), CRm(0b1100), Op2(0b111),
> -   trap_raz_wi },
> +   access_pmceid },
>   /* PMCCNTR_EL0 */
>   { Op0(0b11), Op1(0b011), CRn(0b1001), CRm(0b1101), Op2(0b000),
> trap_raz_wi },
> @@ -937,8 +956,8 @@ static const struct sys_reg_desc cp15_regs[] = {
>   { Op1( 0), CRn( 9), CRm(12), Op2( 2), trap_raz_wi },
>   { Op1( 0), CRn( 9), CRm(12), Op2( 3), trap_raz_wi },
>   { Op1( 0), CRn( 9), CRm(12), Op2( 5), access_pmselr },
> - { Op1( 0), CRn( 9), CRm(12), Op2( 6), trap_raz_wi },
> - { Op1( 0), CRn( 9), CRm(12), Op2( 7), trap_raz_wi },
> + { Op1( 0), CRn( 9), CRm(12), Op2( 6), access_pmceid },
> + { Op1( 0), CRn( 9), CRm(12), Op2( 7), access_pmceid },
>   { Op1( 0), CRn( 9), CRm(13), Op2( 0), trap_raz_wi },
>   { Op1( 0), CRn( 9), CRm(13), Op2( 1), trap_raz_wi },
>   { Op1( 0), CRn( 9), CRm(13), Op2( 2), trap_raz_wi },
> 

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 05/20] KVM: ARM64: Add access handler for PMSELR register

[Marc Zyngier]

On 22/12/15 08:08, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> Since the reset value of PMSELR_EL0 is UNKNOWN, use reset_unknown for
> its reset handler. When reading PMSELR, return the PMSELR.SEL field to
> guest.
> 
> Signed-off-by: Shannon Zhao 
> ---
>  arch/arm64/kvm/sys_regs.c | 16 ++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index c60047e..f9985fc 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -474,6 +474,18 @@ static bool access_pmcr(struct kvm_vcpu *vcpu, struct 
> sys_reg_params *p,
>   return true;
>  }
>  
> +static bool access_pmselr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> +   const struct sys_reg_desc *r)
> +{
> + if (p->is_write)
> + vcpu_sys_reg(vcpu, r->reg) = p->regval;
> + else
> + /* return PMSELR.SEL field */
> + p->regval = vcpu_sys_reg(vcpu, r->reg) & ARMV8_COUNTER_MASK;

Same 32bit bug again.

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 08/20] KVM: ARM64: Add access handler for event typer register

[Shannon Zhao]

On 2016/1/7 19:03, Marc Zyngier wrote:
> On 22/12/15 08:08, Shannon Zhao wrote:
>> > From: Shannon Zhao 
>> > 
>> > These kind of registers include PMEVTYPERn, PMCCFILTR and PMXEVTYPER
>> > which is mapped to PMEVTYPERn or PMCCFILTR.
>> > 
>> > The access handler translates all aarch32 register offsets to aarch64
>> > ones and uses vcpu_sys_reg() to access their values to avoid taking care
>> > of big endian.
>> > 
>> > When writing to these registers, create a perf_event for the selected
>> > event type.
>> > 
>> > Signed-off-by: Shannon Zhao 
>> > ---
>> >  arch/arm64/kvm/sys_regs.c | 156 
>> > +-
>> >  1 file changed, 154 insertions(+), 2 deletions(-)
>> > 
>> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>> > index 2552db1..ed2939b 100644
>> > --- a/arch/arm64/kvm/sys_regs.c
>> > +++ b/arch/arm64/kvm/sys_regs.c
>> > @@ -505,6 +505,70 @@ static bool access_pmceid(struct kvm_vcpu *vcpu, 
>> > struct sys_reg_params *p,
>> >return true;
>> >  }
>> >  
>> > +static inline bool pmu_counter_idx_valid(struct kvm_vcpu *vcpu, u64 idx)
>> > +{
>> > +  u64 pmcr, val;
>> > +
>> > +  pmcr = vcpu_sys_reg(vcpu, PMCR_EL0);
>> > +  val = (pmcr >> ARMV8_PMCR_N_SHIFT) & ARMV8_PMCR_N_MASK;
>> > +  if (idx >= val && idx != ARMV8_CYCLE_IDX)
>> > +  return false;
>> > +
>> > +  return true;
>> > +}
>> > +
>> > +static bool access_pmu_evtyper(struct kvm_vcpu *vcpu, struct 
>> > sys_reg_params *p,
>> > + const struct sys_reg_desc *r)
>> > +{
>> > +  u64 idx, reg;
>> > +
>> > +  if (r->CRn == 9) {
>> > +  /* PMXEVTYPER_EL0 */
>> > +  reg = 0;
> Is there any particular reason why you're not setting reg to PMSELR_EL0,
> since this is what you're using?
> 
>> > +  } else {
>> > +  if (!p->is_aarch32) {
>> > +  /* PMEVTYPERn_EL0 or PMCCFILTR_EL0 */
>> > +  reg = r->reg;
>> > +  } else {
>> > +  if (r->CRn == 14 && r->CRm == 15 && r->Op2 == 7) {
>> > +  reg = PMCCFILTR_EL0;
>> > +  } else {
>> > +  reg = ((r->CRm & 3) << 3) | (r->Op2 & 7);
>> > +  reg += PMEVTYPER0_EL0;
>> > +  }
>> > +  }
>> > +  }
>> > +
>> > +  switch (reg) {
>> > +  case PMEVTYPER0_EL0 ... PMEVTYPER30_EL0:
>> > +  idx = reg - PMEVTYPER0_EL0;
>> > +  if (!pmu_counter_idx_valid(vcpu, idx))
>> > +  return true;
>> > +  break;
>> > +  case PMCCFILTR_EL0:
>> > +  idx = ARMV8_CYCLE_IDX;
>> > +  break;
>> > +  default:
> This would allow this case to be more precise, and we could have the
> default case as a bug handler.
> 
Ah, you're right. Will fix this.

Thanks,
-- 
Shannon

--
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 13/20] KVM: ARM64: Add access handler for PMSWINC register

[Marc Zyngier]

On 22/12/15 08:08, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> Add access handler which emulates writing and reading PMSWINC
> register and add support for creating software increment event.
> 
> Signed-off-by: Shannon Zhao 
> ---
>  arch/arm64/kvm/sys_regs.c | 18 +-
>  include/kvm/arm_pmu.h |  2 ++
>  virt/kvm/arm/pmu.c| 33 +
>  3 files changed, 52 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index d61f271dd..92021dc 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -682,6 +682,21 @@ static bool access_pmovs(struct kvm_vcpu *vcpu, struct 
> sys_reg_params *p,
>   return true;
>  }
>  
> +static bool access_pmswinc(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> +const struct sys_reg_desc *r)
> +{
> + u64 mask;
> +
> + if (p->is_write) {
> + mask = kvm_pmu_valid_counter_mask(vcpu);
> + kvm_pmu_software_increment(vcpu, p->regval & mask);
> + } else {
> + kvm_inject_undefined(vcpu);

"return false;" instead.

> + }
> +
> + return true;
> +}
> +
>  /* Silly macro to expand the DBG{BCR,BVR,WVR,WCR}n_EL1 registers in one go */
>  #define DBG_BCR_BVR_WCR_WVR_EL1(n)   \
>   /* DBGBVRn_EL1 */   \
> @@ -892,7 +907,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> access_pmovs, NULL, PMOVSSET_EL0 },
>   /* PMSWINC_EL0 */
>   { Op0(0b11), Op1(0b011), CRn(0b1001), CRm(0b1100), Op2(0b100),
> -   trap_raz_wi },
> +   access_pmswinc, reset_unknown, PMSWINC_EL0 },
>   /* PMSELR_EL0 */
>   { Op0(0b11), Op1(0b011), CRn(0b1001), CRm(0b1100), Op2(0b101),
> access_pmselr, reset_unknown, PMSELR_EL0 },
> @@ -1231,6 +1246,7 @@ static const struct sys_reg_desc cp15_regs[] = {
>   { Op1( 0), CRn( 9), CRm(12), Op2( 1), access_pmcnten },
>   { Op1( 0), CRn( 9), CRm(12), Op2( 2), access_pmcnten },
>   { Op1( 0), CRn( 9), CRm(12), Op2( 3), access_pmovs },
> + { Op1( 0), CRn( 9), CRm(12), Op2( 4), access_pmswinc },
>   { Op1( 0), CRn( 9), CRm(12), Op2( 5), access_pmselr },
>   { Op1( 0), CRn( 9), CRm(12), Op2( 6), access_pmceid },
>   { Op1( 0), CRn( 9), CRm(12), Op2( 7), access_pmceid },
> diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
> index 244970b..67d168c 100644
> --- a/include/kvm/arm_pmu.h
> +++ b/include/kvm/arm_pmu.h
> @@ -40,6 +40,7 @@ u64 kvm_pmu_valid_counter_mask(struct kvm_vcpu *vcpu);
>  void kvm_pmu_disable_counter(struct kvm_vcpu *vcpu, u64 val);
>  void kvm_pmu_enable_counter(struct kvm_vcpu *vcpu, u64 val);
>  void kvm_pmu_overflow_set(struct kvm_vcpu *vcpu, u64 val);
> +void kvm_pmu_software_increment(struct kvm_vcpu *vcpu, u64 val);
>  void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu, u64 data,
>   u64 select_idx);
>  #else
> @@ -57,6 +58,7 @@ u64 kvm_pmu_valid_counter_mask(struct kvm_vcpu *vcpu)
>  void kvm_pmu_disable_counter(struct kvm_vcpu *vcpu, u64 val) {}
>  void kvm_pmu_enable_counter(struct kvm_vcpu *vcpu, u64 val) {}
>  void kvm_pmu_overflow_set(struct kvm_vcpu *vcpu, u64 val) {}
> +void kvm_pmu_software_increment(struct kvm_vcpu *vcpu, u64 val) {}
>  void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu, u64 data,
>   u64 select_idx) {}
>  #endif
> diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
> index c23d57e..409f3c4 100644
> --- a/virt/kvm/arm/pmu.c
> +++ b/virt/kvm/arm/pmu.c
> @@ -160,6 +160,35 @@ void kvm_pmu_overflow_set(struct kvm_vcpu *vcpu, u64 val)
>   kvm_vcpu_kick(vcpu);
>  }
>  
> +/**
> + * kvm_pmu_software_increment - do software increment
> + * @vcpu: The vcpu pointer
> + * @val: the value guest writes to PMSWINC register
> + */
> +void kvm_pmu_software_increment(struct kvm_vcpu *vcpu, u64 val)
> +{
> + int i;
> + u64 type, enable, reg;
> +
> + if (val == 0)
> + return;
> +
> + for (i = 0; i < ARMV8_CYCLE_IDX; i++) {
> + if (!(val & BIT(i)))
> + continue;
> + type = vcpu_sys_reg(vcpu, PMEVTYPER0_EL0 + i)
> +& ARMV8_EVTYPE_EVENT;
> + enable = vcpu_sys_reg(vcpu, PMCNTENSET_EL0);
> + if ((type == 0) && (enable & BIT(i))) {

nit: Should we have a ARMV8_EVTYPE_EVENT_SW_INCR instead of just
checking for zero?

> + reg = vcpu_sys_reg(vcpu, PMEVCNTR0_EL0 + i) + 1;
> + reg = lower_32_bits(reg);
> + vcpu_sys_reg(vcpu, PMEVCNTR0_EL0 + i) = reg;
> + if (!reg)
> + kvm_pmu_overflow_set(vcpu, BIT(i));
> + }
> + }
> +}
> +
>  static inline bool kvm_pmu_counter_is_enabled(struct kvm_vcpu *vcpu,
>

Re: [PATCH v8 16/20] KVM: ARM64: Add access handler for PMUSERENR register

[Marc Zyngier]

On 22/12/15 08:08, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> This register resets as unknown in 64bit mode while it resets as zero
> in 32bit mode. Here we choose to reset it as zero for consistency.
> 
> PMUSERENR_EL0 holds some bits which decide whether PMU registers can be
> accessed from EL0. Add some check helpers to handle the access from EL0.
> 
> When these bits are zero, only reading PMUSERENR will trap to EL2 and
> writing PMUSERENR or reading/writing other PMU registers will trap to
> EL1 other than EL2 when HCR.TGE==0. To current KVM configuration
> (HCR.TGE==0) there is no way to get these traps. Here we write 0xf to
> physical PMUSERENR register on VM entry, so that it will trap PMU access
> from EL0 to EL2. Within the register access handler we check the real
> value of guest PMUSERENR register to decide whether this access is
> allowed. If not allowed, forward this trap to EL1.
> 
> Signed-off-by: Shannon Zhao 
> ---
>  arch/arm64/include/asm/pmu.h |   9 
>  arch/arm64/kvm/hyp/switch.c  |   3 ++
>  arch/arm64/kvm/sys_regs.c| 122 
> +--
>  3 files changed, 129 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/pmu.h b/arch/arm64/include/asm/pmu.h
> index 2588f9c..1238ade 100644
> --- a/arch/arm64/include/asm/pmu.h
> +++ b/arch/arm64/include/asm/pmu.h
> @@ -67,4 +67,13 @@
>  #define  ARMV8_EXCLUDE_EL0   (1 << 30)
>  #define  ARMV8_INCLUDE_EL2   (1 << 27)
>  
> +/*
> + * PMUSERENR: user enable reg
> + */
> +#define ARMV8_USERENR_MASK   0xf /* Mask for writable bits */
> +#define ARMV8_USERENR_EN (1 << 0) /* PMU regs can be accessed at EL0 */
> +#define ARMV8_USERENR_SW (1 << 1) /* PMSWINC can be written at EL0 */
> +#define ARMV8_USERENR_CR (1 << 2) /* Cycle counter can be read at EL0 */
> +#define ARMV8_USERENR_ER (1 << 3) /* Event counter can be read at EL0 */
> +
>  #endif /* __ASM_PMU_H */
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index ca8f5a5..a85375f 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -37,6 +37,8 @@ static void __hyp_text __activate_traps(struct kvm_vcpu 
> *vcpu)
>   /* Trap on AArch32 cp15 c15 accesses (EL1 or EL0) */
>   write_sysreg(1 << 15, hstr_el2);
>   write_sysreg(CPTR_EL2_TTA | CPTR_EL2_TFP, cptr_el2);
> + /* Make sure we trap PMU access from EL0 to EL2 */
> + write_sysreg(15, pmuserenr_el0);

Please use the ARMV8_USERENR_* constants here instead of a magic number
(since you went through the hassle of defining them!).

>   write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2);
>  }
>  
> @@ -45,6 +47,7 @@ static void __hyp_text __deactivate_traps(struct kvm_vcpu 
> *vcpu)
>   write_sysreg(HCR_RW, hcr_el2);
>   write_sysreg(0, hstr_el2);
>   write_sysreg(read_sysreg(mdcr_el2) & MDCR_EL2_HPMN_MASK, mdcr_el2);
> + write_sysreg(0, pmuserenr_el0);
>   write_sysreg(0, cptr_el2);
>  }
>  
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 04281f1..ac0cbf8 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -453,11 +453,47 @@ static void reset_pmcr(struct kvm_vcpu *vcpu, const 
> struct sys_reg_desc *r)
>   vcpu_sys_reg(vcpu, r->reg) = val;
>  }
>  
> +static inline bool pmu_access_el0_disabled(struct kvm_vcpu *vcpu)

Please drop all the inline attributes. The compiler knows its stuff well
enough to do it automagically, and this is hardly a fast path...

> +{
> + u64 reg = vcpu_sys_reg(vcpu, PMUSERENR_EL0);
> +
> + return !((reg & ARMV8_USERENR_EN) || vcpu_mode_priv(vcpu));
> +}
> +
> +static inline bool pmu_write_swinc_el0_disabled(struct kvm_vcpu *vcpu)
> +{
> + u64 reg = vcpu_sys_reg(vcpu, PMUSERENR_EL0);
> +
> + return !((reg & (ARMV8_USERENR_SW | ARMV8_USERENR_EN))
> +  || vcpu_mode_priv(vcpu));
> +}
> +
> +static inline bool pmu_access_cycle_counter_el0_disabled(struct kvm_vcpu 
> *vcpu)
> +{
> + u64 reg = vcpu_sys_reg(vcpu, PMUSERENR_EL0);
> +
> + return !((reg & (ARMV8_USERENR_CR | ARMV8_USERENR_EN))
> +  || vcpu_mode_priv(vcpu));
> +}
> +
> +static inline bool pmu_access_event_counter_el0_disabled(struct kvm_vcpu 
> *vcpu)
> +{
> + u64 reg = vcpu_sys_reg(vcpu, PMUSERENR_EL0);
> +
> + return !((reg & (ARMV8_USERENR_ER | ARMV8_USERENR_EN))
> +  || vcpu_mode_priv(vcpu));
> +}
> +
>  static bool access_pmcr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>   const struct sys_reg_desc *r)
>  {
>   u64 val;
>  
> + if (pmu_access_el0_disabled(vcpu)) {
> + kvm_forward_trap_to_el1(vcpu);
> + return true;
> + }

So with the patch I posted earlier
(http://www.spinics.net/lists/arm-kernel/msg472693.html), all the
instances similar to that code can be rewritten as

+   if (pmu_access_el0_disabled(vcpu))
+   return 

Re: [PATCH v8 03/20] KVM: ARM64: Add offset defines for PMU registers

[Marc Zyngier]

On 22/12/15 08:07, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> We are about to trap and emulate accesses to each PMU register
> individually. This adds the context offsets for the AArch64 PMU
> registers.
> 
> Signed-off-by: Shannon Zhao 

Reviewed-by: Marc Zyngier 

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] How to reserve guest physical region for ACPI

[Michael S. Tsirkin]

On Thu, Jan 07, 2016 at 11:30:25AM +0100, Igor Mammedov wrote:
> On Tue, 5 Jan 2016 18:43:02 +0200
> "Michael S. Tsirkin"  wrote:
> 
> > On Tue, Jan 05, 2016 at 05:30:25PM +0100, Igor Mammedov wrote:
> > > > > bios-linker-loader is a great interface for initializing some
> > > > > guest owned data and linking it together but I think it adds
> > > > > unnecessary complexity and is misused if it's used to handle
> > > > > device owned data/on device memory in this and VMGID cases.
> > > > 
> > > > I want a generic interface for guest to enumerate these things.  linker
> > > > seems quite reasonable but if you see a reason why it won't do, or want
> > > > to propose a better interface, fine.
> > > > 
> > > > PCI would do, too - though windows guys had concerns about
> > > > returning PCI BARs from ACPI.  
> > > There were potential issues with pSeries bootloader that treated
> > > PCI_CLASS_MEMORY_RAM as conventional RAM but it was fixed.
> > > Could you point out to discussion about windows issues?
> > > 
> > > What VMGEN patches that used PCI for mapping purposes were
> > > stuck at, was that it was suggested to use PCI_CLASS_MEMORY_RAM
> > > class id but we couldn't agree on it.
> > > 
> > > VMGEN v13 with full discussion is here
> > > https://patchwork.ozlabs.org/patch/443554/
> > > So to continue with this route we would need to pick some other
> > > driver less class id so windows won't prompt for driver or
> > > maybe supply our own driver stub to guarantee that no one
> > > would touch it. Any suggestions?  
> > 
> > Pick any device/vendor id pair for which windows specifies no driver.
> > There's a small risk that this will conflict with some
> > guest but I think it's minimal.
> device/vendor id pair was QEMU specific so doesn't conflicts with anything
> issue we were trying to solve was to prevent windows asking for driver
> even though it does so only once if told not to ask again.
> 
> That's why PCI_CLASS_MEMORY_RAM was selected as it's generic driver-less
> device descriptor in INF file which matches as the last resort if
> there isn't any other diver that's matched device by device/vendor id pair.

I think this is the only class in this inf.
If you can't use it, you must use an existing device/vendor id pair,
there's some risk involved but probably not much.

> > 
> > 
> > > > 
> > > >   
> > > > > There was RFC on list to make BIOS boot from NVDIMM already
> > > > > doing some ACPI table lookup/parsing. Now if they were forced
> > > > > to also parse and execute AML to initialize QEMU with guest
> > > > > allocated address that would complicate them quite a bit.
> > > > 
> > > > If they just need to find a table by name, it won't be
> > > > too bad, would it?  
> > > that's what they were doing scanning memory for static NVDIMM table.
> > > However if it were DataTable, BIOS side would have to execute
> > > AML so that the table address could be told to QEMU.  
> > 
> > Not at all. You can find any table by its signature without
> > parsing AML.
> yep, and then BIOS would need to tell its address to QEMU
> writing to IO port which is allocated statically in QEMU
> for this purpose and is described in AML only on guest side.

io ports are an ABI too but they are way easier to
maintain.

> > 
> > 
> > > In case of direct mapping or PCI BAR there is no need to initialize
> > > QEMU side from AML.
> > > That also saves us IO port where this address should be written
> > > if bios-linker-loader approach is used.
> > >   
> > > >   
> > > > > While with NVDIMM control memory region mapped directly by QEMU,
> > > > > respective patches don't need in any way to initialize QEMU,
> > > > > all they would need just read necessary data from control region.
> > > > > 
> > > > > Also using bios-linker-loader takes away some usable RAM
> > > > > from guest and in the end that doesn't scale,
> > > > > the more devices I add the less usable RAM is left for guest OS
> > > > > while all the device needs is a piece of GPA address space
> > > > > that would belong to it.
> > > > 
> > > > I don't get this comment. I don't think it's MMIO that is wanted.
> > > > If it's backed by qemu virtual memory then it's RAM.  
> > > Then why don't allocate video card VRAM the same way and try to explain
> > > user that a guest started with '-m 128 -device cirrus-vga,vgamem_mb=64Mb'
> > > only has 64Mb of available RAM because of we think that on device VRAM
> > > is also RAM.
> > > 
> > > Maybe I've used MMIO term wrongly here but it roughly reflects the idea
> > > that on device memory (whether it's VRAM, NVDIMM control block or VMGEN
> > > area) is not allocated from guest's usable RAM (as described in E820)
> > > but rather directly mapped in guest's GPA and doesn't consume available
> > > RAM as guest sees it. That's also the way it's done on real hardware.
> > > 
> > > What we need in case of VMGEN ID and NVDIMM is on device memory
> > > that could be directly accessed by guest.
> > > Both 

Re: [PATCH v8 04/20] KVM: ARM64: Add access handler for PMCR register

[Shannon Zhao]

On 2016/1/7 18:43, Marc Zyngier wrote:
> On 22/12/15 08:07, Shannon Zhao wrote:
>> > From: Shannon Zhao 
>> > 
>> > Add reset handler which gets host value of PMCR_EL0 and make writable
>> > bits architecturally UNKNOWN except PMCR.E which is zero. Add an access
>> > handler for PMCR.
>> > 
>> > Signed-off-by: Shannon Zhao 
>> > ---
>> >  arch/arm64/kvm/sys_regs.c | 39 +--
>> >  1 file changed, 37 insertions(+), 2 deletions(-)
>> > 
>> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>> > index e8bf374..c60047e 100644
>> > --- a/arch/arm64/kvm/sys_regs.c
>> > +++ b/arch/arm64/kvm/sys_regs.c
>> > @@ -34,6 +34,7 @@
>> >  #include 
>> >  #include 
>> >  #include 
>> > +#include 
>> >  
>> >  #include 
>> >  
>> > @@ -439,6 +440,40 @@ static void reset_mpidr(struct kvm_vcpu *vcpu, const 
>> > struct sys_reg_desc *r)
>> >vcpu_sys_reg(vcpu, MPIDR_EL1) = (1ULL << 31) | mpidr;
>> >  }
>> >  
>> > +static void reset_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc 
>> > *r)
>> > +{
>> > +  u64 pmcr, val;
>> > +
>> > +  asm volatile("mrs %0, pmcr_el0\n" : "=r" (pmcr));
>> > +  /* Writable bits of PMCR_EL0 (ARMV8_PMCR_MASK) is reset to UNKNOWN
>> > +   * except PMCR.E resetting to zero.
>> > +   */
>> > +  val = ((pmcr & ~ARMV8_PMCR_MASK) | (ARMV8_PMCR_MASK & 0xdecafbad))
>> > +& (~ARMV8_PMCR_E);
>> > +  vcpu_sys_reg(vcpu, r->reg) = val;
>> > +}
>> > +
>> > +static bool access_pmcr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>> > +  const struct sys_reg_desc *r)
>> > +{
>> > +  u64 val;
>> > +
>> > +  if (p->is_write) {
>> > +  /* Only update writeable bits of PMCR */
>> > +  val = vcpu_sys_reg(vcpu, r->reg);
>> > +  val &= ~ARMV8_PMCR_MASK;
>> > +  val |= p->regval & ARMV8_PMCR_MASK;
>> > +  vcpu_sys_reg(vcpu, r->reg) = val;
>> > +  } else {
>> > +  /* PMCR.P & PMCR.C are RAZ */
>> > +  val = vcpu_sys_reg(vcpu, r->reg)
>> > +& ~(ARMV8_PMCR_P | ARMV8_PMCR_C);
>> > +  p->regval = val;
>> > +  }
> How can that work for 32bit, where r->reg is not populated from the trap
> table? You *know* that you are accessing PMCR, so just use PMCR_EL0 as
> an index into vcpu_sys_reg() in all cases. You can then drop PMCR_EL0
> from the 64bit trap table entry.
> 
Oh, sorry for this bug. Will fix this and those in other places.

Thanks,
-- 
Shannon

--
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 08/20] KVM: ARM64: Add access handler for event typer register

[Marc Zyngier]

On 07/01/16 12:09, Shannon Zhao wrote:
> 
> 
> On 2015/12/22 16:08, Shannon Zhao wrote:
>> From: Shannon Zhao 
>>
>> These kind of registers include PMEVTYPERn, PMCCFILTR and PMXEVTYPER
>> which is mapped to PMEVTYPERn or PMCCFILTR.
>>
>> The access handler translates all aarch32 register offsets to aarch64
>> ones and uses vcpu_sys_reg() to access their values to avoid taking care
>> of big endian.
>>
>> When writing to these registers, create a perf_event for the selected
>> event type.
>>
>> Signed-off-by: Shannon Zhao 
>> ---
>>  arch/arm64/kvm/sys_regs.c | 156 
>> +-
>>  1 file changed, 154 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>> index 2552db1..ed2939b 100644
>> --- a/arch/arm64/kvm/sys_regs.c
>> +++ b/arch/arm64/kvm/sys_regs.c
>> @@ -505,6 +505,70 @@ static bool access_pmceid(struct kvm_vcpu *vcpu, struct 
>> sys_reg_params *p,
>>  return true;
>>  }
>>  
>> +static inline bool pmu_counter_idx_valid(struct kvm_vcpu *vcpu, u64 idx)
>> +{
>> +u64 pmcr, val;
>> +
>> +pmcr = vcpu_sys_reg(vcpu, PMCR_EL0);
>> +val = (pmcr >> ARMV8_PMCR_N_SHIFT) & ARMV8_PMCR_N_MASK;
>> +if (idx >= val && idx != ARMV8_CYCLE_IDX)
>> +return false;
>> +
>> +return true;
>> +}
>> +
>> +static bool access_pmu_evtyper(struct kvm_vcpu *vcpu, struct sys_reg_params 
>> *p,
>> +   const struct sys_reg_desc *r)
>> +{
>> +u64 idx, reg;
>> +
>> +if (r->CRn == 9) {
>> +/* PMXEVTYPER_EL0 */
>> +reg = 0;
>> +} else {
>> +if (!p->is_aarch32) {
>> +/* PMEVTYPERn_EL0 or PMCCFILTR_EL0 */
>> +reg = r->reg;
>> +} else {
>> +if (r->CRn == 14 && r->CRm == 15 && r->Op2 == 7) {
>> +reg = PMCCFILTR_EL0;
>> +} else {
>> +reg = ((r->CRm & 3) << 3) | (r->Op2 & 7);
>> +reg += PMEVTYPER0_EL0;
>> +}
>> +}
>> +}
>> +
>> +switch (reg) {
>> +case PMEVTYPER0_EL0 ... PMEVTYPER30_EL0:
>> +idx = reg - PMEVTYPER0_EL0;
>> +if (!pmu_counter_idx_valid(vcpu, idx))
>> +return true;
> Hi Marc,
> 
> Here should we return false to inject an UND since there is no
> PMEVTYPER(idx)_EL0? The ARMv8 spec says it should.

The spec says that the following behaviours are valid:
- Accesses to the register are UNDEFINED .
- Accesses to the register behave as RAZ/WI.
- Accesses to the register execute as a NOP .

Same for the counters. So you can either return true (act as a NOP), or
return false (UNDEF), and even zero r->regval on read and return true
(RAZ/WI).

This is entirely up to you. My personal preference is indeed to UNDEF,
but your current implementation is valid.

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 5/6] nvdimm acpi: let qemu handle _DSM method

[Igor Mammedov]

On Tue,  5 Jan 2016 02:52:07 +0800
Xiao Guangrong  wrote:

> If dsm memory is successfully patched, we let qemu fully emulate
> the dsm method
> 
> This patch saves _DSM input parameters into dsm memory, tell dsm
> memory address to QEMU, then fetch the result from the dsm memory
you also need to add NVDR._CRS method that would report
resources used by operation regions.

NVDIMM_COMMON_DSM - probably should be serialized, otherwise
there is a race risk, when several callers would write to
control region.


> 
> Signed-off-by: Xiao Guangrong 
> ---
>  hw/acpi/aml-build.c |  27 ++
>  hw/acpi/nvdimm.c| 124 
> ++--
>  include/hw/acpi/aml-build.h |   2 +
>  3 files changed, 150 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 677c1a6..e65171f 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1013,6 +1013,19 @@ Aml *create_field_common(int opcode, Aml *srcbuf, Aml 
> *index, const char *name)
>  return var;
>  }
>  
> +/* ACPI 1.0b: 16.2.5.2 Named Objects Encoding: DefCreateField */
> +Aml *aml_create_field(Aml *srcbuf, Aml *index, Aml *len, const char *name)
> +{
> +Aml *var = aml_alloc();
> +build_append_byte(var->buf, 0x5B); /* ExtOpPrefix */
> +build_append_byte(var->buf, 0x13); /* CreateFieldOp */
> +aml_append(var, srcbuf);
> +aml_append(var, index);
> +aml_append(var, len);
> +build_append_namestring(var->buf, "%s", name);
> +return var;
> +}
> +
>  /* ACPI 1.0b: 16.2.5.2 Named Objects Encoding: DefCreateDWordField */
>  Aml *aml_create_dword_field(Aml *srcbuf, Aml *index, const char *name)
>  {
> @@ -1439,6 +1452,20 @@ Aml *aml_alias(const char *source_object, const char 
> *alias_object)
>  return var;
>  }
>  
> +/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefConcat */
> +Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target)
> +{
> +Aml *var = aml_opcode(0x73 /* ConcatOp */);
> +aml_append(var, source1);
> +aml_append(var, source2);
> +
> +if (target) {
> +aml_append(var, target);
> +}
> +
> +return var;
> +}
> +
>  void
>  build_header(GArray *linker, GArray *table_data,
>   AcpiTableHeader *h, const char *sig, int len, uint8_t rev,
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index a72104c..dfccbc0 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -369,6 +369,24 @@ static void nvdimm_build_nfit(GSList *device_list, 
> GArray *table_offsets,
>  g_array_free(structures, true);
>  }
>  
> +struct NvdimmDsmIn {
> +uint32_t handle;
> +uint32_t revision;
> +uint32_t function;
> +   /* the remaining size in the page is used by arg3. */
> +union {
> +uint8_t arg3[0];
> +};
> +} QEMU_PACKED;
> +typedef struct NvdimmDsmIn NvdimmDsmIn;
> +
> +struct NvdimmDsmOut {
> +/* the size of buffer filled by QEMU. */
> +uint32_t len;
> +uint8_t data[0];
> +} QEMU_PACKED;
> +typedef struct NvdimmDsmOut NvdimmDsmOut;
> +
>  static uint64_t
>  nvdimm_dsm_read(void *opaque, hwaddr addr, unsigned size)
>  {
> @@ -408,11 +426,21 @@ void nvdimm_init_acpi_state(AcpiNVDIMMState *state, 
> MemoryRegion *io,
>  
>  static void nvdimm_build_common_dsm(Aml *dev)
>  {
> -Aml *method, *ifctx, *function;
> +Aml *method, *ifctx, *function, *unpatched, *field, *high_dsm_mem;
> +Aml *result_size, *dsm_mem;
>  uint8_t byte_list[1];
>  
>  method = aml_method(NVDIMM_COMMON_DSM, 4, AML_NOTSERIALIZED);
>  function = aml_arg(2);
> +dsm_mem = aml_arg(3);
> +
> +aml_append(method, aml_store(aml_call0(NVDIMM_GET_DSM_MEM), dsm_mem));
> +
> +/*
> + * do not support any method if DSM memory address has not been
> + * patched.
> + */
> +unpatched = aml_if(aml_equal(dsm_mem, aml_int64(0x0)));
>  
>  /*
>   * function 0 is called to inquire what functions are supported by
> @@ -421,12 +449,102 @@ static void nvdimm_build_common_dsm(Aml *dev)
>  ifctx = aml_if(aml_equal(function, aml_int(0)));
>  byte_list[0] = 0 /* No function Supported */;
>  aml_append(ifctx, aml_return(aml_buffer(1, byte_list)));
> -aml_append(method, ifctx);
> +aml_append(unpatched, ifctx);
>  
>  /* No function is supported yet. */
>  byte_list[0] = 1 /* Not Supported */;
> -aml_append(method, aml_return(aml_buffer(1, byte_list)));
> +aml_append(unpatched, aml_return(aml_buffer(1, byte_list)));
> +aml_append(method, unpatched);
> +
> +/* map DSM memory and IO into ACPI namespace. */
> +aml_append(method, aml_operation_region("NPIO", AML_SYSTEM_IO,
> +   aml_int(NVDIMM_ACPI_IO_BASE), NVDIMM_ACPI_IO_LEN));
> +aml_append(method, aml_operation_region("NRAM", AML_SYSTEM_MEMORY,
> +dsm_mem, TARGET_PAGE_SIZE));
> +
> +/*
> + * DSM notifier:
> + * LNTF: write 

Re: [PATCH v8 20/20] KVM: ARM64: Add a new kvm ARM PMU device

[Peter Maydell]

On 22 December 2015 at 08:08, Shannon Zhao  wrote:
> From: Shannon Zhao 
>
> Add a new kvm device type KVM_DEV_TYPE_ARM_PMU_V3 for ARM PMU. Implement
> the kvm_device_ops for it.
>
> Signed-off-by: Shannon Zhao 
> ---
>  Documentation/virtual/kvm/devices/arm-pmu.txt |  24 +
>  arch/arm64/include/uapi/asm/kvm.h |   4 +
>  include/linux/kvm_host.h  |   1 +
>  include/uapi/linux/kvm.h  |   2 +
>  virt/kvm/arm/pmu.c| 128 
> ++
>  virt/kvm/kvm_main.c   |   4 +
>  6 files changed, 163 insertions(+)
>  create mode 100644 Documentation/virtual/kvm/devices/arm-pmu.txt
>
> diff --git a/Documentation/virtual/kvm/devices/arm-pmu.txt 
> b/Documentation/virtual/kvm/devices/arm-pmu.txt
> new file mode 100644
> index 000..dda864e
> --- /dev/null
> +++ b/Documentation/virtual/kvm/devices/arm-pmu.txt
> @@ -0,0 +1,24 @@
> +ARM Virtual Performance Monitor Unit (vPMU)
> +===
> +
> +Device types supported:
> +  KVM_DEV_TYPE_ARM_PMU_V3 ARM Performance Monitor Unit v3
> +
> +Instantiate one PMU instance for per VCPU through this API.

Do you mean that userspace has to call this API once per vCPU to
create each PMU, or that calling the device create ioctl once makes
the kernel instantiate a PMU for each vCPU?

(It's a little bit confusing that we say "this API" to mean
"not the API documented in this file at all but actually
the KVM_CREATE_DEVICE ioctl", but I see we do that in the GIC
API docs too.)

> +
> +Groups:
> +  KVM_DEV_ARM_PMU_GRP_IRQ
> +  Attributes:
> +The attr field of kvm_device_attr encodes one value:
> +bits: | 63  32 | 31   0 |
> +values:   |  reserved  | vcpu_index |
> +A value describing the PMU overflow interrupt number for the specified
> +vcpu_index vcpu. This interrupt could be a PPI or SPI, but for one VM the
> +interrupt type must be same for each vcpu. As a PPI, the interrupt 
> number is
> +same for all vcpus, while as a SPI it must be different for each vcpu.

I see we're using vcpu_index rather than MPIDR affinity value
for specifying which CPU we're configuring. Is this in line with
our planned API for GICv3 configuration?

> +  Errors:
> +-ENXIO: Unsupported attribute group
> +-EBUSY: The PMU overflow interrupt is already set
> +-ENODEV: Getting the PMU overflow interrupt number while it's not set
> +-EINVAL: Invalid vcpu_index or PMU overflow interrupt number supplied

What happens if you create a PMU but then never set the IRQ number?
Is there a default or does the VM refuse to run or something?

Do we want an attribute like KVM_DEV_ARM_VGIC_CTRL_INIT so userspace
can say "I have definitely completed configuration of this device now" ?

We wound up with a fair amount of legacy mess to deal with in the
GIC code because we didn't put one of those in from the start.
(Perhaps we should aim to standardize on all kvm devices having one?)

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

Re: [PATCH 3/4] KVM: renumber architecture-dependent requests

[Christian Borntraeger]

On 01/07/2016 03:17 PM, Paolo Bonzini wrote:
> Leave room for 4 more arch-independent requests.
> 


The patch subject is wrong.

"renumber architecture-dependent requests"

--> "renumber kvm requests"

as we also renumber the architecture independent ones.

--
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: use PIT channel index in hpet_legacy_start mode

[P J P]

+-- On Thu, 7 Jan 2016, Paolo Bonzini wrote --+
| > Will this trigger the same issue like CVE-2015-7513 ?
| > 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=0185604c2d82c560dab2f2933a18f797e74ab5a8
| 
| I am not sure (--verbose please :))

  IIUC, it shouldn't, because pit_load_count() does

/*
 * The largest possible initial count is 0; this is equivalent
 * to 216 for binary counting and 104 for BCD counting.
 */
if (val == 0)
val = 0x1;


| but the right fix is to change the caller like this:
 
| diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
| @@ -420,6 +420,7 @@ void kvm_pit_load_count(struct kvm *kvm, int channel, u32 
val, int hpet_legacy_s
|   u8 saved_mode;
|   if (hpet_legacy_start) {
|   /* save existing mode for later reenablement */
| + WARN_ON(channel != 0);
|   saved_mode = kvm->arch.vpit->pit_state.channels[0].mode;
|   kvm->arch.vpit->pit_state.channels[0].mode = 0xff; /* disable 
timer */
|   pit_load_count(kvm, channel, val);

  In that case I guess, 'pit_load_count' could be called as

+   pit_load_count(kvm, 0, val); 


Thank you.
--
 - P J P
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
--
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 19/20] KVM: ARM64: Free perf event of PMU when destroying vcpu

[Marc Zyngier]

On Tue, 22 Dec 2015 16:08:14 +0800
Shannon Zhao  wrote:

> From: Shannon Zhao 
> 
> When KVM frees VCPU, it needs to free the perf_event of PMU.
> 
> Signed-off-by: Shannon Zhao 

Reviewed-by: Marc Zyngier 

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: How to reserve guest physical region for ACPI

[Igor Mammedov]

On Mon, 4 Jan 2016 21:17:31 +0100
Laszlo Ersek  wrote:

> Michael CC'd me on the grandparent of the email below. I'll try to add
> my thoughts in a single go, with regard to OVMF.
> 
> On 12/30/15 20:52, Michael S. Tsirkin wrote:
> > On Wed, Dec 30, 2015 at 04:55:54PM +0100, Igor Mammedov wrote:  
> >> On Mon, 28 Dec 2015 14:50:15 +0200
> >> "Michael S. Tsirkin"  wrote:
> >>  
> >>> On Mon, Dec 28, 2015 at 10:39:04AM +0800, Xiao Guangrong wrote:  
> 
>  Hi Michael, Paolo,
> 
>  Now it is the time to return to the challenge that how to reserve guest
>  physical region internally used by ACPI.
> 
>  Igor suggested that:
>  | An alternative place to allocate reserve from could be high memory.
>  | For pc we have "reserved-memory-end" which currently makes sure
>  | that hotpluggable memory range isn't used by firmware
>  (https://lists.nongnu.org/archive/html/qemu-devel/2015-11/msg00926.html) 
>   
> 
> OVMF has no support for the "reserved-memory-end" fw_cfg file. The
> reason is that nobody wrote that patch, nor asked for the patch to be
> written. (Not implying that just requesting the patch would be
> sufficient for the patch to be written.)
Hijacking this part of thread to check if OVMF would work with memory-hotplug
and if it needs "reserved-memory-end" support at all.

How OVMF determines which GPA ranges to use for initializing PCI BARs
at boot time, more specifically 64-bit BARs.
--
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] [PATCH 0/6] NVDIMM ACPI: introduce the framework of QEMU emulated DSM

[Igor Mammedov]

On Tue,  5 Jan 2016 02:52:02 +0800
Xiao Guangrong  wrote:

> This patchset is against commit 5530427f0ca (acpi: extend aml_and() to
> accept target argument) on pci branch of Michael's git tree
> and can be found at:
>   https://github.com/xiaogr/qemu.git nvdimm-acpi-v1
> 
> This is the second part of vNVDIMM implementation which implements the
> BIOS patched dsm memory and introduces the framework that allows QEMU
> to emulate DSM method
> 
> Thanks to Michael's idea, we do not reserve any memory for NVDIMM ACPI,
> instead we let BIOS allocate the memory and patch the address to the
> offset we want
> 
> IO port is still enabled as it plays as the way to notify QEMU and pass
> the patched dsm memory address, so that IO port region, 0x0a18 - 0xa20,
> is reserved and it is divided into two 32 bits ports and used to pass
> the low 32 bits and high 32 bits of dsm memory address to QEMU
> 
> Thanks Igor's idea, this patchset also extends DSDT/SSDT to revision 2
> to apply 64 bit operations, in order to keeping compatibility, old
> version (<= 2.5) still uses revision 1. Since 64 bit operations breaks
> old guests (such as windows XP), we should keep the 64 bits stuff in
> the private place where common ACPI operation does not touch it
> 

general notes:
1. could you split out AML API additions/changes into separate patches?
   even if series nvdims patches couldn't be accepted on next respin,
   AML API patches could be good and we could pick them up just
   for API completeness. That also would make them easier to review
   and reduces count of patches you'd need to respin.
2. add test case for NVDIMM table blob, see tests/bios-tables-test.c
   at the beginning of series.
3. make V=1 check would show you ASL diff your patches are introducing,
   it will save you from booting real guest and dumping/decompiling
   tables manually.
4. at the end of series add NVDIMM table test blob with new table.
   you can use tests/acpi-test-data/rebuild-expected-aml.sh to make it
5. if make check by some miracle passes with these patches,
   dump NVDIMM table in guest and try to decompile and then compile it
   back with IASL, it will show you what needs to be fixed.
   
PS:
 under NVDIMM table I mean SSDT NVMDIM table.

> Igor Mammedov (1):
>   pc: acpi: bump DSDT/SSDT compliance revision to v2
> 
> Xiao Guangrong (5):
>   nvdimm acpi: initialize the resource used by NVDIMM ACPI
>   nvdimm acpi: introduce patched dsm memory
>   acpi: allow using acpi named offset for OperationRegion
>   nvdimm acpi: let qemu handle _DSM method
>   nvdimm acpi: emulate dsm method
> 
>  hw/acpi/Makefile.objs   |   2 +-
>  hw/acpi/aml-build.c |  45 +++-
>  hw/acpi/ich9.c  |  32 +
>  hw/acpi/nvdimm.c| 276 
> ++--
>  hw/acpi/piix4.c |   3 +
>  hw/i386/acpi-build.c|  41 ---
>  hw/i386/pc.c|   8 +-
>  hw/i386/pc_piix.c   |   5 +
>  hw/i386/pc_q35.c|   8 +-
>  include/hw/acpi/aml-build.h |   6 +-
>  include/hw/acpi/ich9.h  |   2 +
>  include/hw/i386/pc.h|  19 ++-
>  include/hw/mem/nvdimm.h |  44 ++-
>  13 files changed, 449 insertions(+), 42 deletions(-)
> 

--
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 1/2] KVM: Remove unused KVM_REQ_KICK to save a bit in vcpu->requests

[Paolo Bonzini]

On 07/01/2016 12:43, Takuya Yoshikawa wrote:
> Signed-off-by: Takuya Yoshikawa 
> ---
>  include/linux/kvm_host.h | 45 ++---
>  1 file changed, 22 insertions(+), 23 deletions(-)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 61c3e6c..ca9b93e 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -122,29 +122,28 @@ static inline bool is_error_page(struct page *page)
>  #define KVM_REQ_UNHALT 6
>  #define KVM_REQ_MMU_SYNC   7
>  #define KVM_REQ_CLOCK_UPDATE   8
> -#define KVM_REQ_KICK   9

I'd prefer to leave just an /* unused: 9 */ here.

This patch can go in for 4.5.  Regarding the other patch,
KVM_REQ_MCLOCK_INPROGRESS is indeed not really necessary, see
http://www.spinics.net/lists/kvm/msg95944.html and the follow-up.  Were
you thinking of the same?  If so I would prefer to have some comments.

Paolo

> -#define KVM_REQ_DEACTIVATE_FPU10
> -#define KVM_REQ_EVENT 11
> -#define KVM_REQ_APF_HALT  12
> -#define KVM_REQ_STEAL_UPDATE  13
> -#define KVM_REQ_NMI   14
> -#define KVM_REQ_PMU   15
> -#define KVM_REQ_PMI   16
> -#define KVM_REQ_WATCHDOG  17
> -#define KVM_REQ_MASTERCLOCK_UPDATE 18
> -#define KVM_REQ_MCLOCK_INPROGRESS 19
> -#define KVM_REQ_EPR_EXIT  20
> -#define KVM_REQ_SCAN_IOAPIC   21
> -#define KVM_REQ_GLOBAL_CLOCK_UPDATE 22
> -#define KVM_REQ_ENABLE_IBS23
> -#define KVM_REQ_DISABLE_IBS   24
> -#define KVM_REQ_APIC_PAGE_RELOAD  25
> -#define KVM_REQ_SMI   26
> -#define KVM_REQ_HV_CRASH  27
> -#define KVM_REQ_IOAPIC_EOI_EXIT   28
> -#define KVM_REQ_HV_RESET  29
> -#define KVM_REQ_HV_EXIT   30
> -#define KVM_REQ_HV_STIMER 31
> +#define KVM_REQ_DEACTIVATE_FPU 9
> +#define KVM_REQ_EVENT 10
> +#define KVM_REQ_APF_HALT  11
> +#define KVM_REQ_STEAL_UPDATE  12
> +#define KVM_REQ_NMI   13
> +#define KVM_REQ_PMU   14
> +#define KVM_REQ_PMI   15
> +#define KVM_REQ_WATCHDOG  16
> +#define KVM_REQ_MASTERCLOCK_UPDATE 17
> +#define KVM_REQ_MCLOCK_INPROGRESS 18
> +#define KVM_REQ_EPR_EXIT  19
> +#define KVM_REQ_SCAN_IOAPIC   20
> +#define KVM_REQ_GLOBAL_CLOCK_UPDATE 21
> +#define KVM_REQ_ENABLE_IBS22
> +#define KVM_REQ_DISABLE_IBS   23
> +#define KVM_REQ_APIC_PAGE_RELOAD  24
> +#define KVM_REQ_SMI   25
> +#define KVM_REQ_HV_CRASH  26
> +#define KVM_REQ_IOAPIC_EOI_EXIT   27
> +#define KVM_REQ_HV_RESET  28
> +#define KVM_REQ_HV_EXIT   29
> +#define KVM_REQ_HV_STIMER 30
>  
>  #define KVM_USERSPACE_IRQ_SOURCE_ID  0
>  #define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID 1
> 
--
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: use PIT channel index in hpet_legacy_start mode

[Yang Zhang]

On 2016/1/7 20:32, P J P wrote:

From: P J P 

While setting the KVM PIT counters in 'kvm_pit_load_count', if
'hpet_legacy_start' is set, the function disables the timer on
channel[0], instead of the respective index 'channel'. Update it
to use 'channel' index parameter.

Signed-off-by: P J P 
---
  arch/x86/kvm/i8254.c | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 08116ff..154e936 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -420,10 +420,11 @@ void kvm_pit_load_count(struct kvm *kvm, int channel, u32 
val, int hpet_legacy_s
u8 saved_mode;
if (hpet_legacy_start) {
/* save existing mode for later reenablement */
-   saved_mode = kvm->arch.vpit->pit_state.channels[0].mode;
-   kvm->arch.vpit->pit_state.channels[0].mode = 0xff; /* disable 
timer */
+   saved_mode = kvm->arch.vpit->pit_state.channels[channel].mode;
+   /* disable timer */
+   kvm->arch.vpit->pit_state.channels[channel].mode = 0xff;
pit_load_count(kvm, channel, val);
-   kvm->arch.vpit->pit_state.channels[0].mode = saved_mode;
+   kvm->arch.vpit->pit_state.channels[channel].mode = saved_mode;
} else {
pit_load_count(kvm, channel, val);
}



Will this trigger the same issue like CVE-2015-7513 ?

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=0185604c2d82c560dab2f2933a18f797e74ab5a8

--
best regards
yang
--
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 08/20] KVM: ARM64: Add access handler for event typer register

[Shannon Zhao]

On 2016/1/7 19:03, Marc Zyngier wrote:
> On 22/12/15 08:08, Shannon Zhao wrote:
>> > From: Shannon Zhao 
>> > 
>> > These kind of registers include PMEVTYPERn, PMCCFILTR and PMXEVTYPER
>> > which is mapped to PMEVTYPERn or PMCCFILTR.
>> > 
>> > The access handler translates all aarch32 register offsets to aarch64
>> > ones and uses vcpu_sys_reg() to access their values to avoid taking care
>> > of big endian.
>> > 
>> > When writing to these registers, create a perf_event for the selected
>> > event type.
>> > 
>> > Signed-off-by: Shannon Zhao 
>> > ---
>> >  arch/arm64/kvm/sys_regs.c | 156 
>> > +-
>> >  1 file changed, 154 insertions(+), 2 deletions(-)
>> > 
>> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>> > index 2552db1..ed2939b 100644
>> > --- a/arch/arm64/kvm/sys_regs.c
>> > +++ b/arch/arm64/kvm/sys_regs.c
>> > @@ -505,6 +505,70 @@ static bool access_pmceid(struct kvm_vcpu *vcpu, 
>> > struct sys_reg_params *p,
>> >return true;
>> >  }
>> >  
>> > +static inline bool pmu_counter_idx_valid(struct kvm_vcpu *vcpu, u64 idx)
>> > +{
>> > +  u64 pmcr, val;
>> > +
>> > +  pmcr = vcpu_sys_reg(vcpu, PMCR_EL0);
>> > +  val = (pmcr >> ARMV8_PMCR_N_SHIFT) & ARMV8_PMCR_N_MASK;
>> > +  if (idx >= val && idx != ARMV8_CYCLE_IDX)
>> > +  return false;
>> > +
>> > +  return true;
>> > +}
>> > +
>> > +static bool access_pmu_evtyper(struct kvm_vcpu *vcpu, struct 
>> > sys_reg_params *p,
>> > + const struct sys_reg_desc *r)
>> > +{
>> > +  u64 idx, reg;
>> > +
>> > +  if (r->CRn == 9) {
>> > +  /* PMXEVTYPER_EL0 */
>> > +  reg = 0;
> Is there any particular reason why you're not setting reg to PMSELR_EL0,
> since this is what you're using?
> 
>> > +  } else {
>> > +  if (!p->is_aarch32) {
>> > +  /* PMEVTYPERn_EL0 or PMCCFILTR_EL0 */
>> > +  reg = r->reg;
>> > +  } else {
>> > +  if (r->CRn == 14 && r->CRm == 15 && r->Op2 == 7) {
>> > +  reg = PMCCFILTR_EL0;
>> > +  } else {
>> > +  reg = ((r->CRm & 3) << 3) | (r->Op2 & 7);
>> > +  reg += PMEVTYPER0_EL0;
>> > +  }
>> > +  }
>> > +  }
>> > +
>> > +  switch (reg) {
>> > +  case PMEVTYPER0_EL0 ... PMEVTYPER30_EL0:
>> > +  idx = reg - PMEVTYPER0_EL0;
>> > +  if (!pmu_counter_idx_valid(vcpu, idx))
>> > +  return true;
>> > +  break;
>> > +  case PMCCFILTR_EL0:
>> > +  idx = ARMV8_CYCLE_IDX;
>> > +  break;
>> > +  default:
> This would allow this case to be more precise, and we could have the
> default case as a bug handler.
> 

It turns out that I refactor this function like below:

+static bool access_pmu_evtyper(struct kvm_vcpu *vcpu, struct
sys_reg_params *p,
+  const struct sys_reg_desc *r)
+{
+   u64 idx, reg = 0;
+
+   if (r->CRn == 9) {
+   /* PMXEVTYPER_EL0 */
+   idx = vcpu_sys_reg(vcpu, PMSELR_EL0) & ARMV8_COUNTER_MASK;
+   reg = PMEVTYPER0_EL0 + idx;
+   } else {
+   if (r->CRm == 15 && r->Op2 == 7) {
+   idx = ARMV8_CYCLE_IDX;
+   reg = PMCCFILTR_EL0;
+   } else {
+   /* PMEVTYPERn_EL0 */
+   idx = ((r->CRm & 3) << 3) | (r->Op2 & 7);
+   reg = PMEVTYPER0_EL0 + idx;
+   }
+   }
+
+   BUG_ON(reg == 0);
+
+   if (!pmu_counter_idx_valid(vcpu, idx))
+   return false;
+
+   if (p->is_write) {
+   kvm_pmu_set_counter_event_type(vcpu, p->regval, idx);
+   vcpu_sys_reg(vcpu, reg) = p->regval & ARMV8_EVTYPE_MASK;
+   } else {
+   p->regval = vcpu_sys_reg(vcpu, reg) & ARMV8_EVTYPE_MASK;
+   }
+
+   return true;
+}

How about this?

Thanks,
-- 
Shannon

--
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 07/20] KVM: ARM64: PMU: Add perf event map and introduce perf event creating function

[Marc Zyngier]

On Tue, 22 Dec 2015 16:08:02 +0800
Shannon Zhao  wrote:

> From: Shannon Zhao 
> 
> When we use tools like perf on host, perf passes the event type and the
> id of this event type category to kernel, then kernel will map them to
> hardware event number and write this number to PMU PMEVTYPER_EL0
> register. When getting the event number in KVM, directly use raw event
> type to create a perf_event for it.
> 
> Signed-off-by: Shannon Zhao 
> ---
>  arch/arm64/include/asm/pmu.h |   3 ++
>  arch/arm64/kvm/Makefile  |   1 +
>  include/kvm/arm_pmu.h|  11 
>  virt/kvm/arm/pmu.c   | 122 
> +++
>  4 files changed, 137 insertions(+)
>  create mode 100644 virt/kvm/arm/pmu.c
> 
> diff --git a/arch/arm64/include/asm/pmu.h b/arch/arm64/include/asm/pmu.h
> index 4406184..2588f9c 100644
> --- a/arch/arm64/include/asm/pmu.h
> +++ b/arch/arm64/include/asm/pmu.h
> @@ -21,6 +21,7 @@
>  
>  #define ARMV8_MAX_COUNTERS  32
>  #define ARMV8_COUNTER_MASK  (ARMV8_MAX_COUNTERS - 1)
> +#define ARMV8_CYCLE_IDX (ARMV8_MAX_COUNTERS - 1)
>  
>  /*
>   * Per-CPU PMCR: config reg
> @@ -31,6 +32,8 @@
>  #define ARMV8_PMCR_D (1 << 3) /* CCNT counts every 64th cpu cycle */
>  #define ARMV8_PMCR_X (1 << 4) /* Export to ETM */
>  #define ARMV8_PMCR_DP(1 << 5) /* Disable CCNT if 
> non-invasive debug*/
> +/* Determines which PMCCNTR_EL0 bit generates an overflow */
> +#define ARMV8_PMCR_LC(1 << 6)
>  #define  ARMV8_PMCR_N_SHIFT  11   /* Number of counters 
> supported */
>  #define  ARMV8_PMCR_N_MASK   0x1f
>  #define  ARMV8_PMCR_MASK 0x3f /* Mask for writable bits */
> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> index caee9ee..122cff4 100644
> --- a/arch/arm64/kvm/Makefile
> +++ b/arch/arm64/kvm/Makefile
> @@ -26,3 +26,4 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic-v2-emul.o
>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic-v3.o
>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic-v3-emul.o
>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arch_timer.o
> +kvm-$(CONFIG_KVM_ARM_PMU) += $(KVM)/arm/pmu.o
> diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
> index ddcb5b2..14bedb0 100644
> --- a/include/kvm/arm_pmu.h
> +++ b/include/kvm/arm_pmu.h
> @@ -34,9 +34,20 @@ struct kvm_pmu {
>   int irq_num;
>   struct kvm_pmc pmc[ARMV8_MAX_COUNTERS];
>  };
> +
> +u64 kvm_pmu_get_counter_value(struct kvm_vcpu *vcpu, u64 select_idx);
> +void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu, u64 data,
> + u64 select_idx);
>  #else
>  struct kvm_pmu {
>  };
> +
> +u64 kvm_pmu_get_counter_value(struct kvm_vcpu *vcpu, u64 select_idx)
> +{
> + return 0;
> +}
> +void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu, u64 data,
> + u64 select_idx) {}
>  #endif
>  
>  #endif
> diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
> new file mode 100644
> index 000..9d27999
> --- /dev/null
> +++ b/virt/kvm/arm/pmu.c
> @@ -0,0 +1,122 @@
> +/*
> + * Copyright (C) 2015 Linaro Ltd.
> + * Author: Shannon Zhao 
> + *
> + * 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 .
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/**
> + * kvm_pmu_get_counter_value - get PMU counter value
> + * @vcpu: The vcpu pointer
> + * @select_idx: The counter index
> + */
> +u64 kvm_pmu_get_counter_value(struct kvm_vcpu *vcpu, u64 select_idx)
> +{
> + u64 counter, reg, enabled, running;
> + struct kvm_pmu *pmu = >arch.pmu;
> + struct kvm_pmc *pmc = >pmc[select_idx];
> +
> + reg = (select_idx == ARMV8_CYCLE_IDX)
> +   ? PMCCNTR_EL0 : PMEVCNTR0_EL0 + select_idx;
> + counter = vcpu_sys_reg(vcpu, reg);
> +
> + /* The real counter value is equal to the value of counter register plus
> +  * the value perf event counts.
> +  */
> + if (pmc->perf_event)
> + counter += perf_event_read_value(pmc->perf_event, ,
> +  );
> +
> + return counter & pmc->bitmask;
> +}
> +
> +/**
> + * kvm_pmu_stop_counter - stop PMU counter
> + * @pmc: The PMU counter pointer
> + *
> + * If this counter has been configured to monitor some event, release it 
> here.

Re: [PATCH v8 00/20] KVM: ARM64: Add guest PMU support

[Marc Zyngier]

On 07/01/16 14:12, Will Deacon wrote:
> On Thu, Jan 07, 2016 at 02:10:38PM +, Marc Zyngier wrote:
>> On 22/12/15 08:07, Shannon Zhao wrote:
>>> From: Shannon Zhao 
>>>
>>> This patchset adds guest PMU support for KVM on ARM64. It takes
>>> trap-and-emulate approach. When guest wants to monitor one event, it
>>> will be trapped by KVM and KVM will call perf_event API to create a perf
>>> event and call relevant perf_event APIs to get the count value of event.
>>>
>>> Use perf to test this patchset in guest. When using "perf list", it
>>> shows the list of the hardware events and hardware cache events perf
>>> supports. Then use "perf stat -e EVENT" to monitor some event. For
>>> example, use "perf stat -e cycles" to count cpu cycles and
>>> "perf stat -e cache-misses" to count cache misses.
>>
>> I finally feel like we're pretty close to something we could merge. My
>> current concerns are:
> 
> The merge window opens on Monday and I'm not prepared to take further
> PMU changes at this point (unless they're bug fixes, of course).
> 
> This needs to wait until 4.6.

Fair enough. I guess I'll continue the review process and queue that
early in the next cycle.

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 20/20] KVM: ARM64: Add a new kvm ARM PMU device

[Peter Maydell]

On 7 January 2016 at 14:49, Shannon Zhao  wrote:
>
>
> On 2016/1/7 22:36, Peter Maydell wrote:
>> On 22 December 2015 at 08:08, Shannon Zhao  wrote:
>>> From: Shannon Zhao 
>>>
>>> Add a new kvm device type KVM_DEV_TYPE_ARM_PMU_V3 for ARM PMU. Implement
>>> the kvm_device_ops for it.
>>>
>>> Signed-off-by: Shannon Zhao 
>>> ---
>>>  Documentation/virtual/kvm/devices/arm-pmu.txt |  24 +
>>>  arch/arm64/include/uapi/asm/kvm.h |   4 +
>>>  include/linux/kvm_host.h  |   1 +
>>>  include/uapi/linux/kvm.h  |   2 +
>>>  virt/kvm/arm/pmu.c| 128 
>>> ++
>>>  virt/kvm/kvm_main.c   |   4 +
>>>  6 files changed, 163 insertions(+)
>>>  create mode 100644 Documentation/virtual/kvm/devices/arm-pmu.txt
>>>
>>> diff --git a/Documentation/virtual/kvm/devices/arm-pmu.txt 
>>> b/Documentation/virtual/kvm/devices/arm-pmu.txt
>>> new file mode 100644
>>> index 000..dda864e
>>> --- /dev/null
>>> +++ b/Documentation/virtual/kvm/devices/arm-pmu.txt
>>> @@ -0,0 +1,24 @@
>>> +ARM Virtual Performance Monitor Unit (vPMU)
>>> +===
>>> +
>>> +Device types supported:
>>> +  KVM_DEV_TYPE_ARM_PMU_V3 ARM Performance Monitor Unit v3
>>> +
>>> +Instantiate one PMU instance for per VCPU through this API.
>>
>> Do you mean that userspace has to call this API once per vCPU to
>> create each PMU, or that calling the device create ioctl once makes
>> the kernel instantiate a PMU for each vCPU?
>>
> Call the device create ioctl once and kvm will create a PMU for each
> vCPU. But userspace should set the irqs for each PMU since for SPI they
> are different.
>
>> (It's a little bit confusing that we say "this API" to mean
>> "not the API documented in this file at all but actually
>> the KVM_CREATE_DEVICE ioctl", but I see we do that in the GIC
>> API docs too.)
>>
>>> +
>>> +Groups:
>>> +  KVM_DEV_ARM_PMU_GRP_IRQ
>>> +  Attributes:
>>> +The attr field of kvm_device_attr encodes one value:
>>> +bits: | 63  32 | 31   0 |
>>> +values:   |  reserved  | vcpu_index |
>>> +A value describing the PMU overflow interrupt number for the specified
>>> +vcpu_index vcpu. This interrupt could be a PPI or SPI, but for one VM 
>>> the
>>> +interrupt type must be same for each vcpu. As a PPI, the interrupt 
>>> number is
>>> +same for all vcpus, while as a SPI it must be different for each vcpu.
>>
>> I see we're using vcpu_index rather than MPIDR affinity value
>> for specifying which CPU we're configuring. Is this in line with
>> our planned API for GICv3 configuration?
>>
> Here vcpu_index is used to indexing the vCPU, no special use.

Yes, but you can identify the CPU by index, or by its MPIDR.
We had a discussion about which was the best way for doing
the VGIC API, and I can't remember which way round we ended up
going for. Whichever we chose, we should do the same thing here.

>>> +  Errors:
>>> +-ENXIO: Unsupported attribute group
>>> +-EBUSY: The PMU overflow interrupt is already set
>>> +-ENODEV: Getting the PMU overflow interrupt number while it's not set
>>> +-EINVAL: Invalid vcpu_index or PMU overflow interrupt number supplied
>>
>> What happens if you create a PMU but then never set the IRQ number?
>> Is there a default or does the VM refuse to run or something?
>>
> If userspace doesn't specify the irq number, the guest will not receive
> the PMU interrupt because we check if the irq is initialized when we
> inject the interrupt. But guest could still use the vPMU if QEMU
> generates a proper DTB or ACPI.

So is it a valid use case to create a PMU with the interrupt not wired
up to anything? (If it's never valid it would be nice to diagnose it
rather than just silently letting the guest run but not work right.)

thanks
-- PMM
--
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] virtio/s390: use dev_to_virtio

[Cornelia Huck]

On Wed, 30 Dec 2015 22:05:25 +0800
Geliang Tang  wrote:

> Use dev_to_virtio() instead of open-coding it.
> 
> Signed-off-by: Geliang Tang 
> ---
>  drivers/s390/virtio/virtio_ccw.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Thanks, added to my queue.

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

Re: [PATCH 3/4] KVM: renumber architecture-dependent requests

[Paolo Bonzini]

On 07/01/2016 16:27, Christian Borntraeger wrote:
> On 01/07/2016 03:17 PM, Paolo Bonzini wrote:
>> Leave room for 4 more arch-independent requests.
> 
> The patch subject is wrong.
> 
> "renumber architecture-dependent requests"
> 
> --> "renumber kvm requests"
> 
> as we also renumber the architecture independent ones.

Right.  Ok with that change?

Paolo
--
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 4/4] KVM: move architecture-dependent requests to arch/

[Paolo Bonzini]

On 07/01/2016 16:54, Christian Borntraeger wrote:
> On 01/07/2016 03:17 PM, Paolo Bonzini wrote:
> 
> Can you add at least a one line patch description?

Yes, and it will be more than one line. :)

"Since the numbers now overlap, it makes sense to enumerate
them in asm/kvm_host.h rather than linux/kvm_host.h.  Functions
that refer to architecture-specific requests are also moved
to arch/."

Paolo

>> Signed-off-by: Paolo Bonzini 
> 
> 
> Reviewed-by: Christian Borntraeger 
>> ---
>>  arch/powerpc/include/asm/kvm_host.h |  4 
>>  arch/s390/include/asm/kvm_host.h|  4 
>>  arch/x86/include/asm/kvm_host.h | 28 +++
>>  arch/x86/kvm/x86.c  | 10 ++
>>  include/linux/kvm_host.h| 38 
>> ++---
>>  virt/kvm/kvm_main.c | 10 --
>>  6 files changed, 48 insertions(+), 46 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/kvm_host.h 
>> b/arch/powerpc/include/asm/kvm_host.h
>> index cfa758c6b4f6..271fefbbe521 100644
>> --- a/arch/powerpc/include/asm/kvm_host.h
>> +++ b/arch/powerpc/include/asm/kvm_host.h
>> @@ -50,6 +50,10 @@
>>  #define KVM_NR_IRQCHIPS  1
>>  #define KVM_IRQCHIP_NUM_PINS 256
>>
>> +/* PPC-specific vcpu->requests bit members */
>> +#define KVM_REQ_WATCHDOG   8
>> +#define KVM_REQ_EPR_EXIT   9
>> +
>>  #include 
>>
>>  #define KVM_ARCH_WANT_MMU_NOTIFIER
>> diff --git a/arch/s390/include/asm/kvm_host.h 
>> b/arch/s390/include/asm/kvm_host.h
>> index c83144110ea9..31fe20f4d129 100644
>> --- a/arch/s390/include/asm/kvm_host.h
>> +++ b/arch/s390/include/asm/kvm_host.h
>> @@ -39,6 +39,10 @@
>>  #define KVM_IRQCHIP_NUM_PINS 4096
>>  #define KVM_HALT_POLL_NS_DEFAULT 0
>>
>> +/* s390-specific vcpu->requests bit members */
>> +#define KVM_REQ_ENABLE_IBS 8
>> +#define KVM_REQ_DISABLE_IBS9
>> +
>>  #define SIGP_CTRL_C 0x80
>>  #define SIGP_CTRL_SCN_MASK  0x3f
>>
>> diff --git a/arch/x86/include/asm/kvm_host.h 
>> b/arch/x86/include/asm/kvm_host.h
>> index a7c89876698b..44adbb819041 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -46,6 +46,31 @@
>>
>>  #define KVM_IRQCHIP_NUM_PINS  KVM_IOAPIC_NUM_PINS
>>
>> +/* x86-specific vcpu->requests bit members */
>> +#define KVM_REQ_MIGRATE_TIMER  8
>> +#define KVM_REQ_REPORT_TPR_ACCESS  9
>> +#define KVM_REQ_TRIPLE_FAULT  10
>> +#define KVM_REQ_MMU_SYNC  11
>> +#define KVM_REQ_CLOCK_UPDATE  12
>> +#define KVM_REQ_DEACTIVATE_FPU13
>> +#define KVM_REQ_EVENT 14
>> +#define KVM_REQ_APF_HALT  15
>> +#define KVM_REQ_STEAL_UPDATE  16
>> +#define KVM_REQ_NMI   17
>> +#define KVM_REQ_PMU   18
>> +#define KVM_REQ_PMI   19
>> +#define KVM_REQ_SMI   20
>> +#define KVM_REQ_MASTERCLOCK_UPDATE 21
>> +#define KVM_REQ_MCLOCK_INPROGRESS 22
>> +#define KVM_REQ_SCAN_IOAPIC   23
>> +#define KVM_REQ_GLOBAL_CLOCK_UPDATE 24
>> +#define KVM_REQ_APIC_PAGE_RELOAD  25
>> +#define KVM_REQ_HV_CRASH  26
>> +#define KVM_REQ_IOAPIC_EOI_EXIT   27
>> +#define KVM_REQ_HV_RESET  28
>> +#define KVM_REQ_HV_EXIT   29
>> +#define KVM_REQ_HV_STIMER 30
>> +
>>  #define CR0_RESERVED_BITS   \
>>  (~(unsigned long)(X86_CR0_PE | X86_CR0_MP | X86_CR0_EM | X86_CR0_TS \
>>| X86_CR0_ET | X86_CR0_NE | X86_CR0_WP | X86_CR0_AM \
>> @@ -1268,6 +1293,9 @@ u64 kvm_read_l1_tsc(struct kvm_vcpu *vcpu, u64 
>> host_tsc);
>>  unsigned long kvm_get_linear_rip(struct kvm_vcpu *vcpu);
>>  bool kvm_is_linear_rip(struct kvm_vcpu *vcpu, unsigned long linear_rip);
>>
>> +void kvm_make_mclock_inprogress_request(struct kvm *kvm);
>> +void kvm_make_scan_ioapic_request(struct kvm *kvm);
>> +
>>  void kvm_arch_async_page_not_present(struct kvm_vcpu *vcpu,
>>   struct kvm_async_pf *work);
>>  void kvm_arch_async_page_present(struct kvm_vcpu *vcpu,
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 102c3028513f..dc6b37f47cd7 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -1686,6 +1686,11 @@ static void pvclock_update_vm_gtod_copy(struct kvm 
>> *kvm)
>>  #endif
>>  }
>>
>> +void kvm_make_mclock_inprogress_request(struct kvm *kvm)
>> +{
>> +kvm_make_all_cpus_request(kvm, KVM_REQ_MCLOCK_INPROGRESS);
>> +}
>> +
>>  static void kvm_gen_update_masterclock(struct kvm *kvm)
>>  {
>>  #ifdef CONFIG_X86_64
>> @@ -6337,6 +6342,11 @@ static void process_smi(struct kvm_vcpu *vcpu)
>>  kvm_mmu_reset_context(vcpu);
>>  }
>>
>> +void kvm_make_scan_ioapic_request(struct kvm *kvm)
>> +{
>> +kvm_make_all_cpus_request(kvm, KVM_REQ_SCAN_IOAPIC);
>> +}
>> +
>>  static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu)
>>  {
>>  u64 eoi_exit_bitmap[4];
>> diff --git a/include/linux/kvm_host.h 

Re: [PATCH v1 0/6] KVM: Hyper-V SynIC timers migration fixes

[Paolo Bonzini]

On 23/12/2015 12:28, Andrey Smetanin wrote:
> During testing of Windows 2012R2 guest migration with
> Hyper-V SynIC timers enabled we found several bugs
> which lead to restoring guest in a hung state.
> 
> This patch series provides several fixes to make the
> migration of guest with Hyper-V SynIC timers enabled
> succeed.
> 
> The series applies on top of
> 'kvm/x86: Remove Hyper-V SynIC timer stopping'
> previously sent.
> 
> Signed-off-by: Andrey Smetanin 
> Reviewed-by: Roman Kagan 
> CC: Gleb Natapov 
> CC: Paolo Bonzini 
> CC: Roman Kagan 
> CC: Denis V. Lunev 
> CC: qemu-de...@nongnu.org
> 
> Andrey Smetanin (6):
>   kvm/x86: Drop stimer_stop() function
>   kvm/x86: Hyper-V unify stimer_start() and stimer_restart()
>   kvm/x86: Reorg stimer_expiration() to better control timer restart
>   kvm/x86: Hyper-V fix SynIC timer disabling condition
>   kvm/x86: Skip SynIC vector check for QEMU side
>   kvm/x86: Update SynIC timers on guest entry only
> 
>  arch/x86/kvm/hyperv.c | 112 
> +++---
>  arch/x86/kvm/x86.c|   6 +++
>  2 files changed, 58 insertions(+), 60 deletions(-)
> 

Applied, let me know if I should fix up patch 3 (the bug is preexisting
anyway, if it is a bug as I suspect).

Paolo
--
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 4/4] KVM: move architecture-dependent requests to arch/

[Christian Borntraeger]

On 01/07/2016 03:17 PM, Paolo Bonzini wrote:

Can you add at least a one line patch description?

> Signed-off-by: Paolo Bonzini 


Reviewed-by: Christian Borntraeger 
> ---
>  arch/powerpc/include/asm/kvm_host.h |  4 
>  arch/s390/include/asm/kvm_host.h|  4 
>  arch/x86/include/asm/kvm_host.h | 28 +++
>  arch/x86/kvm/x86.c  | 10 ++
>  include/linux/kvm_host.h| 38 
> ++---
>  virt/kvm/kvm_main.c | 10 --
>  6 files changed, 48 insertions(+), 46 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/kvm_host.h 
> b/arch/powerpc/include/asm/kvm_host.h
> index cfa758c6b4f6..271fefbbe521 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -50,6 +50,10 @@
>  #define KVM_NR_IRQCHIPS  1
>  #define KVM_IRQCHIP_NUM_PINS 256
> 
> +/* PPC-specific vcpu->requests bit members */
> +#define KVM_REQ_WATCHDOG   8
> +#define KVM_REQ_EPR_EXIT   9
> +
>  #include 
> 
>  #define KVM_ARCH_WANT_MMU_NOTIFIER
> diff --git a/arch/s390/include/asm/kvm_host.h 
> b/arch/s390/include/asm/kvm_host.h
> index c83144110ea9..31fe20f4d129 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -39,6 +39,10 @@
>  #define KVM_IRQCHIP_NUM_PINS 4096
>  #define KVM_HALT_POLL_NS_DEFAULT 0
> 
> +/* s390-specific vcpu->requests bit members */
> +#define KVM_REQ_ENABLE_IBS 8
> +#define KVM_REQ_DISABLE_IBS9
> +
>  #define SIGP_CTRL_C  0x80
>  #define SIGP_CTRL_SCN_MASK   0x3f
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index a7c89876698b..44adbb819041 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -46,6 +46,31 @@
> 
>  #define KVM_IRQCHIP_NUM_PINS  KVM_IOAPIC_NUM_PINS
> 
> +/* x86-specific vcpu->requests bit members */
> +#define KVM_REQ_MIGRATE_TIMER  8
> +#define KVM_REQ_REPORT_TPR_ACCESS  9
> +#define KVM_REQ_TRIPLE_FAULT  10
> +#define KVM_REQ_MMU_SYNC  11
> +#define KVM_REQ_CLOCK_UPDATE  12
> +#define KVM_REQ_DEACTIVATE_FPU13
> +#define KVM_REQ_EVENT 14
> +#define KVM_REQ_APF_HALT  15
> +#define KVM_REQ_STEAL_UPDATE  16
> +#define KVM_REQ_NMI   17
> +#define KVM_REQ_PMU   18
> +#define KVM_REQ_PMI   19
> +#define KVM_REQ_SMI   20
> +#define KVM_REQ_MASTERCLOCK_UPDATE 21
> +#define KVM_REQ_MCLOCK_INPROGRESS 22
> +#define KVM_REQ_SCAN_IOAPIC   23
> +#define KVM_REQ_GLOBAL_CLOCK_UPDATE 24
> +#define KVM_REQ_APIC_PAGE_RELOAD  25
> +#define KVM_REQ_HV_CRASH  26
> +#define KVM_REQ_IOAPIC_EOI_EXIT   27
> +#define KVM_REQ_HV_RESET  28
> +#define KVM_REQ_HV_EXIT   29
> +#define KVM_REQ_HV_STIMER 30
> +
>  #define CR0_RESERVED_BITS   \
>   (~(unsigned long)(X86_CR0_PE | X86_CR0_MP | X86_CR0_EM | X86_CR0_TS \
> | X86_CR0_ET | X86_CR0_NE | X86_CR0_WP | X86_CR0_AM \
> @@ -1268,6 +1293,9 @@ u64 kvm_read_l1_tsc(struct kvm_vcpu *vcpu, u64 
> host_tsc);
>  unsigned long kvm_get_linear_rip(struct kvm_vcpu *vcpu);
>  bool kvm_is_linear_rip(struct kvm_vcpu *vcpu, unsigned long linear_rip);
> 
> +void kvm_make_mclock_inprogress_request(struct kvm *kvm);
> +void kvm_make_scan_ioapic_request(struct kvm *kvm);
> +
>  void kvm_arch_async_page_not_present(struct kvm_vcpu *vcpu,
>struct kvm_async_pf *work);
>  void kvm_arch_async_page_present(struct kvm_vcpu *vcpu,
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 102c3028513f..dc6b37f47cd7 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1686,6 +1686,11 @@ static void pvclock_update_vm_gtod_copy(struct kvm 
> *kvm)
>  #endif
>  }
> 
> +void kvm_make_mclock_inprogress_request(struct kvm *kvm)
> +{
> + kvm_make_all_cpus_request(kvm, KVM_REQ_MCLOCK_INPROGRESS);
> +}
> +
>  static void kvm_gen_update_masterclock(struct kvm *kvm)
>  {
>  #ifdef CONFIG_X86_64
> @@ -6337,6 +6342,11 @@ static void process_smi(struct kvm_vcpu *vcpu)
>   kvm_mmu_reset_context(vcpu);
>  }
> 
> +void kvm_make_scan_ioapic_request(struct kvm *kvm)
> +{
> + kvm_make_all_cpus_request(kvm, KVM_REQ_SCAN_IOAPIC);
> +}
> +
>  static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu)
>  {
>   u64 eoi_exit_bitmap[4];
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index b0ec0f778192..81c35dff52fd 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -111,46 +111,14 @@ static inline bool is_error_page(struct page *page)
>  }
> 
>  /*
> - * vcpu->requests bit members
> + * Architecture-independent vcpu->requests bit members
> + * Bits 4-7 are reserved for more arch-independent bits.
>   */
>  #define KVM_REQ_TLB_FLUSH  0
>  #define 

Re: [PATCH v2 3/7] kvm/x86: Hyper-V unify stimer_start() and stimer_restart()

[Paolo Bonzini]

On 28/12/2015 16:27, Andrey Smetanin wrote:
> This will be used in future to start Hyper-V SynIC timer
> in several places by one logic in one function.
> 
> Changes v2:
> * drop stimer->count == 0 check inside stimer_start()
> * comment stimer_start() assumptions

Can you replace comments with WARN_ON_ONCE?

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

lovely meet dear

[happie2]

Hi nice to meet you i"m ms happy by name you got me interested on 
serverfault.com ;how are you doing?
--
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