Re: [Qemu-devel] [RISU PATCH v3 06/22] Makefile: include risu_reginfo_$(ARCH) in HDRS
On 06/13/2018 02:55 AM, Alex Bennée wrote: > Otherwise changes to reginfo don't cause the whole thing to be > re-built causing much confusion when bisecting. > > Signed-off-by: Alex Bennée > --- > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [RISU PATCH v3 05/22] risu: add zlib indication to help text
On 06/13/2018 02:55 AM, Alex Bennée wrote: > This is a simple aide-memoir as it can be tricky to determine this > with a simple statically compiled binary. > > Signed-off-by: Alex Bennée > --- > risu.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) Eh, sure. Or we could simply require zlib... Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [RISU PATCH v3 04/22] build-all-arches: do a distclean $(SRC) configured
On 06/13/2018 02:55 AM, Alex Bennée wrote: > This can cause much confusion when you have been building in your > source tree. I've added a distclean so we don't unexpectedly drop the > config for normal make clean invocations. > > Signed-off-by: Alex Bennée > --- > Makefile| 3 +++ > build-all-archs | 10 ++ > 2 files changed, 13 insertions(+) Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [RISU PATCH v3 03/22] build-all-arches: expand the range of docker images
On 06/13/2018 02:55 AM, Alex Bennée wrote: > We won't actually want power, we want ppc64el for the 64 bit version. > Also we will soon have m68k so include that as well. > > Signed-off-by: Alex Bennée > --- > build-all-archs | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [RISU PATCH v3 02/22] comms: include header for writev
On 06/13/2018 02:55 AM, Alex Bennée wrote: > The compiler complains about implicit declarations otherwise. > > Signed-off-by: Alex Bennée > --- > comms.c | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [RISU PATCH v3 01/22] risu_reginfo_aarch64: include signal.h for FPSIMD_MAGIC
On 06/13/2018 02:55 AM, Alex Bennée wrote: > Building with the Bionic Beaver cross compilers fails which probably > means we were getting this as a side effect before. Include the > correct header to get at these bits. > > Signed-off-by: Alex Bennée > --- > risu_reginfo_aarch64.c | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [PATCHv3 6/7] target/ppc: Replace intc pointer with a general machine_data pointer
On 06/14/2018 06:41 AM, David Gibson wrote:
> PowerPCCPU contains an (Object *)intc used to point to the cpu's interrupt
> controller. Or more precisely to the "presentation" component of the
> interrupt controller relevant to this cpu.
>
> Really, this field is machine specific. The machines which use it can
> point it to different types of object depending on their needs, and most
> machines don't use it at all (since they have older style PICs which don't
> have per-cpu presentation components).
>
> There's also other information that's per-cpu, but platform/machine
> specific. So replace the intc pointer with a (void *)machine_data which
> can be managed as the machine type likes to conveniently store per cpu
> information.
I will add the XiveTCXT (Thread interrupt state context) on both machines.
> Signed-off-by: David Gibson
> Reviewed-by: Greg Kurz
Reviewed-by: Cédric Le Goater
Thanks,
C.
> ---
> hw/intc/xics.c | 5 +++--
> hw/intc/xics_spapr.c| 16 +++-
> hw/ppc/pnv.c| 4 ++--
> hw/ppc/pnv_core.c | 11 +--
> hw/ppc/spapr.c | 8
> hw/ppc/spapr_cpu_core.c | 13 ++---
> include/hw/ppc/pnv_core.h | 9 +
> include/hw/ppc/spapr_cpu_core.h | 10 ++
> include/hw/ppc/xics.h | 4 ++--
> target/ppc/cpu.h| 2 +-
> 10 files changed, 61 insertions(+), 21 deletions(-)
>
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index e73e623e3b..689ad44e5f 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -383,7 +383,8 @@ static const TypeInfo icp_info = {
> .class_size = sizeof(ICPStateClass),
> };
>
> -Object *icp_create(Object *cpu, const char *type, XICSFabric *xi, Error
> **errp)
> +ICPState *icp_create(Object *cpu, const char *type, XICSFabric *xi,
> + Error **errp)
> {
> Error *local_err = NULL;
> Object *obj;
> @@ -401,7 +402,7 @@ Object *icp_create(Object *cpu, const char *type,
> XICSFabric *xi, Error **errp)
> obj = NULL;
> }
>
> -return obj;
> +return ICP(obj);
> }
>
> /*
> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> index 2e27b92b87..01c76717cf 100644
> --- a/hw/intc/xics_spapr.c
> +++ b/hw/intc/xics_spapr.c
> @@ -31,6 +31,7 @@
> #include "trace.h"
> #include "qemu/timer.h"
> #include "hw/ppc/spapr.h"
> +#include "hw/ppc/spapr_cpu_core.h"
> #include "hw/ppc/xics.h"
> #include "hw/ppc/fdt.h"
> #include "qapi/visitor.h"
> @@ -43,8 +44,9 @@ static target_ulong h_cppr(PowerPCCPU *cpu,
> sPAPRMachineState *spapr,
> target_ulong opcode, target_ulong *args)
> {
> target_ulong cppr = args[0];
> +sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
>
> -icp_set_cppr(ICP(cpu->intc), cppr);
> +icp_set_cppr(spapr_cpu->icp, cppr);
> return H_SUCCESS;
> }
>
> @@ -65,7 +67,8 @@ static target_ulong h_ipi(PowerPCCPU *cpu,
> sPAPRMachineState *spapr,
> static target_ulong h_xirr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> target_ulong opcode, target_ulong *args)
> {
> -uint32_t xirr = icp_accept(ICP(cpu->intc));
> +sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
> +uint32_t xirr = icp_accept(spapr_cpu->icp);
>
> args[0] = xirr;
> return H_SUCCESS;
> @@ -74,7 +77,8 @@ static target_ulong h_xirr(PowerPCCPU *cpu,
> sPAPRMachineState *spapr,
> static target_ulong h_xirr_x(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> target_ulong opcode, target_ulong *args)
> {
> -uint32_t xirr = icp_accept(ICP(cpu->intc));
> +sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
> +uint32_t xirr = icp_accept(spapr_cpu->icp);
>
> args[0] = xirr;
> args[1] = cpu_get_host_ticks();
> @@ -84,9 +88,10 @@ static target_ulong h_xirr_x(PowerPCCPU *cpu,
> sPAPRMachineState *spapr,
> static target_ulong h_eoi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>target_ulong opcode, target_ulong *args)
> {
> +sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
> target_ulong xirr = args[0];
>
> -icp_eoi(ICP(cpu->intc), xirr);
> +icp_eoi(spapr_cpu->icp, xirr);
> return H_SUCCESS;
> }
>
> @@ -94,7 +99,8 @@ static target_ulong h_ipoll(PowerPCCPU *cpu,
> sPAPRMachineState *spapr,
> target_ulong opcode, target_ulong *args)
> {
> uint32_t mfrr;
> -uint32_t xirr = icp_ipoll(ICP(cpu->intc), );
> +sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
> +uint32_t xirr = icp_ipoll(spapr_cpu->icp, );
>
> args[0] = xirr;
> args[1] = mfrr;
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 0b9508d94d..3a36c6ac6a 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -1013,7 +1013,7 @@ static ICPState *pnv_icp_get(XICSFabric *xi, int pir)
> {
> PowerPCCPU *cpu = ppc_get_vcpu_by_pir(pir);
>
> -return cpu ?
[Qemu-devel] [PATCH] xilinx_spips: Make dma transactions as per dma_burst_size
Qspi dma has a burst length of 64 bytes, So limit transaction length to
64 max.
Signed-off-by: Sai Pavan Boddu
---
hw/ssi/xilinx_spips.c | 18 +++---
include/hw/ssi/xilinx_spips.h | 3 ++-
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
index 03f5fae..ea006c4 100644
--- a/hw/ssi/xilinx_spips.c
+++ b/hw/ssi/xilinx_spips.c
@@ -851,12 +851,17 @@ static void xlnx_zynqmp_qspips_notify(void *opaque)
{
size_t ret;
uint32_t num;
-const void *rxd = pop_buf(recv_fifo, 4, );
+const void *rxd;
+int len;
+
+len = recv_fifo->num >= rq->dma_burst_size ? rq->dma_burst_size :
+ recv_fifo->num;
+rxd = pop_buf(recv_fifo, len, );
memcpy(rq->dma_buf, rxd, num);
-ret = stream_push(rq->dma, rq->dma_buf, 4);
-assert(ret == 4);
+ret = stream_push(rq->dma, rq->dma_buf, num);
+assert(ret == num);
xlnx_zynqmp_qspips_check_flush(rq);
}
}
@@ -1337,6 +1342,7 @@ static void xlnx_zynqmp_qspips_realize(DeviceState *dev,
Error **errp)
fifo8_create(>rx_fifo_g, xsc->rx_fifo_size);
fifo8_create(>tx_fifo_g, xsc->tx_fifo_size);
fifo32_create(>fifo_g, 32);
+s->dma_buf = g_new0(uint8_t, s->dma_burst_size);
}
static void xlnx_zynqmp_qspips_init(Object *obj)
@@ -1411,6 +1417,11 @@ static const VMStateDescription
vmstate_xlnx_zynqmp_qspips = {
}
};
+static Property xilinx_zynqmp_qspips_properties[] = {
+DEFINE_PROP_UINT32("dma-burst-size", XlnxZynqMPQSPIPS, dma_burst_size, 64),
+DEFINE_PROP_END_OF_LIST(),
+};
+
static Property xilinx_qspips_properties[] = {
/* We had to turn this off for 2.10 as it is not compatible with migration.
* It can be enabled but will prevent the device to be migrated.
@@ -1463,6 +1474,7 @@ static void xlnx_zynqmp_qspips_class_init(ObjectClass
*klass, void * data)
dc->realize = xlnx_zynqmp_qspips_realize;
dc->reset = xlnx_zynqmp_qspips_reset;
dc->vmsd = _xlnx_zynqmp_qspips;
+dc->props = xilinx_zynqmp_qspips_properties;
xsc->reg_ops = _zynqmp_qspips_ops;
xsc->rx_fifo_size = RXFF_A_Q;
xsc->tx_fifo_size = TXFF_A_Q;
diff --git a/include/hw/ssi/xilinx_spips.h b/include/hw/ssi/xilinx_spips.h
index d398a4e..cca1813 100644
--- a/include/hw/ssi/xilinx_spips.h
+++ b/include/hw/ssi/xilinx_spips.h
@@ -95,7 +95,8 @@ typedef struct {
XilinxQSPIPS parent_obj;
StreamSlave *dma;
-uint8_t dma_buf[4];
+uint8_t *dma_buf;
+uint32_t dma_burst_size;
int gqspi_irqline;
uint32_t regs[XLNX_ZYNQMP_SPIPS_R_MAX];
--
2.7.4
Re: [Qemu-devel] [PATCHv3 2/7] pnv: Fix some error handling cpu realize()
On 06/14/2018 06:41 AM, David Gibson wrote:
> In pnv_core_realize() we call two functions with an Error * parameter in
> succession, which will go badly if they both cause errors. In fact, a
> failure in either of them indicates a qemu internal error, so we can just
> use _abort in both cases.
Yes. I think that most of the object_property_add_child() could use
_abort, object_property_add_const_link() also.
> Signed-off-by: David Gibson
Reviewed-by: Cédric Le Goater
Thanks,
C.
> ---
> hw/ppc/pnv_core.c | 7 ++-
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> index 13ad7d9e04..01f47c8037 100644
> --- a/hw/ppc/pnv_core.c
> +++ b/hw/ppc/pnv_core.c
> @@ -172,12 +172,9 @@ static void pnv_core_realize(DeviceState *dev, Error
> **errp)
> object_initialize(obj, size, typename);
>
> snprintf(name, sizeof(name), "thread[%d]", i);
> -object_property_add_child(OBJECT(pc), name, obj, _err);
> +object_property_add_child(OBJECT(pc), name, obj, _abort);
> object_property_add_alias(obj, "core-pir", OBJECT(pc),
> - "pir", _err);
> -if (local_err) {
> -goto err;
> -}
> + "pir", _abort);
> object_unref(obj);
> }
>
>
[Qemu-devel] [PATCH] xilinx_spips: Make dma transactions as per dma_burst_size
Qspi dma has a burst length of 64 bytes, So limit transaction length to
64 max.
Signed-off-by: Sai Pavan Boddu
---
hw/ssi/xilinx_spips.c | 18 +++---
include/hw/ssi/xilinx_spips.h | 3 ++-
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
index 03f5fae..ea006c4 100644
--- a/hw/ssi/xilinx_spips.c
+++ b/hw/ssi/xilinx_spips.c
@@ -851,12 +851,17 @@ static void xlnx_zynqmp_qspips_notify(void *opaque)
{
size_t ret;
uint32_t num;
-const void *rxd = pop_buf(recv_fifo, 4, );
+const void *rxd;
+int len;
+
+len = recv_fifo->num >= rq->dma_burst_size ? rq->dma_burst_size :
+ recv_fifo->num;
+rxd = pop_buf(recv_fifo, len, );
memcpy(rq->dma_buf, rxd, num);
-ret = stream_push(rq->dma, rq->dma_buf, 4);
-assert(ret == 4);
+ret = stream_push(rq->dma, rq->dma_buf, num);
+assert(ret == num);
xlnx_zynqmp_qspips_check_flush(rq);
}
}
@@ -1337,6 +1342,7 @@ static void xlnx_zynqmp_qspips_realize(DeviceState *dev,
Error **errp)
fifo8_create(>rx_fifo_g, xsc->rx_fifo_size);
fifo8_create(>tx_fifo_g, xsc->tx_fifo_size);
fifo32_create(>fifo_g, 32);
+s->dma_buf = g_new0(uint8_t, s->dma_burst_size);
}
static void xlnx_zynqmp_qspips_init(Object *obj)
@@ -1411,6 +1417,11 @@ static const VMStateDescription
vmstate_xlnx_zynqmp_qspips = {
}
};
+static Property xilinx_zynqmp_qspips_properties[] = {
+DEFINE_PROP_UINT32("dma-burst-size", XlnxZynqMPQSPIPS, dma_burst_size, 64),
+DEFINE_PROP_END_OF_LIST(),
+};
+
static Property xilinx_qspips_properties[] = {
/* We had to turn this off for 2.10 as it is not compatible with migration.
* It can be enabled but will prevent the device to be migrated.
@@ -1463,6 +1474,7 @@ static void xlnx_zynqmp_qspips_class_init(ObjectClass
*klass, void * data)
dc->realize = xlnx_zynqmp_qspips_realize;
dc->reset = xlnx_zynqmp_qspips_reset;
dc->vmsd = _xlnx_zynqmp_qspips;
+dc->props = xilinx_zynqmp_qspips_properties;
xsc->reg_ops = _zynqmp_qspips_ops;
xsc->rx_fifo_size = RXFF_A_Q;
xsc->tx_fifo_size = TXFF_A_Q;
diff --git a/include/hw/ssi/xilinx_spips.h b/include/hw/ssi/xilinx_spips.h
index d398a4e..cca1813 100644
--- a/include/hw/ssi/xilinx_spips.h
+++ b/include/hw/ssi/xilinx_spips.h
@@ -95,7 +95,8 @@ typedef struct {
XilinxQSPIPS parent_obj;
StreamSlave *dma;
-uint8_t dma_buf[4];
+uint8_t *dma_buf;
+uint32_t dma_burst_size;
int gqspi_irqline;
uint32_t regs[XLNX_ZYNQMP_SPIPS_R_MAX];
--
2.7.4
[Qemu-devel] [PATCHv3 7/7] target/ppc, spapr: Move VPA information to machine_data
CPUPPCState currently contains a number of fields containing the state of
the VPA. The VPA is a PAPR specific concept covering several guest/host
shared memory areas used to communicate some information with the
hypervisor.
As a PAPR concept this is really machine specific information, although it
is per-cpu, so it doesn't really belong in the core CPU state structure.
So, move it to the PAPR specific 'machine_data' structure.
Signed-off-by: David Gibson
Reviewed-by: Greg Kurz
---
hw/ppc/spapr_cpu_core.c | 7 +++
hw/ppc/spapr_hcall.c| 77 ++---
include/hw/ppc/spapr_cpu_core.h | 3 ++
target/ppc/cpu.h| 6 ---
target/ppc/kvm.c| 36 +++
target/ppc/translate_init.inc.c | 8
6 files changed, 70 insertions(+), 67 deletions(-)
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 544bda93e2..f642c95967 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -28,6 +28,7 @@ static void spapr_cpu_reset(void *opaque)
CPUState *cs = CPU(cpu);
CPUPPCState *env = >env;
PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
target_ulong lpcr;
cpu_reset(cs);
@@ -69,6 +70,12 @@ static void spapr_cpu_reset(void *opaque)
/* Set a full AMOR so guest can use the AMR as it sees fit */
env->spr[SPR_AMOR] = 0xull;
+
+spapr_cpu->vpa_addr = 0;
+spapr_cpu->slb_shadow_addr = 0;
+spapr_cpu->slb_shadow_size = 0;
+spapr_cpu->dtl_addr = 0;
+spapr_cpu->dtl_size = 0;
}
void spapr_cpu_set_entry_state(PowerPCCPU *cpu, target_ulong nip, target_ulong
r3)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 8b9a4b577f..ae913d070f 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -8,6 +8,7 @@
#include "exec/exec-all.h"
#include "helper_regs.h"
#include "hw/ppc/spapr.h"
+#include "hw/ppc/spapr_cpu_core.h"
#include "mmu-hash64.h"
#include "cpu-models.h"
#include "trace.h"
@@ -908,9 +909,11 @@ unmap_out:
#define VPA_SHARED_PROC_OFFSET 0x9
#define VPA_SHARED_PROC_VAL0x2
-static target_ulong register_vpa(CPUPPCState *env, target_ulong vpa)
+static target_ulong register_vpa(PowerPCCPU *cpu, target_ulong vpa)
{
-CPUState *cs = CPU(ppc_env_get_cpu(env));
+CPUState *cs = CPU(cpu);
+CPUPPCState *env = >env;
+sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
uint16_t size;
uint8_t tmp;
@@ -935,32 +938,34 @@ static target_ulong register_vpa(CPUPPCState *env,
target_ulong vpa)
return H_PARAMETER;
}
-env->vpa_addr = vpa;
+spapr_cpu->vpa_addr = vpa;
-tmp = ldub_phys(cs->as, env->vpa_addr + VPA_SHARED_PROC_OFFSET);
+tmp = ldub_phys(cs->as, spapr_cpu->vpa_addr + VPA_SHARED_PROC_OFFSET);
tmp |= VPA_SHARED_PROC_VAL;
-stb_phys(cs->as, env->vpa_addr + VPA_SHARED_PROC_OFFSET, tmp);
+stb_phys(cs->as, spapr_cpu->vpa_addr + VPA_SHARED_PROC_OFFSET, tmp);
return H_SUCCESS;
}
-static target_ulong deregister_vpa(CPUPPCState *env, target_ulong vpa)
+static target_ulong deregister_vpa(PowerPCCPU *cpu, target_ulong vpa)
{
-if (env->slb_shadow_addr) {
+sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
+
+if (spapr_cpu->slb_shadow_addr) {
return H_RESOURCE;
}
-if (env->dtl_addr) {
+if (spapr_cpu->dtl_addr) {
return H_RESOURCE;
}
-env->vpa_addr = 0;
+spapr_cpu->vpa_addr = 0;
return H_SUCCESS;
}
-static target_ulong register_slb_shadow(CPUPPCState *env, target_ulong addr)
+static target_ulong register_slb_shadow(PowerPCCPU *cpu, target_ulong addr)
{
-CPUState *cs = CPU(ppc_env_get_cpu(env));
+sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
uint32_t size;
if (addr == 0) {
@@ -968,7 +973,7 @@ static target_ulong register_slb_shadow(CPUPPCState *env,
target_ulong addr)
return H_HARDWARE;
}
-size = ldl_be_phys(cs->as, addr + 0x4);
+size = ldl_be_phys(CPU(cpu)->as, addr + 0x4);
if (size < 0x8) {
return H_PARAMETER;
}
@@ -977,26 +982,28 @@ static target_ulong register_slb_shadow(CPUPPCState *env,
target_ulong addr)
return H_PARAMETER;
}
-if (!env->vpa_addr) {
+if (!spapr_cpu->vpa_addr) {
return H_RESOURCE;
}
-env->slb_shadow_addr = addr;
-env->slb_shadow_size = size;
+spapr_cpu->slb_shadow_addr = addr;
+spapr_cpu->slb_shadow_size = size;
return H_SUCCESS;
}
-static target_ulong deregister_slb_shadow(CPUPPCState *env, target_ulong addr)
+static target_ulong deregister_slb_shadow(PowerPCCPU *cpu, target_ulong addr)
{
-env->slb_shadow_addr = 0;
-env->slb_shadow_size = 0;
+sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
+
+spapr_cpu->slb_shadow_addr = 0;
+spapr_cpu->slb_shadow_size = 0;
return H_SUCCESS;
}
-static target_ulong register_dtl(CPUPPCState *env, target_ulong addr)
[Qemu-devel] [PATCHv3 0/7] Better handling of machine specific per-cpu information
It's moderately common for a machine type to need to keep track of information that is specific to the platform it implements, but per-cpu. While it could keep such information inside the MachineState, this makes lookup from the CPUState awkward. So, this series adds a standard way to stash machine-specific per-cpu information using a void pointer in the PowerPCCPU object. The machine is responsible for alloc()ing, free()ing and (if applicable) migrating this state. The meat of the series is the last two patches. The first 5 clean up a number of minor uglies I encountered while implementing. Chganges since v2: * Fix compile (and other) errors in KVM code. Changes since v1: * Simplify the error handling cleanup in 2/7 to use _abort instead of more propagations David Gibson (7): spapr: Clean up cpu realize/unrealize paths pnv: Fix some error handling cpu realize() pnv_core: Allocate cpu thread objects individually pnv: Clean up cpu realize path pnv: Add cpu unrealize path target/ppc: Replace intc pointer with a general machine_data pointer target/ppc, spapr: Move VPA information to machine_data hw/intc/xics.c | 5 +- hw/intc/xics_spapr.c| 16 +++-- hw/ppc/pnv.c| 8 +-- hw/ppc/pnv_core.c | 100 ++-- hw/ppc/spapr.c | 8 +-- hw/ppc/spapr_cpu_core.c | 85 +-- hw/ppc/spapr_hcall.c| 77 +--- include/hw/ppc/pnv_core.h | 11 +++- include/hw/ppc/spapr_cpu_core.h | 13 + include/hw/ppc/xics.h | 4 +- target/ppc/cpu.h| 8 +-- target/ppc/kvm.c| 36 ++-- target/ppc/translate_init.inc.c | 8 --- 13 files changed, 203 insertions(+), 176 deletions(-) -- 2.17.1
[Qemu-devel] [PATCHv3 4/7] pnv: Clean up cpu realize path
pnv_cpu_init() is only called from the the pnv cpu core realize path, and
really only can be called from there. So fold it into its caller, which
we also rename for brevity.
Signed-off-by: David Gibson
Reviewed-by: Cédric Le Goater
Reviewed-by: Greg Kurz
---
hw/ppc/pnv_core.c | 56 ++-
1 file changed, 21 insertions(+), 35 deletions(-)
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 1e40f01e98..f4c41d89d6 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -54,28 +54,6 @@ static void pnv_cpu_reset(void *opaque)
env->msr |= MSR_HVB; /* Hypervisor mode */
}
-static void pnv_cpu_init(PowerPCCPU *cpu, Error **errp)
-{
-CPUPPCState *env = >env;
-int core_pir;
-int thread_index = 0; /* TODO: TCG supports only one thread */
-ppc_spr_t *pir = >spr_cb[SPR_PIR];
-
-core_pir = object_property_get_uint(OBJECT(cpu), "core-pir", _abort);
-
-/*
- * The PIR of a thread is the core PIR + the thread index. We will
- * need to find a way to get the thread index when TCG supports
- * more than 1. We could use the object name ?
- */
-pir->default_value = core_pir + thread_index;
-
-/* Set time-base frequency to 512 MHz */
-cpu_ppc_tb_init(env, PNV_TIMEBASE_FREQ);
-
-qemu_register_reset(pnv_cpu_reset, cpu);
-}
-
/*
* These values are read by the PowerNV HW monitors under Linux
*/
@@ -121,29 +99,39 @@ static const MemoryRegionOps pnv_core_xscom_ops = {
.endianness = DEVICE_BIG_ENDIAN,
};
-static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp)
+static void pnv_realize_vcpu(PowerPCCPU *cpu, XICSFabric *xi, Error **errp)
{
+CPUPPCState *env = >env;
+int core_pir;
+int thread_index = 0; /* TODO: TCG supports only one thread */
+ppc_spr_t *pir = >spr_cb[SPR_PIR];
Error *local_err = NULL;
-CPUState *cs = CPU(child);
-PowerPCCPU *cpu = POWERPC_CPU(cs);
-object_property_set_bool(child, true, "realized", _err);
+object_property_set_bool(OBJECT(cpu), true, "realized", _err);
if (local_err) {
error_propagate(errp, local_err);
return;
}
-cpu->intc = icp_create(child, TYPE_PNV_ICP, xi, _err);
+cpu->intc = icp_create(OBJECT(cpu), TYPE_PNV_ICP, xi, _err);
if (local_err) {
error_propagate(errp, local_err);
return;
}
-pnv_cpu_init(cpu, _err);
-if (local_err) {
-error_propagate(errp, local_err);
-return;
-}
+core_pir = object_property_get_uint(OBJECT(cpu), "core-pir", _abort);
+
+/*
+ * The PIR of a thread is the core PIR + the thread index. We will
+ * need to find a way to get the thread index when TCG supports
+ * more than 1. We could use the object name ?
+ */
+pir->default_value = core_pir + thread_index;
+
+/* Set time-base frequency to 512 MHz */
+cpu_ppc_tb_init(env, PNV_TIMEBASE_FREQ);
+
+qemu_register_reset(pnv_cpu_reset, cpu);
}
static void pnv_core_realize(DeviceState *dev, Error **errp)
@@ -178,9 +166,7 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
}
for (j = 0; j < cc->nr_threads; j++) {
-obj = OBJECT(pc->threads[j]);
-
-pnv_core_realize_child(obj, XICS_FABRIC(xi), _err);
+pnv_realize_vcpu(pc->threads[j], XICS_FABRIC(xi), _err);
if (local_err) {
goto err;
}
--
2.17.1
[Qemu-devel] [PATCHv3 5/7] pnv: Add cpu unrealize path
Currently we don't have any unrealize path for pnv cpu cores. We get away
with this because we don't yet support cpu hotplug for pnv.
However, we're going to want it eventually, and in the meantime, it makes
it non-obvious why there are a bunch of allocations on the realize() path
that don't have matching frees.
So, implement the missing unrealize path.
Signed-off-by: David Gibson
Reviewed-by: Cédric Le Goater
Reviewed-by: Greg Kurz
---
hw/ppc/pnv_core.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index f4c41d89d6..f7cf33f547 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -186,6 +186,26 @@ err:
error_propagate(errp, local_err);
}
+static void pnv_unrealize_vcpu(PowerPCCPU *cpu)
+{
+qemu_unregister_reset(pnv_cpu_reset, cpu);
+object_unparent(cpu->intc);
+cpu_remove_sync(CPU(cpu));
+object_unparent(OBJECT(cpu));
+}
+
+static void pnv_core_unrealize(DeviceState *dev, Error **errp)
+{
+PnvCore *pc = PNV_CORE(dev);
+CPUCore *cc = CPU_CORE(dev);
+int i;
+
+for (i = 0; i < cc->nr_threads; i++) {
+pnv_unrealize_vcpu(pc->threads[i]);
+}
+g_free(pc->threads);
+}
+
static Property pnv_core_properties[] = {
DEFINE_PROP_UINT32("pir", PnvCore, pir, 0),
DEFINE_PROP_END_OF_LIST(),
@@ -196,6 +216,7 @@ static void pnv_core_class_init(ObjectClass *oc, void *data)
DeviceClass *dc = DEVICE_CLASS(oc);
dc->realize = pnv_core_realize;
+dc->unrealize = pnv_core_unrealize;
dc->props = pnv_core_properties;
}
--
2.17.1
[Qemu-devel] [PATCHv3 2/7] pnv: Fix some error handling cpu realize()
In pnv_core_realize() we call two functions with an Error * parameter in
succession, which will go badly if they both cause errors. In fact, a
failure in either of them indicates a qemu internal error, so we can just
use _abort in both cases.
Signed-off-by: David Gibson
---
hw/ppc/pnv_core.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 13ad7d9e04..01f47c8037 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -172,12 +172,9 @@ static void pnv_core_realize(DeviceState *dev, Error
**errp)
object_initialize(obj, size, typename);
snprintf(name, sizeof(name), "thread[%d]", i);
-object_property_add_child(OBJECT(pc), name, obj, _err);
+object_property_add_child(OBJECT(pc), name, obj, _abort);
object_property_add_alias(obj, "core-pir", OBJECT(pc),
- "pir", _err);
-if (local_err) {
-goto err;
-}
+ "pir", _abort);
object_unref(obj);
}
--
2.17.1
[Qemu-devel] [PATCHv3 1/7] spapr: Clean up cpu realize/unrealize paths
spapr_cpu_init() and spapr_cpu_destroy() are only called from the spapr
cpu core realize/unrealize paths, and really can only be called from there.
Those are all short functions, so fold the pairs together for simplicity.
While we're there rename some functions and change some parameter types
for brevity and clarity.
Signed-off-by: David Gibson
Reviewed-by: Cédric Le Goater
Reviewed-by: Greg Kurz
---
hw/ppc/spapr_cpu_core.c | 69 +++--
1 file changed, 25 insertions(+), 44 deletions(-)
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index f3e9b879b2..7fdb3b6667 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -83,26 +83,6 @@ void spapr_cpu_set_entry_state(PowerPCCPU *cpu, target_ulong
nip, target_ulong r
ppc_store_lpcr(cpu, env->spr[SPR_LPCR] | pcc->lpcr_pm);
}
-static void spapr_cpu_destroy(PowerPCCPU *cpu)
-{
-qemu_unregister_reset(spapr_cpu_reset, cpu);
-}
-
-static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
- Error **errp)
-{
-CPUPPCState *env = >env;
-
-/* Set time-base frequency to 512 MHz */
-cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
-
-cpu_ppc_set_vhyp(cpu, PPC_VIRTUAL_HYPERVISOR(spapr));
-kvmppc_set_papr(cpu);
-
-qemu_register_reset(spapr_cpu_reset, cpu);
-spapr_cpu_reset(cpu);
-}
-
/*
* Return the sPAPR CPU core type for @model which essentially is the CPU
* model specified with -cpu cmdline option.
@@ -122,44 +102,47 @@ const char *spapr_get_cpu_core_type(const char *cpu_type)
return object_class_get_name(oc);
}
-static void spapr_cpu_core_unrealizefn(DeviceState *dev, Error **errp)
+static void spapr_unrealize_vcpu(PowerPCCPU *cpu)
+{
+qemu_unregister_reset(spapr_cpu_reset, cpu);
+object_unparent(cpu->intc);
+cpu_remove_sync(CPU(cpu));
+object_unparent(OBJECT(cpu));
+}
+
+static void spapr_cpu_core_unrealize(DeviceState *dev, Error **errp)
{
sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
CPUCore *cc = CPU_CORE(dev);
int i;
for (i = 0; i < cc->nr_threads; i++) {
-Object *obj = OBJECT(sc->threads[i]);
-DeviceState *dev = DEVICE(obj);
-CPUState *cs = CPU(dev);
-PowerPCCPU *cpu = POWERPC_CPU(cs);
-
-spapr_cpu_destroy(cpu);
-object_unparent(cpu->intc);
-cpu_remove_sync(cs);
-object_unparent(obj);
+spapr_unrealize_vcpu(sc->threads[i]);
}
g_free(sc->threads);
}
-static void spapr_cpu_core_realize_child(Object *child,
- sPAPRMachineState *spapr, Error
**errp)
+static void spapr_realize_vcpu(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+ Error **errp)
{
+CPUPPCState *env = >env;
Error *local_err = NULL;
-CPUState *cs = CPU(child);
-PowerPCCPU *cpu = POWERPC_CPU(cs);
-object_property_set_bool(child, true, "realized", _err);
+object_property_set_bool(OBJECT(cpu), true, "realized", _err);
if (local_err) {
goto error;
}
-spapr_cpu_init(spapr, cpu, _err);
-if (local_err) {
-goto error;
-}
+/* Set time-base frequency to 512 MHz */
+cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
+
+cpu_ppc_set_vhyp(cpu, PPC_VIRTUAL_HYPERVISOR(spapr));
+kvmppc_set_papr(cpu);
-cpu->intc = icp_create(child, spapr->icp_type, XICS_FABRIC(spapr),
+qemu_register_reset(spapr_cpu_reset, cpu);
+spapr_cpu_reset(cpu);
+
+cpu->intc = icp_create(OBJECT(cpu), spapr->icp_type, XICS_FABRIC(spapr),
_err);
if (local_err) {
goto error;
@@ -220,9 +203,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error
**errp)
}
for (j = 0; j < cc->nr_threads; j++) {
-obj = OBJECT(sc->threads[j]);
-
-spapr_cpu_core_realize_child(obj, spapr, _err);
+spapr_realize_vcpu(sc->threads[j], spapr, _err);
if (local_err) {
goto err;
}
@@ -249,7 +230,7 @@ static void spapr_cpu_core_class_init(ObjectClass *oc, void
*data)
sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_CLASS(oc);
dc->realize = spapr_cpu_core_realize;
-dc->unrealize = spapr_cpu_core_unrealizefn;
+dc->unrealize = spapr_cpu_core_unrealize;
dc->props = spapr_cpu_core_properties;
scc->cpu_type = data;
}
--
2.17.1
[Qemu-devel] [PATCHv3 6/7] target/ppc: Replace intc pointer with a general machine_data pointer
PowerPCCPU contains an (Object *)intc used to point to the cpu's interrupt
controller. Or more precisely to the "presentation" component of the
interrupt controller relevant to this cpu.
Really, this field is machine specific. The machines which use it can
point it to different types of object depending on their needs, and most
machines don't use it at all (since they have older style PICs which don't
have per-cpu presentation components).
There's also other information that's per-cpu, but platform/machine
specific. So replace the intc pointer with a (void *)machine_data which
can be managed as the machine type likes to conveniently store per cpu
information.
Signed-off-by: David Gibson
Reviewed-by: Greg Kurz
---
hw/intc/xics.c | 5 +++--
hw/intc/xics_spapr.c| 16 +++-
hw/ppc/pnv.c| 4 ++--
hw/ppc/pnv_core.c | 11 +--
hw/ppc/spapr.c | 8
hw/ppc/spapr_cpu_core.c | 13 ++---
include/hw/ppc/pnv_core.h | 9 +
include/hw/ppc/spapr_cpu_core.h | 10 ++
include/hw/ppc/xics.h | 4 ++--
target/ppc/cpu.h| 2 +-
10 files changed, 61 insertions(+), 21 deletions(-)
diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index e73e623e3b..689ad44e5f 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -383,7 +383,8 @@ static const TypeInfo icp_info = {
.class_size = sizeof(ICPStateClass),
};
-Object *icp_create(Object *cpu, const char *type, XICSFabric *xi, Error **errp)
+ICPState *icp_create(Object *cpu, const char *type, XICSFabric *xi,
+ Error **errp)
{
Error *local_err = NULL;
Object *obj;
@@ -401,7 +402,7 @@ Object *icp_create(Object *cpu, const char *type,
XICSFabric *xi, Error **errp)
obj = NULL;
}
-return obj;
+return ICP(obj);
}
/*
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index 2e27b92b87..01c76717cf 100644
--- a/hw/intc/xics_spapr.c
+++ b/hw/intc/xics_spapr.c
@@ -31,6 +31,7 @@
#include "trace.h"
#include "qemu/timer.h"
#include "hw/ppc/spapr.h"
+#include "hw/ppc/spapr_cpu_core.h"
#include "hw/ppc/xics.h"
#include "hw/ppc/fdt.h"
#include "qapi/visitor.h"
@@ -43,8 +44,9 @@ static target_ulong h_cppr(PowerPCCPU *cpu, sPAPRMachineState
*spapr,
target_ulong opcode, target_ulong *args)
{
target_ulong cppr = args[0];
+sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
-icp_set_cppr(ICP(cpu->intc), cppr);
+icp_set_cppr(spapr_cpu->icp, cppr);
return H_SUCCESS;
}
@@ -65,7 +67,8 @@ static target_ulong h_ipi(PowerPCCPU *cpu, sPAPRMachineState
*spapr,
static target_ulong h_xirr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
target_ulong opcode, target_ulong *args)
{
-uint32_t xirr = icp_accept(ICP(cpu->intc));
+sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
+uint32_t xirr = icp_accept(spapr_cpu->icp);
args[0] = xirr;
return H_SUCCESS;
@@ -74,7 +77,8 @@ static target_ulong h_xirr(PowerPCCPU *cpu, sPAPRMachineState
*spapr,
static target_ulong h_xirr_x(PowerPCCPU *cpu, sPAPRMachineState *spapr,
target_ulong opcode, target_ulong *args)
{
-uint32_t xirr = icp_accept(ICP(cpu->intc));
+sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
+uint32_t xirr = icp_accept(spapr_cpu->icp);
args[0] = xirr;
args[1] = cpu_get_host_ticks();
@@ -84,9 +88,10 @@ static target_ulong h_xirr_x(PowerPCCPU *cpu,
sPAPRMachineState *spapr,
static target_ulong h_eoi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
target_ulong opcode, target_ulong *args)
{
+sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
target_ulong xirr = args[0];
-icp_eoi(ICP(cpu->intc), xirr);
+icp_eoi(spapr_cpu->icp, xirr);
return H_SUCCESS;
}
@@ -94,7 +99,8 @@ static target_ulong h_ipoll(PowerPCCPU *cpu,
sPAPRMachineState *spapr,
target_ulong opcode, target_ulong *args)
{
uint32_t mfrr;
-uint32_t xirr = icp_ipoll(ICP(cpu->intc), );
+sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
+uint32_t xirr = icp_ipoll(spapr_cpu->icp, );
args[0] = xirr;
args[1] = mfrr;
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 0b9508d94d..3a36c6ac6a 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1013,7 +1013,7 @@ static ICPState *pnv_icp_get(XICSFabric *xi, int pir)
{
PowerPCCPU *cpu = ppc_get_vcpu_by_pir(pir);
-return cpu ? ICP(cpu->intc) : NULL;
+return cpu ? pnv_cpu_state(cpu)->icp : NULL;
}
static void pnv_pic_print_info(InterruptStatsProvider *obj,
@@ -1026,7 +1026,7 @@ static void pnv_pic_print_info(InterruptStatsProvider
*obj,
CPU_FOREACH(cs) {
PowerPCCPU *cpu = POWERPC_CPU(cs);
-icp_pic_print_info(ICP(cpu->intc), mon);
+icp_pic_print_info(pnv_cpu_state(cpu)->icp, mon);
}
for (i = 0; i <
[Qemu-devel] [PATCHv3 3/7] pnv_core: Allocate cpu thread objects individually
Currently, we allocate space for all the cpu objects within a single core
in one big block. This was copied from an older version of the spapr code
and requires some ugly pointer manipulation to extract the individual
objects.
This design was due to a misunderstanding of qemu lifetime conventions and
has already been changed in spapr (in 94ad93bd "spapr_cpu_core: instantiate
CPUs separately".
Make an equivalent change in pnv_core to get rid of the nasty pointer
arithmetic.
Signed-off-by: David Gibson
Reviewed-by: Cédric Le Goater
Reviewed-by: Greg Kurz
---
hw/ppc/pnv.c | 4 ++--
hw/ppc/pnv_core.c | 11 +--
include/hw/ppc/pnv_core.h | 2 +-
3 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 0314881316..0b9508d94d 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -121,9 +121,9 @@ static int get_cpus_node(void *fdt)
*/
static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
{
-CPUState *cs = CPU(DEVICE(pc->threads));
+PowerPCCPU *cpu = pc->threads[0];
+CPUState *cs = CPU(cpu);
DeviceClass *dc = DEVICE_GET_CLASS(cs);
-PowerPCCPU *cpu = POWERPC_CPU(cs);
int smt_threads = CPU_CORE(pc)->nr_threads;
CPUPPCState *env = >env;
PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 01f47c8037..1e40f01e98 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -151,7 +151,6 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
PnvCore *pc = PNV_CORE(OBJECT(dev));
CPUCore *cc = CPU_CORE(OBJECT(dev));
const char *typename = pnv_core_cpu_typename(pc);
-size_t size = object_type_get_instance_size(typename);
Error *local_err = NULL;
void *obj;
int i, j;
@@ -165,11 +164,11 @@ static void pnv_core_realize(DeviceState *dev, Error
**errp)
return;
}
-pc->threads = g_malloc0(size * cc->nr_threads);
+pc->threads = g_new(PowerPCCPU *, cc->nr_threads);
for (i = 0; i < cc->nr_threads; i++) {
-obj = pc->threads + i * size;
+obj = object_new(typename);
-object_initialize(obj, size, typename);
+pc->threads[i] = POWERPC_CPU(obj);
snprintf(name, sizeof(name), "thread[%d]", i);
object_property_add_child(OBJECT(pc), name, obj, _abort);
@@ -179,7 +178,7 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
}
for (j = 0; j < cc->nr_threads; j++) {
-obj = pc->threads + j * size;
+obj = OBJECT(pc->threads[j]);
pnv_core_realize_child(obj, XICS_FABRIC(xi), _err);
if (local_err) {
@@ -194,7 +193,7 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
err:
while (--i >= 0) {
-obj = pc->threads + i * size;
+obj = OBJECT(pc->threads[i]);
object_unparent(obj);
}
g_free(pc->threads);
diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
index e337af7a3a..447ae761f7 100644
--- a/include/hw/ppc/pnv_core.h
+++ b/include/hw/ppc/pnv_core.h
@@ -34,7 +34,7 @@ typedef struct PnvCore {
CPUCore parent_obj;
/*< public >*/
-void *threads;
+PowerPCCPU **threads;
uint32_t pir;
MemoryRegion xscom_regs;
--
2.17.1
Re: [Qemu-devel] [PATCHv2 0/7] BROKEN, SORRY: Better handling of machine specific per-cpu information
On Thu, Jun 14, 2018 at 11:04:39AM +1000, David Gibson wrote: > It's moderately common for a machine type to need to keep track of > information that is specific to the platform it implements, but > per-cpu. > > While it could keep such information inside the MachineState, this > makes lookup from the CPUState awkward. So, this series adds a > standard way to stash machine-specific per-cpu information using a > void pointer in the PowerPCCPU object. The machine is responsible for > alloc()ing, free()ing and (if applicable) migrating this state. > > The meat of the series is the last two patches. The first 5 clean up > a number of minor uglies I encountered while implementing. Oops, realized 7/7 is totally broken on a KVM build. I'll send a new spin. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 1/4] spapr: remove irq_hint parameter from spapr_irq_alloc()
On Wed, Jun 13, 2018 at 09:24:12AM +0200, Cédric Le Goater wrote:
> On 06/13/2018 06:22 AM, David Gibson wrote:
> > On Tue, Jun 05, 2018 at 08:41:13AM +0200, Cédric Le Goater wrote:
> >> On 06/05/2018 05:34 AM, David Gibson wrote:
> >>> On Mon, May 28, 2018 at 09:06:12AM +0200, Cédric Le Goater wrote:
> On 05/28/2018 08:17 AM, Thomas Huth wrote:
> > On 25.05.2018 16:02, Greg Kurz wrote:
> >> On Fri, 18 May 2018 18:44:02 +0200
> >> Cédric Le Goater wrote:
> >>
> >>> This IRQ number hint can possibly be used by the VIO devices if the
> >>> "irq" property is defined on the command line but it seems it is never
> >>> the case. It is not used in libvirt for instance. So, let's remove it
> >>> to simplify future changes.
> >>>
> >>
> >> Setting an irq manually looks a bit anachronistic. I doubt anyone would
> >> do that nowadays, and the patch does a nice cleanup. So this looks like
> >> a good idea.
> > [...]
> >>> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> >>> index 472dd6f33a96..cc064f64fccf 100644
> >>> --- a/hw/ppc/spapr_vio.c
> >>> +++ b/hw/ppc/spapr_vio.c
> >>> @@ -455,7 +455,7 @@ static void spapr_vio_busdev_realize(DeviceState
> >>> *qdev, Error **errp)
> >>> dev->qdev.id = id;
> >>> }
> >>>
> >>> -dev->irq = spapr_irq_alloc(spapr, dev->irq, false, _err);
> >>> +dev->irq = spapr_irq_alloc(spapr, false, _err);
> >>
> >> Silently breaking "irq" like this looks wrong. I'd rather officially
> >> remove
> >> it first (ie, kill spapr_vio_props, -5 lines in spapr_vio.c).
> >>
> >> Of course, this raises the question of interface deprecation, and it
> >> should
> >> theoretically follow the process described at:
> >>
> >> https://wiki.qemu.org/Features/LegacyRemoval#Rules_for_removing_an_interface
> >>
> >> Cc'ing Thomas, our Chief Deprecation Officer, for insights :)
> >
> > The property is a public interface. Just because it's not used by
> > libvirt does not mean that nobody is using it. So yes, please follow the
> > rules and mark it as deprecated first for two release, before you really
> > remove it.
>
> This "irq" property is a problem to introduce a new static layout of IRQ
> numbers. It is in complete opposition.
>
> Can we keep it as it is for old pseries machine (settable) and ignore it
> for newer ? Would that be fine ?
> >>>
> >>> So, Thomas is right that we need to keep the interface while we go
> >>> through the deprecation process, even though it's a bit of a pain
> >>> (like you, I seriously doubt anyone ever used it).
> >>
> >> That's OK. The patch is simple. But it means that we have to keep the
> >> irq_hint parameter for 2 QEMU versions.
> >
> > No.. the suggestion below is designed to avoid that..
> >
> >>> But, I think there's a way to avoid that getting in the way of your
> >>> cleanups too much.
> >>>
> >>> A bunch of the current problems are caused because spapr_irq_alloc()
> >>> conflates two meanings of "allocate": 1) finding a free irq to use for
> >>> this device and 2) assigning that irq exclusively to this device.
> >>>
> >>> I think the first thing to do is to split those two parts. (1) will
> >>> never take an irq parameter, (2) will always take an irq parameter.
> >>> To implement the (to be deprecated) "irq" property on vio devices you
> >>> should skip (1) and just call (2) with the given irq number.
> >>
> >> well, we need to call both because if "irq" is zero then when we
> >> fallback to "1) finding a free irq to use."
> >
> > No, basically in the VIO code itself you'd have:
> > irq = ;
> > if (!irq)
> > irq = find_irq()
> > claim_irq(irq);
> >
> > find_irq() never takes a hint, claim_irq() always does (except it's
> > not really a hint).
>
> ok. I add something like that in mind :
>
> if (dev->irq) {
> spapr_irq_assign(spapr, SPAPR_IRQ_VIO, dev->irq, _err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> }
> } else {
> dev->irq = spapr_irq_alloc(spapr, SPAPR_IRQ_VIO, vio_index++,
>_err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> }
>
> and spapr_irq_assign() would die when the vio "irq" property does.
Right, but this obscures the fact that the function of assign/claim is
also performed at the end of the alloc function.
> >> But we can move the exclusive IRQ assignment (2) under the VIO model
> >> which is the only one using it and start deprecating the property.
> >
> > No.. the exclusive claim would be global - everything would use that.
>
> Yes, I see the model. I am not sure it's useful to have two routines
> in the long term.
Well, we won't, in a sense, since we want to phase out the find()
part.
> >>>
Re: [Qemu-devel] [PATCH 04/12] migration: introduce migration_update_rates
On 06/14/2018 12:17 AM, Dr. David Alan Gilbert wrote: * guangrong.x...@gmail.com (guangrong.x...@gmail.com) wrote: From: Xiao Guangrong It is used to slightly clean the code up, no logic is changed Actually, there is a slight change; iterations_prev is always updated when previously it was only updated with xbzrle on; still the change makes more sense. Yes, indeed. I will update the changelog.
Re: [Qemu-devel] [PATCH 02/12] migration: fix counting normal page for compression
On 06/13/2018 11:51 PM, Dr. David Alan Gilbert wrote: * guangrong.x...@gmail.com (guangrong.x...@gmail.com) wrote: From: Xiao Guangrong The compressed page is not normal page Is this the right reason? I think the 'normal' page shouldn't include the compressed page and XBZRLE-ed page (the current code does not treat xbzrle pages are normal as well). I think we always increment some counter for a page - so what gets incremented for a compressed page? In the later patch, we will introduce the statistics of compression which contains "pages": @pages: amount of pages compressed and transferred to the target VM Is the real answer that we do: ram_save_target_page control_save_page compress_page_with_multi_thread and control_save_page already increments the counter? No :), control_save_page increments the counter only if it posted data out, under that case, the compression path is not invoked. Thanks!
Re: [Qemu-devel] [RFC v2 1/3] pci_expander_bridge: add type TYPE_PXB_PCIE_HOST
Michael S. Tsirkin 于2018年6月13日周三 下午10:46写道： > > On Wed, Jun 13, 2018 at 04:23:40PM +0800, Zihan Yang wrote: > > Michael S. Tsirkin 于2018年6月12日周二 下午9:43写道： > > > > > > On Tue, Jun 12, 2018 at 05:13:22PM +0800, Zihan Yang wrote: > > > > The inner host bridge created by pxb-pcie is TYPE_PXB_PCI_HOST by > > > > default, > > > > add a new type TYPE_PXB_PCIE_HOST to better utilize the ECAM of PCIe > > > > > > > > Signed-off-by: Zihan Yang > > > > > > I have a concern that there are lots of new properties > > > added here, I'm not sure how are upper layers supposed to > > > manage them all. > > > > > > E.g. bus_nr supplied in several places, domain_nr for which > > > it's not clear how it is supposed to be allocated, etc. > > > > Indeed they seem to double the properties, but the pxb host is > > an internal structure of pxb-pcie device, created in pxb-pcie's > > realization procedure, and acpi-build queries host bridges instead > > of pxb-pcie devices. This means that users can not directly specify > > the property of pxb host bridge, but must 'inherit' from pxb-pcie > > devices. I had thought about changing the acpi-build process, > > but that would require more modifications. > > As for the properties, bus_nr means the start bus number > > of this host bridge. It is used when pxb-pcie is in pci domain 0 > > with q35 host to avoid bus number confliction. When it is placed > > in a separate pci domain, it is not used and should be 0. > > > > max_bus means how many buses the user desires, EACH bus in > > PCIe requires 1MB configuration space, thus specifying it could > > reduce the reserved memory in MMCFG as suggested by Marcel. > > Typically, the user can specify > > > > -device pxb-pcie,id=br1,bus="pcie.0",sep_domain=on,domain_nr=1,max_bus=130 > > > > this will place the buses under this pxb host bridge in pci domain > > 1, and reserve (130 + 1) = 131 buses for it. The start bus number > > is always 0 currently for simplicity. > > > > > Can the management interface be simplified? > > > Ideally we wouldn't have to teach libvirt new tricks, > > > just generalize pxb support slightly. > > > > We can delete 'sep_domain' property, I just find 'domain_nr' > > already indicates domain number. But domain_nr and > > max_bus seems unremovable, although they look 'redundant' > > because they appear twice. > > > > I'm not familiar with libvirt, but from the perspective of user, > > only 2 properties are added(domain_nr and max_bus, if we > > delete sep_domain), though the internal structure actually has > > changed. > > If you want a property for an internal purpose, > you can have a property starting with "x-" this way > we don't commit to maintaining it. OK, I'll change that in pxb-pcie-host. > -- > MST
Re: [Qemu-devel] [PATCH 01/12] migration: do not wait if no free thread
On 06/13/2018 11:43 PM, Dr. David Alan Gilbert wrote: * Peter Xu (pet...@redhat.com) wrote: On Tue, Jun 12, 2018 at 10:42:25AM +0800, Xiao Guangrong wrote: On 06/11/2018 03:39 PM, Peter Xu wrote: On Mon, Jun 04, 2018 at 05:55:09PM +0800, guangrong.x...@gmail.com wrote: From: Xiao Guangrong Instead of putting the main thread to sleep state to wait for free compression thread, we can directly post it out as normal page that reduces the latency and uses CPUs more efficiently The feature looks good, though I'm not sure whether we should make a capability flag for this feature since otherwise it'll be hard to switch back to the old full-compression way no matter for what reason. Would that be a problem? We assume this optimization should always be optimistic for all cases, particularly, we introduced the statistics of compression, then the user should adjust its parameters based on those statistics if anything works worse. Ah, that'll be good. Furthermore, we really need to improve this optimization if it hurts any case rather than leaving a option to the user. :) Yeah, even if we make it a parameter/capability we can still turn that on by default in new versions but keep the old behavior in old versions. :) The major difference is that, then we can still _have_ a way to compress every page. I'm just thinking if we don't have a switch for that then if someone wants to measure e.g. how a new compression algo could help VM migration, then he/she won't be possible to do that again since the numbers will be meaningless if that bit is out of control on which page will be compressed. Though I don't know how much use it'll bring... But if that won't be too hard, it still seems good. Not a strong opinion. I think that is needed; it might be that some users have really awful networking and need the compression; I'd expect that for people who turn on compression they really expect the slowdown because they need it for their network, so changing that is a bit odd. People should make sure the system has enough CPU resource to do compression as well, so the perfect usage is that the 'busy-rate' is low enough i think. However, it's not a big deal, i will introduce a parameter, maybe, compress-wait-free-thread. Thank you all, Dave and Peter! :)
Re: [Qemu-devel] [PATCH] monitor: postpone monitor_qmp_cleanup_queues
On Wed, Jun 13, 2018 at 03:30:05PM +0100, Stefan Hajnoczi wrote: > On Tue, Jun 12, 2018 at 01:47:00PM +0800, Peter Xu wrote: > > On Mon, Jun 11, 2018 at 05:45:49PM +0100, Stefan Hajnoczi wrote: > > > On Fri, Jun 08, 2018 at 10:18:25AM +0200, Markus Armbruster wrote: > > > > Stefan Hajnoczi writes: > > > > > On Fri, Jun 08, 2018 at 12:42:35PM +0800, Peter Xu wrote: > > > > >> On Thu, Jun 07, 2018 at 01:53:01PM +0200, Markus Armbruster wrote: > > > > >> > Peter Xu writes: > > > > >> > > > > > >> > > Previously we cleanup the queues when we got CLOSED event. It > > > > >> > > was used > > > > >> > > > > > >> > we clean up > > > > >> > > > > > >> > > to make sure we won't leftover replies/events of a old client to > > > > >> > > a new > > > > >> > > > > > >> > we won't send leftover replies/events of an old client > > > > >> > > > > > >> > > client. Now this patch postpones that until OPENED. > > > > >> > > > > > >> > What if OPENED never comes? > > > > >> > > > > >> Then we clean that up until destruction of the monitor. IMHO it's > > > > >> fine, but I'm not sure whether that's an overall acceptable approach. > > > > > > > > > > I think this patch fixes the problem at the wrong level. Marc-André's > > > > > fix seemed like a cleaner solution. > > > > > > > > Is it the right solution? > > > > > > > > I proposed another one: > > > > > > Sorry, I won't be able to participate in this because I'm behind on > > > other patches and tasks. Therefore, feel free to disregard but I'll > > > give my 2 cents: > > > > > > This seems like a chardev bug. The solution should probably be in the > > > chardev layer (Marc-André's patch or something else), not in the monitor > > > (this patch). > > > > Yes that's why I said I like Marc-Andre's patch. :) I just don't know > > an reliable way to achieve what we want there. > > > > The thing is that we don't really monitor the ioc_out for fd-typed > > chardevs. We only do that when we call qemu_chr_fe_add_watch() (e.g., > > in monitor_flush_locked()) when the writting buffer is full. But > > normally we can't detect any event from the output side, hence no way > > to deliever a CLOSED event when the output fd is the last fd that is > > closed. > > > > Maybe we can keep that output watch for the whole lifecycle of the > > chardev? I'm not sure yet. > > It's not possible to keep a POLLOUT fd watch since poll(2) would return > instantly and the event loop would spin. However, POLLHUP (G_IO_HUP) > might work. It seems so. But I am not confident enough to quickly provide a mature patch for chardev. I'll just work around for monitor right now. I'll comment with your ideas there and also on the events. We can rework upon when we have time with it. Regards, -- Peter Xu
Re: [Qemu-devel] [RFC for-2.13 0/7] spapr: Clean up pagesize handling
On Mon, May 07, 2018 at 03:48:54PM +0200, Andrea Bolognani wrote: > On Fri, 2018-04-27 at 22:17 +1000, David Gibson wrote: > > On Fri, Apr 27, 2018 at 10:31:10AM +0200, Andrea Bolognani wrote: > > > On Fri, 2018-04-27 at 12:14 +1000, David Gibson wrote: > > > > Right.. note that with the draft qemu patches a TCG guest will be > > > > prevented from using hugepages *by default* (the default value of the > > > > capability is 16). You have to explicitly change it to allow > > > > hugepages to be used in a TCG guest (but you don't have to supply > > > > hugepage backing). > > > > > > ... this will already happen. That's okay[1], we can't really > > > avoid it if we want to ensure consistent behavior between KVM and > > > TCG. > > > > So.. regarding [1]. The draft patches *do* change the behaviour on > > older machine types. I'll consider revisiting that, but I'd need to > > be convinced. Basically we have to choose between consistency between > > accelerator and consistency between versions. I think the former is > > the better choice; at least I think it is given that we *can* get both > > for the overwhelmingly common case in production (KVM HV). > > Forgot to answer this point. > > I agree that consistency between accelerators is the sane option > going forward, but changing the behavior for old machine types will > cause existing guests which have been using hugepages to lose the > ability to do so after being restarted on a newer QEMU. > > Isn't that exactly the kind of scenario versioned machine types are > supposed to prevent? Yeah, it is. What I was questioning was whether it was important enough for the case of TCG and PR guests (which have never been as well supported) to justify keeping the other inconsistency. On reflection, I think it probably does.. and I also think I have a way to preserve it without having to keep around masses of the old code, so I'll adjust that for the next spin. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v3 6/9] sm501: Do not clear read only bits when writing registers
On Thu, Jun 14, 2018 at 02:17:00AM +0200, BALATON Zoltan wrote:
> When writing registers that have read only bits we have to avoid
> changing these bits as they may have non zero values. Make sure we use
> the correct masks to mask out read only and reserved bits when
> changing registers.
>
> Also remove extra spaces from dram_control and arbitration_control
> assignments.
>
> Signed-off-by: BALATON Zoltan
> ---
> v3: Not only preserve read only bits but also allow clearing r/w
> bits
Applied to ppc-for-3.0, thanks.
>
> hw/display/sm501.c | 15 +--
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/hw/display/sm501.c b/hw/display/sm501.c
> index e47be99..ca0840f 100644
> --- a/hw/display/sm501.c
> +++ b/hw/display/sm501.c
> @@ -836,27 +836,30 @@ static void sm501_system_config_write(void *opaque,
> hwaddr addr,
>
> switch (addr) {
> case SM501_SYSTEM_CONTROL:
> -s->system_control = value & 0xE300B8F7;
> +s->system_control &= 0x10DB;
> +s->system_control |= value & 0xEF00B8F7;
> break;
> case SM501_MISC_CONTROL:
> -s->misc_control = value & 0xFF7FFF20;
> +s->misc_control &= 0xEF;
> +s->misc_control |= value & 0xFF7FFF10;
> break;
> case SM501_GPIO31_0_CONTROL:
> s->gpio_31_0_control = value;
> break;
> case SM501_GPIO63_32_CONTROL:
> -s->gpio_63_32_control = value;
> +s->gpio_63_32_control = value & 0xFF80;
> break;
> case SM501_DRAM_CONTROL:
> s->local_mem_size_index = (value >> 13) & 0x7;
> /* TODO : check validity of size change */
> -s->dram_control |= value & 0x7FC3;
> +s->dram_control &= 0x8000;
> +s->dram_control |= value & 0x7FC3;
> break;
> case SM501_ARBTRTN_CONTROL:
> -s->arbitration_control = value & 0x3777;
> +s->arbitration_control = value & 0x3777;
> break;
> case SM501_IRQ_MASK:
> -s->irq_mask = value;
> +s->irq_mask = value & 0xFFDF3F5F;
> break;
> case SM501_MISC_TIMING:
> s->misc_timing = value & 0xF31F1FFF;
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH v3 9/9] target/ppc: Add missing opcode for icbt on PPC440
On Thu, Jun 14, 2018 at 02:17:00AM +0200, BALATON Zoltan wrote: > PPC440 has two opcodes for icbt, add the missing one. A document reference to confim this would be nice. > Signed-off-by: BALATON Zoltan > --- > target/ppc/translate.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/target/ppc/translate.c b/target/ppc/translate.c > index 5fe1ba6..3a215a1 100644 > --- a/target/ppc/translate.c > +++ b/target/ppc/translate.c > @@ -6707,6 +6707,8 @@ GEN_HANDLER_E(mbar, 0x1F, 0x16, 0x1a, 0x001FF801, > GEN_HANDLER(msync_4xx, 0x1F, 0x16, 0x12, 0x03FFF801, PPC_BOOKE), > GEN_HANDLER2_E(icbt_440, "icbt", 0x1F, 0x16, 0x00, 0x03E1, > PPC_BOOKE, PPC2_BOOKE206), > +GEN_HANDLER2(icbt_440, "icbt", 0x1F, 0x06, 0x08, 0x03E1, > + PPC_440_SPEC), > GEN_HANDLER(lvsl, 0x1f, 0x06, 0x00, 0x0001, PPC_ALTIVEC), > GEN_HANDLER(lvsr, 0x1f, 0x06, 0x01, 0x0001, PPC_ALTIVEC), > GEN_HANDLER(mfvscr, 0x04, 0x2, 0x18, 0x001ff800, PPC_ALTIVEC), -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v3 5/9] sam460ex: Add RTC device
On Thu, Jun 14, 2018 at 02:17:00AM +0200, BALATON Zoltan wrote: > The Sam460ex has an M41T80 serial RTC chip on I2C bus 0 at address 0x68. > > Signed-off-by: BALATON Zoltan Reviewed-by: David Gibson > --- > hw/ppc/sam460ex.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c > index bdc53d2..dc730cc 100644 > --- a/hw/ppc/sam460ex.c > +++ b/hw/ppc/sam460ex.c > @@ -457,6 +457,7 @@ static void sam460ex_init(MachineState *machine) > object_property_set_bool(OBJECT(dev), true, "realized", NULL); > smbus_eeprom_init(i2c[0]->bus, 8, smbus_eeprom_buf, smbus_eeprom_size); > g_free(smbus_eeprom_buf); > +i2c_create_slave(i2c[0]->bus, "m41t80", 0x68); > > dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600800, uic[0][3]); > i2c[1] = PPC4xx_I2C(dev); -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v3 8/9] sm501: Perform a full update after palette change
On Thu, Jun 14, 2018 at 02:17:00AM +0200, BALATON Zoltan wrote:
> From: Sebastian Bauer
>
> Signed-off-by: Sebastian Bauer
> Signed-off-by: BALATON Zoltan
Commit message. Why is this necessary?
> ---
> hw/display/sm501.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/hw/display/sm501.c b/hw/display/sm501.c
> index 0625cf5..a2ee6e3 100644
> --- a/hw/display/sm501.c
> +++ b/hw/display/sm501.c
> @@ -479,6 +479,7 @@ typedef struct SM501State {
> MemoryRegion twoD_engine_region;
> uint32_t last_width;
> uint32_t last_height;
> +uint32_t do_full_update; /* perform a full update next time */
> I2CBus *i2c_bus;
>
> /* mmio registers */
> @@ -1032,6 +1033,7 @@ static void sm501_palette_write(void *opaque, hwaddr
> addr,
>
> assert(range_covers_byte(0, 0x400 * 3, addr));
> *(uint32_t *)>dc_palette[addr] = value;
> +s->do_full_update = 1;
> }
>
> static uint64_t sm501_disp_ctrl_read(void *opaque, hwaddr addr,
> @@ -1620,6 +1622,12 @@ static void sm501_update_display(void *opaque)
> full_update = 1;
> }
>
> +/* someone else requested a full update */
> +if (s->do_full_update) {
> +s->do_full_update = 0;
> +full_update = 1;
> +}
> +
> /* draw each line according to conditions */
> snap = memory_region_snapshot_and_clear_dirty(>local_mem_region,
>offset, width * height * src_bpp, DIRTY_MEMORY_VGA);
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH v3 4/9] hw/timer: Add basic M41T80 emulation
On Thu, Jun 14, 2018 at 02:17:00AM +0200, BALATON Zoltan wrote:
> Basic emulation of the M41T80 serial (I2C) RTC chip. Only getting time
> of day is implemented. Setting time and RTC alarm are not supported.
>
> Signed-off-by: BALATON Zoltan
Acked-by: David Gibson
> ---
>
> Notes:
> v3: Fixed \n-s in log messages
>
> MAINTAINERS | 1 +
> default-configs/ppc-softmmu.mak | 1 +
> hw/timer/Makefile.objs | 1 +
> hw/timer/m41t80.c | 117
>
> 4 files changed, 120 insertions(+)
> create mode 100644 hw/timer/m41t80.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8a94517..74ae589 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -829,6 +829,7 @@ M: BALATON Zoltan
> L: qemu-...@nongnu.org
> S: Maintained
> F: hw/ide/sii3112.c
> +F: hw/timer/m41t80.c
>
> SH4 Machines
>
> diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
> index 7d0dc2f..9fbaadc 100644
> --- a/default-configs/ppc-softmmu.mak
> +++ b/default-configs/ppc-softmmu.mak
> @@ -27,6 +27,7 @@ CONFIG_SM501=y
> CONFIG_IDE_SII3112=y
> CONFIG_I2C=y
> CONFIG_BITBANG_I2C=y
> +CONFIG_M41T80=y
>
> # For Macs
> CONFIG_MAC=y
> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
> index 8b27a4b..e16b2b9 100644
> --- a/hw/timer/Makefile.objs
> +++ b/hw/timer/Makefile.objs
> @@ -6,6 +6,7 @@ common-obj-$(CONFIG_CADENCE) += cadence_ttc.o
> common-obj-$(CONFIG_DS1338) += ds1338.o
> common-obj-$(CONFIG_HPET) += hpet.o
> common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
> +common-obj-$(CONFIG_M41T80) += m41t80.o
> common-obj-$(CONFIG_M48T59) += m48t59.o
> ifeq ($(CONFIG_ISA_BUS),y)
> common-obj-$(CONFIG_M48T59) += m48t59-isa.o
> diff --git a/hw/timer/m41t80.c b/hw/timer/m41t80.c
> new file mode 100644
> index 000..734d7d9
> --- /dev/null
> +++ b/hw/timer/m41t80.c
> @@ -0,0 +1,117 @@
> +/*
> + * M41T80 serial rtc emulation
> + *
> + * Copyright (c) 2018 BALATON Zoltan
> + *
> + * This work is licensed under the GNU GPL license version 2 or later.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "qemu/timer.h"
> +#include "qemu/bcd.h"
> +#include "hw/i2c/i2c.h"
> +
> +#define TYPE_M41T80 "m41t80"
> +#define M41T80(obj) OBJECT_CHECK(M41t80State, (obj), TYPE_M41T80)
> +
> +typedef struct M41t80State {
> +I2CSlave parent_obj;
> +int8_t addr;
> +} M41t80State;
> +
> +static void m41t80_realize(DeviceState *dev, Error **errp)
> +{
> +M41t80State *s = M41T80(dev);
> +
> +s->addr = -1;
> +}
> +
> +static int m41t80_send(I2CSlave *i2c, uint8_t data)
> +{
> +M41t80State *s = M41T80(i2c);
> +
> +if (s->addr < 0) {
> +s->addr = data;
> +} else {
> +s->addr++;
> +}
> +return 0;
> +}
> +
> +static int m41t80_recv(I2CSlave *i2c)
> +{
> +M41t80State *s = M41T80(i2c);
> +struct tm now;
> +qemu_timeval tv;
> +
> +if (s->addr < 0) {
> +s->addr = 0;
> +}
> +if (s->addr >= 1 && s->addr <= 7) {
> +qemu_get_timedate(, -1);
> +}
> +switch (s->addr++) {
> +case 0:
> +qemu_gettimeofday();
> +return to_bcd(tv.tv_usec / 1);
> +case 1:
> +return to_bcd(now.tm_sec);
> +case 2:
> +return to_bcd(now.tm_min);
> +case 3:
> +return to_bcd(now.tm_hour);
> +case 4:
> +return to_bcd(now.tm_wday);
> +case 5:
> +return to_bcd(now.tm_mday);
> +case 6:
> +return to_bcd(now.tm_mon + 1);
> +case 7:
> +return to_bcd(now.tm_year % 100);
> +case 8 ... 19:
> +qemu_log_mask(LOG_UNIMP, "%s: unimplemented register: %d\n",
> + __func__, s->addr - 1);
> +return 0;
> +default:
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid register: %d\n",
> + __func__, s->addr - 1);
> +return 0;
> +}
> +}
> +
> +static int m41t80_event(I2CSlave *i2c, enum i2c_event event)
> +{
> +M41t80State *s = M41T80(i2c);
> +
> +if (event == I2C_START_SEND) {
> +s->addr = -1;
> +}
> +return 0;
> +}
> +
> +static void m41t80_class_init(ObjectClass *klass, void *data)
> +{
> +DeviceClass *dc = DEVICE_CLASS(klass);
> +I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass);
> +
> +dc->realize = m41t80_realize;
> +sc->send = m41t80_send;
> +sc->recv = m41t80_recv;
> +sc->event = m41t80_event;
> +}
> +
> +static const TypeInfo m41t80_info = {
> +.name = TYPE_M41T80,
> +.parent= TYPE_I2C_SLAVE,
> +.instance_size = sizeof(M41t80State),
> +.class_init= m41t80_class_init,
> +};
> +
> +static void m41t80_register_types(void)
> +{
> +type_register_static(_info);
> +}
> +
> +type_init(m41t80_register_types)
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
Re: [Qemu-devel] [PATCH v3 7/9] sm501: Implement i2c part for reading monitor EDID
On Thu, Jun 14, 2018 at 02:17:00AM +0200, BALATON Zoltan wrote:
> Signed-off-by: BALATON Zoltan
Again needs a commit message expanding on what this is and why it's
useful.
[snip]
> static const VMStateDescription vmstate_sm501_sysbus = {
> .name = TYPE_SYSBUS_SM501,
> -.version_id = 1,
> -.minimum_version_id = 1,
> +.version_id = 2,
> +.minimum_version_id = 2,
> .fields = (VMStateField[]) {
> VMSTATE_STRUCT(state, SM501SysBusState, 1,
> vmstate_sm501_state, SM501State),
Additionally, since you're changing the migration stream, you need to
point out that this is not used in any machine types which support
cross-version migration.
> @@ -1843,8 +1971,8 @@ static void sm501_reset_pci(DeviceState *dev)
>
> static const VMStateDescription vmstate_sm501_pci = {
> .name = TYPE_PCI_SM501,
> -.version_id = 1,
> -.minimum_version_id = 1,
> +.version_id = 2,
> +.minimum_version_id = 2,
> .fields = (VMStateField[]) {
> VMSTATE_PCI_DEVICE(parent_obj, SM501PCIState),
> VMSTATE_STRUCT(state, SM501PCIState, 1,
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] Is there a way to package QEMU binaries?
On Wed, Jun 13, 2018 at 10:28:09AM +0100, Daniel P. Berrangé wrote: > On Wed, Jun 13, 2018 at 12:02:59PM +0800, Peter Xu wrote: > > On Tue, Jun 12, 2018 at 09:52:45AM +0100, Peter Maydell wrote: > > > On 12 June 2018 at 07:24, Peter Xu wrote: > > > > For example, I wanted to compile QEMU once and install it on multiple > > > > systems. What would be the suggested way to do so? > > > > > > For this, I would recommend using whatever the packaging > > > format for those systems is. Eg for Debian use the existing > > > Debian QEMU packages, for Redhat systems use RPMs, etc. > > > If you want a newer version of QEMU than is in the distro's > > > packages, you can probably forward port the packaging parts > > > to a newer QEMU without too much pain. > > > > > > Or you can use a distro-agnostic packaging tool of some sort; > > > there are a few out there but I have no particular recommendations. > > > > I'll start my investigation with RPM first. Thanks Peter. > > If you're interested in Fedora, I maintain a Copr repository which > provides RPMs for every QEMU version since 1.4.0 and every libvirt > version since 1.2.0... > > https://copr.fedorainfracloud.org/coprs/berrange/virt-ark/ > > Yeah, Fedora 28 is missing, but I'll be adding it real soon. Good to know this. Then is there an easy way to port the specfile and tools to QEMU repository so that we can pack that even with a git tree? Regards, -- Peter Xu
Re: [Qemu-devel] [PATCH v5 09/10] migration: poll the cm event while wait RDMA work request completion
On Wed, Jun 13, 2018 at 10:24 PM, Dr. David Alan Gilbert
wrote:
> * Lidong Chen (jemmy858...@gmail.com) wrote:
>> If the peer qemu is crashed, the qemu_rdma_wait_comp_channel function
>> maybe loop forever. so we should also poll the cm event fd, and when
>> receive RDMA_CM_EVENT_DISCONNECTED and RDMA_CM_EVENT_DEVICE_REMOVAL,
>> we consider some error happened.
>>
>> Signed-off-by: Lidong Chen
>
> Was there a reply which explained/pointed to docs for cm_event?
https://linux.die.net/man/3/rdma_get_cm_event
> Or a Review-by from one of the Infiniband people would be fine.
yes, I should add Gal Shachaf ,Aviad Yehezkel
we are working together on RDMA live migration.
Thanks.
>
> Dave
>
>> ---
>> migration/rdma.c | 33 ++---
>> 1 file changed, 30 insertions(+), 3 deletions(-)
>>
>> diff --git a/migration/rdma.c b/migration/rdma.c
>> index f12e8d5..bb6989e 100644
>> --- a/migration/rdma.c
>> +++ b/migration/rdma.c
>> @@ -1489,6 +1489,9 @@ static uint64_t qemu_rdma_poll(RDMAContext *rdma,
>> uint64_t *wr_id_out,
>> */
>> static int qemu_rdma_wait_comp_channel(RDMAContext *rdma)
>> {
>> +struct rdma_cm_event *cm_event;
>> +int ret = -1;
>> +
>> /*
>> * Coroutine doesn't start until migration_fd_process_incoming()
>> * so don't yield unless we know we're running inside of a coroutine.
>> @@ -1505,13 +1508,37 @@ static int qemu_rdma_wait_comp_channel(RDMAContext
>> *rdma)
>> * without hanging forever.
>> */
>> while (!rdma->error_state && !rdma->received_error) {
>> -GPollFD pfds[1];
>> +GPollFD pfds[2];
>> pfds[0].fd = rdma->comp_channel->fd;
>> pfds[0].events = G_IO_IN | G_IO_HUP | G_IO_ERR;
>> +pfds[0].revents = 0;
>> +
>> +pfds[1].fd = rdma->channel->fd;
>> +pfds[1].events = G_IO_IN | G_IO_HUP | G_IO_ERR;
>> +pfds[1].revents = 0;
>> +
>> /* 0.1s timeout, should be fine for a 'cancel' */
>> -switch (qemu_poll_ns(pfds, 1, 100 * 1000 * 1000)) {
>> +switch (qemu_poll_ns(pfds, 2, 100 * 1000 * 1000)) {
>> +case 2:
>> case 1: /* fd active */
>> -return 0;
>> +if (pfds[0].revents) {
>> +return 0;
>> +}
>> +
>> +if (pfds[1].revents) {
>> +ret = rdma_get_cm_event(rdma->channel, _event);
>> +if (!ret) {
>> +rdma_ack_cm_event(cm_event);
>> +}
>> +
>> +error_report("receive cm event while wait comp channel,"
>> + "cm event is %d", cm_event->event);
>> +if (cm_event->event == RDMA_CM_EVENT_DISCONNECTED ||
>> +cm_event->event == RDMA_CM_EVENT_DEVICE_REMOVAL) {
>> +return -EPIPE;
>> +}
>> +}
>> +break;
>>
>> case 0: /* Timeout, go around again */
>> break;
>> --
>> 1.8.3.1
>>
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v14 1/6] i386: Set TOPOEXT unconditionally for comapatibility
On Wed, Jun 13, 2018 at 09:18:22PM -0400, Babu Moger wrote:
> Enabling TOPOEXT feature might cause compatibility issues if
> older kernels does not set this feature. Lets set this feature
> unconditionally.
>
> Signed-off-by: Babu Moger
> ---
> target/i386/kvm.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 445e0e0..6f2cca7 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -372,6 +372,12 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s,
> uint32_t function,
> if (host_tsx_blacklisted()) {
> ret &= ~(CPUID_7_0_EBX_RTM | CPUID_7_0_EBX_HLE);
> }
> +} else if (function == 0x8001 && reg == R_ECX) {
> +/* Enabling topoext feature might cause compatibility issues if
> + * older kernel does not set this feature. Lets set this feature
> + * unconditionally.
> + */
Thanks. I will apply and rewrite the comment as:
/*
* It's safe to enable TOPOEXT even if it's not returned
* by GET_SUPPORTED_CPUID. Unconditionally enabling
* TOPOEXT here let us keep CPU models runnable on
* older kernels even when TOPOEXT is enabled.
*/
> +ret |= CPUID_EXT3_TOPOEXT;
> } else if (function == 0x8001 && reg == R_EDX) {
> /* On Intel, kvm returns cpuid according to the Intel spec,
> * so add missing bits according to the AMD spec:
> --
> 1.8.3.1
>
--
Eduardo
Re: [Qemu-devel] [PATCH 2/3] migration: Wake rate limiting for urgent requests
On Wed, Jun 13, 2018 at 11:26:41AM +0100, Dr. David Alan Gilbert (git) wrote:
[...]
> @@ -2932,10 +2943,24 @@ static void *migration_thread(void *opaque)
>
> migration_update_counters(s, current_time);
>
> +urgent = false;
> if (qemu_file_rate_limit(s->to_dst_file)) {
> -/* usleep expects microseconds */
> -g_usleep((s->iteration_start_time + BUFFER_DELAY -
> - current_time) * 1000);
> +/* Wait for a delay to do rate limiting OR
> + * something urgent to post the semaphore.
> + */
> +int ms = s->iteration_start_time + BUFFER_DELAY - current_time;
> +trace_migration_thread_ratelimit_pre(ms);
> +if (qemu_sem_timedwait(>rate_limit_sem, ms) == 0) {
> +/* We were worken by one or more urgent things but
> + * the timedwait will have consumed one of them.
> + * The service routine for the urgent wake will dec
> + * the semaphore itself for each item it consumes,
> + * so add this one we just eat back.
> + */
> +qemu_sem_post(>rate_limit_sem);
Is it possible that we just avoid eating that in the next patch? Then
we only provide a helper to "trigger an urgent request" but we only
consume the point here?
--
Peter Xu
Re: [Qemu-devel] [PATCH 1/3] migration/postcopy: Add max-postcopy-bandwidth parameter
On Wed, Jun 13, 2018 at 11:26:40AM +0100, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" > > Limit the background transfer bandwidth during the postcopy > phase to the value set on this new parameter. The default, 0, > corresponds to the existing behaviour which is unlimited bandwidth. > > Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Peter Xu -- Peter Xu
[Qemu-devel] TCG region page guards and transparent huge pages
After a discussion with Richard yesterday on IRC, I ran some numbers on the performance impact of the guard pages added at the end of TCG regions. These 4kB guard pages prevent the use of transparent huge pages for the last 2MB of a region. The benchmark I used is the bootup+shutdown of debian on aarch64. This generates code at a high rate (for a total of ~350MB), which performance-wise makes it sensitive to changes to page lookup latency for memory accesses to the TB cache. Find here a chart with several -smp 1 runs for different configurations, with the error bars delimiting a 95% confidence interval for the average (IOW, there's a 95% chance that the true average is within the error bars): https://imgur.com/G5Gd39O I used -tb-size 1024, to make sure there aren't any flushes. Configurations: - master: single region. It does use huge pages whenever pages are written to (checked via /procs/$pid/smaps). - 2/4M no guards: forced region size to 2/4 MB, and removed the guard pages at the end. - 2/4M guards: same as above, but keeping the guard pages. - nohuge: single region, but passing MADV_NOHUGEPAGE to madvise. As seen in the chart, 2M-guards performs similarly to nohuge. This makes sense, because they both result in not using hugepages. Master, 2M-no-guards and 4M-no-guards have similar performance, about 3% better than nohuge/2M-guards. (There's quite a bit of noise because I only ran these 7 times each, but the confidence intervals have a wide overlap.) 4M-guards performs in between the above two groups. With 4M-guards, one half of each region is a huge page (2M), while the other half is broken into 4K pages due to the guard page. The conclusion I draw from the above is that as long as we keep regions sufficiently large (>=4M), we won't be able to measure performance regressions due to less huge page use. Given the above, I plotted what region sizes we obtain for different -smp and -tb-size combinations: https://imgur.com/RDF56Nv We can see how region_size == 2 only occurs when we either have very large -smp's, or small TB cache sizes (<= 256 MB). So, what to do based on all this? I think the current implementation makes sense. That said, two things we could consider doing are: - Remove the guard pages. I'd rather keep them in though, since their cost is negligible in most scenarios. If we really wanted to recover this performance, we could just enable the guards (by calling mprotect) only under TCG_DEBUG. - Change tcg_n_regions() to assign larger regions. I am not convinced of this, since at most we'd gain 3% in performance, but we might end up wasting more space (recall that not all vCPUs translate code at the same rate), or flushing more often (probably with a perf impact larger than 3%). It is also possible that other workloads might be more sensitive to the use of huge pages for the TB cache, but I couldn't think of any. Besides, the bootup+shutdown test is very easy to run =) So based on the above numbers, those are my thoughts. Thanks, Emilio PS. You can see both charts side-by-side here: https://imgur.com/a/cu6g0pA
Re: [Qemu-devel] [PATCH v2] exec: Fix MAP_RAM for cached access
On Wed, Jun 13, 2018 at 04:20:34PM +0200, Auger Eric wrote: > Hi Paolo, > > On 06/13/2018 03:53 PM, Paolo Bonzini wrote: > > On 13/06/2018 15:44, Auger Eric wrote: > >>> Queuing this patch. I'm not sure how I missed this, I have actually > >>> tested it with SMMU. > >> no problem. Strange also I was the only one facing the issue. > > > > No, I must have blundered it between testing and posting the patches. > > > >>> Do you also need the MemTxAttrs so that the right PCI requestor id is > >>> used, or do you get it from somewhere else? > >> which call site do you have in mind, sorry? > > > > I'm wondering if the MemoryRegionCache needs to store the MemTxAttrs. > > They would be passed to address_space_init_cache. > > I acknowledge I don't master this code enough but I would say MSI > wouldn't work already (vITS wouldn't translate them properly) if the > proper requester_id wasn't conveyed properly. MSI writes to the doorbell > are not cached I guess? I might be wrong, but I guess Paolo means the DMA part. In address_space_cache_init() now we are with MEMTXATTRS_UNSPECIFIED when translate the first time (to be cached). But I'd also guess we're fine with that now since after all we're not even passing the attrs into IOMMUMemoryRegionClass.translate() yet. Regards, -- Peter Xu
Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 5/8] hw/timer: Add basic M41T80 emulation
On Wed, Jun 13, 2018 at 04:13:57PM +0200, BALATON Zoltan wrote:
> On Wed, 13 Jun 2018, David Gibson wrote:
> > On Wed, Jun 13, 2018 at 10:50:59AM +0200, BALATON Zoltan wrote:
> > > On Wed, 13 Jun 2018, David Gibson wrote:
> > > > On Wed, Jun 06, 2018 at 07:35:28PM +0200, BALATON Zoltan wrote:
> > > > > On Wed, 6 Jun 2018, Philippe Mathieu-Daudé wrote:
> > > > > > On 06/06/2018 10:31 AM, BALATON Zoltan wrote:
> > > > > > > Basic emulation of the M41T80 serial (I2C) RTC chip. Only getting
> > > > > > > time
> > > > > > > of day is implemented. Setting time and RTC alarm are not
> > > > > > > supported.
> > > > > [...]
> > > > > > > diff --git a/hw/timer/m41t80.c b/hw/timer/m41t80.c
> > > > > > > new file mode 100644
> > > > > > > index 000..9dbdb1b
> > > > > > > --- /dev/null
> > > > > > > +++ b/hw/timer/m41t80.c
> > > > > > > @@ -0,0 +1,117 @@
> > > > > > > +/*
> > > > > > > + * M41T80 serial rtc emulation
> > > > > > > + *
> > > > > > > + * Copyright (c) 2018 BALATON Zoltan
> > > > > > > + *
> > > > > > > + * This work is licensed under the GNU GPL license version 2 or
> > > > > > > later.
> > > > > > > + *
> > > > > > > + */
> > > > > > > +
> > > > > > > +#include "qemu/osdep.h"
> > > > > > > +#include "qemu/log.h"
> > > > > > > +#include "qemu/timer.h"
> > > > > > > +#include "qemu/bcd.h"
> > > > > > > +#include "hw/i2c/i2c.h"
> > > > > > > +
> > > > > > > +#define TYPE_M41T80 "m41t80"
> > > > > > > +#define M41T80(obj) OBJECT_CHECK(M41t80State, (obj), TYPE_M41T80)
> > > > > > > +
> > > > > > > +typedef struct M41t80State {
> > > > > > > +I2CSlave parent_obj;
> > > > > > > +int8_t addr;
> > > > > > > +} M41t80State;
> > > > > > > +
> > > > > > > +static void m41t80_realize(DeviceState *dev, Error **errp)
> > > > > > > +{
> > > > > > > +M41t80State *s = M41T80(dev);
> > > > > > > +
> > > > > > > +s->addr = -1;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static int m41t80_send(I2CSlave *i2c, uint8_t data)
> > > > > > > +{
> > > > > > > +M41t80State *s = M41T80(i2c);
> > > > > > > +
> > > > > > > +if (s->addr < 0) {
> > > > > > > +s->addr = data;
> > > > > > > +} else {
> > > > > > > +s->addr++;
> > > > > > > +}
> > > > > >
> > > > > > What about adding enum i2c_event in M41t80State and use the enum
> > > > > > here
> > > > > > rather than the addr < 0? Also this wrap at INT8_MAX = 127, is this
> > > > > > expected?
> > > > >
> > > > > Thanks for the review. I guess we could add enum for device bytes and
> > > > > the
> > > > > special case -1 meaning no register address selected yet but this is
> > > > > a very
> > > > > simple device with only 20 bytes and the datasheet also lists them by
> > > > > number
> > > > > without naming them so I think we can also refer to them by number.
> > > > > Since
> > > > > the device has only this 20 bytes the case with 127 should also not
> > > > > be a
> > > > > problem as that's invalid address anyway. Or did you mean something
> > > > > else?
> > > >
> > > > So, I'm not particularly in favour of adding extra state variables.
> > > >
> > > > But is using addr < 0 safe here? You're assigning the uint8_t data to
> > > > addr - could that result in a negative value?
> > >
> > > Why wouldn't it be safe with the expected values for register address
> > > between 0-19? If the guest sends garbage values over 127 it will either
> > > result in invalid register or unselected register and lead to an error
> > > when
> > > trying to read/write that register so I don't see what other problem this
> > > may cause.
> >
> > Ok, but where is that enforced?
>
> I don't see the problem. The addr register selects the register to read or
> write. It is set by the first write when the device is accessed the first
> time (this is denoted by addr == -1 (or really any negative value). The
> device has 20 registers and trying to read any register outside addr between
> 0-19 will result in returning 0 and logging a warning about invalid register
> in m41t80_recv. What could fail here when guest sends garbage? It will set
> addr to invalid value and try to read non-exitent register and get an error
> just like for any other nonexistent value of addr (or start to read from
> register 0 if it manages to set a negative value). All writes of registers
> are ignored currently (except setting addr by the first write). What should
> be enforced here?
Ah, I see your point. I mean strictly we should match the hardware
behaviour if you write garbage addresses here, but really I don't
think it matters much.
Ok, it should be fine.
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH 3/3] target/ppc: filter out non-zero PCR values when using TCG
On Wed, Jun 13, 2018 at 04:26:39PM +0200, Greg Kurz wrote:
> On Wed, 13 Jun 2018 22:05:02 +1000
> David Gibson wrote:
>
> > On Wed, Jun 13, 2018 at 10:19:15AM +0200, Greg Kurz wrote:
> > > On Wed, 13 Jun 2018 10:45:06 +1000
> > > David Gibson wrote:
> > >
> > > > On Tue, Jun 12, 2018 at 07:04:15PM +0200, Greg Kurz wrote:
> > > > > Bits set in the PCR disable features of the processor. TCG currently
> > > > > doesn't implement that, ie, we always act like if PCR is all zeros.
> > > > >
> > > > > But it is still possible for the PCR to have a non-null value. This
> > > > > may
> > > > > confuse the guest.
> > > > >
> > > > > There are three distinct cases:
> > > > >
> > > > > 1) a powernv guest doing mtspr SPR_PCR
> > > > >
> > > > > 2) reset of a pseries guest if the max-cpu-compat machine property is
> > > > > set
> > > > >
> > > > > 3) CAS of a pseries guest
> > > > >
> > > > > This patch adds a ppc_store_pcr() helper that ensures we cannot put
> > > > > a non-null value in the PCR when using TCG. This helper also has
> > > > > error propagation support, so that each case listed above can be
> > > > > handled appropriately:
> > > > >
> > > > > 1) since the powernv machine is mostly used for OpenPOWER FW devel,
> > > > >we just print an error and let QEMU continue execution
> > > > >
> > > > > 2) an error is printed and QEMU exits, ie, same behaviour as when
> > > > >KVM doesn't support the requested compat mode
> > > > >
> > > > > 3) an error is printed and QEMU reports H_HARDWARE to the guest
> > > > >
> > > > > Signed-off-by: Greg Kurz
> > > >
> > > > I'm not really convinced this is a good idea. Printing a (non fatal)
> > > > error if the guest attempts to write a non-zero value to the PCR
> > > > should be ok. However, you're generating a fatal error if the machine
> > > > tries to set the PCR in TCG mode. That could easily happen using,
> > > > e.g. the cap-htm flag on a TCG guest. That would take TCG from mostly
> > > > working, to refusing to run at all.
> > > >
> > >
> > > I'm confused... I don't see anything related to HTM in TCG. Also we have
> > > the following in cap_htm_apply():
> > >
> > > if (tcg_enabled()) {
> > > error_setg(errp,
> > >"No Transactional Memory support in TCG, try
> > > cap-htm=off");
> > >
> > > I'm probably missing something... can you enlighten me ?
> >
> > Ok, so right now when cap-htm=off we don't actually enforce that, we
> > just don't advertise it to the guest. We probably _should_ enforce
> > that, and the way we'd do it is to set the appropriate bit in the
> > PCR. That'll do the right thing for KVM (well, once we update KVM to
> > actually pass on the PCR value) but would break TCG in conjunction
> > with your patch above.
>
> Hmm... The granularity of the PCR bits is PowerISA versions, not individual
> facilities AFAIK... or am I missing something again ?
Huh.. so. In the 3.0 ISA, there are only ISA version bits. But in
the 2.07 ISA, there are ISA version bits *and* a bit to control HTM.
I'm not quite sure what to make of that. I kind of love the fact that
they incompatibly change the compatibility register.
> Anyway, with the current code, if we start the guest with:
>
> -machine accel=tcg,max-cpu-compat=power8 -cpu POWER9
>
> We get this in the guest:
>
> # grep ^cpu /proc/cpuinfo
> cpu : POWER8 (architected), altivec supported
>
> This is the expected result of the CAS negotiation, but
> the guest can still execute PowerISA 3.0 instructions
> that should cause an invalid instruction exception.
Right, this is a bug, albeit not a very high priority one.
> I agree this shouldn't cause QEMU to exit, but I guess we should
> at least leave the PCR to all zeroes, so that the guest view
> is consistent with what TGC does (ie, raw mode). And maybe an
> error message to indicate that the PCR is ignored in TCG... but
> since that could be guest triggered, and the user cannot do anything
> about it, I'm wondering if this should rather be a trace actually.
>
> Does it make sense for you ?
TBH, I don't care all that much. There are a bunch of cases where TCG
doesn't behave quite right, most without warnings. One more or less
doesn't make a huge difference.
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
[Qemu-devel] [PATCH v14 6/6] i386: Remove generic SMT thread check
Remove generic non-intel check while validating hyperthreading support.
Certain AMD CPUs can support hyperthreading now.
CPU family with TOPOEXT feature can support hyperthreading now.
Signed-off-by: Babu Moger
Tested-by: Geoffrey McRae
Reviewed-by: Eduardo Habkost
---
target/i386/cpu.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 637d8eb..6783305 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4963,17 +4963,22 @@ static void x86_cpu_realizefn(DeviceState *dev, Error
**errp)
qemu_init_vcpu(cs);
-/* Only Intel CPUs support hyperthreading. Even though QEMU fixes this
- * issue by adjusting CPUID__0001_EBX and CPUID_8000_0008_ECX
- * based on inputs (sockets,cores,threads), it is still better to gives
+/*
+ * Most Intel and certain AMD CPUs support hyperthreading. Even though QEMU
+ * fixes this issue by adjusting CPUID__0001_EBX and
CPUID_8000_0008_ECX
+ * based on inputs (sockets,cores,threads), it is still better to give
* users a warning.
*
* NOTE: the following code has to follow qemu_init_vcpu(). Otherwise
* cs->nr_threads hasn't be populated yet and the checking is incorrect.
*/
-if (!IS_INTEL_CPU(env) && cs->nr_threads > 1 && !ht_warned) {
-error_report("AMD CPU doesn't support hyperthreading. Please configure"
- " -smp options properly.");
+ if (IS_AMD_CPU(env) &&
+ !(env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) &&
+ cs->nr_threads > 1 && !ht_warned) {
+error_report("This family of AMD CPU doesn't support "
+ "hyperthreading(%d). Please configure -smp "
+ "options properly or try enabling topoext feature.",
+ cs->nr_threads);
ht_warned = true;
}
--
1.8.3.1
[Qemu-devel] [PATCH v14 4/6] cpus: Add new function topology_supports_topoext
Add new function topology_supports_topoext to verify
if we can support topoext feature. Will be used to enable/disable
topoext feature.
Signed-off-by: Babu Moger
---
accel/tcg/user-exec-stub.c | 5 +
cpus.c | 13 +
include/qom/cpu.h | 9 +
3 files changed, 27 insertions(+)
diff --git a/accel/tcg/user-exec-stub.c b/accel/tcg/user-exec-stub.c
index dbcf1ad..715818a 100644
--- a/accel/tcg/user-exec-stub.c
+++ b/accel/tcg/user-exec-stub.c
@@ -11,6 +11,11 @@ void qemu_init_vcpu(CPUState *cpu)
{
}
+int topology_supports_topoext(int max_cores, int max_threads)
+{
+return true;
+}
+
/* User mode emulation does not support record/replay yet. */
bool replay_exception(void)
diff --git a/cpus.c b/cpus.c
index d1f1629..17f6f4c 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1979,6 +1979,19 @@ static void qemu_dummy_start_vcpu(CPUState *cpu)
QEMU_THREAD_JOINABLE);
}
+/*
+ * Check if we can support topoext feature with this topology
+ * Fail if number of cores are beyond the supported cores
+ * or number of threads are more than supported threads
+ */
+int topology_supports_topoext(int max_cores, int max_threads)
+{
+if ((smp_cores > max_cores) || (smp_threads > max_threads)) {
+return false;
+}
+return true;
+}
+
void qemu_init_vcpu(CPUState *cpu)
{
cpu->nr_cores = smp_cores;
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 9d3afc6..4ac4d49 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -1004,6 +1004,15 @@ void end_exclusive(void);
*/
void qemu_init_vcpu(CPUState *cpu);
+/**
+ * topology_supports_topoext:
+ * @max_cores: Max cores topoext feature can support
+ * @max_threads: Max threads topoext feature can support
+ *
+ * Return true if topology can be supported else return false
+ */
+int topology_supports_topoext(int max_cores, int max_threads);
+
#define SSTEP_ENABLE 0x1 /* Enable simulated HW single stepping */
#define SSTEP_NOIRQ 0x2 /* Do not use IRQ while single stepping */
#define SSTEP_NOTIMER 0x4 /* Do not Timers while single stepping */
--
1.8.3.1
[Qemu-devel] [PATCH v14 5/6] i386: Disable TOPOEXT feature if it cannot be supported
Disable the TOPOEXT feature if it cannot be supported.
We cannot support this feature with more than 2 nr_threads
or more than 32 cores in a socket.
Signed-off-by: Babu Moger
---
target/i386/cpu.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 2eb26da..637d8eb 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4765,7 +4765,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error
**errp)
X86CPUClass *xcc = X86_CPU_GET_CLASS(dev);
CPUX86State *env = >env;
Error *local_err = NULL;
-static bool ht_warned;
+static bool ht_warned, topo_warned;
if (xcc->host_cpuid_required && !accel_uses_host_cpuid()) {
char *name = x86_cpu_class_get_model_name(xcc);
@@ -4779,6 +4779,21 @@ static void x86_cpu_realizefn(DeviceState *dev, Error
**errp)
return;
}
+/* Disable TOPOEXT if topology cannot be supported */
+if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) {
+if (!topology_supports_topoext(MAX_CORES_IN_NODE *
MAX_NODES_PER_SOCKET,
+ 2)) {
+env->features[FEAT_8000_0001_ECX] &= !CPUID_EXT3_TOPOEXT;
+if (!topo_warned) {
+error_report("TOPOEXT feature cannot be supported with more"
+ " than %d cores or more than 2 threads per
socket."
+ " Disabling the feature.",
+ (MAX_CORES_IN_NODE * MAX_NODES_PER_SOCKET));
+topo_warned = true;
+}
+}
+}
+
x86_cpu_expand_features(cpu, _err);
if (local_err) {
goto out;
--
1.8.3.1
[Qemu-devel] [PATCH v14 1/6] i386: Set TOPOEXT unconditionally for comapatibility
Enabling TOPOEXT feature might cause compatibility issues if
older kernels does not set this feature. Lets set this feature
unconditionally.
Signed-off-by: Babu Moger
---
target/i386/kvm.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 445e0e0..6f2cca7 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -372,6 +372,12 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s,
uint32_t function,
if (host_tsx_blacklisted()) {
ret &= ~(CPUID_7_0_EBX_RTM | CPUID_7_0_EBX_HLE);
}
+} else if (function == 0x8001 && reg == R_ECX) {
+/* Enabling topoext feature might cause compatibility issues if
+ * older kernel does not set this feature. Lets set this feature
+ * unconditionally.
+ */
+ret |= CPUID_EXT3_TOPOEXT;
} else if (function == 0x8001 && reg == R_EDX) {
/* On Intel, kvm returns cpuid according to the Intel spec,
* so add missing bits according to the AMD spec:
--
1.8.3.1
[Qemu-devel] [PATCH v14 0/6] i386: Enable TOPOEXT to support hyperthreading on AMD CPU
This series enables the TOPOEXT feature for AMD CPUs. This is required to support hyperthreading on kvm guests. This addresses the issues reported in these bugs: https://bugzilla.redhat.com/show_bug.cgi?id=1481253 https://bugs.launchpad.net/qemu/+bug/1703506 v14: Patches are based off of Eduardo's git://github.com/ehabkost/qemu.git x86-next. Some of the patches are queued already. Submitting remaining series. Summary of changes. 1. Always set TOPOEXT feature in kvm_arch_get_supported_cpuid 2. Implemented topology_supports_topoext bit differently. Reason for this is, if we need to disable this feature before the x86_cpu_expand_features. But problem is nr_cores and nr_threads are not populated at this time. It is populated in qemu_init_vcpus. 3. Removed auto-topoext feature completely. The can cause lots of compatibility issues. v13: Patches are based off of Eduardo's git://github.com/ehabkost/qemu.git x86-next. Some of the patches are queued already. Submitting remaining series. Summary of changes. 1.Fixed the error format if the topology cannot be supported. 2.Fixed the compatibility issues with old cpu models and new machine types. Here is the discussion thread. Here is the discussion thread. https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg01239.html 3.I am still testing it. But sending it to get review feedback. v12: Patches are based off of Eduardo's git://github.com/ehabkost/qemu.git x86-next. Some of the patches are queued already. Submitting remaining series. Summary of changes. 1.Added more comments explaining CPUID_Fn801E bit definitions. 2.Split the patch into separate patch to check the topology. Moved the code to x86_cpu_realizefn. Display the error if topoext feature cannot be enabled. 3.Few more text corrections. v11: Patches are based off of Eduardo's git://github.com/ehabkost/qemu.git x86-next. Summary of changes. 1.Added more comments explaining different constants and variables. 2.Removed NUM_SHARING_CACHE macro and made the code simpler. 3.Changed the function name num_sharing_l3_cache to cores_in_core_complex. This function is actually finding the number of cores in a core complex. Purpose here is to re-use the code in couple more places. 4.Added new function nodes_in_socket to find number of nodes in the config. Purpose here is to re-use the code. 5.Used DIV_ROUND_UP wherever applicable. 6.Renamed few constants and functions to generic names. 7.Few more text corrections. v10: Based the patches on Eduardo's git://github.com/ehabkost/qemu.git x86-next Some of the earlier patches are already queued. So, submitting the rest of the series here. This series adds complete redesign of the cpu topology. Based on user given parameter, we try to build topology very close to the hardware. Maintains symmetry as much as possible. Added new function epyc_build_topology to build the topology based on user given nr_cores, nr_threads. Summary of changes. 1. Build the topology dinamically based on nr_cores and nr_threads 2. Added new epyc_build_topology to build the new topology. 3. Added new function num_sharing_l3_cache to calculate the L3 sharing 4. Added a check to verify the topology. Disabled the TOPOEXT if the topology cannot be built. v9: Based the patches on Eduardo's git://github.com/ehabkost/qemu.git x86-next tree. Following 3 patches from v8 are already queued. i386: Add cache information in X86CPUDefinition i386: Initialize cache information for EPYC family processors i386: Helpers to encode cache information consistently So, submitting the rest of the series here. Changes: 1. Included Eduardo's clean up patch 2. Added 2.13 machine types 3. Disabled topoext for 2.12 and below versions. 4. Added the assert to core_id as discussed. v8: Addressed feedback from Eduardo. Thanks Eduardo for being patient with me. Tested on AMD EPYC server and also did some basic testing on intel box. Summary of changes. 1. Reverted back l2 cache associativity. Kept it same as legacy. 2. Changed cache_info structure in X86CPUDefinition and CPUX86State to pointers. 3. Added legacy_cache property in PC_COMPAT_2_12 and initialized legacy_cache based on static cache_info availability. 4. Squashed patch 4 and 5 and applied it before patch 3. 5. Added legacy cache check for cpuid[2] and cpuid[4] for consistancy. 6. Simplified NUM_SHARING_CACHE definition for readability, 7. Removed assert for core_id as it appeared redundant. 8. Simplified encode_cache_cpuid801d little bit. 9. Few more minor changes v7: Rebased on top of latest tree after 2.12 release and done few basic tests. There are no changes except for few minor hunks. Hopefully this gets pulled into 2.13 release. Please review, let me know of any feedback. v6: 1.Fixed problem with patch#4(Add new property to control cache info). The parameter legacy_cache should be "on" by default on machine type "pc-q35-2.10". This was
Re: [Qemu-devel] [PATCH v3 1/9] ppc4xx_i2c: Remove unimplemented sdata and intr registers
On Thu, Jun 14, 2018 at 02:17:00AM +0200, BALATON Zoltan wrote:
> Signed-off-by: BALATON Zoltan
But.. they are implemented. Albeit as an entirely software controlled
register.
I'm guessing that's not what they're supposed to do, which is why
you're removing them, but that needs to be explained in the commit
message.
As a general rule cases where a one line commit message is acceptable
are *very* rare.
> ---
> hw/i2c/ppc4xx_i2c.c | 16 +---
> include/hw/i2c/ppc4xx_i2c.h | 4 +---
> 2 files changed, 2 insertions(+), 18 deletions(-)
>
> diff --git a/hw/i2c/ppc4xx_i2c.c b/hw/i2c/ppc4xx_i2c.c
> index d1936db..4e0aaae 100644
> --- a/hw/i2c/ppc4xx_i2c.c
> +++ b/hw/i2c/ppc4xx_i2c.c
> @@ -3,7 +3,7 @@
> *
> * Copyright (c) 2007 Jocelyn Mayer
> * Copyright (c) 2012 François Revol
> - * Copyright (c) 2016 BALATON Zoltan
> + * Copyright (c) 2016-2018 BALATON Zoltan
> *
> * Permission is hereby granted, free of charge, to any person obtaining a
> copy
> * of this software and associated documentation files (the "Software"), to
> deal
> @@ -63,7 +63,6 @@ static void ppc4xx_i2c_reset(DeviceState *s)
> i2c->mdcntl = 0;
> i2c->sts = 0;
> i2c->extsts = 0x8f;
> -i2c->sdata = 0;
> i2c->lsadr = 0;
> i2c->hsadr = 0;
> i2c->clkdiv = 0;
> @@ -71,7 +70,6 @@ static void ppc4xx_i2c_reset(DeviceState *s)
> i2c->xfrcnt = 0;
> i2c->xtcntlss = 0;
> i2c->directcntl = 0xf;
> -i2c->intr = 0;
> }
>
> static inline bool ppc4xx_i2c_is_master(PPC4xxI2CState *i2c)
> @@ -139,9 +137,6 @@ static uint64_t ppc4xx_i2c_readb(void *opaque, hwaddr
> addr, unsigned int size)
>TYPE_PPC4xx_I2C, __func__);
> }
> break;
> -case 2:
> -ret = i2c->sdata;
> -break;
> case 4:
> ret = i2c->lmadr;
> break;
> @@ -181,9 +176,6 @@ static uint64_t ppc4xx_i2c_readb(void *opaque, hwaddr
> addr, unsigned int size)
> case 16:
> ret = i2c->directcntl;
> break;
> -case 17:
> -ret = i2c->intr;
> -break;
> default:
> if (addr < PPC4xx_I2C_MEM_SIZE) {
> qemu_log_mask(LOG_UNIMP, "%s: Unimplemented register 0x%"
> @@ -229,9 +221,6 @@ static void ppc4xx_i2c_writeb(void *opaque, hwaddr addr,
> uint64_t value,
> }
> }
> break;
> -case 2:
> -i2c->sdata = value;
> -break;
> case 4:
> i2c->lmadr = value;
> if (i2c_bus_busy(i2c->bus)) {
> @@ -302,9 +291,6 @@ static void ppc4xx_i2c_writeb(void *opaque, hwaddr addr,
> uint64_t value,
> case 16:
> i2c->directcntl = value & 0x7;
> break;
> -case 17:
> -i2c->intr = value;
> -break;
> default:
> if (addr < PPC4xx_I2C_MEM_SIZE) {
> qemu_log_mask(LOG_UNIMP, "%s: Unimplemented register 0x%"
> diff --git a/include/hw/i2c/ppc4xx_i2c.h b/include/hw/i2c/ppc4xx_i2c.h
> index 3c60307..e4b6ded 100644
> --- a/include/hw/i2c/ppc4xx_i2c.h
> +++ b/include/hw/i2c/ppc4xx_i2c.h
> @@ -3,7 +3,7 @@
> *
> * Copyright (c) 2007 Jocelyn Mayer
> * Copyright (c) 2012 François Revol
> - * Copyright (c) 2016 BALATON Zoltan
> + * Copyright (c) 2016-2018 BALATON Zoltan
> *
> * Permission is hereby granted, free of charge, to any person obtaining a
> copy
> * of this software and associated documentation files (the "Software"), to
> deal
> @@ -49,7 +49,6 @@ typedef struct PPC4xxI2CState {
> uint8_t mdcntl;
> uint8_t sts;
> uint8_t extsts;
> -uint8_t sdata;
> uint8_t lsadr;
> uint8_t hsadr;
> uint8_t clkdiv;
> @@ -57,7 +56,6 @@ typedef struct PPC4xxI2CState {
> uint8_t xfrcnt;
> uint8_t xtcntlss;
> uint8_t directcntl;
> -uint8_t intr;
> } PPC4xxI2CState;
>
> #endif /* PPC4XX_I2C_H */
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
[Qemu-devel] [PATCH v14 2/6] i386: Enable TOPOEXT feature on AMD EPYC CPU
Enable TOPOEXT feature on EPYC CPU. This is required to support
hyperthreading on VM guests. Also extend xlevel to 0x801E.
Signed-off-by: Babu Moger
---
target/i386/cpu.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 86fb1a4..2eb26da 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2554,7 +2554,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
.features[FEAT_8000_0001_ECX] =
CPUID_EXT3_OSVW | CPUID_EXT3_3DNOWPREFETCH |
CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A | CPUID_EXT3_ABM |
-CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
+CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM |
+CPUID_EXT3_TOPOEXT,
.features[FEAT_7_0_EBX] =
CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_AVX2 |
CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_RDSEED |
@@ -2599,7 +2600,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
.features[FEAT_8000_0001_ECX] =
CPUID_EXT3_OSVW | CPUID_EXT3_3DNOWPREFETCH |
CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A | CPUID_EXT3_ABM |
-CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
+CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM |
+CPUID_EXT3_TOPOEXT,
.features[FEAT_8000_0008_EBX] =
CPUID_8000_0008_EBX_IBPB,
.features[FEAT_7_0_EBX] =
@@ -4667,6 +4669,11 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error
**errp)
x86_cpu_adjust_level(cpu, >cpuid_min_xlevel, 0x800A);
}
+/* TOPOEXT feature requires 0x801E */
+if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) {
+x86_cpu_adjust_level(cpu, >cpuid_min_xlevel, 0x801E);
+}
+
/* SEV requires CPUID[0x801F] */
if (sev_enabled()) {
x86_cpu_adjust_level(cpu, >cpuid_min_xlevel, 0x801F);
--
1.8.3.1
[Qemu-devel] [PATCH v14 3/6] i386: Disable TOPOEXT feature on pc-2.12
Disable TOPOEXT feature for older machines.
Signed-off-by: Babu Moger
---
include/hw/i386/pc.h | 4
1 file changed, 4 insertions(+)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 04d1f8c..ecccf6b 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -303,6 +303,10 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
.driver = TYPE_X86_CPU,\
.property = "legacy-cache",\
.value= "on",\
+},{\
+.driver = TYPE_X86_CPU,\
+.property = "topoext",\
+.value= "off",\
},
#define PC_COMPAT_2_11 \
--
1.8.3.1
Re: [Qemu-devel] [PATCH v3 2/9] ppc4xx_i2c: Implement directcntl register
On Thu, Jun 14, 2018 at 02:17:00AM +0200, BALATON Zoltan wrote:
> Signed-off-by: BALATON Zoltan
Patch looks good, but it needs a commit message. What is the
directcntl register? Now does the bitbang interface play into that?
(Note that I know the answer to those questions right now, but it
needs to be in the commit message for the benefit of people looking
back in the future).
> ---
> default-configs/ppc-softmmu.mak| 1 +
> default-configs/ppcemb-softmmu.mak | 1 +
> hw/i2c/ppc4xx_i2c.c| 13 -
> include/hw/i2c/ppc4xx_i2c.h| 4
> 4 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
> index 4d7be45..7d0dc2f 100644
> --- a/default-configs/ppc-softmmu.mak
> +++ b/default-configs/ppc-softmmu.mak
> @@ -26,6 +26,7 @@ CONFIG_USB_EHCI_SYSBUS=y
> CONFIG_SM501=y
> CONFIG_IDE_SII3112=y
> CONFIG_I2C=y
> +CONFIG_BITBANG_I2C=y
>
> # For Macs
> CONFIG_MAC=y
> diff --git a/default-configs/ppcemb-softmmu.mak
> b/default-configs/ppcemb-softmmu.mak
> index 67d18b2..37af193 100644
> --- a/default-configs/ppcemb-softmmu.mak
> +++ b/default-configs/ppcemb-softmmu.mak
> @@ -19,3 +19,4 @@ CONFIG_USB_EHCI_SYSBUS=y
> CONFIG_SM501=y
> CONFIG_IDE_SII3112=y
> CONFIG_I2C=y
> +CONFIG_BITBANG_I2C=y
> diff --git a/hw/i2c/ppc4xx_i2c.c b/hw/i2c/ppc4xx_i2c.c
> index 4e0aaae..c0a1930 100644
> --- a/hw/i2c/ppc4xx_i2c.c
> +++ b/hw/i2c/ppc4xx_i2c.c
> @@ -30,6 +30,7 @@
> #include "cpu.h"
> #include "hw/hw.h"
> #include "hw/i2c/ppc4xx_i2c.h"
> +#include "bitbang_i2c.h"
>
> #define PPC4xx_I2C_MEM_SIZE 18
>
> @@ -46,6 +47,11 @@
>
> #define IIC_XTCNTLSS_SRST (1 << 0)
>
> +#define IIC_DIRECTCNTL_SDAC (1 << 3)
> +#define IIC_DIRECTCNTL_SCLC (1 << 2)
> +#define IIC_DIRECTCNTL_MSDA (1 << 1)
> +#define IIC_DIRECTCNTL_MSCL (1 << 0)
> +
> static void ppc4xx_i2c_reset(DeviceState *s)
> {
> PPC4xxI2CState *i2c = PPC4xx_I2C(s);
> @@ -289,7 +295,11 @@ static void ppc4xx_i2c_writeb(void *opaque, hwaddr addr,
> uint64_t value,
> i2c->xtcntlss = value;
> break;
> case 16:
> -i2c->directcntl = value & 0x7;
> +i2c->directcntl = value & (IIC_DIRECTCNTL_SDAC &
> IIC_DIRECTCNTL_SCLC);
> +i2c->directcntl |= (value & IIC_DIRECTCNTL_SCLC ? 1 : 0);
> +bitbang_i2c_set(i2c->bitbang, BITBANG_I2C_SCL, i2c->directcntl & 1);
> +i2c->directcntl |= bitbang_i2c_set(i2c->bitbang, BITBANG_I2C_SDA,
> + (value & IIC_DIRECTCNTL_SDAC) != 0) << 1;
> break;
> default:
> if (addr < PPC4xx_I2C_MEM_SIZE) {
> @@ -322,6 +332,7 @@ static void ppc4xx_i2c_init(Object *o)
> sysbus_init_mmio(SYS_BUS_DEVICE(s), >iomem);
> sysbus_init_irq(SYS_BUS_DEVICE(s), >irq);
> s->bus = i2c_init_bus(DEVICE(s), "i2c");
> +s->bitbang = bitbang_i2c_init(s->bus);
> }
>
> static void ppc4xx_i2c_class_init(ObjectClass *klass, void *data)
> diff --git a/include/hw/i2c/ppc4xx_i2c.h b/include/hw/i2c/ppc4xx_i2c.h
> index e4b6ded..ea6c8e1 100644
> --- a/include/hw/i2c/ppc4xx_i2c.h
> +++ b/include/hw/i2c/ppc4xx_i2c.h
> @@ -31,6 +31,9 @@
> #include "hw/sysbus.h"
> #include "hw/i2c/i2c.h"
>
> +/* from hw/i2c/bitbang_i2c.h */
> +typedef struct bitbang_i2c_interface bitbang_i2c_interface;
> +
> #define TYPE_PPC4xx_I2C "ppc4xx-i2c"
> #define PPC4xx_I2C(obj) OBJECT_CHECK(PPC4xxI2CState, (obj), TYPE_PPC4xx_I2C)
>
> @@ -42,6 +45,7 @@ typedef struct PPC4xxI2CState {
> I2CBus *bus;
> qemu_irq irq;
> MemoryRegion iomem;
> +bitbang_i2c_interface *bitbang;
> uint8_t mdata;
> uint8_t lmadr;
> uint8_t hmadr;
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
[Qemu-devel] [PATCHv2 7/7] target/ppc, spapr: Move VPA information to machine_data
CPUPPCState currently contains a number of fields containing the state of
the VPA. The VPA is a PAPR specific concept covering several guest/host
shared memory areas used to communicate some information with the
hypervisor.
As a PAPR concept this is really machine specific information, although it
is per-cpu, so it doesn't really belong in the core CPU state structure.
So, move it to the PAPR specific 'machine_data' structure.
Signed-off-by: David Gibson
Reviewed-by: Greg Kurz
---
hw/ppc/spapr_cpu_core.c | 7 +++
hw/ppc/spapr_hcall.c| 77 ++---
include/hw/ppc/spapr_cpu_core.h | 3 ++
target/ppc/cpu.h| 6 ---
target/ppc/translate_init.inc.c | 8
5 files changed, 52 insertions(+), 49 deletions(-)
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 544bda93e2..f642c95967 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -28,6 +28,7 @@ static void spapr_cpu_reset(void *opaque)
CPUState *cs = CPU(cpu);
CPUPPCState *env = >env;
PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
target_ulong lpcr;
cpu_reset(cs);
@@ -69,6 +70,12 @@ static void spapr_cpu_reset(void *opaque)
/* Set a full AMOR so guest can use the AMR as it sees fit */
env->spr[SPR_AMOR] = 0xull;
+
+spapr_cpu->vpa_addr = 0;
+spapr_cpu->slb_shadow_addr = 0;
+spapr_cpu->slb_shadow_size = 0;
+spapr_cpu->dtl_addr = 0;
+spapr_cpu->dtl_size = 0;
}
void spapr_cpu_set_entry_state(PowerPCCPU *cpu, target_ulong nip, target_ulong
r3)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 8b9a4b577f..ae913d070f 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -8,6 +8,7 @@
#include "exec/exec-all.h"
#include "helper_regs.h"
#include "hw/ppc/spapr.h"
+#include "hw/ppc/spapr_cpu_core.h"
#include "mmu-hash64.h"
#include "cpu-models.h"
#include "trace.h"
@@ -908,9 +909,11 @@ unmap_out:
#define VPA_SHARED_PROC_OFFSET 0x9
#define VPA_SHARED_PROC_VAL0x2
-static target_ulong register_vpa(CPUPPCState *env, target_ulong vpa)
+static target_ulong register_vpa(PowerPCCPU *cpu, target_ulong vpa)
{
-CPUState *cs = CPU(ppc_env_get_cpu(env));
+CPUState *cs = CPU(cpu);
+CPUPPCState *env = >env;
+sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
uint16_t size;
uint8_t tmp;
@@ -935,32 +938,34 @@ static target_ulong register_vpa(CPUPPCState *env,
target_ulong vpa)
return H_PARAMETER;
}
-env->vpa_addr = vpa;
+spapr_cpu->vpa_addr = vpa;
-tmp = ldub_phys(cs->as, env->vpa_addr + VPA_SHARED_PROC_OFFSET);
+tmp = ldub_phys(cs->as, spapr_cpu->vpa_addr + VPA_SHARED_PROC_OFFSET);
tmp |= VPA_SHARED_PROC_VAL;
-stb_phys(cs->as, env->vpa_addr + VPA_SHARED_PROC_OFFSET, tmp);
+stb_phys(cs->as, spapr_cpu->vpa_addr + VPA_SHARED_PROC_OFFSET, tmp);
return H_SUCCESS;
}
-static target_ulong deregister_vpa(CPUPPCState *env, target_ulong vpa)
+static target_ulong deregister_vpa(PowerPCCPU *cpu, target_ulong vpa)
{
-if (env->slb_shadow_addr) {
+sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
+
+if (spapr_cpu->slb_shadow_addr) {
return H_RESOURCE;
}
-if (env->dtl_addr) {
+if (spapr_cpu->dtl_addr) {
return H_RESOURCE;
}
-env->vpa_addr = 0;
+spapr_cpu->vpa_addr = 0;
return H_SUCCESS;
}
-static target_ulong register_slb_shadow(CPUPPCState *env, target_ulong addr)
+static target_ulong register_slb_shadow(PowerPCCPU *cpu, target_ulong addr)
{
-CPUState *cs = CPU(ppc_env_get_cpu(env));
+sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
uint32_t size;
if (addr == 0) {
@@ -968,7 +973,7 @@ static target_ulong register_slb_shadow(CPUPPCState *env,
target_ulong addr)
return H_HARDWARE;
}
-size = ldl_be_phys(cs->as, addr + 0x4);
+size = ldl_be_phys(CPU(cpu)->as, addr + 0x4);
if (size < 0x8) {
return H_PARAMETER;
}
@@ -977,26 +982,28 @@ static target_ulong register_slb_shadow(CPUPPCState *env,
target_ulong addr)
return H_PARAMETER;
}
-if (!env->vpa_addr) {
+if (!spapr_cpu->vpa_addr) {
return H_RESOURCE;
}
-env->slb_shadow_addr = addr;
-env->slb_shadow_size = size;
+spapr_cpu->slb_shadow_addr = addr;
+spapr_cpu->slb_shadow_size = size;
return H_SUCCESS;
}
-static target_ulong deregister_slb_shadow(CPUPPCState *env, target_ulong addr)
+static target_ulong deregister_slb_shadow(PowerPCCPU *cpu, target_ulong addr)
{
-env->slb_shadow_addr = 0;
-env->slb_shadow_size = 0;
+sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
+
+spapr_cpu->slb_shadow_addr = 0;
+spapr_cpu->slb_shadow_size = 0;
return H_SUCCESS;
}
-static target_ulong register_dtl(CPUPPCState *env, target_ulong addr)
+static target_ulong register_dtl(PowerPCCPU *cpu,
[Qemu-devel] [PATCHv2 4/7] pnv: Clean up cpu realize path
pnv_cpu_init() is only called from the the pnv cpu core realize path, and
really only can be called from there. So fold it into its caller, which
we also rename for brevity.
Signed-off-by: David Gibson
Reviewed-by: Cédric Le Goater
Reviewed-by: Greg Kurz
---
hw/ppc/pnv_core.c | 56 ++-
1 file changed, 21 insertions(+), 35 deletions(-)
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 1e40f01e98..f4c41d89d6 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -54,28 +54,6 @@ static void pnv_cpu_reset(void *opaque)
env->msr |= MSR_HVB; /* Hypervisor mode */
}
-static void pnv_cpu_init(PowerPCCPU *cpu, Error **errp)
-{
-CPUPPCState *env = >env;
-int core_pir;
-int thread_index = 0; /* TODO: TCG supports only one thread */
-ppc_spr_t *pir = >spr_cb[SPR_PIR];
-
-core_pir = object_property_get_uint(OBJECT(cpu), "core-pir", _abort);
-
-/*
- * The PIR of a thread is the core PIR + the thread index. We will
- * need to find a way to get the thread index when TCG supports
- * more than 1. We could use the object name ?
- */
-pir->default_value = core_pir + thread_index;
-
-/* Set time-base frequency to 512 MHz */
-cpu_ppc_tb_init(env, PNV_TIMEBASE_FREQ);
-
-qemu_register_reset(pnv_cpu_reset, cpu);
-}
-
/*
* These values are read by the PowerNV HW monitors under Linux
*/
@@ -121,29 +99,39 @@ static const MemoryRegionOps pnv_core_xscom_ops = {
.endianness = DEVICE_BIG_ENDIAN,
};
-static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp)
+static void pnv_realize_vcpu(PowerPCCPU *cpu, XICSFabric *xi, Error **errp)
{
+CPUPPCState *env = >env;
+int core_pir;
+int thread_index = 0; /* TODO: TCG supports only one thread */
+ppc_spr_t *pir = >spr_cb[SPR_PIR];
Error *local_err = NULL;
-CPUState *cs = CPU(child);
-PowerPCCPU *cpu = POWERPC_CPU(cs);
-object_property_set_bool(child, true, "realized", _err);
+object_property_set_bool(OBJECT(cpu), true, "realized", _err);
if (local_err) {
error_propagate(errp, local_err);
return;
}
-cpu->intc = icp_create(child, TYPE_PNV_ICP, xi, _err);
+cpu->intc = icp_create(OBJECT(cpu), TYPE_PNV_ICP, xi, _err);
if (local_err) {
error_propagate(errp, local_err);
return;
}
-pnv_cpu_init(cpu, _err);
-if (local_err) {
-error_propagate(errp, local_err);
-return;
-}
+core_pir = object_property_get_uint(OBJECT(cpu), "core-pir", _abort);
+
+/*
+ * The PIR of a thread is the core PIR + the thread index. We will
+ * need to find a way to get the thread index when TCG supports
+ * more than 1. We could use the object name ?
+ */
+pir->default_value = core_pir + thread_index;
+
+/* Set time-base frequency to 512 MHz */
+cpu_ppc_tb_init(env, PNV_TIMEBASE_FREQ);
+
+qemu_register_reset(pnv_cpu_reset, cpu);
}
static void pnv_core_realize(DeviceState *dev, Error **errp)
@@ -178,9 +166,7 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
}
for (j = 0; j < cc->nr_threads; j++) {
-obj = OBJECT(pc->threads[j]);
-
-pnv_core_realize_child(obj, XICS_FABRIC(xi), _err);
+pnv_realize_vcpu(pc->threads[j], XICS_FABRIC(xi), _err);
if (local_err) {
goto err;
}
--
2.17.1
[Qemu-devel] [PATCHv2 6/7] target/ppc: Replace intc pointer with a general machine_data pointer
PowerPCCPU contains an (Object *)intc used to point to the cpu's interrupt
controller. Or more precisely to the "presentation" component of the
interrupt controller relevant to this cpu.
Really, this field is machine specific. The machines which use it can
point it to different types of object depending on their needs, and most
machines don't use it at all (since they have older style PICs which don't
have per-cpu presentation components).
There's also other information that's per-cpu, but platform/machine
specific. So replace the intc pointer with a (void *)machine_data which
can be managed as the machine type likes to conveniently store per cpu
information.
Signed-off-by: David Gibson
Reviewed-by: Greg Kurz
---
hw/intc/xics.c | 5 +++--
hw/intc/xics_spapr.c| 16 +++-
hw/ppc/pnv.c| 4 ++--
hw/ppc/pnv_core.c | 11 +--
hw/ppc/spapr.c | 8
hw/ppc/spapr_cpu_core.c | 13 ++---
include/hw/ppc/pnv_core.h | 9 +
include/hw/ppc/spapr_cpu_core.h | 10 ++
include/hw/ppc/xics.h | 4 ++--
target/ppc/cpu.h| 2 +-
10 files changed, 61 insertions(+), 21 deletions(-)
diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index e73e623e3b..689ad44e5f 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -383,7 +383,8 @@ static const TypeInfo icp_info = {
.class_size = sizeof(ICPStateClass),
};
-Object *icp_create(Object *cpu, const char *type, XICSFabric *xi, Error **errp)
+ICPState *icp_create(Object *cpu, const char *type, XICSFabric *xi,
+ Error **errp)
{
Error *local_err = NULL;
Object *obj;
@@ -401,7 +402,7 @@ Object *icp_create(Object *cpu, const char *type,
XICSFabric *xi, Error **errp)
obj = NULL;
}
-return obj;
+return ICP(obj);
}
/*
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index 2e27b92b87..01c76717cf 100644
--- a/hw/intc/xics_spapr.c
+++ b/hw/intc/xics_spapr.c
@@ -31,6 +31,7 @@
#include "trace.h"
#include "qemu/timer.h"
#include "hw/ppc/spapr.h"
+#include "hw/ppc/spapr_cpu_core.h"
#include "hw/ppc/xics.h"
#include "hw/ppc/fdt.h"
#include "qapi/visitor.h"
@@ -43,8 +44,9 @@ static target_ulong h_cppr(PowerPCCPU *cpu, sPAPRMachineState
*spapr,
target_ulong opcode, target_ulong *args)
{
target_ulong cppr = args[0];
+sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
-icp_set_cppr(ICP(cpu->intc), cppr);
+icp_set_cppr(spapr_cpu->icp, cppr);
return H_SUCCESS;
}
@@ -65,7 +67,8 @@ static target_ulong h_ipi(PowerPCCPU *cpu, sPAPRMachineState
*spapr,
static target_ulong h_xirr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
target_ulong opcode, target_ulong *args)
{
-uint32_t xirr = icp_accept(ICP(cpu->intc));
+sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
+uint32_t xirr = icp_accept(spapr_cpu->icp);
args[0] = xirr;
return H_SUCCESS;
@@ -74,7 +77,8 @@ static target_ulong h_xirr(PowerPCCPU *cpu, sPAPRMachineState
*spapr,
static target_ulong h_xirr_x(PowerPCCPU *cpu, sPAPRMachineState *spapr,
target_ulong opcode, target_ulong *args)
{
-uint32_t xirr = icp_accept(ICP(cpu->intc));
+sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
+uint32_t xirr = icp_accept(spapr_cpu->icp);
args[0] = xirr;
args[1] = cpu_get_host_ticks();
@@ -84,9 +88,10 @@ static target_ulong h_xirr_x(PowerPCCPU *cpu,
sPAPRMachineState *spapr,
static target_ulong h_eoi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
target_ulong opcode, target_ulong *args)
{
+sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
target_ulong xirr = args[0];
-icp_eoi(ICP(cpu->intc), xirr);
+icp_eoi(spapr_cpu->icp, xirr);
return H_SUCCESS;
}
@@ -94,7 +99,8 @@ static target_ulong h_ipoll(PowerPCCPU *cpu,
sPAPRMachineState *spapr,
target_ulong opcode, target_ulong *args)
{
uint32_t mfrr;
-uint32_t xirr = icp_ipoll(ICP(cpu->intc), );
+sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
+uint32_t xirr = icp_ipoll(spapr_cpu->icp, );
args[0] = xirr;
args[1] = mfrr;
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 0b9508d94d..3a36c6ac6a 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1013,7 +1013,7 @@ static ICPState *pnv_icp_get(XICSFabric *xi, int pir)
{
PowerPCCPU *cpu = ppc_get_vcpu_by_pir(pir);
-return cpu ? ICP(cpu->intc) : NULL;
+return cpu ? pnv_cpu_state(cpu)->icp : NULL;
}
static void pnv_pic_print_info(InterruptStatsProvider *obj,
@@ -1026,7 +1026,7 @@ static void pnv_pic_print_info(InterruptStatsProvider
*obj,
CPU_FOREACH(cs) {
PowerPCCPU *cpu = POWERPC_CPU(cs);
-icp_pic_print_info(ICP(cpu->intc), mon);
+icp_pic_print_info(pnv_cpu_state(cpu)->icp, mon);
}
for (i = 0; i <
[Qemu-devel] [PATCHv2 2/7] pnv: Fix some error handling cpu realize()
In pnv_core_realize() we call two functions with an Error * parameter in
succession, which will go badly if they both cause errors. In fact, a
failure in either of them indicates a qemu internal error, so we can just
use _abort in both cases.
Signed-off-by: David Gibson
---
hw/ppc/pnv_core.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 13ad7d9e04..01f47c8037 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -172,12 +172,9 @@ static void pnv_core_realize(DeviceState *dev, Error
**errp)
object_initialize(obj, size, typename);
snprintf(name, sizeof(name), "thread[%d]", i);
-object_property_add_child(OBJECT(pc), name, obj, _err);
+object_property_add_child(OBJECT(pc), name, obj, _abort);
object_property_add_alias(obj, "core-pir", OBJECT(pc),
- "pir", _err);
-if (local_err) {
-goto err;
-}
+ "pir", _abort);
object_unref(obj);
}
--
2.17.1
[Qemu-devel] [PATCHv2 1/7] spapr: Clean up cpu realize/unrealize paths
spapr_cpu_init() and spapr_cpu_destroy() are only called from the spapr
cpu core realize/unrealize paths, and really can only be called from there.
Those are all short functions, so fold the pairs together for simplicity.
While we're there rename some functions and change some parameter types
for brevity and clarity.
Signed-off-by: David Gibson
Reviewed-by: Cédric Le Goater
Reviewed-by: Greg Kurz
---
hw/ppc/spapr_cpu_core.c | 69 +++--
1 file changed, 25 insertions(+), 44 deletions(-)
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index f3e9b879b2..7fdb3b6667 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -83,26 +83,6 @@ void spapr_cpu_set_entry_state(PowerPCCPU *cpu, target_ulong
nip, target_ulong r
ppc_store_lpcr(cpu, env->spr[SPR_LPCR] | pcc->lpcr_pm);
}
-static void spapr_cpu_destroy(PowerPCCPU *cpu)
-{
-qemu_unregister_reset(spapr_cpu_reset, cpu);
-}
-
-static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
- Error **errp)
-{
-CPUPPCState *env = >env;
-
-/* Set time-base frequency to 512 MHz */
-cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
-
-cpu_ppc_set_vhyp(cpu, PPC_VIRTUAL_HYPERVISOR(spapr));
-kvmppc_set_papr(cpu);
-
-qemu_register_reset(spapr_cpu_reset, cpu);
-spapr_cpu_reset(cpu);
-}
-
/*
* Return the sPAPR CPU core type for @model which essentially is the CPU
* model specified with -cpu cmdline option.
@@ -122,44 +102,47 @@ const char *spapr_get_cpu_core_type(const char *cpu_type)
return object_class_get_name(oc);
}
-static void spapr_cpu_core_unrealizefn(DeviceState *dev, Error **errp)
+static void spapr_unrealize_vcpu(PowerPCCPU *cpu)
+{
+qemu_unregister_reset(spapr_cpu_reset, cpu);
+object_unparent(cpu->intc);
+cpu_remove_sync(CPU(cpu));
+object_unparent(OBJECT(cpu));
+}
+
+static void spapr_cpu_core_unrealize(DeviceState *dev, Error **errp)
{
sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
CPUCore *cc = CPU_CORE(dev);
int i;
for (i = 0; i < cc->nr_threads; i++) {
-Object *obj = OBJECT(sc->threads[i]);
-DeviceState *dev = DEVICE(obj);
-CPUState *cs = CPU(dev);
-PowerPCCPU *cpu = POWERPC_CPU(cs);
-
-spapr_cpu_destroy(cpu);
-object_unparent(cpu->intc);
-cpu_remove_sync(cs);
-object_unparent(obj);
+spapr_unrealize_vcpu(sc->threads[i]);
}
g_free(sc->threads);
}
-static void spapr_cpu_core_realize_child(Object *child,
- sPAPRMachineState *spapr, Error
**errp)
+static void spapr_realize_vcpu(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+ Error **errp)
{
+CPUPPCState *env = >env;
Error *local_err = NULL;
-CPUState *cs = CPU(child);
-PowerPCCPU *cpu = POWERPC_CPU(cs);
-object_property_set_bool(child, true, "realized", _err);
+object_property_set_bool(OBJECT(cpu), true, "realized", _err);
if (local_err) {
goto error;
}
-spapr_cpu_init(spapr, cpu, _err);
-if (local_err) {
-goto error;
-}
+/* Set time-base frequency to 512 MHz */
+cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
+
+cpu_ppc_set_vhyp(cpu, PPC_VIRTUAL_HYPERVISOR(spapr));
+kvmppc_set_papr(cpu);
-cpu->intc = icp_create(child, spapr->icp_type, XICS_FABRIC(spapr),
+qemu_register_reset(spapr_cpu_reset, cpu);
+spapr_cpu_reset(cpu);
+
+cpu->intc = icp_create(OBJECT(cpu), spapr->icp_type, XICS_FABRIC(spapr),
_err);
if (local_err) {
goto error;
@@ -220,9 +203,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error
**errp)
}
for (j = 0; j < cc->nr_threads; j++) {
-obj = OBJECT(sc->threads[j]);
-
-spapr_cpu_core_realize_child(obj, spapr, _err);
+spapr_realize_vcpu(sc->threads[j], spapr, _err);
if (local_err) {
goto err;
}
@@ -249,7 +230,7 @@ static void spapr_cpu_core_class_init(ObjectClass *oc, void
*data)
sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_CLASS(oc);
dc->realize = spapr_cpu_core_realize;
-dc->unrealize = spapr_cpu_core_unrealizefn;
+dc->unrealize = spapr_cpu_core_unrealize;
dc->props = spapr_cpu_core_properties;
scc->cpu_type = data;
}
--
2.17.1
[Qemu-devel] [PATCHv2 5/7] pnv: Add cpu unrealize path
Currently we don't have any unrealize path for pnv cpu cores. We get away
with this because we don't yet support cpu hotplug for pnv.
However, we're going to want it eventually, and in the meantime, it makes
it non-obvious why there are a bunch of allocations on the realize() path
that don't have matching frees.
So, implement the missing unrealize path.
Signed-off-by: David Gibson
Reviewed-by: Cédric Le Goater
Reviewed-by: Greg Kurz
---
hw/ppc/pnv_core.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index f4c41d89d6..f7cf33f547 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -186,6 +186,26 @@ err:
error_propagate(errp, local_err);
}
+static void pnv_unrealize_vcpu(PowerPCCPU *cpu)
+{
+qemu_unregister_reset(pnv_cpu_reset, cpu);
+object_unparent(cpu->intc);
+cpu_remove_sync(CPU(cpu));
+object_unparent(OBJECT(cpu));
+}
+
+static void pnv_core_unrealize(DeviceState *dev, Error **errp)
+{
+PnvCore *pc = PNV_CORE(dev);
+CPUCore *cc = CPU_CORE(dev);
+int i;
+
+for (i = 0; i < cc->nr_threads; i++) {
+pnv_unrealize_vcpu(pc->threads[i]);
+}
+g_free(pc->threads);
+}
+
static Property pnv_core_properties[] = {
DEFINE_PROP_UINT32("pir", PnvCore, pir, 0),
DEFINE_PROP_END_OF_LIST(),
@@ -196,6 +216,7 @@ static void pnv_core_class_init(ObjectClass *oc, void *data)
DeviceClass *dc = DEVICE_CLASS(oc);
dc->realize = pnv_core_realize;
+dc->unrealize = pnv_core_unrealize;
dc->props = pnv_core_properties;
}
--
2.17.1
[Qemu-devel] [PATCHv2 0/7] Better handling of machine specific per-cpu information
It's moderately common for a machine type to need to keep track of information that is specific to the platform it implements, but per-cpu. While it could keep such information inside the MachineState, this makes lookup from the CPUState awkward. So, this series adds a standard way to stash machine-specific per-cpu information using a void pointer in the PowerPCCPU object. The machine is responsible for alloc()ing, free()ing and (if applicable) migrating this state. The meat of the series is the last two patches. The first 5 clean up a number of minor uglies I encountered while implementing. Changes since v1: * Simplify the error handling cleanup in 2/7 to use _abort instead of more propagations David Gibson (7): spapr: Clean up cpu realize/unrealize paths pnv: Fix some error handling cpu realize() pnv_core: Allocate cpu thread objects individually pnv: Clean up cpu realize path pnv: Add cpu unrealize path target/ppc: Replace intc pointer with a general machine_data pointer target/ppc, spapr: Move VPA information to machine_data hw/intc/xics.c | 5 +- hw/intc/xics_spapr.c| 16 +++-- hw/ppc/pnv.c| 8 +-- hw/ppc/pnv_core.c | 100 ++-- hw/ppc/spapr.c | 8 +-- hw/ppc/spapr_cpu_core.c | 85 +-- hw/ppc/spapr_hcall.c| 77 +--- include/hw/ppc/pnv_core.h | 11 +++- include/hw/ppc/spapr_cpu_core.h | 13 + include/hw/ppc/xics.h | 4 +- target/ppc/cpu.h| 8 +-- target/ppc/translate_init.inc.c | 8 --- 12 files changed, 185 insertions(+), 158 deletions(-) -- 2.17.1
[Qemu-devel] [PATCHv2 3/7] pnv_core: Allocate cpu thread objects individually
Currently, we allocate space for all the cpu objects within a single core
in one big block. This was copied from an older version of the spapr code
and requires some ugly pointer manipulation to extract the individual
objects.
This design was due to a misunderstanding of qemu lifetime conventions and
has already been changed in spapr (in 94ad93bd "spapr_cpu_core: instantiate
CPUs separately".
Make an equivalent change in pnv_core to get rid of the nasty pointer
arithmetic.
Signed-off-by: David Gibson
Reviewed-by: Cédric Le Goater
Reviewed-by: Greg Kurz
---
hw/ppc/pnv.c | 4 ++--
hw/ppc/pnv_core.c | 11 +--
include/hw/ppc/pnv_core.h | 2 +-
3 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 0314881316..0b9508d94d 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -121,9 +121,9 @@ static int get_cpus_node(void *fdt)
*/
static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
{
-CPUState *cs = CPU(DEVICE(pc->threads));
+PowerPCCPU *cpu = pc->threads[0];
+CPUState *cs = CPU(cpu);
DeviceClass *dc = DEVICE_GET_CLASS(cs);
-PowerPCCPU *cpu = POWERPC_CPU(cs);
int smt_threads = CPU_CORE(pc)->nr_threads;
CPUPPCState *env = >env;
PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 01f47c8037..1e40f01e98 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -151,7 +151,6 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
PnvCore *pc = PNV_CORE(OBJECT(dev));
CPUCore *cc = CPU_CORE(OBJECT(dev));
const char *typename = pnv_core_cpu_typename(pc);
-size_t size = object_type_get_instance_size(typename);
Error *local_err = NULL;
void *obj;
int i, j;
@@ -165,11 +164,11 @@ static void pnv_core_realize(DeviceState *dev, Error
**errp)
return;
}
-pc->threads = g_malloc0(size * cc->nr_threads);
+pc->threads = g_new(PowerPCCPU *, cc->nr_threads);
for (i = 0; i < cc->nr_threads; i++) {
-obj = pc->threads + i * size;
+obj = object_new(typename);
-object_initialize(obj, size, typename);
+pc->threads[i] = POWERPC_CPU(obj);
snprintf(name, sizeof(name), "thread[%d]", i);
object_property_add_child(OBJECT(pc), name, obj, _abort);
@@ -179,7 +178,7 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
}
for (j = 0; j < cc->nr_threads; j++) {
-obj = pc->threads + j * size;
+obj = OBJECT(pc->threads[j]);
pnv_core_realize_child(obj, XICS_FABRIC(xi), _err);
if (local_err) {
@@ -194,7 +193,7 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
err:
while (--i >= 0) {
-obj = pc->threads + i * size;
+obj = OBJECT(pc->threads[i]);
object_unparent(obj);
}
g_free(pc->threads);
diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
index e337af7a3a..447ae761f7 100644
--- a/include/hw/ppc/pnv_core.h
+++ b/include/hw/ppc/pnv_core.h
@@ -34,7 +34,7 @@ typedef struct PnvCore {
CPUCore parent_obj;
/*< public >*/
-void *threads;
+PowerPCCPU **threads;
uint32_t pir;
MemoryRegion xscom_regs;
--
2.17.1
Re: [Qemu-devel] [virtio-dev] Re: [PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net
On Tue, Jun 12, 2018 at 5:08 PM, Samudrala, Sridhar
wrote:
> On 6/12/2018 4:34 AM, Michael S. Tsirkin wrote:
>>
>> On Mon, Jun 11, 2018 at 10:02:45PM -0700, Samudrala, Sridhar wrote:
>>>
>>> On 6/11/2018 7:17 PM, Michael S. Tsirkin wrote:
On Tue, Jun 12, 2018 at 09:54:44AM +0800, Jason Wang wrote:
>
> On 2018年06月12日 01:26, Michael S. Tsirkin wrote:
>>
>> On Mon, May 07, 2018 at 04:09:54PM -0700, Sridhar Samudrala wrote:
>>>
>>> This feature bit can be used by hypervisor to indicate virtio_net
>>> device to
>>> act as a standby for another device with the same MAC address.
>>>
>>> I tested this with a small change to the patch to mark the STANDBY
>>> feature 'true'
>>> by default as i am using libvirt to start the VMs.
>>> Is there a way to pass the newly added feature bit 'standby' to qemu
>>> via libvirt
>>> XML file?
>>>
>>> Signed-off-by: Sridhar Samudrala
>>
>> So I do not think we can commit to this interface: we
>> really need to control visibility of the primary device.
>
> The problem is legacy guest won't use primary device at all if we do
> this.
And that's by design - I think it's the only way to ensure the
legacy guest isn't confused.
>>>
>>> Yes. I think so. But i am not sure if Qemu is the right place to control
>>> the visibility
>>> of the primary device. The primary device may not be specified as an
>>> argument to Qemu. It
>>> may be plugged in later.
>>> The cloud service provider is providing a feature that enables low
>>> latency datapath and live
>>> migration capability.
>>> A tenant can use this feature only if he is running a VM that has
>>> virtio-net with failover support.
>>
>> Well live migration is there already. The new feature is low latency
>> data path.
>
>
> we get live migration with just virtio. But I meant live migration with VF
> as
> primary device.
>
>>
>> And it's the guest that needs failover support not the VM.
>
>
> Isn't guest and VM synonymous?
>
>
>>
>>
>>> I think Qemu should check if guest virtio-net supports this feature and
>>> provide a mechanism for
>>> an upper layer indicating if the STANDBY feature is successfully
>>> negotiated or not.
>>> The upper layer can then decide if it should hot plug a VF with the same
>>> MAC and manage the 2 links.
>>> If VF is successfully hot plugged, virtio-net link should be disabled.
>>
>> Did you even talk to upper layer management about it?
>> Just list the steps they need to do and you will see
>> that's a lot of machinery to manage by the upper layer.
>>
>> What do we gain in flexibility? As far as I can see the
>> only gain is some resources saved for legacy VMs.
>>
>> That's not a lot as tenant of the upper layer probably already has
>> at least a hunch that it's a new guest otherwise
>> why bother specifying the feature at all - you
>> save even more resources without it.
>>
>
> I am not all that familiar with how Qemu manages network devices. If we can
> do all the
> required management of the primary/standby devices within Qemu, that is
> definitely a better
> approach without upper layer involvement.
Right. I would imagine in the extreme case the upper layer doesn't
have to be involved at all if QEMU manages all hot plug/unplug logic.
The management tool can supply passthrough device and virtio with the
same group UUID, QEMU auto-manages the presence of the primary, and
hot plug the device as needed before or after the migration.
-Siwei
>
>
>>
>>
> How about control the visibility of standby device?
>
> Thanks
standy the always there to guarantee no downtime.
>> However just for testing purposes, we could add a non-stable
>> interface "x-standby" with the understanding that as any
>> x- prefix it's unstable and will be changed down the road,
>> likely in the next release.
>>
>>
>>> ---
>>> hw/net/virtio-net.c | 2 ++
>>> include/standard-headers/linux/virtio_net.h | 3 +++
>>> 2 files changed, 5 insertions(+)
>>>
>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>>> index 90502fca7c..38b3140670 100644
>>> --- a/hw/net/virtio-net.c
>>> +++ b/hw/net/virtio-net.c
>>> @@ -2198,6 +2198,8 @@ static Property virtio_net_properties[] = {
>>> true),
>>> DEFINE_PROP_INT32("speed", VirtIONet, net_conf.speed,
>>> SPEED_UNKNOWN),
>>> DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_str),
>>> +DEFINE_PROP_BIT64("standby", VirtIONet, host_features,
>>> VIRTIO_NET_F_STANDBY,
>>> + false),
>>> DEFINE_PROP_END_OF_LIST(),
>>> };
>>> diff --git a/include/standard-headers/linux/virtio_net.h
>>> b/include/standard-headers/linux/virtio_net.h
>>> index e9f255ea3f..01ec09684c 100644
>>> --- a/include/standard-headers/linux/virtio_net.h
>>>
Re: [Qemu-devel] [PATCH 2/7] pnv: Add missing error check during cpu realize()
On Wed, Jun 13, 2018 at 07:53:29PM +1000, David Gibson wrote:
> On Wed, Jun 13, 2018 at 11:42:07AM +0200, Greg Kurz wrote:
> > On Wed, 13 Jun 2018 11:14:57 +0200
> > Cédric Le Goater wrote:
> >
> > > >> index 13ad7d9e04..efb68226bb 100644
> > > >> --- a/hw/ppc/pnv_core.c
> > > >> +++ b/hw/ppc/pnv_core.c
> > > >> @@ -173,6 +173,9 @@ static void pnv_core_realize(DeviceState *dev,
> > > >> Error **errp)
> > > >>
> > > >> snprintf(name, sizeof(name), "thread[%d]", i);
> > > >> object_property_add_child(OBJECT(pc), name, obj, _err);
> > > >> +if (local_err) {
> > > >> +goto err;
> > > >> +}
> > > >> object_property_add_alias(obj, "core-pir", OBJECT(pc),
> > > >>"pir", _err);
> > > >> if (local_err) {
> > > >
> > > > Hmm... the current error path seems to assume failures to be
> > > > caused by object_property_add_child(). It hence unparents the
> > > > previously parented CPUs, but not the current one. So we'll
> > > > miss one call to object_unparent() if object_property_add_alias()
> > > > fails.
> > >
> > > yes, let's just put NULL or _abort instead.
> > >
> >
> > NULL means we really don't care if the call fails or succeeds.
> >
> > _abort means we consider a failure to be a unrecoverable bug.
> >
> > So I would rather pass _abort here.
> >
> > But if the guest is already running and functional, and we hit
> > the error during hotplug, does the guest really deserve to be
> > aborted or should we just fail the hotplug ?
>
> Ah, dammit, that's why it wasn't an abort in the first place. Yeah,
> we'd better propagate the errors.
No.. thinking about this yet again, we should be ok with error_abort.
These really aren't things that should fail. If they do something has
gone so horribly wrong, that I think an abort() is a reasonable
reaction, even on hotplug.
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH v13 3/5] i386: Enable TOPOEXT feature on AMD EPYC CPU
> -Original Message- > From: Eduardo Habkost [mailto:ehabk...@redhat.com] > Sent: Wednesday, June 13, 2018 1:49 PM > To: Moger, Babu > Cc: m...@redhat.com; marcel.apfelb...@gmail.com; pbonz...@redhat.com; > r...@twiddle.net; mtosa...@redhat.com; qemu-devel@nongnu.org; > k...@vger.kernel.org; k...@tripleback.net; ge...@hostfission.com; Jiri > Denemark > Subject: Re: [PATCH v13 3/5] i386: Enable TOPOEXT feature on AMD EPYC > CPU > > On Wed, Jun 13, 2018 at 06:21:58PM +, Moger, Babu wrote: > > > > > > > -Original Message- > > > From: Eduardo Habkost [mailto:ehabk...@redhat.com] > > > Sent: Wednesday, June 13, 2018 1:18 PM > > > To: Moger, Babu > > > Cc: m...@redhat.com; marcel.apfelb...@gmail.com; > pbonz...@redhat.com; > > > r...@twiddle.net; mtosa...@redhat.com; qemu-devel@nongnu.org; > > > k...@vger.kernel.org; k...@tripleback.net; ge...@hostfission.com; Jiri > > > Denemark > > > Subject: Re: [PATCH v13 3/5] i386: Enable TOPOEXT feature on AMD EPYC > > > CPU > > > > > > On Wed, Jun 13, 2018 at 06:10:30PM +, Moger, Babu wrote: > > > > > -Original Message- > > > > > From: Eduardo Habkost [mailto:ehabk...@redhat.com] > > > > > Sent: Wednesday, June 13, 2018 12:18 PM > > > > > To: Moger, Babu > > > > > Cc: m...@redhat.com; marcel.apfelb...@gmail.com; > > > pbonz...@redhat.com; > > > > > r...@twiddle.net; mtosa...@redhat.com; qemu-devel@nongnu.org; > > > > > k...@vger.kernel.org; k...@tripleback.net; ge...@hostfission.com; > Jiri > > > > > Denemark > > > > > Subject: Re: [PATCH v13 3/5] i386: Enable TOPOEXT feature on AMD > EPYC > > > > > CPU > > > > > > > > > > On Wed, Jun 13, 2018 at 04:52:18PM +, Moger, Babu wrote: > > > > > [...] > > > > > > > What do you think our options are here? > > > > > > > > > > > > Should we drop automatic topoext completely and move forward? > > > > > > What are your thoughts? > > > > > > > > > > Let's drop automatic topoext by now, and see if we find solutions > > > > > later. I don't want to hold the rest of the patches because of > > > > > this. > > > > > > > > Ok. I will drop topoext. > > > > > > > > > > > > > > I'm thinking we could simply make kvm_arch_get_supported_cpuid() > > > > > always return TOPOEXT on AMD CPUs, because the feature flag don't > > > > > really depend on any KVM code to work (is that correct?). > > > > > > > > Yes, that is correct. I don't see any dependent code on TOPOEXT in > KVM > > > driver. > > > > > > > > Ok. Let me add TOPOEXT flag for all the AMD cpus and see how it goes. > > > > > > Hmm, this could actually solve all of our problems, then: > > > > > > We can forget about auto-topoext: just add TOPOEXT in > > > kvm_arch_get_supported_cpuid(), add TOPOEXT unconditionally to > > > the CPU models where you are interested into (EPYC only?), and > > > add topoext=off to pc-2.12 compat_props. > > > > > > > Ok Sure. > > Sorry, I forgot we still need to decide what to do if TOPOEXT is > enabled in the CPU model (or command-line) but the -smp options > are not compatible with it. Yes. I have kept that check. But, I had to implement topology_supports_topoext bit differently. Reason for this is we need to disable this feature before the x86_cpu_expand_features. But problem is nr_cores and nr_threads are not populated at this time. It is populated in qemu_init_vcpus. Please take a look at topology_supports_topoext again. > > In other words, what should guest see on CPUID if using: > > "-machine pc-q35-3.0 -cpu EPYC -smp 64,cores=64" > or: > "-machine pc-q35-3.0 -cpu Opteron_G5,+topoext -smp 64,cores=64" > Tested both these cases. It works fine with some warning messages. > I wonder what would happen if we just return zeroes on > CPUID[0x81E] if !topology_supports_topoext(), instead of > trying to clear/set TOPOEXT depending on the -smp option? It > would make things much simpler for QEMU and libvirt. I did not see that difference in behavior if we clear the bit versus return 0s. Sending new patches now. Please review. One note: I will going on vacation from June 20th for couple of weeks. If possible I would like to close this feature. If we cannot that is fine. Just an FYI. > > -- > Eduardo
Re: [Qemu-devel] [virtio-dev] Re: [PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net
On Tue, Jun 12, 2018 at 4:47 AM, Michael S. Tsirkin wrote: > On Tue, Jun 05, 2018 at 03:09:26PM -0700, Siwei Liu wrote: >> The thing is cloud service provider might prefer sticking to the same >> level of service agreement (SLA) of keeping VF over migration, > > That requirement is trivially satisfied by just a single VF :) If your > SLA does not require live migration, you should do just that. The requirement is enable live migration by default without getting user too much attention. Migration should be attempted if guest is live migratable. If not, present a single VF to legacy guest. You seem to think it's not a valid use case? Or impossible to do so? What if I have something in mind that can support my theory as well? -Siwei > > -- > MST
[Qemu-devel] [PATCH v3 2/9] ppc4xx_i2c: Implement directcntl register
Signed-off-by: BALATON Zoltan
---
default-configs/ppc-softmmu.mak| 1 +
default-configs/ppcemb-softmmu.mak | 1 +
hw/i2c/ppc4xx_i2c.c| 13 -
include/hw/i2c/ppc4xx_i2c.h| 4
4 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index 4d7be45..7d0dc2f 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -26,6 +26,7 @@ CONFIG_USB_EHCI_SYSBUS=y
CONFIG_SM501=y
CONFIG_IDE_SII3112=y
CONFIG_I2C=y
+CONFIG_BITBANG_I2C=y
# For Macs
CONFIG_MAC=y
diff --git a/default-configs/ppcemb-softmmu.mak
b/default-configs/ppcemb-softmmu.mak
index 67d18b2..37af193 100644
--- a/default-configs/ppcemb-softmmu.mak
+++ b/default-configs/ppcemb-softmmu.mak
@@ -19,3 +19,4 @@ CONFIG_USB_EHCI_SYSBUS=y
CONFIG_SM501=y
CONFIG_IDE_SII3112=y
CONFIG_I2C=y
+CONFIG_BITBANG_I2C=y
diff --git a/hw/i2c/ppc4xx_i2c.c b/hw/i2c/ppc4xx_i2c.c
index 4e0aaae..c0a1930 100644
--- a/hw/i2c/ppc4xx_i2c.c
+++ b/hw/i2c/ppc4xx_i2c.c
@@ -30,6 +30,7 @@
#include "cpu.h"
#include "hw/hw.h"
#include "hw/i2c/ppc4xx_i2c.h"
+#include "bitbang_i2c.h"
#define PPC4xx_I2C_MEM_SIZE 18
@@ -46,6 +47,11 @@
#define IIC_XTCNTLSS_SRST (1 << 0)
+#define IIC_DIRECTCNTL_SDAC (1 << 3)
+#define IIC_DIRECTCNTL_SCLC (1 << 2)
+#define IIC_DIRECTCNTL_MSDA (1 << 1)
+#define IIC_DIRECTCNTL_MSCL (1 << 0)
+
static void ppc4xx_i2c_reset(DeviceState *s)
{
PPC4xxI2CState *i2c = PPC4xx_I2C(s);
@@ -289,7 +295,11 @@ static void ppc4xx_i2c_writeb(void *opaque, hwaddr addr,
uint64_t value,
i2c->xtcntlss = value;
break;
case 16:
-i2c->directcntl = value & 0x7;
+i2c->directcntl = value & (IIC_DIRECTCNTL_SDAC & IIC_DIRECTCNTL_SCLC);
+i2c->directcntl |= (value & IIC_DIRECTCNTL_SCLC ? 1 : 0);
+bitbang_i2c_set(i2c->bitbang, BITBANG_I2C_SCL, i2c->directcntl & 1);
+i2c->directcntl |= bitbang_i2c_set(i2c->bitbang, BITBANG_I2C_SDA,
+ (value & IIC_DIRECTCNTL_SDAC) != 0) << 1;
break;
default:
if (addr < PPC4xx_I2C_MEM_SIZE) {
@@ -322,6 +332,7 @@ static void ppc4xx_i2c_init(Object *o)
sysbus_init_mmio(SYS_BUS_DEVICE(s), >iomem);
sysbus_init_irq(SYS_BUS_DEVICE(s), >irq);
s->bus = i2c_init_bus(DEVICE(s), "i2c");
+s->bitbang = bitbang_i2c_init(s->bus);
}
static void ppc4xx_i2c_class_init(ObjectClass *klass, void *data)
diff --git a/include/hw/i2c/ppc4xx_i2c.h b/include/hw/i2c/ppc4xx_i2c.h
index e4b6ded..ea6c8e1 100644
--- a/include/hw/i2c/ppc4xx_i2c.h
+++ b/include/hw/i2c/ppc4xx_i2c.h
@@ -31,6 +31,9 @@
#include "hw/sysbus.h"
#include "hw/i2c/i2c.h"
+/* from hw/i2c/bitbang_i2c.h */
+typedef struct bitbang_i2c_interface bitbang_i2c_interface;
+
#define TYPE_PPC4xx_I2C "ppc4xx-i2c"
#define PPC4xx_I2C(obj) OBJECT_CHECK(PPC4xxI2CState, (obj), TYPE_PPC4xx_I2C)
@@ -42,6 +45,7 @@ typedef struct PPC4xxI2CState {
I2CBus *bus;
qemu_irq irq;
MemoryRegion iomem;
+bitbang_i2c_interface *bitbang;
uint8_t mdata;
uint8_t lmadr;
uint8_t hmadr;
--
2.7.6
[Qemu-devel] [PATCH v3 4/9] hw/timer: Add basic M41T80 emulation
Basic emulation of the M41T80 serial (I2C) RTC chip. Only getting time
of day is implemented. Setting time and RTC alarm are not supported.
Signed-off-by: BALATON Zoltan
---
Notes:
v3: Fixed \n-s in log messages
MAINTAINERS | 1 +
default-configs/ppc-softmmu.mak | 1 +
hw/timer/Makefile.objs | 1 +
hw/timer/m41t80.c | 117
4 files changed, 120 insertions(+)
create mode 100644 hw/timer/m41t80.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 8a94517..74ae589 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -829,6 +829,7 @@ M: BALATON Zoltan
L: qemu-...@nongnu.org
S: Maintained
F: hw/ide/sii3112.c
+F: hw/timer/m41t80.c
SH4 Machines
diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index 7d0dc2f..9fbaadc 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -27,6 +27,7 @@ CONFIG_SM501=y
CONFIG_IDE_SII3112=y
CONFIG_I2C=y
CONFIG_BITBANG_I2C=y
+CONFIG_M41T80=y
# For Macs
CONFIG_MAC=y
diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index 8b27a4b..e16b2b9 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -6,6 +6,7 @@ common-obj-$(CONFIG_CADENCE) += cadence_ttc.o
common-obj-$(CONFIG_DS1338) += ds1338.o
common-obj-$(CONFIG_HPET) += hpet.o
common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
+common-obj-$(CONFIG_M41T80) += m41t80.o
common-obj-$(CONFIG_M48T59) += m48t59.o
ifeq ($(CONFIG_ISA_BUS),y)
common-obj-$(CONFIG_M48T59) += m48t59-isa.o
diff --git a/hw/timer/m41t80.c b/hw/timer/m41t80.c
new file mode 100644
index 000..734d7d9
--- /dev/null
+++ b/hw/timer/m41t80.c
@@ -0,0 +1,117 @@
+/*
+ * M41T80 serial rtc emulation
+ *
+ * Copyright (c) 2018 BALATON Zoltan
+ *
+ * This work is licensed under the GNU GPL license version 2 or later.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/timer.h"
+#include "qemu/bcd.h"
+#include "hw/i2c/i2c.h"
+
+#define TYPE_M41T80 "m41t80"
+#define M41T80(obj) OBJECT_CHECK(M41t80State, (obj), TYPE_M41T80)
+
+typedef struct M41t80State {
+I2CSlave parent_obj;
+int8_t addr;
+} M41t80State;
+
+static void m41t80_realize(DeviceState *dev, Error **errp)
+{
+M41t80State *s = M41T80(dev);
+
+s->addr = -1;
+}
+
+static int m41t80_send(I2CSlave *i2c, uint8_t data)
+{
+M41t80State *s = M41T80(i2c);
+
+if (s->addr < 0) {
+s->addr = data;
+} else {
+s->addr++;
+}
+return 0;
+}
+
+static int m41t80_recv(I2CSlave *i2c)
+{
+M41t80State *s = M41T80(i2c);
+struct tm now;
+qemu_timeval tv;
+
+if (s->addr < 0) {
+s->addr = 0;
+}
+if (s->addr >= 1 && s->addr <= 7) {
+qemu_get_timedate(, -1);
+}
+switch (s->addr++) {
+case 0:
+qemu_gettimeofday();
+return to_bcd(tv.tv_usec / 1);
+case 1:
+return to_bcd(now.tm_sec);
+case 2:
+return to_bcd(now.tm_min);
+case 3:
+return to_bcd(now.tm_hour);
+case 4:
+return to_bcd(now.tm_wday);
+case 5:
+return to_bcd(now.tm_mday);
+case 6:
+return to_bcd(now.tm_mon + 1);
+case 7:
+return to_bcd(now.tm_year % 100);
+case 8 ... 19:
+qemu_log_mask(LOG_UNIMP, "%s: unimplemented register: %d\n",
+ __func__, s->addr - 1);
+return 0;
+default:
+qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid register: %d\n",
+ __func__, s->addr - 1);
+return 0;
+}
+}
+
+static int m41t80_event(I2CSlave *i2c, enum i2c_event event)
+{
+M41t80State *s = M41T80(i2c);
+
+if (event == I2C_START_SEND) {
+s->addr = -1;
+}
+return 0;
+}
+
+static void m41t80_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass);
+
+dc->realize = m41t80_realize;
+sc->send = m41t80_send;
+sc->recv = m41t80_recv;
+sc->event = m41t80_event;
+}
+
+static const TypeInfo m41t80_info = {
+.name = TYPE_M41T80,
+.parent= TYPE_I2C_SLAVE,
+.instance_size = sizeof(M41t80State),
+.class_init= m41t80_class_init,
+};
+
+static void m41t80_register_types(void)
+{
+type_register_static(_info);
+}
+
+type_init(m41t80_register_types)
--
2.7.6
[Qemu-devel] [PATCH v3 7/9] sm501: Implement i2c part for reading monitor EDID
Signed-off-by: BALATON Zoltan
---
default-configs/ppc-softmmu.mak| 1 +
default-configs/ppcemb-softmmu.mak | 1 +
default-configs/sh4-softmmu.mak| 1 +
default-configs/sh4eb-softmmu.mak | 1 +
hw/display/sm501.c | 136 +++--
5 files changed, 136 insertions(+), 4 deletions(-)
diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index 9fbaadc..860de80 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -24,6 +24,7 @@ CONFIG_ETSEC=y
# For Sam460ex
CONFIG_USB_EHCI_SYSBUS=y
CONFIG_SM501=y
+CONFIG_DDC=y
CONFIG_IDE_SII3112=y
CONFIG_I2C=y
CONFIG_BITBANG_I2C=y
diff --git a/default-configs/ppcemb-softmmu.mak
b/default-configs/ppcemb-softmmu.mak
index 37af193..ac44f15 100644
--- a/default-configs/ppcemb-softmmu.mak
+++ b/default-configs/ppcemb-softmmu.mak
@@ -17,6 +17,7 @@ CONFIG_XILINX=y
CONFIG_XILINX_ETHLITE=y
CONFIG_USB_EHCI_SYSBUS=y
CONFIG_SM501=y
+CONFIG_DDC=y
CONFIG_IDE_SII3112=y
CONFIG_I2C=y
CONFIG_BITBANG_I2C=y
diff --git a/default-configs/sh4-softmmu.mak b/default-configs/sh4-softmmu.mak
index 546d855..72d8fca 100644
--- a/default-configs/sh4-softmmu.mak
+++ b/default-configs/sh4-softmmu.mak
@@ -9,6 +9,7 @@ CONFIG_PFLASH_CFI02=y
CONFIG_SH4=y
CONFIG_IDE_MMIO=y
CONFIG_SM501=y
+CONFIG_DDC=y
CONFIG_ISA_TESTDEV=y
CONFIG_I82378=y
CONFIG_I8259=y
diff --git a/default-configs/sh4eb-softmmu.mak
b/default-configs/sh4eb-softmmu.mak
index 2d3fd49..c686637 100644
--- a/default-configs/sh4eb-softmmu.mak
+++ b/default-configs/sh4eb-softmmu.mak
@@ -9,6 +9,7 @@ CONFIG_PFLASH_CFI02=y
CONFIG_SH4=y
CONFIG_IDE_MMIO=y
CONFIG_SM501=y
+CONFIG_DDC=y
CONFIG_ISA_TESTDEV=y
CONFIG_I82378=y
CONFIG_I8259=y
diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index ca0840f..0625cf5 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -26,6 +26,7 @@
#include "qemu/osdep.h"
#include "qemu/cutils.h"
#include "qapi/error.h"
+#include "qemu/log.h"
#include "qemu-common.h"
#include "cpu.h"
#include "hw/hw.h"
@@ -34,6 +35,8 @@
#include "hw/devices.h"
#include "hw/sysbus.h"
#include "hw/pci/pci.h"
+#include "hw/i2c/i2c.h"
+#include "hw/i2c/i2c-ddc.h"
#include "qemu/range.h"
#include "ui/pixel_ops.h"
@@ -471,10 +474,12 @@ typedef struct SM501State {
MemoryRegion local_mem_region;
MemoryRegion mmio_region;
MemoryRegion system_config_region;
+MemoryRegion i2c_region;
MemoryRegion disp_ctrl_region;
MemoryRegion twoD_engine_region;
uint32_t last_width;
uint32_t last_height;
+I2CBus *i2c_bus;
/* mmio registers */
uint32_t system_control;
@@ -487,6 +492,11 @@ typedef struct SM501State {
uint32_t misc_timing;
uint32_t power_mode_control;
+uint8_t i2c_byte_count;
+uint8_t i2c_status;
+uint8_t i2c_addr;
+uint8_t i2c_data[16];
+
uint32_t uart0_ier;
uint32_t uart0_lcr;
uint32_t uart0_mcr;
@@ -897,6 +907,107 @@ static const MemoryRegionOps sm501_system_config_ops = {
.endianness = DEVICE_LITTLE_ENDIAN,
};
+static uint64_t sm501_i2c_read(void *opaque, hwaddr addr, unsigned size)
+{
+SM501State *s = (SM501State *)opaque;
+uint8_t ret = 0;
+
+switch (addr) {
+case SM501_I2C_BYTE_COUNT:
+ret = s->i2c_byte_count;
+break;
+case SM501_I2C_STATUS:
+ret = s->i2c_status;
+break;
+case SM501_I2C_SLAVE_ADDRESS:
+ret = s->i2c_addr;
+break;
+case SM501_I2C_DATA ... SM501_I2C_DATA + 15:
+ret = s->i2c_data[addr - SM501_I2C_DATA];
+break;
+default:
+qemu_log_mask(LOG_UNIMP, "sm501 i2c : not implemented register read."
+ " addr=0x%" HWADDR_PRIx "\n", addr);
+}
+
+SM501_DPRINTF("sm501 i2c regs : read addr=%" HWADDR_PRIx " val=%x\n",
+ addr, ret);
+return ret;
+}
+
+static void sm501_i2c_write(void *opaque, hwaddr addr, uint64_t value,
+unsigned size)
+{
+SM501State *s = (SM501State *)opaque;
+SM501_DPRINTF("sm501 i2c regs : write addr=%" HWADDR_PRIx
+ " val=%" PRIx64 "\n", addr, value);
+
+switch (addr) {
+case SM501_I2C_BYTE_COUNT:
+s->i2c_byte_count = value & 0xf;
+break;
+case SM501_I2C_CONTROL:
+if (value & 1) {
+if (value & 4) {
+int res = i2c_start_transfer(s->i2c_bus,
+ s->i2c_addr >> 1,
+ s->i2c_addr & 1);
+s->i2c_status |= (res ? 1 << 2 : 0);
+if (!res) {
+int i;
+SM501_DPRINTF("sm501 i2c : transferring %d bytes to
0x%x\n",
+ s->i2c_byte_count + 1, s->i2c_addr >> 1);
+for (i = 0; i <= s->i2c_byte_count; i++) {
+res = i2c_send_recv(s->i2c_bus, >i2c_data[i],
+
[Qemu-devel] [PATCH v3 0/9] Misc sam460ex improvements
This is v3 of sam460ex improvements addressing review comments and adding two additional patches that allow AmigaOS to progress further (does not boot fully yet). BALATON Zoltan (8): ppc4xx_i2c: Remove unimplemented sdata and intr registers ppc4xx_i2c: Implement directcntl register ppc4xx_i2c: Rewrite to model hardware more closely hw/timer: Add basic M41T80 emulation sam460ex: Add RTC device sm501: Do not clear read only bits when writing registers sm501: Implement i2c part for reading monitor EDID target/ppc: Add missing opcode for icbt on PPC440 Sebastian Bauer (1): sm501: Perform a full update after palette change MAINTAINERS| 1 + default-configs/ppc-softmmu.mak| 3 + default-configs/ppcemb-softmmu.mak | 2 + default-configs/sh4-softmmu.mak| 1 + default-configs/sh4eb-softmmu.mak | 1 + hw/display/sm501.c | 159 +-- hw/i2c/ppc4xx_i2c.c| 251 ++--- hw/ppc/sam460ex.c | 1 + hw/timer/Makefile.objs | 1 + hw/timer/m41t80.c | 117 + include/hw/i2c/ppc4xx_i2c.h| 11 +- target/ppc/translate.c | 2 + 12 files changed, 406 insertions(+), 144 deletions(-) create mode 100644 hw/timer/m41t80.c -- 2.7.6
[Qemu-devel] [PATCH v3 3/9] ppc4xx_i2c: Rewrite to model hardware more closely
Rewrite to make it closer to how real device works so that guest OS
drivers can access I2C devices. Previously this was only a hack to
allow U-Boot to get past accessing SPD EEPROMs but to support other
I2C devices and allow guests to access them we need to model real
device more properly.
Signed-off-by: BALATON Zoltan
---
hw/i2c/ppc4xx_i2c.c | 222 +---
include/hw/i2c/ppc4xx_i2c.h | 3 +-
2 files changed, 110 insertions(+), 115 deletions(-)
diff --git a/hw/i2c/ppc4xx_i2c.c b/hw/i2c/ppc4xx_i2c.c
index c0a1930..73d6ba5 100644
--- a/hw/i2c/ppc4xx_i2c.c
+++ b/hw/i2c/ppc4xx_i2c.c
@@ -38,13 +38,26 @@
#define IIC_CNTL_READ (1 << 1)
#define IIC_CNTL_CHT(1 << 2)
#define IIC_CNTL_RPST (1 << 3)
+#define IIC_CNTL_AMD(1 << 6)
+#define IIC_CNTL_HMT(1 << 7)
+
+#define IIC_MDCNTL_EINT (1 << 2)
+#define IIC_MDCNTL_ESM (1 << 3)
+#define IIC_MDCNTL_FMDB (1 << 6)
#define IIC_STS_PT (1 << 0)
+#define IIC_STS_IRQA(1 << 1)
#define IIC_STS_ERR (1 << 2)
+#define IIC_STS_MDBF(1 << 4)
#define IIC_STS_MDBS(1 << 5)
#define IIC_EXTSTS_XFRA (1 << 0)
+#define IIC_INTRMSK_EIMTC (1 << 0)
+#define IIC_INTRMSK_EITA(1 << 1)
+#define IIC_INTRMSK_EIIC(1 << 2)
+#define IIC_INTRMSK_EIHE(1 << 3)
+
#define IIC_XTCNTLSS_SRST (1 << 0)
#define IIC_DIRECTCNTL_SDAC (1 << 3)
@@ -56,21 +69,13 @@ static void ppc4xx_i2c_reset(DeviceState *s)
{
PPC4xxI2CState *i2c = PPC4xx_I2C(s);
-/* FIXME: Should also reset bus?
- *if (s->address != ADDR_RESET) {
- *i2c_end_transfer(s->bus);
- *}
- */
-
-i2c->mdata = 0;
-i2c->lmadr = 0;
-i2c->hmadr = 0;
+i2c->mdidx = -1;
+memset(i2c->mdata, 0, ARRAY_SIZE(i2c->mdata));
+/* [hl][ms]addr are not affected by reset */
i2c->cntl = 0;
i2c->mdcntl = 0;
i2c->sts = 0;
-i2c->extsts = 0x8f;
-i2c->lsadr = 0;
-i2c->hsadr = 0;
+i2c->extsts = (1 << 6);
i2c->clkdiv = 0;
i2c->intrmsk = 0;
i2c->xfrcnt = 0;
@@ -78,69 +83,29 @@ static void ppc4xx_i2c_reset(DeviceState *s)
i2c->directcntl = 0xf;
}
-static inline bool ppc4xx_i2c_is_master(PPC4xxI2CState *i2c)
-{
-return true;
-}
-
static uint64_t ppc4xx_i2c_readb(void *opaque, hwaddr addr, unsigned int size)
{
PPC4xxI2CState *i2c = PPC4xx_I2C(opaque);
uint64_t ret;
+int i;
switch (addr) {
case 0:
-ret = i2c->mdata;
-if (ppc4xx_i2c_is_master(i2c)) {
+if (i2c->mdidx < 0) {
ret = 0xff;
-
-if (!(i2c->sts & IIC_STS_MDBS)) {
-qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Trying to read "
- "without starting transfer\n",
- TYPE_PPC4xx_I2C, __func__);
-} else {
-int pending = (i2c->cntl >> 4) & 3;
-
-/* get the next byte */
-int byte = i2c_recv(i2c->bus);
-
-if (byte < 0) {
-qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: read failed "
- "for device 0x%02x\n", TYPE_PPC4xx_I2C,
- __func__, i2c->lmadr);
-ret = 0xff;
-} else {
-ret = byte;
-/* Raise interrupt if enabled */
-/*ppc4xx_i2c_raise_interrupt(i2c)*/;
-}
-
-if (!pending) {
-i2c->sts &= ~IIC_STS_MDBS;
-/*i2c_end_transfer(i2c->bus);*/
-/*} else if (i2c->cntl & (IIC_CNTL_RPST | IIC_CNTL_CHT)) {*/
-} else if (pending) {
-/* current smbus implementation doesn't like
- multibyte xfer repeated start */
-i2c_end_transfer(i2c->bus);
-if (i2c_start_transfer(i2c->bus, i2c->lmadr >> 1, 1)) {
-/* if non zero is returned, the adress is not valid */
-i2c->sts &= ~IIC_STS_PT;
-i2c->sts |= IIC_STS_ERR;
-i2c->extsts |= IIC_EXTSTS_XFRA;
-} else {
-/*i2c->sts |= IIC_STS_PT;*/
-i2c->sts |= IIC_STS_MDBS;
-i2c->sts &= ~IIC_STS_ERR;
-i2c->extsts = 0;
-}
-}
-pending--;
-i2c->cntl = (i2c->cntl & 0xcf) | (pending << 4);
-}
-} else {
-qemu_log_mask(LOG_UNIMP, "[%s]%s: slave mode not implemented\n",
- TYPE_PPC4xx_I2C, __func__);
+break;
+}
+ret = i2c->mdata[0];
+if (i2c->mdidx == 3) {
+i2c->sts &= ~IIC_STS_MDBF;
+} else if (i2c->mdidx == 0) {
+i2c->sts &= ~IIC_STS_MDBS;
+}
+for (i = 0; i < i2c->mdidx;
[Qemu-devel] [PATCH v3 8/9] sm501: Perform a full update after palette change
From: Sebastian Bauer
Signed-off-by: Sebastian Bauer
Signed-off-by: BALATON Zoltan
---
hw/display/sm501.c | 8
1 file changed, 8 insertions(+)
diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index 0625cf5..a2ee6e3 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -479,6 +479,7 @@ typedef struct SM501State {
MemoryRegion twoD_engine_region;
uint32_t last_width;
uint32_t last_height;
+uint32_t do_full_update; /* perform a full update next time */
I2CBus *i2c_bus;
/* mmio registers */
@@ -1032,6 +1033,7 @@ static void sm501_palette_write(void *opaque, hwaddr addr,
assert(range_covers_byte(0, 0x400 * 3, addr));
*(uint32_t *)>dc_palette[addr] = value;
+s->do_full_update = 1;
}
static uint64_t sm501_disp_ctrl_read(void *opaque, hwaddr addr,
@@ -1620,6 +1622,12 @@ static void sm501_update_display(void *opaque)
full_update = 1;
}
+/* someone else requested a full update */
+if (s->do_full_update) {
+s->do_full_update = 0;
+full_update = 1;
+}
+
/* draw each line according to conditions */
snap = memory_region_snapshot_and_clear_dirty(>local_mem_region,
offset, width * height * src_bpp, DIRTY_MEMORY_VGA);
--
2.7.6
[Qemu-devel] [PATCH v3 1/9] ppc4xx_i2c: Remove unimplemented sdata and intr registers
Signed-off-by: BALATON Zoltan
---
hw/i2c/ppc4xx_i2c.c | 16 +---
include/hw/i2c/ppc4xx_i2c.h | 4 +---
2 files changed, 2 insertions(+), 18 deletions(-)
diff --git a/hw/i2c/ppc4xx_i2c.c b/hw/i2c/ppc4xx_i2c.c
index d1936db..4e0aaae 100644
--- a/hw/i2c/ppc4xx_i2c.c
+++ b/hw/i2c/ppc4xx_i2c.c
@@ -3,7 +3,7 @@
*
* Copyright (c) 2007 Jocelyn Mayer
* Copyright (c) 2012 François Revol
- * Copyright (c) 2016 BALATON Zoltan
+ * Copyright (c) 2016-2018 BALATON Zoltan
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to
deal
@@ -63,7 +63,6 @@ static void ppc4xx_i2c_reset(DeviceState *s)
i2c->mdcntl = 0;
i2c->sts = 0;
i2c->extsts = 0x8f;
-i2c->sdata = 0;
i2c->lsadr = 0;
i2c->hsadr = 0;
i2c->clkdiv = 0;
@@ -71,7 +70,6 @@ static void ppc4xx_i2c_reset(DeviceState *s)
i2c->xfrcnt = 0;
i2c->xtcntlss = 0;
i2c->directcntl = 0xf;
-i2c->intr = 0;
}
static inline bool ppc4xx_i2c_is_master(PPC4xxI2CState *i2c)
@@ -139,9 +137,6 @@ static uint64_t ppc4xx_i2c_readb(void *opaque, hwaddr addr,
unsigned int size)
TYPE_PPC4xx_I2C, __func__);
}
break;
-case 2:
-ret = i2c->sdata;
-break;
case 4:
ret = i2c->lmadr;
break;
@@ -181,9 +176,6 @@ static uint64_t ppc4xx_i2c_readb(void *opaque, hwaddr addr,
unsigned int size)
case 16:
ret = i2c->directcntl;
break;
-case 17:
-ret = i2c->intr;
-break;
default:
if (addr < PPC4xx_I2C_MEM_SIZE) {
qemu_log_mask(LOG_UNIMP, "%s: Unimplemented register 0x%"
@@ -229,9 +221,6 @@ static void ppc4xx_i2c_writeb(void *opaque, hwaddr addr,
uint64_t value,
}
}
break;
-case 2:
-i2c->sdata = value;
-break;
case 4:
i2c->lmadr = value;
if (i2c_bus_busy(i2c->bus)) {
@@ -302,9 +291,6 @@ static void ppc4xx_i2c_writeb(void *opaque, hwaddr addr,
uint64_t value,
case 16:
i2c->directcntl = value & 0x7;
break;
-case 17:
-i2c->intr = value;
-break;
default:
if (addr < PPC4xx_I2C_MEM_SIZE) {
qemu_log_mask(LOG_UNIMP, "%s: Unimplemented register 0x%"
diff --git a/include/hw/i2c/ppc4xx_i2c.h b/include/hw/i2c/ppc4xx_i2c.h
index 3c60307..e4b6ded 100644
--- a/include/hw/i2c/ppc4xx_i2c.h
+++ b/include/hw/i2c/ppc4xx_i2c.h
@@ -3,7 +3,7 @@
*
* Copyright (c) 2007 Jocelyn Mayer
* Copyright (c) 2012 François Revol
- * Copyright (c) 2016 BALATON Zoltan
+ * Copyright (c) 2016-2018 BALATON Zoltan
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to
deal
@@ -49,7 +49,6 @@ typedef struct PPC4xxI2CState {
uint8_t mdcntl;
uint8_t sts;
uint8_t extsts;
-uint8_t sdata;
uint8_t lsadr;
uint8_t hsadr;
uint8_t clkdiv;
@@ -57,7 +56,6 @@ typedef struct PPC4xxI2CState {
uint8_t xfrcnt;
uint8_t xtcntlss;
uint8_t directcntl;
-uint8_t intr;
} PPC4xxI2CState;
#endif /* PPC4XX_I2C_H */
--
2.7.6
[Qemu-devel] [PATCH v3 9/9] target/ppc: Add missing opcode for icbt on PPC440
PPC440 has two opcodes for icbt, add the missing one. Signed-off-by: BALATON Zoltan --- target/ppc/translate.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/target/ppc/translate.c b/target/ppc/translate.c index 5fe1ba6..3a215a1 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -6707,6 +6707,8 @@ GEN_HANDLER_E(mbar, 0x1F, 0x16, 0x1a, 0x001FF801, GEN_HANDLER(msync_4xx, 0x1F, 0x16, 0x12, 0x03FFF801, PPC_BOOKE), GEN_HANDLER2_E(icbt_440, "icbt", 0x1F, 0x16, 0x00, 0x03E1, PPC_BOOKE, PPC2_BOOKE206), +GEN_HANDLER2(icbt_440, "icbt", 0x1F, 0x06, 0x08, 0x03E1, + PPC_440_SPEC), GEN_HANDLER(lvsl, 0x1f, 0x06, 0x00, 0x0001, PPC_ALTIVEC), GEN_HANDLER(lvsr, 0x1f, 0x06, 0x01, 0x0001, PPC_ALTIVEC), GEN_HANDLER(mfvscr, 0x04, 0x2, 0x18, 0x001ff800, PPC_ALTIVEC), -- 2.7.6
[Qemu-devel] [PATCH v3 6/9] sm501: Do not clear read only bits when writing registers
When writing registers that have read only bits we have to avoid
changing these bits as they may have non zero values. Make sure we use
the correct masks to mask out read only and reserved bits when
changing registers.
Also remove extra spaces from dram_control and arbitration_control
assignments.
Signed-off-by: BALATON Zoltan
---
v3: Not only preserve read only bits but also allow clearing r/w bits
hw/display/sm501.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index e47be99..ca0840f 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -836,27 +836,30 @@ static void sm501_system_config_write(void *opaque,
hwaddr addr,
switch (addr) {
case SM501_SYSTEM_CONTROL:
-s->system_control = value & 0xE300B8F7;
+s->system_control &= 0x10DB;
+s->system_control |= value & 0xEF00B8F7;
break;
case SM501_MISC_CONTROL:
-s->misc_control = value & 0xFF7FFF20;
+s->misc_control &= 0xEF;
+s->misc_control |= value & 0xFF7FFF10;
break;
case SM501_GPIO31_0_CONTROL:
s->gpio_31_0_control = value;
break;
case SM501_GPIO63_32_CONTROL:
-s->gpio_63_32_control = value;
+s->gpio_63_32_control = value & 0xFF80;
break;
case SM501_DRAM_CONTROL:
s->local_mem_size_index = (value >> 13) & 0x7;
/* TODO : check validity of size change */
-s->dram_control |= value & 0x7FC3;
+s->dram_control &= 0x8000;
+s->dram_control |= value & 0x7FC3;
break;
case SM501_ARBTRTN_CONTROL:
-s->arbitration_control = value & 0x3777;
+s->arbitration_control = value & 0x3777;
break;
case SM501_IRQ_MASK:
-s->irq_mask = value;
+s->irq_mask = value & 0xFFDF3F5F;
break;
case SM501_MISC_TIMING:
s->misc_timing = value & 0xF31F1FFF;
--
2.7.6
[Qemu-devel] [PATCH v3 5/9] sam460ex: Add RTC device
The Sam460ex has an M41T80 serial RTC chip on I2C bus 0 at address 0x68. Signed-off-by: BALATON Zoltan --- hw/ppc/sam460ex.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c index bdc53d2..dc730cc 100644 --- a/hw/ppc/sam460ex.c +++ b/hw/ppc/sam460ex.c @@ -457,6 +457,7 @@ static void sam460ex_init(MachineState *machine) object_property_set_bool(OBJECT(dev), true, "realized", NULL); smbus_eeprom_init(i2c[0]->bus, 8, smbus_eeprom_buf, smbus_eeprom_size); g_free(smbus_eeprom_buf); +i2c_create_slave(i2c[0]->bus, "m41t80", 0x68); dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600800, uic[0][3]); i2c[1] = PPC4xx_I2C(dev); -- 2.7.6
[Qemu-devel] [PATCH] ui: darwin: gtk: Add missing input keymap
In appears the input keymap for osx was forgotten in the commit that
converted the gtk frontend to keycodemapdb. Add it.
Fixes: 2ec78706 ("ui: convert GTK and SDL1 frontends to keycodemapdb")
CC: Daniel P. Berrange
Signed-off-by: Keno Fischer
---
Makefile | 1 +
include/ui/input.h | 3 +++
ui/input-keymap.c | 1 +
3 files changed, 5 insertions(+)
diff --git a/Makefile b/Makefile
index 6a5fff0..1e79914 100644
--- a/Makefile
+++ b/Makefile
@@ -322,6 +322,7 @@ KEYCODEMAP_FILES = \
ui/input-keymap-xorgkbd-to-qcode.c \
ui/input-keymap-xorgxquartz-to-qcode.c \
ui/input-keymap-xorgxwin-to-qcode.c \
+ui/input-keymap-osx-to-qcode.c \
$(NULL)
GENERATED_FILES += $(KEYCODEMAP_FILES)
diff --git a/include/ui/input.h b/include/ui/input.h
index 16395ab..34ebc67 100644
--- a/include/ui/input.h
+++ b/include/ui/input.h
@@ -116,4 +116,7 @@ extern const guint16 qemu_input_map_xorgxquartz_to_qcode[];
extern const guint qemu_input_map_xorgxwin_to_qcode_len;
extern const guint16 qemu_input_map_xorgxwin_to_qcode[];
+extern const guint qemu_input_map_osx_to_qcode_len;
+extern const guint16 qemu_input_map_osx_to_qcode[];
+
#endif /* INPUT_H */
diff --git a/ui/input-keymap.c b/ui/input-keymap.c
index 87cc301..db5ccff 100644
--- a/ui/input-keymap.c
+++ b/ui/input-keymap.c
@@ -21,6 +21,7 @@
#include "ui/input-keymap-xorgkbd-to-qcode.c"
#include "ui/input-keymap-xorgxquartz-to-qcode.c"
#include "ui/input-keymap-xorgxwin-to-qcode.c"
+#include "ui/input-keymap-osx-to-qcode.c"
int qemu_input_linux_to_qcode(unsigned int lnx)
{
--
2.8.1
Re: [Qemu-devel] [PATCH v2 0/2] kvm: x86 CPU power management
On Wed, Jun 13, 2018 at 07:34:53PM -0300, Marcelo Tosatti wrote: > On Tue, Jun 12, 2018 at 09:47:11PM +0300, Michael S. Tsirkin wrote: > > This adds ability to expose host CPU power management capabilities to > > guests. For intel guests, this is sufficient for guest to enable > > low power CPU power management. For AMD guests it isn't sufficient, > > deeper C-states are entered using System-IO. > > > > mwait based power management is tied closely to specifics of CPUID, > > making migration challenging. At this point only the non-migrateable > > -cpu host is supported. > > > > With this patch applied, VM latency is within the noise of > > baremetal for some benchmarks. > > > > perf bench sched pipe results: > > Before: > > 6.452 sec > > After: > > 4.382 sec > > Baremetal: > > 4.136 sec > > > > Michael S. Tsirkin (2): > > kvm: support -realtime cpu-pm=on|off > > i386/cpu: make -cpu host support monitor/mwait > > > > include/sysemu/sysemu.h | 1 + > > target/i386/cpu.h | 9 + > > target/i386/cpu.c | 19 ++- > > target/i386/kvm.c | 30 ++ > > vl.c| 6 ++ > > qemu-options.hx | 9 +++-- > > 6 files changed, 67 insertions(+), 7 deletions(-) > > > > -- > > MST > > Hi Michael, > > 1) Command line option interface > > Why is this not an optional cpu feature such as the other features? > > > -cpu CPU,+mwait > > rather than a separate, architecture independent "-realtime cpu-pm=on|off" > command line option? Because it's not just a guest flag. With guest pm on, one guest can severely affect the latency of others on the same host CPU. > 2) Migration > > Isnt it sufficient to check that both CPUID leafs are the same, > to allow migration ? Not at the moment since linux guests use mwait hints and latency values from a table in intel_idle. If the host and guest models do not match, mwait will get a wrong hint. It will not do the right thing then! You want exactly the same host CPU for it to work. This isn't different from how -host cpu works generally. > 1. Check that the processor supports MONITOR and MWAIT. If > CPUID.01H:ECX.MONITOR[bit 3] = 1, MONITOR and MWAIT are available at > ring 0. > > 2. Query the smallest and largest line size that MONITOR uses. > Use CPUID.05H:EAX.smallest[bits 15:0];EBX.largest[bits15:0]. >
Re: [Qemu-devel] [PATCH v4 3/3] hw/vfio/display: add ramfb support
On Wed, Jun 13, 2018 at 01:50:47PM -0600, Alex Williamson wrote:
> On Wed, 13 Jun 2018 10:41:49 +0200
> Gerd Hoffmann wrote:
>
> > So we have a boot display when using a vgpu as primary display.
> >
> > Use vfio-pci-ramfb instead of vfio-pci to enable it.
>
> Using a different device here seems like it almost guarantees a very
> complicated path to support under libvirt. What necessitates this
> versus a simple ramfb=on option to vfio-pci?
Well, it's simliar to qxl vs. qxl-vga. It's not qxl,vga={on,off} and
libvirt has no problems to deal with that ...
Another more technical reason is (again) hotplug. ramfb needs an fw_cfg
entry for configuration, and fw_cfg entries can't be hotplugged. So
hotplugging vfio-pci with ramfb=on isn't going to fly. So we need a
separate device with hotplug turned off.
> I'm also not sure I understand the usage model, SeaBIOS and OVMF know
> how to write to this display, but it seems that the guest does not.
Yes.
> I suppose in the UEFI case runtime services can be used to continue
> writing this display,
Yes.
> but BIOS doesn't have such an option, unless we're somehow emulating
> VGA here.
vgabios support is in the pipeline, including text mode emulation (at
vgabios level, direct access to vga window @ 0xa doesn't work).
> So for UEFI, I can imagine this
> covers us from power on through firmware boot and up to guest drivers
> initializing the GPU (assuming the vGPU supports a kernel mode driver,
> does NVIDIA?),
Yes. Shouldn't matter whenever the driver is kernel or userspace.
> but for BIOS it seems we likely still have a break from
> the bootloader to GPU driver initialization.
Depends. vgacon (text mode console) doesn't work. fbcon @ vesafb works.
> For instance, what driver
> is used to draw the boot animation (or blue screen) on SeaBIOS Windows
> VM?
Windows depends on vgabios for that and it works fine.
> I'm assuming that this display and the vGPU display are one in the
> same, so there's some cut from one to the other.
Yes. If the vfio query plane ioctl reports a valid guest video mode
configuration the vgpu display will be used, ramfb otherwise.
cheers,
Gerd
Re: [Qemu-devel] [PATCH v2 0/2] kvm: x86 CPU power management
On Tue, Jun 12, 2018 at 09:47:11PM +0300, Michael S. Tsirkin wrote: > This adds ability to expose host CPU power management capabilities to > guests. For intel guests, this is sufficient for guest to enable > low power CPU power management. For AMD guests it isn't sufficient, > deeper C-states are entered using System-IO. > > mwait based power management is tied closely to specifics of CPUID, > making migration challenging. At this point only the non-migrateable > -cpu host is supported. > > With this patch applied, VM latency is within the noise of > baremetal for some benchmarks. > > perf bench sched pipe results: > Before: > 6.452 sec > After: > 4.382 sec > Baremetal: > 4.136 sec > > Michael S. Tsirkin (2): > kvm: support -realtime cpu-pm=on|off > i386/cpu: make -cpu host support monitor/mwait > > include/sysemu/sysemu.h | 1 + > target/i386/cpu.h | 9 + > target/i386/cpu.c | 19 ++- > target/i386/kvm.c | 30 ++ > vl.c| 6 ++ > qemu-options.hx | 9 +++-- > 6 files changed, 67 insertions(+), 7 deletions(-) > > -- > MST Hi Michael, 1) Command line option interface Why is this not an optional cpu feature such as the other features? -cpu CPU,+mwait rather than a separate, architecture independent "-realtime cpu-pm=on|off" command line option? 2) Migration Isnt it sufficient to check that both CPUID leafs are the same, to allow migration ? 1. Check that the processor supports MONITOR and MWAIT. If CPUID.01H:ECX.MONITOR[bit 3] = 1, MONITOR and MWAIT are available at ring 0. 2. Query the smallest and largest line size that MONITOR uses. Use CPUID.05H:EAX.smallest[bits 15:0];EBX.largest[bits15:0].
Re: [Qemu-devel] [PULL 0/3] Miscellaneous patches for 2018-06-13
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20180613131045.17379-1-arm...@redhat.com
Subject: [Qemu-devel] [PULL 0/3] Miscellaneous patches for 2018-06-13
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
5cb103bc21 Purge uses of banned g_assert_FOO()
3aa2ecc4d4 coverity-model: replay data is considered trusted
572b561a1a Revert "Makefile: add target to print generated files"
=== OUTPUT BEGIN ===
Checking PATCH 1/3: Revert "Makefile: add target to print generated files"...
Checking PATCH 2/3: coverity-model: replay data is considered trusted...
ERROR: suspect code indent for conditional statements (5, 9)
#28: FILE: scripts/coverity-model.c:110:
+ if (replay_file) {
+ uint8_t c;
total: 1 errors, 0 warnings, 18 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 3/3: Purge uses of banned g_assert_FOO()...
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH v4 04/14] pc: prepare for multi stage hotplug handlers
On Wed, Jun 13, 2018 at 09:37:54PM +0200, David Hildenbrand wrote:
> >>> [ ... specific to machine_foo wiring ...]
> >>>
> >>> virtio_mem_plug() {
> >>> [... some machine specific wiring ...]
> >>>
> >>> bus_hotplug_ctrl = qdev_get_bus_hotplug_handler()
> >>> bus_hotplug_ctrl->plug(bus_hotplug_ctrl, dev)
> >>>
> >>> [... some more machine specific wiring ...]
> >>> }
> >>>
> >>> [ ... specific to machine_foo wiring ...]
> >>>
> >>> i.e. device itself doesn't participate in attaching to external entities,
> >>> those entities (machine or bus controller virtio device is attached to)
> >>> do wiring on their own within their own domain.
> >>
> >> I am fine with this, but Michael asked if this ("virtio_mem_plug()")
> >> could go via new DeviceClass functions. Michael, would that also work
> >> for your use case?
> >
> > It's not virtio specifically, I'm interested in how this will work for
> > PCI generally. Right now we call do_pci_register_device which
> > immediately makes it guest visible.
>
> So you're telling me that a PCI device exposes itself to the system in
> pci_qdev_realize() instead of letting a hotplug handler handle that? My
> assumption is that the PCI bridge hotplug handler handles this.
Well given how things work in qemu that's not exactly
the case. See below.
> What am
> I missing?
>
> I can see that e.g. for a virtio device the realize call chain is
>
> pci_qdev_realize() -> virtio_pci_realize() -> virtio_XXX__pci_realize ->
> virtio_XXX_realize()
>
> If any realization in pci_qdev_realize() fails, we do a
> do_pci_unregister_device().
>
> So if it is true what you're saying than we're already exposing
> partially realized (and possibly unrealized again) devices via PCI. I
> *guess* because we're holding the iothread mutex this is okay and
> actually not visible.
For now but we need ability to have separate new commands for
realize and plug, so we will drop the mutex.
> And we only seem to be sending events in the PCI
> bridge hotplug handlers, so my assumption is that this is fine.
For core PCI, it's mostly just this line:
bus->devices[devfn] = pci_dev;
which makes it accessible to pci config cycles.
But failover also cares about vfio, which seems to set up
e.g. irqfs on realize.
> >
> >
> >>
> >> --
> >>
> >> Thanks,
> >>
> >> David / dhildenb
>
>
> --
>
> Thanks,
>
> David / dhildenb
Re: [Qemu-devel] [PATCH v2 1/2] kvm: support -realtime cpu-pm=on|off
On Wed, Jun 13, 2018 at 06:00:22PM -0300, Eduardo Habkost wrote:
> On Tue, Jun 12, 2018 at 09:47:11PM +0300, Michael S. Tsirkin wrote:
> > With this flag, kvm allows guest to control host CPU power state. This
> > increases latency for other processes using same host CPU in an
> > unpredictable way, but if decreases idle entry/exit times for the
> > running VCPU.
> >
> > Follow-up patches will expose this capability to guest
> > (using mwait leaf).
> >
> > Based on a patch by Wanpeng Li .
> >
> > Signed-off-by: Michael S. Tsirkin
> > ---
> > include/sysemu/sysemu.h | 1 +
> > target/i386/kvm.c | 22 ++
> > vl.c| 6 ++
> > qemu-options.hx | 9 +++--
> > 4 files changed, 36 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > index e893f72f3b..b921c6f3b7 100644
> > --- a/include/sysemu/sysemu.h
> > +++ b/include/sysemu/sysemu.h
> > @@ -128,6 +128,7 @@ extern bool boot_strict;
> > extern uint8_t *boot_splash_filedata;
> > extern size_t boot_splash_filedata_size;
> > extern bool enable_mlock;
> > +extern bool enable_cpu_pm;
>
> After looking at patch 2/2, I see that the global variable is
> useful, and it's consistent with the existing enable_mlock
> variable.
>
> > extern uint8_t qemu_extra_params_fw[2];
> > extern QEMUClockType rtc_clock;
> > extern const char *mem_path;
> > diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> > index 44f70733e7..f093d55209 100644
> > --- a/target/i386/kvm.c
> > +++ b/target/i386/kvm.c
> > @@ -1357,6 +1357,28 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
> > smram_machine_done.notify = register_smram_listener;
> > qemu_add_machine_init_done_notifier(_machine_done);
> > }
> > +
> > +if (enable_cpu_pm) {
> > +int disable_exits = kvm_check_extension(s,
> > KVM_CAP_X86_DISABLE_EXITS);
> > +int ret;
> > +
> > +/* Work around for kernel header with a typo. TODO: fix header and drop. */
> > +#if defined(KVM_X86_DISABLE_EXITS_HTL) &&
> > !defined(KVM_X86_DISABLE_EXITS_HLT)
> > +#define KVM_X86_DISABLE_EXITS_HLT KVM_X86_DISABLE_EXITS_HTL
> > +#endif
> > +if (disable_exits) {
> > +disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> > + KVM_X86_DISABLE_EXITS_HLT |
> > + KVM_X86_DISABLE_EXITS_PAUSE);
> > +}
> > +
> > +ret = kvm_vm_enable_cap(s, KVM_CAP_X86_DISABLE_EXITS, 0,
> > +disable_exits);
>
> Isn't the kvm_vm_enable_cap() call supposed to be inside the "if
> (disable_exits)" block?
Doing it like this causes a warning if pm is requested but
disable halt is not supported.
But I can move it, sure - let me know.
> > +if (ret < 0) {
> > +error_report("kvm: guest stopping CPU not supported: %s",
> > strerror(-ret));
> > +}
> > +}
> > +
> > return 0;
> > }
> [...]
>
> --
> Eduardo
Re: [Qemu-devel] [PATCH QEMU] Patches for new AMD CPU bits.
On Fri, Jun 01, 2018 at 11:38:07AM -0400, Konrad Rzeszutek Wilk wrote: > Hi! > > > I was reading the AMD whitepaper on SSBD and noticed that they have added > two new bits in the 8000_0008 CPUID. EBX: > 1) Bit[26] - similar to Intel's SSB_NO not needed anymore. > 2) Bit[24] - use SPEC_CTRL MSR (0x48) instead of VIRT SPEC_CTRL MSR > (0xC001_011f). > > See 124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf > A copy of this document is available at > https://bugzilla.kernel.org/show_bug.cgi?id=199889 > > These two patches along with the kernel ones allow us to expose those > two bits to the guest. Queued on x86-next, thanks! -- Eduardo
Re: [Qemu-devel] [PATCH v1 06/11] pc-dimm: don't allow to access "size" before the device was realized
On Wed, Jun 13, 2018 at 04:03:35PM +0200, David Hildenbrand wrote: > On 13.06.2018 14:56, Igor Mammedov wrote: > > On Mon, 11 Jun 2018 14:16:50 +0200 > > David Hildenbrand wrote: > > > >> "size" should not be queried before the device was realized. Let' make > >> that explicit. > >> > >> Signed-off-by: David Hildenbrand > > It's generic property getter, it should work even before realize is called. > > That's the point, as we can see in NVDIMM code , it is *not* a generic > property getter. If it were a writeable property, I would agree with Igor because it would make it impossible to find out what's the default value for the property. As it's a read-only property, returning an error seems harmless. Igor, why exactly do you think it's a problem? -- Eduardo
Re: [Qemu-devel] [PATCH v2 2/2] i386/cpu: make -cpu host support monitor/mwait
On Tue, Jun 12, 2018 at 09:47:13PM +0300, Michael S. Tsirkin wrote: > When guest CPU PM is enabled, and with -cpu host, expose the host CPU > MWAIT leaf in the CPUID so guest can make good PM decisions. > > Note: the result is 100% CPU utilization reported by host as host > no longer knows that the CPU is halted. > > Signed-off-by: Michael S. Tsirkin Reviewed-by: Eduardo Habkost -- Eduardo
Re: [Qemu-devel] [PATCH 9/9] hw/arm/virt: Add virt-3.0 machine type
On 06/13/18 10:48, Eric Auger wrote: > PATCH: merge of ECAM and VCPU extension > - Laszlo reviewed the ECAM changes but I dropped his R-b > due to the squash Was there any particular reason why the previous patch set (with only the ECAM enlargement) couldn't be merged first? To be honest I'm not super happy when my R-b is dropped for non-technical reasons; it seems like wasted work for both of us. Obviously if there's a technical dependency or some other reason why committing the ECAM enlargement in separation would be *wrong*, that's different. Even in that case, wouldn't it be possible to keep the initial virt-3.0 machtype addition as I reviewed it, and then add the rest in an incremental patch? Thanks, Laszlo
[Qemu-devel] [Bug 1776760] [NEW] Loading from a saved state results in blue screen due to qxl_dod driver
Public bug reported: Versions: Arch Linux (kernel 4.16.13) Qemu 2.12 libvirt 4.3.0 Windows 10 1803 latest updates installed under libvirt management Spice tools 0.132 QXL DOD driver 0.18 (from redhat server) Steps to reproduce: 1. Boot Windows (xml is attached) 2. Save VM state through libvirt interface 3. Restore VM state through libvirt interface Result: Blue screen. Previously, the result was high CPU usage and a black screen, nonresponsive VM; I could only force shut down or force reset it. The blue screen mentioned the qxl DOD driver as the culprit, the created minidump shows "SYSTEM_THREAD_EXCEPTION_NOT_HANDLED" as error. I can provide the memory.dmp file if it's at all helpful (it's around 250 MB in size). libvirt domain logs for the above actions: 2018-06-13 18:59:49.913+: starting up libvirt version: 4.3.0, qemu version: 2.12.0, hostname: arch-vaio.localdomain LC_ALL=C PATH=/usr/local/sbin:/usr/local/bin:/usr/bin QEMU_AUDIO_DRV=spice /usr/bin/qemu-system-x86_64 -name guest=Windows,debug-threads=on -S -object secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-4-Windows/master-key.aes -machine pc-i440fx-2.7,accel=kvm,usb=off,vmport=off,dump-guest-core=off -cpu Nehalem,vme=on,ss=on,pcid=on,x2apic=on,tsc-deadline=on,hypervisor=on,arat=on,tsc_adjust=on,rdtscp=on,hv_time,hv_relaxed,hv_vapic,hv_spinlocks=0x1fff -m 2048 -realtime mlock=off -smp 4,sockets=1,cores=2,threads=2 -uuid f14684d3-5f81-4743-8512-e516d85ca2c9 -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/domain-4-Windows/monitor.sock,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=localtime,driftfix=slew -global kvm-pit.lost_tick_policy=delay -no-hpet -no-shutdown -global PIIX4_PM.disable_s3=1 -global PIIX4_PM.disable_s4=1 -boot strict=on -device nec-usb-xhci,id=usb,bus=pci.0,addr=0x6 -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x5 -drive file=/mnt/media/Qemu/windows10.qcow2,format=qcow2,if=none,id=drive-virtio-disk0 -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x7,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -drive file=/usr/share/spice-guest-tools/spice-guest-tools.iso,format=raw,if=none,id=drive-ide0-0-1,readonly=on -device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 -netdev user,id=hostnet0 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:44:08:31,bus=pci.0,addr=0x3 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -chardev spicevmc,id=charchannel0,name=vdagent -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=com.redhat.spice.0 -device usb-tablet,id=input2,bus=usb.0,port=3 -spice port=5900,addr=127.0.0.1,disable-ticketing,image-compression=off,seamless-migration=on -device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,vram64_size_mb=0,vgamem_mb=16,max_outputs=1,bus=pci.0,addr=0x2 -device intel-hda,id=sound0,bus=pci.0,addr=0x4 -device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 -chardev spicevmc,id=charredir0,name=usbredir -device usb-redir,chardev=charredir0,id=redir0,bus=usb.0,port=1 -chardev spicevmc,id=charredir1,name=usbredir -device usb-redir,chardev=charredir1,id=redir1,bus=usb.0,port=2 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x8 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny -msg timestamp=on 2018-06-13T18:59:50.018852Z qemu-system-x86_64: -chardev pty,id=charserial0: char device redirected to /dev/pts/6 (label charserial0) main_channel_link: add main channel client inputs_connect: inputs channel client create red_qxl_set_cursor_peer: main_channel_handle_message: agent start main_channel_handle_message: agent start main_channel_handle_message: agent start main_channel_handle_message: agent start main_channel_handle_message: agent start main_channel_handle_message: agent start main_channel_handle_message: agent start 2018-06-13 20:28:19.077+: shutting down, reason=saved 2018-06-13T20:28:19.118226Z qemu-system-x86_64: terminating on signal 15 from pid 457 (/usr/bin/libvirtd) red_channel_client_disconnect: rcc=0x7f7eaa3d8a30 (channel=0x7f7eaa34d300 type=5 id=0) red_channel_client_disconnect: rcc=0x7f7e22bf04b0 (channel=0x7f7eaa34d3c0 type=6 id=0) red_channel_client_disconnect: rcc=0x7f7e22bd89b0 (channel=0x7f7e214599a0 type=9 id=0) red_channel_client_disconnect: rcc=0x7f7eaa3de270 (channel=0x7f7e21459a70 type=9 id=1) 2018-06-13 20:29:04.933+: starting up libvirt version: 4.3.0, qemu version: 2.12.0, hostname: arch-vaio.localdomain LC_ALL=C PATH=/usr/local/sbin:/usr/local/bin:/usr/bin QEMU_AUDIO_DRV=spice /usr/bin/qemu-system-x86_64 -name guest=Windows,debug-threads=on -S -object secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-5-Windows/master-key.aes -machine pc-i440fx-2.7,accel=kvm,usb=off,vmport=off,dump-guest-core=off -cpu
[Qemu-devel] [Bug 1776760] Re: Loading from a saved state results in blue screen due to qxl_dod driver
** Attachment added: "minidump of the bluescreen on resume" https://bugs.launchpad.net/qemu/+bug/1776760/+attachment/5152236/+files/MEMORY.DMP -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1776760 Title: Loading from a saved state results in blue screen due to qxl_dod driver Status in QEMU: New Bug description: Versions: Arch Linux (kernel 4.16.13) Qemu 2.12 libvirt 4.3.0 Windows 10 1803 latest updates installed under libvirt management Spice tools 0.132 QXL DOD driver 0.18 (from redhat server) Steps to reproduce: 1. Boot Windows (xml is attached) 2. Save VM state through libvirt interface 3. Restore VM state through libvirt interface Result: Blue screen. Previously, the result was high CPU usage and a black screen, nonresponsive VM; I could only force shut down or force reset it. The blue screen mentioned the qxl DOD driver as the culprit, the created minidump shows "SYSTEM_THREAD_EXCEPTION_NOT_HANDLED" as error. I can provide the memory.dmp file if it's at all helpful (it's around 250 MB in size). libvirt domain logs for the above actions: 2018-06-13 18:59:49.913+: starting up libvirt version: 4.3.0, qemu version: 2.12.0, hostname: arch-vaio.localdomain LC_ALL=C PATH=/usr/local/sbin:/usr/local/bin:/usr/bin QEMU_AUDIO_DRV=spice /usr/bin/qemu-system-x86_64 -name guest=Windows,debug-threads=on -S -object secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-4-Windows/master-key.aes -machine pc-i440fx-2.7,accel=kvm,usb=off,vmport=off,dump-guest-core=off -cpu Nehalem,vme=on,ss=on,pcid=on,x2apic=on,tsc-deadline=on,hypervisor=on,arat=on,tsc_adjust=on,rdtscp=on,hv_time,hv_relaxed,hv_vapic,hv_spinlocks=0x1fff -m 2048 -realtime mlock=off -smp 4,sockets=1,cores=2,threads=2 -uuid f14684d3-5f81-4743-8512-e516d85ca2c9 -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/domain-4-Windows/monitor.sock,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=localtime,driftfix=slew -global kvm-pit.lost_tick_policy=delay -no-hpet -no-shutdown -global PIIX4_PM.disable_s3=1 -global PIIX4_PM.disable_s4=1 -boot strict=on -device nec-usb-xhci,id=usb,bus=pci.0,addr=0x6 -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x5 -drive file=/mnt/media/Qemu/windows10.qcow2,format=qcow2,if=none,id=drive-virtio-disk0 -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x7,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -drive file=/usr/share/spice-guest-tools/spice-guest-tools.iso,format=raw,if=none,id=drive-ide0-0-1,readonly=on -device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 -netdev user,id=hostnet0 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:44:08:31,bus=pci.0,addr=0x3 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -chardev spicevmc,id=charchannel0,name=vdagent -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=com.redhat.spice.0 -device usb-tablet,id=input2,bus=usb.0,port=3 -spice port=5900,addr=127.0.0.1,disable-ticketing,image-compression=off,seamless-migration=on -device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,vram64_size_mb=0,vgamem_mb=16,max_outputs=1,bus=pci.0,addr=0x2 -device intel-hda,id=sound0,bus=pci.0,addr=0x4 -device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 -chardev spicevmc,id=charredir0,name=usbredir -device usb-redir,chardev=charredir0,id=redir0,bus=usb.0,port=1 -chardev spicevmc,id=charredir1,name=usbredir -device usb-redir,chardev=charredir1,id=redir1,bus=usb.0,port=2 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x8 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny -msg timestamp=on 2018-06-13T18:59:50.018852Z qemu-system-x86_64: -chardev pty,id=charserial0: char device redirected to /dev/pts/6 (label charserial0) main_channel_link: add main channel client inputs_connect: inputs channel client create red_qxl_set_cursor_peer: main_channel_handle_message: agent start main_channel_handle_message: agent start main_channel_handle_message: agent start main_channel_handle_message: agent start main_channel_handle_message: agent start main_channel_handle_message: agent start main_channel_handle_message: agent start 2018-06-13 20:28:19.077+: shutting down, reason=saved 2018-06-13T20:28:19.118226Z qemu-system-x86_64: terminating on signal 15 from pid 457 (/usr/bin/libvirtd) red_channel_client_disconnect: rcc=0x7f7eaa3d8a30 (channel=0x7f7eaa34d300 type=5 id=0) red_channel_client_disconnect: rcc=0x7f7e22bf04b0 (channel=0x7f7eaa34d3c0 type=6 id=0) red_channel_client_disconnect: rcc=0x7f7e22bd89b0 (channel=0x7f7e214599a0 type=9 id=0) red_channel_client_disconnect: rcc=0x7f7eaa3de270 (channel=0x7f7e21459a70 type=9 id=1) 2018-06-13 20:29:04.933+: starting up
Re: [Qemu-devel] [PATCH v2 1/2] kvm: support -realtime cpu-pm=on|off
On Tue, Jun 12, 2018 at 09:47:11PM +0300, Michael S. Tsirkin wrote:
> With this flag, kvm allows guest to control host CPU power state. This
> increases latency for other processes using same host CPU in an
> unpredictable way, but if decreases idle entry/exit times for the
> running VCPU.
>
> Follow-up patches will expose this capability to guest
> (using mwait leaf).
>
> Based on a patch by Wanpeng Li .
>
> Signed-off-by: Michael S. Tsirkin
> ---
> include/sysemu/sysemu.h | 1 +
> target/i386/kvm.c | 22 ++
> vl.c| 6 ++
> qemu-options.hx | 9 +++--
> 4 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index e893f72f3b..b921c6f3b7 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -128,6 +128,7 @@ extern bool boot_strict;
> extern uint8_t *boot_splash_filedata;
> extern size_t boot_splash_filedata_size;
> extern bool enable_mlock;
> +extern bool enable_cpu_pm;
After looking at patch 2/2, I see that the global variable is
useful, and it's consistent with the existing enable_mlock
variable.
> extern uint8_t qemu_extra_params_fw[2];
> extern QEMUClockType rtc_clock;
> extern const char *mem_path;
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 44f70733e7..f093d55209 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -1357,6 +1357,28 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
> smram_machine_done.notify = register_smram_listener;
> qemu_add_machine_init_done_notifier(_machine_done);
> }
> +
> +if (enable_cpu_pm) {
> +int disable_exits = kvm_check_extension(s,
> KVM_CAP_X86_DISABLE_EXITS);
> +int ret;
> +
> +/* Work around for kernel header with a typo. TODO: fix header and drop. */
> +#if defined(KVM_X86_DISABLE_EXITS_HTL) && !defined(KVM_X86_DISABLE_EXITS_HLT)
> +#define KVM_X86_DISABLE_EXITS_HLT KVM_X86_DISABLE_EXITS_HTL
> +#endif
> +if (disable_exits) {
> +disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> + KVM_X86_DISABLE_EXITS_HLT |
> + KVM_X86_DISABLE_EXITS_PAUSE);
> +}
> +
> +ret = kvm_vm_enable_cap(s, KVM_CAP_X86_DISABLE_EXITS, 0,
> +disable_exits);
Isn't the kvm_vm_enable_cap() call supposed to be inside the "if
(disable_exits)" block?
> +if (ret < 0) {
> +error_report("kvm: guest stopping CPU not supported: %s",
> strerror(-ret));
> +}
> +}
> +
> return 0;
> }
[...]
--
Eduardo
[Qemu-devel] [Bug 1769189] Re: Issue with qemu 2.12.0 + SATA
Oughtta be fixed in current master, will be fixed in 2.12.1 and 3.0. ** Changed in: qemu Status: Confirmed => Fix Committed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1769189 Title: Issue with qemu 2.12.0 + SATA Status in QEMU: Fix Committed Bug description: [EDIT: I first thought that OVMF was the issue, but it turns out to be SATA] I had a Windows 10 VM running perfectly fine with a SATA drive, since I upgraded to qemu 2.12, the guests hangs for a couple of minutes, works for a few seconds, and hangs again, etc. By "hang" I mean it doesn't freeze, but it looks like it's waiting on IO or something, I can move the mouse but everything needing disk access is unresponsive. What doesn't work: qemu 2.12 with SATA What works: using VirIO-SCSI with qemu 2.12 or downgrading qemu to 2.11.1 and keep using SATA. Platform is arch linux 4.16.7 on skylake and Haswell, I have attached the vm xml file. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1769189/+subscriptions
[Qemu-devel] [PATCH v2 1/2] crypto/virtio-crypto: Read crypto services and algorithm masks
Read the crypto services and algorithm masks which provides
information about the services and algorithms supported by
virtio-crypto backend.
Signed-off-by: Farhan Ali
Acked-by: Gonglei
---
drivers/crypto/virtio/virtio_crypto_common.h | 14 ++
drivers/crypto/virtio/virtio_crypto_core.c | 29
2 files changed, 43 insertions(+)
diff --git a/drivers/crypto/virtio/virtio_crypto_common.h
b/drivers/crypto/virtio/virtio_crypto_common.h
index 66501a5..931a3bd 100644
--- a/drivers/crypto/virtio/virtio_crypto_common.h
+++ b/drivers/crypto/virtio/virtio_crypto_common.h
@@ -55,6 +55,20 @@ struct virtio_crypto {
/* Number of queue currently used by the driver */
u32 curr_queue;
+ /*
+* Specifies the services mask which the device support,
+* see VIRTIO_CRYPTO_SERVICE_*
+*/
+ u32 crypto_services;
+
+ /* Detailed algorithms mask */
+ u32 cipher_algo_l;
+ u32 cipher_algo_h;
+ u32 hash_algo;
+ u32 mac_algo_l;
+ u32 mac_algo_h;
+ u32 aead_algo;
+
/* Maximum length of cipher key */
u32 max_cipher_key_len;
/* Maximum length of authenticated key */
diff --git a/drivers/crypto/virtio/virtio_crypto_core.c
b/drivers/crypto/virtio/virtio_crypto_core.c
index 8332698..8f745f2 100644
--- a/drivers/crypto/virtio/virtio_crypto_core.c
+++ b/drivers/crypto/virtio/virtio_crypto_core.c
@@ -303,6 +303,13 @@ static int virtcrypto_probe(struct virtio_device *vdev)
u32 max_data_queues = 0, max_cipher_key_len = 0;
u32 max_auth_key_len = 0;
u64 max_size = 0;
+ u32 cipher_algo_l = 0;
+ u32 cipher_algo_h = 0;
+ u32 hash_algo = 0;
+ u32 mac_algo_l = 0;
+ u32 mac_algo_h = 0;
+ u32 aead_algo = 0;
+ u32 crypto_services = 0;
if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
return -ENODEV;
@@ -339,6 +346,20 @@ static int virtcrypto_probe(struct virtio_device *vdev)
max_auth_key_len, _auth_key_len);
virtio_cread(vdev, struct virtio_crypto_config,
max_size, _size);
+ virtio_cread(vdev, struct virtio_crypto_config,
+ crypto_services, _services);
+ virtio_cread(vdev, struct virtio_crypto_config,
+ cipher_algo_l, _algo_l);
+ virtio_cread(vdev, struct virtio_crypto_config,
+ cipher_algo_h, _algo_h);
+ virtio_cread(vdev, struct virtio_crypto_config,
+ hash_algo, _algo);
+ virtio_cread(vdev, struct virtio_crypto_config,
+ mac_algo_l, _algo_l);
+ virtio_cread(vdev, struct virtio_crypto_config,
+ mac_algo_h, _algo_h);
+ virtio_cread(vdev, struct virtio_crypto_config,
+ aead_algo, _algo);
/* Add virtio crypto device to global table */
err = virtcrypto_devmgr_add_dev(vcrypto);
@@ -358,6 +379,14 @@ static int virtcrypto_probe(struct virtio_device *vdev)
vcrypto->max_cipher_key_len = max_cipher_key_len;
vcrypto->max_auth_key_len = max_auth_key_len;
vcrypto->max_size = max_size;
+ vcrypto->crypto_services = crypto_services;
+ vcrypto->cipher_algo_l = cipher_algo_l;
+ vcrypto->cipher_algo_h = cipher_algo_h;
+ vcrypto->mac_algo_l = mac_algo_l;
+ vcrypto->mac_algo_h = mac_algo_h;
+ vcrypto->hash_algo = hash_algo;
+ vcrypto->aead_algo = aead_algo;
+
dev_info(>dev,
"max_queues: %u, max_cipher_key_len: %u, max_auth_key_len: %u,
max_size 0x%llx\n",
--
2.7.4
[Qemu-devel] [PATCH v2 0/2] Detect & register virtio-crypto algos only if it can be supported by backend
Hi, Currently the Linux virtio-crypto driver registers the crypto algorithm without verifying if the backend actually supports the algorithm. This kernel patch series adds support for registering algorithm with Linux crypto layer, only if the algorithm is supported by the backend device. This also makes the driver more compliant with the virtio-crypto spec [1]. I would appreciate any feedback or comments on this. Thank you Farhan Reference - [1] Virtio crypto spec proposal https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg00816.html ChangeLog - v1 -> v2 - Modify comment as suggested by Arei (patch 1) - Modify error message as suggested by Arei (patch 2) Farhan Ali (2): crypto/virtio-crypto: Read crypto services and algorithm masks crypto/virtio-crypto: Register an algo only if it's supported drivers/crypto/virtio/virtio_crypto_algs.c | 112 ++- drivers/crypto/virtio/virtio_crypto_common.h | 25 +- drivers/crypto/virtio/virtio_crypto_core.c | 29 +++ drivers/crypto/virtio/virtio_crypto_mgr.c| 81 +-- 4 files changed, 202 insertions(+), 45 deletions(-) -- 2.7.4
[Qemu-devel] [PATCH v2 2/2] crypto/virtio-crypto: Register an algo only if it's supported
From: Farhan Ali
Register a crypto algo with the Linux crypto layer only if
the algorithm is supported by the backend virtio-crypto
device.
Also route crypto requests to a virtio-crypto
device, only if it can support the requested service and
algorithm.
Signed-off-by: Farhan Ali
Acked-by: Gonglei
---
drivers/crypto/virtio/virtio_crypto_algs.c | 112 ++-
drivers/crypto/virtio/virtio_crypto_common.h | 11 ++-
drivers/crypto/virtio/virtio_crypto_mgr.c| 81 +--
3 files changed, 159 insertions(+), 45 deletions(-)
diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c
b/drivers/crypto/virtio/virtio_crypto_algs.c
index ba190cf..11db62f 100644
--- a/drivers/crypto/virtio/virtio_crypto_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_algs.c
@@ -49,12 +49,18 @@ struct virtio_crypto_sym_request {
bool encrypt;
};
+struct virtio_crypto_algo {
+ uint32_t algonum;
+ uint32_t service;
+ unsigned int active_devs;
+ struct crypto_alg algo;
+};
+
/*
* The algs_lock protects the below global virtio_crypto_active_devs
* and crypto algorithms registion.
*/
static DEFINE_MUTEX(algs_lock);
-static unsigned int virtio_crypto_active_devs;
static void virtio_crypto_ablkcipher_finalize_req(
struct virtio_crypto_sym_request *vc_sym_req,
struct ablkcipher_request *req,
@@ -312,15 +318,21 @@ static int virtio_crypto_ablkcipher_setkey(struct
crypto_ablkcipher *tfm,
unsigned int keylen)
{
struct virtio_crypto_ablkcipher_ctx *ctx = crypto_ablkcipher_ctx(tfm);
+ uint32_t alg;
int ret;
+ ret = virtio_crypto_alg_validate_key(keylen, );
+ if (ret)
+ return ret;
+
if (!ctx->vcrypto) {
/* New key */
int node = virtio_crypto_get_current_node();
struct virtio_crypto *vcrypto =
- virtcrypto_get_dev_node(node);
+ virtcrypto_get_dev_node(node,
+ VIRTIO_CRYPTO_SERVICE_CIPHER, alg);
if (!vcrypto) {
- pr_err("virtio_crypto: Could not find a virtio device
in the system\n");
+ pr_err("virtio_crypto: Could not find a virtio device
in the system or unsupported algo\n");
return -ENODEV;
}
@@ -571,57 +583,85 @@ static void virtio_crypto_ablkcipher_finalize_req(
virtcrypto_clear_request(_sym_req->base);
}
-static struct crypto_alg virtio_crypto_algs[] = { {
- .cra_name = "cbc(aes)",
- .cra_driver_name = "virtio_crypto_aes_cbc",
- .cra_priority = 150,
- .cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER | CRYPTO_ALG_ASYNC,
- .cra_blocksize = AES_BLOCK_SIZE,
- .cra_ctxsize = sizeof(struct virtio_crypto_ablkcipher_ctx),
- .cra_alignmask = 0,
- .cra_module = THIS_MODULE,
- .cra_type = _ablkcipher_type,
- .cra_init = virtio_crypto_ablkcipher_init,
- .cra_exit = virtio_crypto_ablkcipher_exit,
- .cra_u = {
- .ablkcipher = {
- .setkey = virtio_crypto_ablkcipher_setkey,
- .decrypt = virtio_crypto_ablkcipher_decrypt,
- .encrypt = virtio_crypto_ablkcipher_encrypt,
- .min_keysize = AES_MIN_KEY_SIZE,
- .max_keysize = AES_MAX_KEY_SIZE,
- .ivsize = AES_BLOCK_SIZE,
+static struct virtio_crypto_algo virtio_crypto_algs[] = { {
+ .algonum = VIRTIO_CRYPTO_CIPHER_AES_CBC,
+ .service = VIRTIO_CRYPTO_SERVICE_CIPHER,
+ .algo = {
+ .cra_name = "cbc(aes)",
+ .cra_driver_name = "virtio_crypto_aes_cbc",
+ .cra_priority = 150,
+ .cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER | CRYPTO_ALG_ASYNC,
+ .cra_blocksize = AES_BLOCK_SIZE,
+ .cra_ctxsize = sizeof(struct virtio_crypto_ablkcipher_ctx),
+ .cra_alignmask = 0,
+ .cra_module = THIS_MODULE,
+ .cra_type = _ablkcipher_type,
+ .cra_init = virtio_crypto_ablkcipher_init,
+ .cra_exit = virtio_crypto_ablkcipher_exit,
+ .cra_u = {
+ .ablkcipher = {
+ .setkey = virtio_crypto_ablkcipher_setkey,
+ .decrypt = virtio_crypto_ablkcipher_decrypt,
+ .encrypt = virtio_crypto_ablkcipher_encrypt,
+ .min_keysize = AES_MIN_KEY_SIZE,
+ .max_keysize = AES_MAX_KEY_SIZE,
+ .ivsize = AES_BLOCK_SIZE,
+ },
},
},
} };
-int virtio_crypto_algs_register(void)
+int virtio_crypto_algs_register(struct virtio_crypto *vcrypto)
{
int ret = 0;
+ int i = 0;
Re: [Qemu-devel] [PATCH v2 1/2] kvm: support -realtime cpu-pm=on|off
On Tue, Jun 12, 2018 at 09:47:11PM +0300, Michael S. Tsirkin wrote: > With this flag, kvm allows guest to control host CPU power state. This > increases latency for other processes using same host CPU in an > unpredictable way, but if decreases idle entry/exit times for the > running VCPU. > > Follow-up patches will expose this capability to guest > (using mwait leaf). > > Based on a patch by Wanpeng Li . > > Signed-off-by: Michael S. Tsirkin The interface makes sense to me, but: > +extern bool enable_cpu_pm; Why do we need a global variable if initialization code can call qemu_opt_get_bool() directly? -- Eduardo
Re: [Qemu-devel] [RFC v1 1/1] virtio-crypto: Allow disabling of cipher algorithms for virtio-crypto device
Hi Arei
On 06/12/2018 08:57 PM, Gonglei (Arei) wrote:
-Original Message-
From: Farhan Ali [mailto:al...@linux.ibm.com]
Sent: Wednesday, June 13, 2018 3:49 AM
To: qemu-devel@nongnu.org
Cc: m...@redhat.com; Gonglei (Arei) ; longpeng
; pa...@linux.ibm.com; borntrae...@de.ibm.com;
fran...@linux.ibm.com; al...@linux.ibm.com
Subject: [RFC v1 1/1] virtio-crypto: Allow disabling of cipher algorithms for
virtio-crypto device
The virtio-crypto driver currently propagates to the guest
all the cipher algorithms that the backend cryptodev can
support. But in certain cases where the guest has more
performant mechanism to handle some algorithms, it would be
useful to propagate only a subset of the algorithms.
It makes sense to me. E.g. current Intel CPU has the AES-NI instruction for
accelerating
AES algo. We don't need to propagate AES algos.
This patch adds support for disabling the cipher
algorithms of the backend cryptodev.
eg:
-object cryptodev-backend-builtin,id=cryptodev0
-device virtio-crypto-ccw,id=crypto0,cryptodev=cryptodev0,cipher-aes-cbc=off
Signed-off-by: Farhan Ali
---
Please note this patch is not complete, and there are TODOs to handle
for other types of algorithms such Hash, AEAD and MAC algorithms.
This is mainly intended to get some feedback on the design approach
from the community.
hw/virtio/virtio-crypto.c | 46
---
include/hw/virtio/virtio-crypto.h | 3 +++
2 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index 9a9fa49..4aed9ca 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -754,12 +754,22 @@ static void virtio_crypto_reset(VirtIODevice *vdev)
static void virtio_crypto_init_config(VirtIODevice *vdev)
{
VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(vdev);
+uint32_t user_crypto_services = (1u <<
VIRTIO_CRYPTO_SERVICE_CIPHER) |
+(1u <<
VIRTIO_CRYPTO_SERVICE_HASH) |
+(1u <<
VIRTIO_CRYPTO_SERVICE_AEAD) |
+(1u <<
VIRTIO_CRYPTO_SERVICE_MAC);
+
+if (vcrypto->user_cipher_algo_l & (1u << VIRTIO_CRYPTO_NO_CIPHER)) {
+vcrypto->user_cipher_algo_l = 1u << VIRTIO_CRYPTO_NO_CIPHER;
+vcrypto->user_cipher_algo_h = 0;
+user_crypto_services &= ~(1u <<
VIRTIO_CRYPTO_SERVICE_CIPHER);
+}
-vcrypto->conf.crypto_services =
+vcrypto->conf.crypto_services = user_crypto_services &
vcrypto->conf.cryptodev->conf.crypto_services;
-vcrypto->conf.cipher_algo_l =
+vcrypto->conf.cipher_algo_l = vcrypto->user_cipher_algo_l &
vcrypto->conf.cryptodev->conf.cipher_algo_l;
-vcrypto->conf.cipher_algo_h =
+vcrypto->conf.cipher_algo_h = vcrypto->user_cipher_algo_h &
vcrypto->conf.cryptodev->conf.cipher_algo_h;
vcrypto->conf.hash_algo = vcrypto->conf.cryptodev->conf.hash_algo;
vcrypto->conf.mac_algo_l = vcrypto->conf.cryptodev->conf.mac_algo_l;
@@ -853,6 +863,34 @@ static const VMStateDescription
vmstate_virtio_crypto = {
static Property virtio_crypto_properties[] = {
DEFINE_PROP_LINK("cryptodev", VirtIOCrypto, conf.cryptodev,
TYPE_CRYPTODEV_BACKEND, CryptoDevBackend
*),
+DEFINE_PROP_BIT("no-cipher", VirtIOCrypto, user_cipher_algo_l,
+VIRTIO_CRYPTO_CIPHER_ARC4, false),
s/ VIRTIO_CRYPTO_CIPHER_ARC4/VIRTIO_CRYPTO_NO_CIPHER/
Thanks for catching that. I will change.
+DEFINE_PROP_BIT("cipher-arc4", VirtIOCrypto, user_cipher_algo_l,
+VIRTIO_CRYPTO_CIPHER_ARC4, false),
+DEFINE_PROP_BIT("cipher-aes-ecb", VirtIOCrypto, user_cipher_algo_l,
+VIRTIO_CRYPTO_CIPHER_AES_ECB, false),
+DEFINE_PROP_BIT("cipher-aes-cbc", VirtIOCrypto, user_cipher_algo_l,
+VIRTIO_CRYPTO_CIPHER_AES_CBC, false),
+DEFINE_PROP_BIT("cipher-aes-ctr", VirtIOCrypto, user_cipher_algo_l,
+VIRTIO_CRYPTO_CIPHER_AES_CTR, false),
+DEFINE_PROP_BIT("cipher-des-ecb", VirtIOCrypto, user_cipher_algo_l,
+VIRTIO_CRYPTO_CIPHER_DES_ECB, false),
+DEFINE_PROP_BIT("cipher-3des-ecb", VirtIOCrypto, user_cipher_algo_l,
+VIRTIO_CRYPTO_CIPHER_3DES_ECB, false),
+DEFINE_PROP_BIT("cipher-3des-cbc", VirtIOCrypto, user_cipher_algo_l,
+VIRTIO_CRYPTO_CIPHER_3DES_CBC, false),
+DEFINE_PROP_BIT("cipher-3des-ctr", VirtIOCrypto, user_cipher_algo_l,
+VIRTIO_CRYPTO_CIPHER_3DES_CTR, false),
+DEFINE_PROP_BIT("cipher-kasumi-f8", VirtIOCrypto, user_cipher_algo_l,
+VIRTIO_CRYPTO_CIPHER_KASUMI_F8, false),
+DEFINE_PROP_BIT("cipher-snow3g-uea2", VirtIOCrypto,
user_cipher_algo_l,
+VIRTIO_CRYPTO_CIPHER_SNOW3G_UEA2, false),
+DEFINE_PROP_BIT("cipher-aes-f8", VirtIOCrypto, user_cipher_algo_l,
+
Re: [Qemu-devel] [PATCH] configure: Enable out-of-tree acceptance tests
On Tue, Jun 12, 2018 at 02:34:37PM -0300, Philippe Mathieu-Daudé wrote: > Currently to run Avocado acceptance tests in an out-of-tree > build directory, we need to use the full path to the test: > > build_dir$ avocado run > /full/path/to/sources/qemu/tests/acceptance/boot_linux_console.py > > This patch adds a symlink in the build tree to simplify the > tests invocation, allowing the same command than in in-tree builds: > > build_dir$ avocado run tests/acceptance/boot_linux_console.py > > Signed-off-by: Philippe Mathieu-Daudé Queued on python-next, thanks! -- Eduardo
Re: [Qemu-devel] [PATCH V6 0/7] nvdimm: guarantee persistence of QEMU writes to persistent memory
On Tue, Jun 12, 2018 at 01:38:08PM +, Junyan He wrote: > He have pmem_persist and pmem_memcpy_persist stub functions. > > If no libpmem and user really specify pmem=on, we just do nothing or just > memcpy. > > Real persistent memory always require libpmem support its load/save. > > If pmem=on and without libpmem, we can think that user want to imitate > > pmem=on while the HW environment is without real persistent memory existing. > > It may help debug on some machine without real pmem. What exactly it would help debug? Is there any difference between testing the code with pmem=off and pmem=on if libptest is unavailable? I would prefer to simply not let the user set pmem=on if libptest is not available. -- Eduardo
Re: [Qemu-devel] [PATCH v4 11/40] hw/xen: Use the IEC binary prefix definitions
On 06/13/2018 04:31 PM, Richard Henderson wrote:
> On 06/13/2018 02:13 AM, Eric Blake wrote:
>> Or spell it UINT64_C(1) if you don't want a cast.
>
> Not unsigned is what I want most.
I used both of your suggestions, but now new format string errors
appeared due to ram_addr_t being unsigned, so code cleaned using
MachineState->ram_size now complains.
include/exec/cpu-common.h:53:typedef uintptr_t ram_addr_t;
include/hw/boards.h:259:ram_addr_t ram_size;
Is the following snippet OK?
/* allocate RAM */
-if (ram_size > (2048u << 20)) {
-error_report("Too much memory for this machine: %dMB, maximum
2048MB",
- ((unsigned int)ram_size / (1 << 20)));
+if (ram_size > 2 * GiB) {
+error_report("Too much memory for this machine: %luMB, maximum
2048MB",
+ ram_size / MiB);
exit(1);
}
Re: [Qemu-devel] [PATCH v4 04/40] checkpatch: Recognize IEC binary prefix definitions
On 06/10/2018 10:14 PM, Philippe Mathieu-Daudé wrote:
> This fixes:
>
> ERROR: "foo * bar" should be "foo *bar"
> #310: FILE: hw/ppc/ppc440_uc.c:564:
> +size = 8 * MiB * sh;
> total: 1 errors, 0 warnings, 433 lines checked
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> scripts/checkpatch.pl | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index e3d8c2cdfc..4923674c71 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -322,6 +322,7 @@ sub build_types {
> (?:${all}\b)
> )
> (?:\s+$Modifier|\s+const)*
> + (?:[KMGTPE]iB)
> }x;
> $Type = qr{
> $NonptrType
>
Well this patch doesn't work like expected...
$ git diff --cached | \
scripts/checkpatch.pl --no-signoff --debug values=2 -
<_>WS( )
<_>IDENT(size)
<_>WS( )
<_>ASSIGN(=)
<_>WS( )
<_>IDENT(8)
<_>WS( )
<_>OPV(*)
<_>WS( )
<_>DECLARE(MiB * ) <
<_>IDENT(sh)
<_>END(;)
<_>WS(
)
22 > . size = 8 * MiB * sh;
22 > EEVNNVVNNTTVVEE
22 > __B___
Which looks OK, but then (example):
$ git diff --cached
diff --git a/include/net/net.h b/include/net/net.h
-void qemu_del_nic(NICState *nic);
+void qemu_del_nic(NICState *test);
$ git diff --cached | \
scripts/checkpatch.pl --no-signoff --debug values=2 -
void qemu_del_nic(NICState *test);
<_>WS( )
<_>IDENT(void)
<_>WS( )
<_>FUNC(qemu_del_nic)
PAREN('(')
<_>IDENT(NICState)
<_>WS( )
<_>OPV(*)
<_>IDENT(test)
<_>PAREN(')') -> V
<_>END(;)
<_>WS(
)
10 > . void qemu_del_nic(NICState *test);
10 > EEVNVNVEE
10 > B___
ERROR: spaces required around that '*' (ctx:WxV)
#10: FILE: include/net/net.h:136:
+void qemu_del_nic(NICState *test);
^
Re: [Qemu-devel] [PATCH v4 3/3] hw/vfio/display: add ramfb support
On Wed, 13 Jun 2018 10:41:49 +0200
Gerd Hoffmann wrote:
> So we have a boot display when using a vgpu as primary display.
>
> Use vfio-pci-ramfb instead of vfio-pci to enable it.
Using a different device here seems like it almost guarantees a very
complicated path to support under libvirt. What necessitates this
versus a simple ramfb=on option to vfio-pci?
I'm also not sure I understand the usage model, SeaBIOS and OVMF know
how to write to this display, but it seems that the guest does not. I
suppose in the UEFI case runtime services can be used to continue
writing this display, but BIOS doesn't have such an option, unless
we're somehow emulating VGA here. So for UEFI, I can imagine this
covers us from power on through firmware boot and up to guest drivers
initializing the GPU (assuming the vGPU supports a kernel mode driver,
does NVIDIA?), but for BIOS it seems we likely still have a break from
the bootloader to GPU driver initialization. For instance, what driver
is used to draw the boot animation (or blue screen) on SeaBIOS Windows
VM? I'm assuming that this display and the vGPU display are one in the
same, so there's some cut from one to the other. Thanks,
Alex
> Signed-off-by: Gerd Hoffmann
> ---
> include/hw/vfio/vfio-common.h | 2 ++
> hw/vfio/display.c | 10 ++
> hw/vfio/pci.c | 15 +++
> 3 files changed, 27 insertions(+)
>
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index a9036929b2..a58d7e7e77 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -26,6 +26,7 @@
> #include "qemu/queue.h"
> #include "qemu/notify.h"
> #include "ui/console.h"
> +#include "hw/display/ramfb.h"
> #ifdef CONFIG_LINUX
> #include
> #endif
> @@ -143,6 +144,7 @@ typedef struct VFIODMABuf {
>
> typedef struct VFIODisplay {
> QemuConsole *con;
> +RAMFBState *ramfb;
> struct {
> VFIORegion buffer;
> DisplaySurface *surface;
> diff --git a/hw/vfio/display.c b/hw/vfio/display.c
> index 59c0e5d1d7..409d5a2e3a 100644
> --- a/hw/vfio/display.c
> +++ b/hw/vfio/display.c
> @@ -124,6 +124,9 @@ static void vfio_display_dmabuf_update(void *opaque)
>
> primary = vfio_display_get_dmabuf(vdev, DRM_PLANE_TYPE_PRIMARY);
> if (primary == NULL) {
> +if (dpy->ramfb) {
> +ramfb_display_update(dpy->con, dpy->ramfb);
> +}
> return;
> }
>
> @@ -181,6 +184,8 @@ static int vfio_display_dmabuf_init(VFIOPCIDevice *vdev,
> Error **errp)
> vdev->dpy->con = graphic_console_init(DEVICE(vdev), 0,
>_display_dmabuf_ops,
>vdev);
> +if (strcmp(object_get_typename(OBJECT(vdev)), "vfio-pci-ramfb") == 0)
> +vdev->dpy->ramfb = ramfb_setup(errp);
> return 0;
> }
>
> @@ -228,6 +233,9 @@ static void vfio_display_region_update(void *opaque)
> return;
> }
> if (!plane.drm_format || !plane.size) {
> +if (dpy->ramfb) {
> +ramfb_display_update(dpy->con, dpy->ramfb);
> +}
> return;
> }
> format = qemu_drm_format_to_pixman(plane.drm_format);
> @@ -300,6 +308,8 @@ static int vfio_display_region_init(VFIOPCIDevice *vdev,
> Error **errp)
> vdev->dpy->con = graphic_console_init(DEVICE(vdev), 0,
>_display_region_ops,
>vdev);
> +if (strcmp(object_get_typename(OBJECT(vdev)), "vfio-pci-ramfb") == 0)
> +vdev->dpy->ramfb = ramfb_setup(errp);
> return 0;
> }
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 18c493b49e..6a2b42a595 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3234,9 +3234,24 @@ static const TypeInfo vfio_pci_dev_info = {
> },
> };
>
> +static void vfio_pci_ramfb_dev_class_init(ObjectClass *klass, void *data)
> +{
> +DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +dc->hotpluggable = false;
> +}
> +
> +static const TypeInfo vfio_pci_ramfb_dev_info = {
> +.name = "vfio-pci-ramfb",
> +.parent = "vfio-pci",
> +.instance_size = sizeof(VFIOPCIDevice),
> +.class_init = vfio_pci_ramfb_dev_class_init,
> +};
> +
> static void register_vfio_pci_dev_type(void)
> {
> type_register_static(_pci_dev_info);
> +type_register_static(_pci_ramfb_dev_info);
> }
>
> type_init(register_vfio_pci_dev_type)
Re: [Qemu-devel] [PATCH v4 04/14] pc: prepare for multi stage hotplug handlers
>>> [ ... specific to machine_foo wiring ...]
>>>
>>> virtio_mem_plug() {
>>> [... some machine specific wiring ...]
>>>
>>> bus_hotplug_ctrl = qdev_get_bus_hotplug_handler()
>>> bus_hotplug_ctrl->plug(bus_hotplug_ctrl, dev)
>>>
>>> [... some more machine specific wiring ...]
>>> }
>>>
>>> [ ... specific to machine_foo wiring ...]
>>>
>>> i.e. device itself doesn't participate in attaching to external entities,
>>> those entities (machine or bus controller virtio device is attached to)
>>> do wiring on their own within their own domain.
>>
>> I am fine with this, but Michael asked if this ("virtio_mem_plug()")
>> could go via new DeviceClass functions. Michael, would that also work
>> for your use case?
>
> It's not virtio specifically, I'm interested in how this will work for
> PCI generally. Right now we call do_pci_register_device which
> immediately makes it guest visible.
So you're telling me that a PCI device exposes itself to the system in
pci_qdev_realize() instead of letting a hotplug handler handle that? My
assumption is that the PCI bridge hotplug handler handles this. What am
I missing?
I can see that e.g. for a virtio device the realize call chain is
pci_qdev_realize() -> virtio_pci_realize() -> virtio_XXX__pci_realize ->
virtio_XXX_realize()
If any realization in pci_qdev_realize() fails, we do a
do_pci_unregister_device().
So if it is true what you're saying than we're already exposing
partially realized (and possibly unrealized again) devices via PCI. I
*guess* because we're holding the iothread mutex this is okay and
actually not visible. And we only seem to be sending events in the PCI
bridge hotplug handlers, so my assumption is that this is fine.
>
>
>>
>> --
>>
>> Thanks,
>>
>> David / dhildenb
--
Thanks,
David / dhildenb
Re: [Qemu-devel] [PATCH v4 11/40] hw/xen: Use the IEC binary prefix definitions
On 06/13/2018 02:13 AM, Eric Blake wrote: > Or spell it UINT64_C(1) if you don't want a cast. Not unsigned is what I want most. r~
Re: [Qemu-devel] [PATCH v13 3/5] i386: Enable TOPOEXT feature on AMD EPYC CPU
On Wed, Jun 13, 2018 at 06:21:58PM +, Moger, Babu wrote: > > > > -Original Message- > > From: Eduardo Habkost [mailto:ehabk...@redhat.com] > > Sent: Wednesday, June 13, 2018 1:18 PM > > To: Moger, Babu > > Cc: m...@redhat.com; marcel.apfelb...@gmail.com; pbonz...@redhat.com; > > r...@twiddle.net; mtosa...@redhat.com; qemu-devel@nongnu.org; > > k...@vger.kernel.org; k...@tripleback.net; ge...@hostfission.com; Jiri > > Denemark > > Subject: Re: [PATCH v13 3/5] i386: Enable TOPOEXT feature on AMD EPYC > > CPU > > > > On Wed, Jun 13, 2018 at 06:10:30PM +, Moger, Babu wrote: > > > > -Original Message- > > > > From: Eduardo Habkost [mailto:ehabk...@redhat.com] > > > > Sent: Wednesday, June 13, 2018 12:18 PM > > > > To: Moger, Babu > > > > Cc: m...@redhat.com; marcel.apfelb...@gmail.com; > > pbonz...@redhat.com; > > > > r...@twiddle.net; mtosa...@redhat.com; qemu-devel@nongnu.org; > > > > k...@vger.kernel.org; k...@tripleback.net; ge...@hostfission.com; Jiri > > > > Denemark > > > > Subject: Re: [PATCH v13 3/5] i386: Enable TOPOEXT feature on AMD EPYC > > > > CPU > > > > > > > > On Wed, Jun 13, 2018 at 04:52:18PM +, Moger, Babu wrote: > > > > [...] > > > > > > What do you think our options are here? > > > > > > > > > > Should we drop automatic topoext completely and move forward? > > > > > What are your thoughts? > > > > > > > > Let's drop automatic topoext by now, and see if we find solutions > > > > later. I don't want to hold the rest of the patches because of > > > > this. > > > > > > Ok. I will drop topoext. > > > > > > > > > > > I'm thinking we could simply make kvm_arch_get_supported_cpuid() > > > > always return TOPOEXT on AMD CPUs, because the feature flag don't > > > > really depend on any KVM code to work (is that correct?). > > > > > > Yes, that is correct. I don't see any dependent code on TOPOEXT in KVM > > driver. > > > > > > Ok. Let me add TOPOEXT flag for all the AMD cpus and see how it goes. > > > > Hmm, this could actually solve all of our problems, then: > > > > We can forget about auto-topoext: just add TOPOEXT in > > kvm_arch_get_supported_cpuid(), add TOPOEXT unconditionally to > > the CPU models where you are interested into (EPYC only?), and > > add topoext=off to pc-2.12 compat_props. > > > > Ok Sure. Sorry, I forgot we still need to decide what to do if TOPOEXT is enabled in the CPU model (or command-line) but the -smp options are not compatible with it. In other words, what should guest see on CPUID if using: "-machine pc-q35-3.0 -cpu EPYC -smp 64,cores=64" or: "-machine pc-q35-3.0 -cpu Opteron_G5,+topoext -smp 64,cores=64" I wonder what would happen if we just return zeroes on CPUID[0x81E] if !topology_supports_topoext(), instead of trying to clear/set TOPOEXT depending on the -smp option? It would make things much simpler for QEMU and libvirt. -- Eduardo
