Re: [PATCH 00/13] arm64: Virtualization Host Extension support
On Wed, 26 Aug 2015 13:16:52 +0200 Antonios Motakis antonios.mota...@huawei.com wrote: On 26-Aug-15 11:59, Marc Zyngier wrote: [...] Unfortunately, there is more to downgrading to EL1 than just interrupts. You need to migrate the whole VM context from EL2 to EL1 in an atomic fashion, clear the HCR_EL2.E2H and HCR_EL2.TGE bits while running at EL2 (which is a bit like pulling the rug from under your own feet so you need to transition via a low mapping or an idmap), reinstall the EL2 stub and do an exception return into EL1. When enabling Jailhouse, we already do most of that. We already use identity mapping, since we need to switch on the MMU for EL2, switch the exception level, etc. Jailhouse entry looks a lot like initializing a new kernel; we just save the state of what was running before it and restore it as the root cell. So I think we could handle the cpu context switch, with changes only in the Jailhouse entry code. But then of course, Linux would be expecting to be in EL2, while it is running in EL1, so we would have to emulate the differences in behavior. But... There would be (almost) no difference in behaviour - VHE is designed for the kernel to be unchanged, and the only difference is the timer interrupt as you noticed. What is really tricky is to perform the downgrade, because you're completely changing the way the code is executed *while running it*. This is not just about changing the memory map, but also changing the effect of most system registers. And that's only for the CPU. Downgrading to EL1 has other fun consequences at the system level (SMMUs listening to TLB traffic would need to be reconfigured on the flight - it's a joke, don't even think of it). ...but then there's that. Hm... even if the kernel is running in EL2, it will still be configuring stage 1 on the SMMU, no? I wonder if this could still be handled somehow... The root cell would be restored with identity mapping, too... Just thinking out loud :) Stage-1 and EL2 are two vastly unrelated concept. The main issue is that it is likely that your SMMU knows about VHE as well (it listens to EL2-VHE DVM messages), and need to be reconfigured as well. Good luck with that. [...] As far as I can see, the only practical solution to this is to have a VHE config option, and Jailhouse that can be set to conflict it (depends on !VHE). Having a toggle to turn VHE off at build time would definitely be the easy way out. Then we can just tell the user that we only support kernels built without it (the Jailhouse driver is out of tree atm). I don't have access to a VHE model though. Are you considering to add a config option for VHE in the next version of your patches? Yes, that's the plan. 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] KVM: arm/arm64: BUG FIX: Do not inject spurious interrupts
Hello! It's a bit weird to just sned this as a new patch without replying to my mail from yesterday with feedback Sorry. But changes are actually minimal, and i remember that i replied to you with the promise of testing your suggestion. So, done, works fine. :) I think you're missing a potential change to the irq_pending_on_cpu field here, which you have to compute by calling vgic_update_state() like we do elsewhere when we change status bits I have just checked this. vgic_update_state() never resets this bit. This bit is reset only in __kvm_vgic_flush_hwstate() and only if we have consumed completely everything. I have followed through the code and looks like it's perfectly safe to have this bit set while nothing is actually pendng. Following __kvm_vgic_flush_hwstate(), having this bit cleared is actually a shorthand for no interrupt is pending at all. If it is set without any interrupt actually being pending (this ends up in pa_percpu and pa_shared being all zeroes), all three for_each_set_bit() loops will just not do anything, and we still get to epilog: label, just after a bit longer check. And, since we are here, the guest has already been disturbed. different from the incorrect approach I suggested yesterday where we always just clear the bit for that vcpu). Yes, it is extremely bad idea to clear it because this bit summarizes all interrupts for this vcpu, and clearing it means that we are going to lose everything. An alternate would be: clear the bit, THEN call vgic_update_state() which would set it back if necessary. But does this extra bit of complexity worth anything, given one paragraph above? Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia -- 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 v2 13/18] nvdimm: build namespace config data
On Wed, Aug 26, 2015 at 06:42:01PM +0800, Xiao Guangrong wrote: On 08/26/2015 12:16 AM, Stefan Hajnoczi wrote: On Fri, Aug 14, 2015 at 10:52:06PM +0800, Xiao Guangrong wrote: +#ifdef NVDIMM_DEBUG +#define nvdebug(fmt, ...) fprintf(stderr, nvdimm: fmt, ## __VA_ARGS__) +#else +#define nvdebug(...) +#endif The following allows the compiler to check format strings and syntax check the argument expressions: #define NVDIMM_DEBUG 0 /* set to 1 for debug output */ #define nvdebug(fmt, ...) \ if (NVDIMM_DEBUG) { \ fprintf(stderr, nvdimm: fmt, ## __VA_ARGS__); \ } This approach avoids bitrot (e.g. debug format string arguments have become outdated). Really good tips, thanks for your sharing. I forgot the do { ... } while (0) in the macro to make nvdebug(hello world); work like a normal C statement. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 14/18] nvdimm: support NFIT_CMD_IMPLEMENTED function
On Wed, Aug 26, 2015 at 06:46:35PM +0800, Xiao Guangrong wrote: On 08/26/2015 12:23 AM, Stefan Hajnoczi wrote: On Fri, Aug 14, 2015 at 10:52:07PM +0800, Xiao Guangrong wrote: static void dsm_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) { +struct MemoryRegion *dsm_ram_mr = opaque; +struct dsm_buffer *dsm; +struct dsm_out *out; +void *buf; + assert(val == NOTIFY_VALUE); The guest should not be able to cause an abort(3). If val != NOTIFY_VALUE we can do nvdebug() and then return. The ACPI code and emulation code both are from qemu, if that happens, it's really a bug, aborting the VM is better than throwing a debug message under this case to avoid potential data corruption. abort(3) is dangerous because it can create a core dump. If a malicious guest triggers this repeatedly it could consume a lot of disk space and I/O or CPU while performing the core dumps. We cannot trust anything inside the guest, even if the guest code comes from QEMU because a malicious guest can still read/write to the same hardware registers. Stefan -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] KVM: arm64: Implement API for vGICv3 live migration
This patchset adds necessary userspace API in order to support vGICv3 live migration. This includes accessing GIC distributor and redistributor memory regions using device attribute ioctls, and system registers of CPU interface using register get/set ioctls. Pavel Fedin (3): KVM: arm64: Implement vGICv3 distributor and redistributor access from userspace KVM: arm64: Allow to use accessors in KVM_SET_ONE_REG and KVM_GET_ONE_REG KVM: arm64: Implement accessors for vGIC CPU interface registers arch/arm64/include/uapi/asm/kvm.h | 1 + arch/arm64/kvm/sys_regs.c | 223 - include/linux/irqchip/arm-gic-v3.h | 18 ++- virt/kvm/arm/vgic-v3-emul.c| 186 --- 4 files changed, 405 insertions(+), 23 deletions(-) -- 2.4.4 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 08/18] nvdimm: init backend memory mapping and config data area
On Wed, Aug 26, 2015 at 06:40:26PM +0800, Xiao Guangrong wrote: On 08/26/2015 12:03 AM, Stefan Hajnoczi wrote: On Fri, Aug 14, 2015 at 10:52:01PM +0800, Xiao Guangrong wrote: +if (fd 0) { +error_setg(errp, can not open %s, nvdimm-file); s/can not/cannot/ +return; +} + +size = get_file_size(fd); +buf = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); I guess the user will want to choose between MAP_SHARED and MAP_PRIVATE. This can be added in the future. Good idea, it will allow guest to write data but discards its content after it exits. Will implement O_RDONLY + MAP_PRIVATE in the near future. Great. +goto do_unmap; +} + +nvdimm-device_index = new_device_index(); +sprintf(name, NVDIMM-%d, nvdimm-device_index); +memory_region_init_ram_ptr(nvdimm-mr, OBJECT(dev), name, nvdimm_size, + buf); How is the autogenerated name used? Why not just use pc-nvdimm.memory? Ah. Just for debug proposal :) and i am not sure if a name used for multiple MRs (MemoryRegion) is a good idea. Other devices use a constant name too (git grep memory_region_init_ram_ptr) so it seems to be okay. The unique thing is the OBJECT(dev) which differs for each NVDIMM instance. -- 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 3/3] KVM: arm64: Implement accessors for vGIC CPU interface registers
This commit adds accessors for all registers, being part of saved vGIC context in the form of ICH_VMCR_EL2. This is necessary for enabling vGICv3 live migration. Signed-off-by: Pavel Fedin p.fe...@samsung.com --- arch/arm64/kvm/sys_regs.c | 176 + include/linux/irqchip/arm-gic-v3.h | 18 +++- 2 files changed, 192 insertions(+), 2 deletions(-) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 8cc4a5e..7a4f982 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -23,6 +23,7 @@ #include linux/kvm_host.h #include linux/mm.h #include linux/uaccess.h +#include linux/irqchip/arm-gic-v3.h #include asm/cacheflush.h #include asm/cputype.h @@ -136,6 +137,162 @@ static bool access_gic_sgi(struct kvm_vcpu *vcpu, return true; } +static bool access_gic_ctlr(struct kvm_vcpu *vcpu, + const struct sys_reg_params *p, + const struct sys_reg_desc *r) +{ + u64 val; + struct vgic_v3_cpu_if *vgicv3 = vcpu-arch.vgic_cpu.vgic_v3; + + if (vcpu-kvm-arch.vgic.vgic_model != KVM_DEV_TYPE_ARM_VGIC_V3) + return false; + + if (p-is_write) { + val = *vcpu_reg(vcpu, p-Rt); + + vgicv3-vgic_vmcr = ~(ICH_VMCR_CBPR|ICH_VMCR_EOIM); + vgicv3-vgic_vmcr |= (val (ICH_VMCR_CBPR_SHIFT - + ICC_CTLR_EL1_CBPR_SHIFT)) + ICH_VMCR_CBPR; + vgicv3-vgic_vmcr |= (val (ICH_VMCR_EOIM_SHIFT - + ICC_CTLR_EL1_EOImode_SHIFT)) + ICH_VMCR_EOIM; + } else { + asm volatile(mrs_s %0, __stringify(ICC_IAR1_EL1) +: =r (val)); + val = (ICC_CTLR_EL1_A3V | ICC_CTLR_EL1_SEIS | + ICC_CTLR_EL1_IDbits_MASK | ICC_CTLR_EL1_PRIbits_MASK); + val |= (vgicv3-vgic_vmcr ICH_VMCR_CBPR) + (ICH_VMCR_CBPR_SHIFT - ICC_CTLR_EL1_CBPR_SHIFT); + val |= (vgicv3-vgic_vmcr ICH_VMCR_EOIM) + (ICH_VMCR_EOIM_SHIFT - ICC_CTLR_EL1_EOImode_SHIFT); + + *vcpu_reg(vcpu, p-Rt) = val; + } + + return true; +} + +static bool access_gic_pmr(struct kvm_vcpu *vcpu, + const struct sys_reg_params *p, + const struct sys_reg_desc *r) +{ + u64 val; + struct vgic_v3_cpu_if *vgicv3 = vcpu-arch.vgic_cpu.vgic_v3; + + if (vcpu-kvm-arch.vgic.vgic_model != KVM_DEV_TYPE_ARM_VGIC_V3) + return false; + + if (p-is_write) { + val = *vcpu_reg(vcpu, p-Rt); + vgicv3-vgic_vmcr = ~ICH_VMCR_PMR_MASK; + vgicv3-vgic_vmcr |= (val ICH_VMCR_PMR_SHIFT) + ICH_VMCR_PMR_MASK; + } else { + val = (vgicv3-vgic_vmcr ICH_VMCR_PMR_MASK) + ICH_VMCR_PMR_SHIFT; + *vcpu_reg(vcpu, p-Rt) = val; + } + + return true; +} + +static bool access_gic_bpr0(struct kvm_vcpu *vcpu, + const struct sys_reg_params *p, + const struct sys_reg_desc *r) +{ + u64 val; + struct vgic_v3_cpu_if *vgicv3 = vcpu-arch.vgic_cpu.vgic_v3; + + if (vcpu-kvm-arch.vgic.vgic_model != KVM_DEV_TYPE_ARM_VGIC_V3) + return false; + + if (p-is_write) { + val = *vcpu_reg(vcpu, p-Rt); + vgicv3-vgic_vmcr = ~ICH_VMCR_BPR0_MASK; + vgicv3-vgic_vmcr |= (val ICH_VMCR_BPR0_SHIFT) + ICH_VMCR_BPR0_MASK; + } else { + val = (vgicv3-vgic_vmcr ICH_VMCR_BPR0_MASK) + ICH_VMCR_BPR0_SHIFT; + *vcpu_reg(vcpu, p-Rt) = val; + } + + return true; +} + +static bool access_gic_bpr1(struct kvm_vcpu *vcpu, + const struct sys_reg_params *p, + const struct sys_reg_desc *r) +{ + u64 val; + struct vgic_v3_cpu_if *vgicv3 = vcpu-arch.vgic_cpu.vgic_v3; + + if (vcpu-kvm-arch.vgic.vgic_model != KVM_DEV_TYPE_ARM_VGIC_V3) + return false; + + if (p-is_write) { + val = *vcpu_reg(vcpu, p-Rt); + vgicv3-vgic_vmcr = ~ICH_VMCR_BPR1_MASK; + vgicv3-vgic_vmcr |= (val ICH_VMCR_BPR1_SHIFT) + ICH_VMCR_BPR1_MASK; + } else { + val = (vgicv3-vgic_vmcr ICH_VMCR_BPR1_MASK) + ICH_VMCR_BPR1_SHIFT; + *vcpu_reg(vcpu, p-Rt) = val; + } + + return true; +} + +static bool access_gic_grpen0(struct kvm_vcpu *vcpu, + const struct sys_reg_params *p, + const struct sys_reg_desc *r) +{ + u64 val; +
[PATCH 2/3] KVM: arm64: Allow to use accessors in KVM_SET_ONE_REG and KVM_GET_ONE_REG
Call accessor functions if the register is defined in the list but does not have accociated cell in CPU context. This allows to implement accessors for vGIC CPU interface, necessary for live migration. Signed-off-by: Pavel Fedin p.fe...@samsung.com --- arch/arm64/kvm/sys_regs.c | 47 +-- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index b41607d..8cc4a5e 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -1306,10 +1306,6 @@ static const struct sys_reg_desc *index_to_sys_reg_desc(struct kvm_vcpu *vcpu, if (!r) r = find_reg(params, sys_reg_descs, ARRAY_SIZE(sys_reg_descs)); - /* Not saved in the sys_reg array? */ - if (r !r-reg) - r = NULL; - return r; } @@ -1533,6 +1529,7 @@ int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg { const struct sys_reg_desc *r; void __user *uaddr = (void __user *)(unsigned long)reg-addr; + u64 val; if ((reg-id KVM_REG_ARM_COPROC_MASK) == KVM_REG_ARM_DEMUX) return demux_c15_get(reg-id, uaddr); @@ -1547,13 +1544,31 @@ int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg if (r-get_user) return (r-get_user)(vcpu, r, reg, uaddr); - return reg_to_user(uaddr, vcpu_sys_reg(vcpu, r-reg), reg-id); + if (r-reg) { + val = vcpu_sys_reg(vcpu, r-reg); + } else { + struct sys_reg_params params; + + if (!index_to_params(reg-id, params)) + return -ENOENT; + + params.is_write = false; + params.is_aarch32 = false; + params.is_32bit = false; + + if (!r-access(vcpu, params, r)) + return -EINVAL; + } + + return reg_to_user(uaddr, val, reg-id); } int kvm_arm_sys_reg_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) { const struct sys_reg_desc *r; void __user *uaddr = (void __user *)(unsigned long)reg-addr; + u64 val; + int ret; if ((reg-id KVM_REG_ARM_COPROC_MASK) == KVM_REG_ARM_DEMUX) return demux_c15_set(reg-id, uaddr); @@ -1568,7 +1583,27 @@ int kvm_arm_sys_reg_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg if (r-set_user) return (r-set_user)(vcpu, r, reg, uaddr); - return reg_from_user(vcpu_sys_reg(vcpu, r-reg), uaddr, reg-id); + ret = reg_from_user(val, uaddr, reg-id); + if (ret) + return ret; + + if (r-reg) { + vcpu_sys_reg(vcpu, r-reg) = val; + } else { + struct sys_reg_params params; + + if (!index_to_params(reg-id, params)) + return -ENOENT; + + params.is_write = true; + params.is_aarch32 = false; + params.is_32bit = false; + + if (!r-access(vcpu, params, r)) + return -EINVAL; + } + + return 0; } static unsigned int num_demux_regs(void) -- 2.4.4 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] KVM: arm64: Implement vGICv3 distributor and redistributor access from userspace
The access is done similar to GICv2, using KVM_DEV_ARM_VGIC_GRP_DIST_REGS and KVM_DEV_ARM_VGIC_GRP_REDIST_REGS with KVM_SET_DEVICE_ATTR and KVM_GET_DEVICE_ATTR ioctls. Registers are always assumed to be of their native size, 4 or 8 bytes. Signed-off-by: Pavel Fedin p.fe...@samsung.com --- arch/arm64/include/uapi/asm/kvm.h | 1 + virt/kvm/arm/vgic-v3-emul.c | 186 +++--- 2 files changed, 172 insertions(+), 15 deletions(-) diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h index 0cd7b59..2936651 100644 --- a/arch/arm64/include/uapi/asm/kvm.h +++ b/arch/arm64/include/uapi/asm/kvm.h @@ -203,6 +203,7 @@ struct kvm_arch_memory_slot { #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS 3 #define KVM_DEV_ARM_VGIC_GRP_CTRL 4 #define KVM_DEV_ARM_VGIC_CTRL_INIT 0 +#define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5 /* KVM_IRQ_LINE irq field index values */ #define KVM_ARM_IRQ_TYPE_SHIFT 24 diff --git a/virt/kvm/arm/vgic-v3-emul.c b/virt/kvm/arm/vgic-v3-emul.c index e661e7f..b3847e1 100644 --- a/virt/kvm/arm/vgic-v3-emul.c +++ b/virt/kvm/arm/vgic-v3-emul.c @@ -39,6 +39,7 @@ #include linux/kvm.h #include linux/kvm_host.h #include linux/interrupt.h +#include linux/uaccess.h #include linux/irqchip/arm-gic-v3.h #include kvm/arm_vgic.h @@ -990,6 +991,107 @@ void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg) vgic_kick_vcpus(vcpu-kvm); } +static int vgic_v3_attr_regs_access(struct kvm_device *dev, +struct kvm_device_attr *attr, +void *reg, u32 len, bool is_write) +{ + const struct vgic_io_range *r = NULL, *ranges; + phys_addr_t offset; + int ret, cpuid, c; + struct kvm_vcpu *vcpu, *tmp_vcpu; + struct vgic_dist *vgic; + struct kvm_exit_mmio mmio; + u64 data; + + offset = attr-attr KVM_DEV_ARM_VGIC_OFFSET_MASK; + cpuid = (attr-attr KVM_DEV_ARM_VGIC_CPUID_MASK) + KVM_DEV_ARM_VGIC_CPUID_SHIFT; + + mutex_lock(dev-kvm-lock); + + ret = vgic_init(dev-kvm); + if (ret) + goto out; + + if (cpuid = atomic_read(dev-kvm-online_vcpus)) { + ret = -EINVAL; + goto out; + } + + vcpu = kvm_get_vcpu(dev-kvm, cpuid); + vgic = dev-kvm-arch.vgic; + + mmio.len = len; + mmio.is_write = is_write; + mmio.data = data; + if (is_write) { + if (len == 8) + data = cpu_to_le64(*((u64 *)reg)); + else + mmio_data_write(mmio, ~0, *((u32 *)reg)); + } + switch (attr-group) { + case KVM_DEV_ARM_VGIC_GRP_DIST_REGS: + mmio.phys_addr = vgic-vgic_dist_base + offset; + ranges = vgic_v3_dist_ranges; + break; + case KVM_DEV_ARM_VGIC_GRP_REDIST_REGS: + mmio.phys_addr = vgic-vgic_redist_base + offset; + ranges = vgic_redist_ranges; + break; + default: + BUG(); + } + r = vgic_find_range(ranges, 4, offset); + + if (unlikely(!r || !r-handle_mmio)) { + ret = -ENXIO; + goto out; + } + + + spin_lock(vgic-lock); + + /* +* Ensure that no other VCPU is running by checking the vcpu-cpu +* field. If no other VPCUs are running we can safely access the VGIC +* state, because even if another VPU is run after this point, that +* VCPU will not touch the vgic state, because it will block on +* getting the vgic-lock in kvm_vgic_sync_hwstate(). +*/ + kvm_for_each_vcpu(c, tmp_vcpu, dev-kvm) { + if (unlikely(tmp_vcpu-cpu != -1)) { + ret = -EBUSY; + goto out_vgic_unlock; + } + } + + /* +* Move all pending IRQs from the LRs on all VCPUs so the pending +* state can be properly represented in the register state accessible +* through this API. +*/ + kvm_for_each_vcpu(c, tmp_vcpu, dev-kvm) + vgic_unqueue_irqs(tmp_vcpu); + + offset -= r-base; + r-handle_mmio(vcpu, mmio, offset); + + if (!is_write) { + if (len == 8) + *(u64 *)reg = le64_to_cpu(data); + else + *(u32 *)reg = mmio_data_read(mmio, ~0); + } + + ret = 0; +out_vgic_unlock: + spin_unlock(vgic-lock); +out: + mutex_unlock(dev-kvm-lock); + return ret; +} + static int vgic_v3_create(struct kvm_device *dev, u32 type) { return kvm_vgic_create(dev-kvm, type); @@ -1000,40 +1102,95 @@ static void vgic_v3_destroy(struct kvm_device *dev) kfree(dev); } +static u32 vgic_v3_get_reg_size(struct kvm_device_attr *attr) +{ + u32 offset; + + switch (attr-group) { + case KVM_DEV_ARM_VGIC_GRP_DIST_REGS: +
Re: [PATCH v2 07/18] nvdimm: reserve address range for NVDIMM
On Fri, Aug 14, 2015 at 10:52:00PM +0800, Xiao Guangrong wrote: NVDIMM reserves all the free range above 4G to do: - Persistent Memory (PMEM) mapping - implement NVDIMM ACPI device _DSM method Signed-off-by: Xiao Guangrong guangrong.x...@linux.intel.com [...] @@ -1302,6 +1303,7 @@ FWCfgState *pc_memory_init(MachineState *machine, MemoryRegion *ram_below_4g, *ram_above_4g; FWCfgState *fw_cfg; PCMachineState *pcms = PC_MACHINE(machine); +ram_addr_t offset; offset is a very generic name. I suggest naming it nvdimm_offset. assert(machine-ram_size == below_4g_mem_size + above_4g_mem_size); @@ -1339,6 +1341,8 @@ FWCfgState *pc_memory_init(MachineState *machine, exit(EXIT_FAILURE); } +offset = 0x1ULL + above_4g_mem_size; + /* initialize hotplug memory address space */ if (guest_info-has_reserved_memory (machine-ram_size machine-maxram_size)) { @@ -1358,8 +1362,7 @@ FWCfgState *pc_memory_init(MachineState *machine, exit(EXIT_FAILURE); } -pcms-hotplug_memory.base = -ROUND_UP(0x1ULL + above_4g_mem_size, 1ULL 30); +pcms-hotplug_memory.base = ROUND_UP(offset, 1ULL 30); if (pcms-enforce_aligned_dimm) { /* size hotplug region assuming 1G page max alignment per slot */ -- Eduardo -- 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