Re: [PATCH-for-8.0 03/10] hw/virtio: Constify qmp_virtio_feature_map_t[]

[Philippe Mathieu-Daudé]

On 13/12/22 01:02, Richard Henderson wrote:

On 12/12/22 17:05, Philippe Mathieu-Daudé wrote:
@@ -161,7 +161,7 @@ static qmp_virtio_feature_map_t 
vhost_user_protocol_map[] = {

  };
  /* virtio device configuration statuses */
-static qmp_virtio_feature_map_t virtio_config_status_map[] = {
+static const qmp_virtio_feature_map_t virtio_config_status_map[] = {
  FEATURE_ENTRY(VIRTIO_CONFIG_S_DRIVER_OK, \
  "VIRTIO_CONFIG_S_DRIVER_OK: Driver setup and ready"),
  FEATURE_ENTRY(VIRTIO_CONFIG_S_FEATURES_OK, \
@@ -179,7 +179,7 @@ static qmp_virtio_feature_map_t 
virtio_config_status_map[] = {

  };
  /* virtio-blk features mapping */
-qmp_virtio_feature_map_t virtio_blk_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_blk_feature_map[] = {
  FEATURE_ENTRY(VIRTIO_BLK_F_SIZE_MAX, \
  "VIRTIO_BLK_F_SIZE_MAX: Max segment size is size_max"),
  FEATURE_ENTRY(VIRTIO_BLK_F_SEG_MAX, \


It appears all of the ones not marked static can be?


It seems some are not static to avoid "declared static but not used"
warnings due to how they are conditionally used with the
CONFIG_VIRTIO_xxx guards in qmp_decode_features():

/* device features */
switch (device_id) {
#ifdef CONFIG_VIRTIO_SERIAL
case VIRTIO_ID_CONSOLE:
features->dev_features =
CONVERT_FEATURES(strList, virtio_serial_feature_map, 0, 
bitmap);

break;
#endif
#ifdef CONFIG_VIRTIO_BLK
case VIRTIO_ID_BLOCK:
features->dev_features =
CONVERT_FEATURES(strList, virtio_blk_feature_map, 0, bitmap);
break;
#endif
#ifdef CONFIG_VIRTIO_GPU
case VIRTIO_ID_GPU:
features->dev_features =
CONVERT_FEATURES(strList, virtio_gpu_feature_map, 0, bitmap);
break;
#endif


Otherwise you should have needed to adjust some header file as well.


OK I'll guard them with the corresponding #ifdef'ry.

Re: [RFC PATCH-for-8.0 06/10] hw/virtio: Cache access_is_big_endian value in VirtIODevice state

[Philippe Mathieu-Daudé]

On 13/12/22 01:14, Richard Henderson wrote:

On 12/12/22 17:05, Philippe Mathieu-Daudé wrote:

The device endianness doesn't change during runtime.


What are you talking about?  Of course it does.


The host CPU certainly does, but the virtio device doesn't... Does it?

This check only consider the device, not the CPU:

bool virtio_access_is_big_endian(VirtIODevice *vdev)
{
#if defined(LEGACY_VIRTIO_IS_BIENDIAN)
return virtio_is_big_endian(vdev);
#elif TARGET_BIG_ENDIAN
if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
/*Devices conforming to VIRTIO 1.0 or later are always LE.*/
return false;
}
return true;
#else
return false;
#endif
}

static inline bool virtio_is_big_endian(VirtIODevice *vdev)
{
if (!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN);
return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_BIG;
}
/* Devices conforming to VIRTIO 1.0 or later are always LE. */
return false;
}

and once the features are negotiated it doesn't seem to change.

I mean, it doesn't often in practice, because the Linux kernel is 
compiled for one endianness and doesn't keep toggling state, but the 
hooks that you're replacing test for the *current* endianness state of 
the cpu.  So this is a behaviour change.


I agree. Note however currently the CPU endianness is only checked once
upon virtio device reset (or from a migration stream):

void virtio_reset(void *opaque)
{
VirtIODevice *vdev = opaque;
VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
int i;

virtio_set_status(vdev, 0);
if (current_cpu) {
/* Guest initiated reset */
vdev->device_endian = virtio_current_cpu_endian();
} else {
/* System reset */
vdev->device_endian = virtio_default_endian();
}

bool cpu_virtio_is_big_endian(CPUState *cpu)
{
CPUClass *cc = CPU_GET_CLASS(cpu);

if (cc->sysemu_ops->virtio_is_big_endian) {
return cc->sysemu_ops->virtio_is_big_endian(cpu);
}
return target_words_bigendian();
}

ARM being the single arch implementing a runtime endianness check:

static bool arm_cpu_virtio_is_big_endian(CPUState *cs)
{
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;

cpu_synchronize_state(cs);
return arm_cpu_data_is_big_endian(env);
}

Have you considered that the bootloader and the kernel may use different 
endianness?


Certainly, but I'll revisit the code more thoughtfully.

Thanks,

Phil.

Re: [PATCH v2] target/sh4: Fix TB_FLAG_UNALIGN

[Guenter Roeck]

On Mon, Dec 12, 2022 at 08:30:42AM -0600, Richard Henderson wrote:
> On 12/11/22 19:13, Guenter Roeck wrote:
> > On Sat, Dec 10, 2022 at 07:27:46AM -0800, Guenter Roeck wrote:
> > > Hi,
> > > 
> > > On Thu, Sep 01, 2022 at 11:15:09AM +0100, Richard Henderson wrote:
> > > > The value previously chosen overlaps GUSA_MASK.
> > > > 
> > > > Rename all DELAY_SLOT_* and GUSA_* defines to emphasize
> > > > that they are included in TB_FLAGs.  Add aliases for the
> > > > FPSCR and SR bits that are included in TB_FLAGS, so that
> > > > we don't accidentally reassign those bits.
> > > > 
> > > > Fixes: 4da06fb3062 ("target/sh4: Implement prctl_unalign_sigbus")
> > > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/856
> > > > Signed-off-by: Richard Henderson 
> > > 
> > > I noticed that my sh4 emulations crash randomly with qemu v7.2-rc4.
> > > This happens with all Linux kernel versions. Testing shows that this
> > > patch is responsible. Reverting it fixes the problem.
> > > 
> > 
> > The patch below fixes the problem for me.
> 
> Thanks for the investigation.
> 
> 
> > +++ b/target/sh4/cpu.c
> > @@ -47,7 +47,7 @@ static void superh_cpu_synchronize_from_tb(CPUState *cs,
> >   SuperHCPU *cpu = SUPERH_CPU(cs);
> >   cpu->env.pc = tb_pc(tb);
> > -cpu->env.flags = tb->flags;
> > +cpu->env.flags = tb->flags & TB_FLAG_ENVFLAGS_MASK;
> 
> Only this hunk should be necessary.
> 

Confirmed.

Do you plan to send a formal patch, or do you want me to do it ?

Thanks,
Guenter

Re: [RFC PATCH v2 02/22] xen: add CONFIG_XENFV_MACHINE and CONFIG_XEN_EMU options for Xen emulation

[David Woodhouse]

On Tue, 2022-12-13 at 01:39 +0100, Paolo Bonzini wrote:
> 
> 
> Il lun 12 dic 2022, 23:23 David Woodhouse  ha
> scritto:
> > On Mon, 2022-12-12 at 18:07 +0100, Paolo Bonzini wrote:
> > > On 12/9/22 10:55, David Woodhouse wrote:
> > > >   config KVM
> > > >   bool
> > > > +imply XEN_EMU if (I386 || X86_64)
> > > 
> > > No need for the "imply", just make it "default y" below and it
> > will have 
> > > the same effect.
> > > 
> > > > 
> > > > diff --git a/target/Kconfig b/target/Kconfig
> > > > index 83da0bd293..e19c9d77b5 100644
> > > > --- a/target/Kconfig
> > > > +++ b/target/Kconfig
> > > > @@ -18,3 +18,7 @@ source sh4/Kconfig
> > > >  source sparc/Kconfig
> > > >  source tricore/Kconfig
> > > >  source xtensa/Kconfig
> > > > +
> > > > +config XEN_EMU
> > > > +bool
> > > > +depends on KVM && (I386 || X86_64)
> > > 
> > > Please place it in hw/xen/Kconfig.
> > 
> > Perhaps I misunderstand, but I'm not sure that is consistent with
> > what
> > Philippe was asking for in  
> > https://lore.kernel.org/qemu-devel/d203e13d-e2f9-5816-030d-c1449bde3...@linaro.org/
> > specifically:
> > 
> > >> I rather have hw/ and target/ features disentangled, so I'd use
> > >> CONFIG_XEN_EMU under target/ and CONFIG_XENFV_MACHINE under hw/,
> > >> eventually having CONFIG_XEN_EMU depending on
> > CONFIG_XENFV_MACHINE
> > >> and -- for now -- CONFIG_KVM.
> 
> 
> However the dependency of the xenfv machine is misguided. In
> principle there is no reason to depend on KVM as opposed to TCG, too,
> which is why I didn't suggest hw/i386/kvm for either the devices or
> the Kconfig file.
> 

That was my initial thought too.

But those devices are there primarily as hooks to save/restore state,
and that means they want to actually program that state back into KVM
on restore; they *have* ended up using KVM directly, which is why I
moved them into hw/i386/kvm.

Contriving some pretence at a "generic" way for the target to set those
features seemed a bit like overengineering.

Supporting TCG (at least if we want to run on non-x86 hosts) isn't just
a case of reimplementing the bits that are already done in-kernel,
either. The structure layouts may differ too (it's bad enough between
i686 and x86_64 with the alignment of uint64_t). So I just don't see
TCG support happening any time soon.

> > The idea there seems to be that XEN_EMU is a *target* feature since
> > it covers the support in target/i386/kvm.
> > 
> > But yes, it *also* covers the devices I'm adding to hw/i386/kvm. Do
> > I want a *separate* config symbol for that? Or just make those
> > depend on XENFV_MACHINE && XEN_EMU ? 
> > 
> > I'll move XEN_EMU to hw/i386/Kconfig for now, thereby doing what
> > *neither* of you said (I don't think hw/xen/Kconfig is the best
> > choice when the *code* it enables is under hw/i386/kvm?)
> 
> 
> Yeah there is no especially better match. I guess hw/i386 is fine,
> since there will be code in mc->kvm_type. It would be either there or
> in target/i386/Kconfig, but not target/Kconfig.

I put pc_machine_kvm_type into hw/i386/pc.c and made DEFINE_PC_MACHINE
add it unconditionally. Then it registers those devices if
xen_mode==XEN_EMULATE. Which is *almost* pretending to be generic,
apart from the hook being KVM-specific.

There was *also* a call to xen_emulated_machine_init() added to
pc_init1() by the 'pc_piix: handle XEN_EMULATE backend init' patch.
I've dropped that for now; once we are ready to hook up the xenbus and
PV drivers, that seems like it can go into mc->kvm_type too. Or maybe I
should have kept that, and initialised the overlay and evtchn devices
from xen_emulated_machine_init() instead of mc->kvm_type() ?




smime.p7s
Description: S/MIME cryptographic signature

Re: [RFC PATCH v2 02/22] xen: add CONFIG_XENFV_MACHINE and CONFIG_XEN_EMU options for Xen emulation

[Paolo Bonzini]

Il lun 12 dic 2022, 23:23 David Woodhouse  ha scritto:

> On Mon, 2022-12-12 at 18:07 +0100, Paolo Bonzini wrote:
> > On 12/9/22 10:55, David Woodhouse wrote:
> > >   config KVM
> > >   bool
> > > +imply XEN_EMU if (I386 || X86_64)
> >
> > No need for the "imply", just make it "default y" below and it will have
> > the same effect.
> >
> > >
> > > diff --git a/target/Kconfig b/target/Kconfig
> > > index 83da0bd293..e19c9d77b5 100644
> > > --- a/target/Kconfig
> > > +++ b/target/Kconfig
> > > @@ -18,3 +18,7 @@ source sh4/Kconfig
> > >  source sparc/Kconfig
> > >  source tricore/Kconfig
> > >  source xtensa/Kconfig
> > > +
> > > +config XEN_EMU
> > > +bool
> > > +depends on KVM && (I386 || X86_64)
> >
> > Please place it in hw/xen/Kconfig.
>
> Perhaps I misunderstand, but I'm not sure that is consistent with what
> Philippe was asking for in
>
> https://lore.kernel.org/qemu-devel/d203e13d-e2f9-5816-030d-c1449bde3...@linaro.org/
> specifically:
>
> >> I rather have hw/ and target/ features disentangled, so I'd use
> >> CONFIG_XEN_EMU under target/ and CONFIG_XENFV_MACHINE under hw/,
> >> eventually having CONFIG_XEN_EMU depending on CONFIG_XENFV_MACHINE
> >> and -- for now -- CONFIG_KVM.
>

However the dependency of the xenfv machine is misguided. In principle
there is no reason to depend on KVM as opposed to TCG, too, which is why I
didn't suggest hw/i386/kvm for either the devices or the Kconfig file.

The idea there seems to be that XEN_EMU is a *target* feature since it
> covers the support in target/i386/kvm.
>
> But yes, it *also* covers the devices I'm adding to hw/i386/kvm. Do I
> want a *separate* config symbol for that? Or just make those depend on
> XENFV_MACHINE && XEN_EMU ?
>
> I'll move XEN_EMU to hw/i386/Kconfig for now, thereby doing what
> *neither* of you said (I don't think hw/xen/Kconfig is the best choice
> when the *code* it enables is under hw/i386/kvm?)
>

Yeah there is no especially better match. I guess hw/i386 is fine, since
there will be code in mc->kvm_type. It would be either there or in
target/i386/Kconfig, but not target/Kconfig.

Paolo

>

Re: [PATCH 4/4] coroutine: Break inclusion loop

[Paolo Bonzini]

dropped qemu-devel by mistake.

Paolo


Il lun 12 dic 2022, 23:16 Paolo Bonzini  ha scritto:

> On 12/8/22 15:23, Markus Armbruster wrote:
> > qemu/coroutine.h and qemu/lockable.h include each other.  Neither
> > header actually needs the other one.
>
> qemu/lockable.h wants qemu/coroutine.h because of the reference to
> qemu_co_mutex_lock/unlock in the QEMU_MAKE_LOCKABLE macro.  Said
> reference only happens when the macro is used, so strictly speaking
> only code that uses of qemu/lockable.h's functionality needs to
> include qemu/coroutine.h.  The order doesn't matter.
>
> qemu/coroutine.h similarly wants qemu/lockable.h only for a macro: it
> uses QemuLockable for the prototype of qemu_co_queue_wait_impl, but
> QemuLockable is defined in qemu/typedefs.h.  On the other hand, the
> qemu_co_queue_wait macro needs QEMU_MAKE_LOCKABLE.  Again, the order
> does not matter but callers of qemu_co_queue_wait appreciate it if
> both files are included.
>
> So, this is why the inclusion loop works.  This patch makes some
> files include qemu/coroutine.h even if they only need qemu/lockable.h
> for QEMU_LOCK_GUARD of a "regular" QemuMutex; for example, linux-user/
> does not use coroutines, so I'd like to avoid that it includes
> qemu/coroutine.h.
>
> One way is to just keep the cycle.  Another is to break the cycle is
> as follows:
>
> 1) qemu/coroutine.h keeps including qemu/lockable.h
>
> 2) qemu/lockable.h is modified as follows to omit the reference to CoMutex:
>
> diff --git a/include/qemu/lockable.h b/include/qemu/lockable.h
> index 86db7cb04c9c..db59656538a4 100644
> --- a/include/qemu/lockable.h
> +++ b/include/qemu/lockable.h
> @@ -71,9 +71,11 @@ qemu_null_lockable(void *x)
>void *: qemu_null_lockable(x), \
>QemuMutex *: qemu_make_lockable(x, QML_OBJ_(x, mutex)),\
>QemuRecMutex *: qemu_make_lockable(x, QML_OBJ_(x,
> rec_mutex)), \
> - CoMutex *: qemu_make_lockable(x, QML_OBJ_(x, co_mutex)),   \
> + QEMU_MAKE_CO_MUTEX_LOCKABLE(x) \
>QemuSpin *: qemu_make_lockable(x, QML_OBJ_(x, spin)))
>
> +#define QEMU_MAKE_CO_MUTEX_LOCKABLE(x)
> +
>   /**
>* QEMU_MAKE_LOCKABLE_NONNULL - Make a polymorphic QemuLockable
>*
>
> 3) the following hack is added in qemu/coroutine.h, right after including
> qemu/lockable.h:
>
> #undef QEMU_MAKE_CO_MUTEX_LOCKABLE(x)
> #define QEMU_MAKE_CO_MUTEX_LOCKABLE(x) \
>   CoMutex *: qemu_make_lockable(x, QML_OBJ_(x, co_mutex)),
>
>
> Neither is particularly pretty, so I vote for leaving things as is with
> a comment above the two #include directives.
>
> Paolo
>

Re: [RFC PATCH-for-8.0 06/10] hw/virtio: Cache access_is_big_endian value in VirtIODevice state

[Richard Henderson]

On 12/12/22 17:05, Philippe Mathieu-Daudé wrote:

The device endianness doesn't change during runtime.


What are you talking about?  Of course it does.

I mean, it doesn't often in practice, because the Linux kernel is compiled for one 
endianness and doesn't keep toggling state, but the hooks that you're replacing test for 
the *current* endianness state of the cpu.  So this is a behaviour change.


Have you considered that the bootloader and the kernel may use different 
endianness?


r~

Re: [RFC PATCH v2 03/22] i386/xen: Add xen-version machine property and init KVM Xen support

[David Woodhouse]

On Mon, 2022-12-12 at 18:30 +0100, Paolo Bonzini wrote:
> On 12/9/22 10:55, David Woodhouse wrote:
> > -m->default_machine_opts = "accel=xen,suppress-vmdesc=on";
> > +if (xen_enabled())
> > +m->default_machine_opts = "accel=xen,suppress-vmdesc=on";
> > +else
> > +m->default_machine_opts = "accel=kvm,xen-version=0x30001";
> 
> Please do not modify pc_xen_hvm_init().
> 
> "-M xenfv" should be the same as "-M pc-i440fx-...,suppress-vmdesc=on 
> -accel xen -device xen-platform".  It must work *without* "-accel xen", 
> while here you're basically requiring it.  For now, please make 
> KVM-emulated Xen use "-M pc -device xen-platform".  We can figure out 
> "-M xenfv" later.
> 
> You can instead have:
> 
> - a check in xen_init() that checks that xen_mode is XEN_ATTACH.  If 
> not, fail.
> 
> - an extra enum value for xen_mode, XEN_DISABLED, which is the default 
> instead of XEN_EMULATE;
> 
> - an accelerator property "-accel kvm,xen-version=...", added in 
> kvm_accel_class_init() instead of the machine property.  The property, 
> when set to a nonzero value, flips xen_mode from XEN_DISABLED to 
> XEN_EMULATE.
> 
> The Xen overlay device can be created using the mc->kvm_type function 
> (which you can set in DEFINE_PC_MACHINE); at that point, xen_mode has 
> switched from XEN_DISABLED to XEN_EMULATE.  Those xen_enabled() checks 
> that apply to KVM then become xen_mode != XEN_DISABLED, as long as they 
> run during mc->kvm_type or afterwards.
> 
> The platform device can be created either in mc->kvm_type or manually 
> (not sure if it makes sense to have a "XenVMMXenVMM" CPUID + emulated 
> hypercalls but no platform device---would it still use pvclock for 
> example?).

That works; thanks. I won't spam the list with another round just yet,
but have pushed it to
https://git.infradead.org/users/dwmw2/qemu.git/shortlog/refs/heads/xenfv

The guest now correctly panics because I haven't implemented event
channel hypercalls yet (got to fix up a bit more of the 32-bit compat
first, and some other parts of Paul's feedback I haven't yet got to).

$ ./build/qemu-system-x86_64 -serial mon:stdio -M pc -accel 
kvm,xen-version=0x4000a  -cpu host  -display none -kernel 
vmlinuz-5.17.8-200.fc35.x86_64 -append "console=ttyS0 earlyprintk=ttyS0 
panic=1" --trace "kvm_xen*" -d unimp -m 1G -smp 4 -device xen-platform
Probing EDD (edd=off to disable)... ok
[0.00] Linux version 5.17.8-200.fc35.x86_64 
(mockbu...@bkernel02.iad2.fedoraproject.org) (gcc (GCC) 11.3.1 20220421 (Red 
Hat 11.3.1-2), GNU ld version 2.37-17.fc35) #1 SMP PREEMPT Mon May 16 01:01:02 
UTC 2022
[0.00] Command line: console=ttyS0 earlyprintk=ttyS0 panic=1
[0.00] x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point 
registers'
[0.00] x86/fpu: Supporting XSAVE feature 0x002: 'SSE registers'
[0.00] x86/fpu: Supporting XSAVE feature 0x004: 'AVX registers'
[0.00] x86/fpu: Supporting XSAVE feature 0x008: 'MPX bounds registers'
[0.00] x86/fpu: Supporting XSAVE feature 0x010: 'MPX CSR'
[0.00] x86/fpu: xstate_offset[2]:  576, xstate_sizes[2]:  256
[0.00] x86/fpu: xstate_offset[3]:  832, xstate_sizes[3]:   64
[0.00] x86/fpu: xstate_offset[4]:  896, xstate_sizes[4]:   64
[0.00] x86/fpu: Enabled xstate features 0x1f, context size is 960 
bytes, using 'compacted' format.
[0.00] signal: max sigframe size: 2032
[0.00] BIOS-provided physical RAM map:
[0.00] BIOS-e820: [mem 0x-0x0009fbff] usable
[0.00] BIOS-e820: [mem 0x0009fc00-0x0009] reserved
[0.00] BIOS-e820: [mem 0x000f-0x000f] reserved
[0.00] BIOS-e820: [mem 0x0010-0x3ffd] usable
[0.00] BIOS-e820: [mem 0x3ffe-0x3fff] reserved
[0.00] BIOS-e820: [mem 0xfeffc000-0xfeff] reserved
[0.00] BIOS-e820: [mem 0xfffc-0x] reserved
[0.00] printk: bootconsole [earlyser0] enabled
[0.00] NX (Execute Disable) protection: active
[0.00] extended physical RAM map:
[0.00] reserve setup_data: [mem 0x-0x0009fbff] 
usable
[0.00] reserve setup_data: [mem 0x0009fc00-0x0009] 
reserved
[0.00] reserve setup_data: [mem 0x000f-0x000f] 
reserved
[0.00] reserve setup_data: [mem 0x0010-0x00bf98ef] 
usable
[0.00] reserve setup_data: [mem 0x00bf98f0-0x00bf991f] 
usable
[0.00] reserve setup_data: [mem 0x00bf9920-0x3ffd] 
usable
[0.00] reserve setup_data: [mem 0x3ffe-0x3fff] 
reserved
[0.00] reserve setup_data: [mem 0xfeffc000-0xfeff] 
reserved
[0.00] reserve setup_data: [mem 0xfffc-0x] 
reserved
[0.0

Re: [RFC PATCH v2 16/22] i386/xen: handle VCPUOP_register_vcpu_info

[David Woodhouse]

On Mon, 2022-12-12 at 14:58 +, Paul Durrant wrote:
> On 09/12/2022 09:56, David Woodhouse wrote:
> > From: Joao Martins <
> > joao.m.mart...@oracle.com
> > >
> > 
> > Handle the hypercall to set a per vcpu info, and also wire up the
> > default
> > vcpu_info in the shared_info page for the first 32 vCPUs.
> > 
> > To avoid deadlock within KVM a vCPU thread must set its *own*
> > vcpu_info
> > rather than it being set from the context in which the hypercall is
> > invoked.
> > 
> > Add the vcpu_info (and default) GPA to the vmstate_x86_cpu for
> > migration,
> > and restore it in kvm_arch_put_registers() appropriately.
> > 
> > Signed-off-by: Joao Martins <
> > joao.m.mart...@oracle.com
> > >
> > Signed-off-by: David Woodhouse <
> > d...@amazon.co.uk
> > >
> > ---
> >   target/i386/cpu.h|  2 ++
> >   target/i386/kvm/kvm.c| 19 +++
> >   target/i386/machine.c| 21 
> >   target/i386/trace-events |  1 +
> >   target/i386/xen.c| 74
> > +---
> >   target/i386/xen.h|  1 +
> >   6 files changed, 113 insertions(+), 5 deletions(-)
> > 
> > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > index c6c57baed5..109b2e5669 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -1788,6 +1788,8 @@ typedef struct CPUArchState {
> >   #endif
> >   #if defined(CONFIG_KVM)
> >   struct kvm_nested_state *nested_state;
> > +uint64_t xen_vcpu_info_gpa;
> > +uint64_t xen_vcpu_info_default_gpa;
> >   #endif
> >   #if defined(CONFIG_HVF)
> >   HVFX86LazyFlags hvf_lflags;
> > diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> > index ebde6bc204..fa45e2f99a 100644
> > --- a/target/i386/kvm/kvm.c
> > +++ b/target/i386/kvm/kvm.c
> > @@ -1811,6 +1811,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
> >   has_msr_hv_hypercall = true;
> >   }
> >   
> > +env->xen_vcpu_info_gpa = UINT64_MAX;
> > +env->xen_vcpu_info_default_gpa = UINT64_MAX;
> 
> 
> There was an INVALID_GPA definition for shared info. Looks like we
> could use it here too.

There was, and I started trying to use it, but it fell foul of the "is
this going to live in target/ or hw/ and who can include what from
where?" and I decided to just use UINT64_MAX for now and keep typing.

That will work out in the end, I'm sure.

> Some sanity checks wouldn't go a miss here...
> 
> rvi.offset should:
> a) be < TARGET_PAGE_SIZE, and
> b) ba aligned to vcpu_info_t size

Ack.


smime.p7s
Description: S/MIME cryptographic signature

Re: [RFC PATCH v2 15/22] i386/xen: implement HYPERVISOR_vcpu_op

[David Woodhouse]

On Mon, 2022-12-12 at 14:51 +, Paul Durrant wrote:
> Again, should this patch be deferred until we actually implement 
> something useful here? I.e. folding it into the subsequent patch? It's 
> not like the boilerplate is massive.

That's how Joao did it; it seems sane enough to do bite-sized pieces so
I didn't change it. As I've been through and rebased/reworked it all
about ten times so far, it's been useful for it to be in small chunks.



smime.p7s
Description: S/MIME cryptographic signature

Re: [RFC PATCH v2 13/22] i386/xen: implement HYPERVISOR_memory_op

[David Woodhouse]

On Mon, 2022-12-12 at 14:38 +, Paul Durrant wrote:
> 
> > +switch (xatp.space) {
> > +case XENMAPSPACE_shared_info:
> > +break;
> > +default:
> > +err = -ENOSYS;
> > +break;
> 
> Don't you want to return false here?

It doesn't make a lot of difference right now. Once we believe we've
implemented everything a sane guest would use, then the distinction
between {true, -ENOSYS} and just returning false actually starts to
matter.


smime.p7s
Description: S/MIME cryptographic signature

Re: [RFC PATCH v2 11/22] i386/xen: implement HYPERCALL_xen_version

[David Woodhouse]

On Mon, 2022-12-12 at 14:17 +, Paul Durrant wrote:
> Shouldn't this be (sz - i)?

Turns out I hate that loop. Have refactored it to be
while (sz) { ... sz -= len; ... }.

Thanks.



smime.p7s
Description: S/MIME cryptographic signature

Re: [PATCH-for-8.0 03/10] hw/virtio: Constify qmp_virtio_feature_map_t[]

[Richard Henderson]

On 12/12/22 17:05, Philippe Mathieu-Daudé wrote:

@@ -161,7 +161,7 @@ static qmp_virtio_feature_map_t vhost_user_protocol_map[] = 
{
  };
  
  /* virtio device configuration statuses */

-static qmp_virtio_feature_map_t virtio_config_status_map[] = {
+static const qmp_virtio_feature_map_t virtio_config_status_map[] = {
  FEATURE_ENTRY(VIRTIO_CONFIG_S_DRIVER_OK, \
  "VIRTIO_CONFIG_S_DRIVER_OK: Driver setup and ready"),
  FEATURE_ENTRY(VIRTIO_CONFIG_S_FEATURES_OK, \
@@ -179,7 +179,7 @@ static qmp_virtio_feature_map_t virtio_config_status_map[] 
= {
  };
  
  /* virtio-blk features mapping */

-qmp_virtio_feature_map_t virtio_blk_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_blk_feature_map[] = {
  FEATURE_ENTRY(VIRTIO_BLK_F_SIZE_MAX, \
  "VIRTIO_BLK_F_SIZE_MAX: Max segment size is size_max"),
  FEATURE_ENTRY(VIRTIO_BLK_F_SEG_MAX, \


It appears all of the ones not marked static can be?
Otherwise you should have needed to adjust some header file as well.


r~

[PATCH-for-8.0 01/10] hw/virtio: Add missing "hw/core/cpu.h" include

[Philippe Mathieu-Daudé]

virtio.c uses target_words_bigendian() which is declared in
"hw/core/cpu.h". Add the missing header to avoid when refactoring:

  hw/virtio/virtio.c:2451:9: error: implicit declaration of function 
'target_words_bigendian' is invalid in C99 
[-Werror,-Wimplicit-function-declaration]
if (target_words_bigendian()) {
^

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/virtio/virtio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index eb6347ab5d..5817f4cbc9 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -25,6 +25,7 @@
 #include "qemu/main-loop.h"
 #include "qemu/module.h"
 #include "qom/object_interfaces.h"
+#include "hw/core/cpu.h"
 #include "hw/virtio/virtio.h"
 #include "migration/qemu-file-types.h"
 #include "qemu/atomic.h"
-- 
2.38.1

[RFC PATCH-for-8.0 09/10] hw/virtio: Extract vhost_user_ram_slots_max() to vhost-user-target.c

[Philippe Mathieu-Daudé]

The current definition of VHOST_USER_MAX_RAM_SLOTS is
target specific. By converting this definition to a runtime
vhost_user_ram_slots_max() helper declared in a target
specific unit, we can have the rest of vhost-user.c target
independent.

To avoid variable length array or using the heap to store
arrays of vhost_user_ram_slots_max() elements, we simply
declare an array of the biggest VHOST_USER_MAX_RAM_SLOTS,
and each target uses up to vhost_user_ram_slots_max()
elements of it. Ensure arrays are big enough by adding an
assertion in vhost_user_init().

Signed-off-by: Philippe Mathieu-Daudé 
---
RFC: Should I add VHOST_USER_MAX_RAM_SLOTS to vhost-user.h
 or create an internal header for it?
---
 hw/virtio/meson.build  |  1 +
 hw/virtio/vhost-user-target.c  | 29 +
 hw/virtio/vhost-user.c | 26 +-
 include/hw/virtio/vhost-user.h |  7 +++
 4 files changed, 42 insertions(+), 21 deletions(-)
 create mode 100644 hw/virtio/vhost-user-target.c

diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index eb7ee8ea92..bf7e35fa8a 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -11,6 +11,7 @@ if have_vhost
   specific_virtio_ss.add(files('vhost.c', 'vhost-backend.c', 
'vhost-iova-tree.c'))
   if have_vhost_user
 specific_virtio_ss.add(files('vhost-user.c'))
+specific_virtio_ss.add(files('vhost-user-target.c'))
   endif
   if have_vhost_vdpa
 specific_virtio_ss.add(files('vhost-vdpa.c', 'vhost-shadow-virtqueue.c'))
diff --git a/hw/virtio/vhost-user-target.c b/hw/virtio/vhost-user-target.c
new file mode 100644
index 00..6a0d0f53d0
--- /dev/null
+++ b/hw/virtio/vhost-user-target.c
@@ -0,0 +1,29 @@
+/*
+ * vhost-user target-specific helpers
+ *
+ * Copyright (c) 2013 Virtual Open Systems Sarl.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/virtio/vhost-user.h"
+
+#if defined(TARGET_X86) || defined(TARGET_X86_64) || \
+defined(TARGET_ARM) || defined(TARGET_ARM_64)
+#include "hw/acpi/acpi.h"
+#elif defined(TARGET_PPC) || defined(TARGET_PPC64)
+#include "hw/ppc/spapr.h"
+#endif
+
+unsigned int vhost_user_ram_slots_max(void)
+{
+#if defined(TARGET_X86) || defined(TARGET_X86_64) || \
+defined(TARGET_ARM) || defined(TARGET_ARM_64)
+return ACPI_MAX_RAM_SLOTS;
+#elif defined(TARGET_PPC) || defined(TARGET_PPC64)
+return SPAPR_MAX_RAM_SLOTS;
+#else
+return 512;
+#endif
+}
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 8f635844af..21fc176725 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -41,24 +41,7 @@
 #define VHOST_MEMORY_BASELINE_NREGIONS8
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
 #define VHOST_USER_SLAVE_MAX_FDS 8
-
-/*
- * Set maximum number of RAM slots supported to
- * the maximum number supported by the target
- * hardware plaform.
- */
-#if defined(TARGET_X86) || defined(TARGET_X86_64) || \
-defined(TARGET_ARM) || defined(TARGET_ARM_64)
-#include "hw/acpi/acpi.h"
-#define VHOST_USER_MAX_RAM_SLOTS ACPI_MAX_RAM_SLOTS
-
-#elif defined(TARGET_PPC) || defined(TARGET_PPC64)
-#include "hw/ppc/spapr.h"
-#define VHOST_USER_MAX_RAM_SLOTS SPAPR_MAX_RAM_SLOTS
-
-#else
 #define VHOST_USER_MAX_RAM_SLOTS 512
-#endif
 
 /*
  * Maximum size of virtio device config space
@@ -935,7 +918,7 @@ static int vhost_user_add_remove_regions(struct vhost_dev 
*dev,
 
 if (track_ramblocks) {
 memcpy(u->postcopy_client_bases, shadow_pcb,
-   sizeof(uint64_t) * VHOST_USER_MAX_RAM_SLOTS);
+   sizeof(uint64_t) * vhost_user_ram_slots_max());
 /*
  * Now we've registered this with the postcopy code, we ack to the
  * client, because now we're in the position to be able to deal with
@@ -956,7 +939,7 @@ static int vhost_user_add_remove_regions(struct vhost_dev 
*dev,
 err:
 if (track_ramblocks) {
 memcpy(u->postcopy_client_bases, shadow_pcb,
-   sizeof(uint64_t) * VHOST_USER_MAX_RAM_SLOTS);
+   sizeof(uint64_t) * vhost_user_ram_slots_max());
 }
 
 return ret;
@@ -1030,7 +1013,7 @@ static int vhost_user_set_mem_table_postcopy(struct 
vhost_dev *dev,
 }
 
 memset(u->postcopy_client_bases, 0,
-   sizeof(uint64_t) * VHOST_USER_MAX_RAM_SLOTS);
+   sizeof(uint64_t) * vhost_user_ram_slots_max());
 
 /*
  * They're in the same order as the regions that were sent
@@ -2169,7 +2152,7 @@ static int vhost_user_backend_init(struct vhost_dev *dev, 
void *opaque,
 return -EINVAL;
 }
 
-u->user->memory_slots = MIN(ram_slots, VHOST_USER_MAX_RAM_SLOTS);
+u->user->memory_slots = MIN(ram_slots, vhost_user_ram_slots_max());
 }
 }
 
@@ -2649,6 +2632,7 @@ static void vhost_user_state_destroy(gpointer data)
 
 bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error **errp)
 {
+assert(vhost_user_ram_slots_max() <= 

[RFC PATCH-for-8.0 10/10] hw/virtio: Make most of virtio devices target-independent

[Philippe Mathieu-Daudé]

Except the following files:
- virtio-config.c
- virtio-qmp.c
- virtio-iommu.c
- virtio-mem.c
- vhost-user-target.c
- vhost-vdpa.c
all other virtio related files are target independent and
can be compiled only once for a system emulation build,
avoiding compiling hundreds of objects.

Signed-off-by: Philippe Mathieu-Daudé 
---
RFC: Cross-built on ppc64le/s390x but not tested there.
---
 hw/9pfs/meson.build|  2 +-
 hw/block/dataplane/meson.build |  2 +-
 hw/block/meson.build   |  4 ++--
 hw/char/meson.build|  2 +-
 hw/net/meson.build |  2 +-
 hw/virtio/meson.build  | 38 +-
 6 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/hw/9pfs/meson.build b/hw/9pfs/meson.build
index 12443b6ad5..ef37532dbf 100644
--- a/hw/9pfs/meson.build
+++ b/hw/9pfs/meson.build
@@ -18,4 +18,4 @@ fs_ss.add(when: 'CONFIG_DARWIN', if_true: 
files('9p-util-darwin.c'))
 fs_ss.add(when: 'CONFIG_XEN', if_true: files('xen-9p-backend.c'))
 softmmu_ss.add_all(when: 'CONFIG_FSDEV_9P', if_true: fs_ss)
 
-specific_ss.add(when: 'CONFIG_VIRTIO_9P', if_true: files('virtio-9p-device.c'))
+softmmu_ss.add(when: 'CONFIG_VIRTIO_9P', if_true: files('virtio-9p-device.c'))
diff --git a/hw/block/dataplane/meson.build b/hw/block/dataplane/meson.build
index 12c6a264f1..e2f3721ce2 100644
--- a/hw/block/dataplane/meson.build
+++ b/hw/block/dataplane/meson.build
@@ -1,2 +1,2 @@
-specific_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c'))
+softmmu_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c'))
 specific_ss.add(when: 'CONFIG_XEN', if_true: files('xen-block.c'))
diff --git a/hw/block/meson.build b/hw/block/meson.build
index b434d5654c..8a3ca43a5c 100644
--- a/hw/block/meson.build
+++ b/hw/block/meson.build
@@ -17,7 +17,7 @@ softmmu_ss.add(when: 'CONFIG_SWIM', if_true: files('swim.c'))
 softmmu_ss.add(when: 'CONFIG_XEN', if_true: files('xen-block.c'))
 softmmu_ss.add(when: 'CONFIG_TC58128', if_true: files('tc58128.c'))
 
-specific_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c', 
'virtio-blk-common.c'))
-specific_ss.add(when: 'CONFIG_VHOST_USER_BLK', if_true: 
files('vhost-user-blk.c', 'virtio-blk-common.c'))
+softmmu_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c', 
'virtio-blk-common.c'))
+softmmu_ss.add(when: 'CONFIG_VHOST_USER_BLK', if_true: 
files('vhost-user-blk.c', 'virtio-blk-common.c'))
 
 subdir('dataplane')
diff --git a/hw/char/meson.build b/hw/char/meson.build
index 7b594f51b8..afd35649cd 100644
--- a/hw/char/meson.build
+++ b/hw/char/meson.build
@@ -18,6 +18,7 @@ softmmu_ss.add(when: 'CONFIG_SERIAL_PCI', if_true: 
files('serial-pci.c'))
 softmmu_ss.add(when: 'CONFIG_SERIAL_PCI_MULTI', if_true: 
files('serial-pci-multi.c'))
 softmmu_ss.add(when: 'CONFIG_SHAKTI_UART', if_true: files('shakti_uart.c'))
 softmmu_ss.add(when: 'CONFIG_VIRTIO_SERIAL', if_true: 
files('virtio-console.c'))
+softmmu_ss.add(when: 'CONFIG_VIRTIO', if_true: files('virtio-serial-bus.c'))
 softmmu_ss.add(when: 'CONFIG_XEN', if_true: files('xen_console.c'))
 softmmu_ss.add(when: 'CONFIG_XILINX', if_true: files('xilinx_uartlite.c'))
 
@@ -35,7 +36,6 @@ softmmu_ss.add(when: 'CONFIG_MCHP_PFSOC_MMUART', if_true: 
files('mchp_pfsoc_mmua
 
 specific_ss.add(when: 'CONFIG_HTIF', if_true: files('riscv_htif.c'))
 specific_ss.add(when: 'CONFIG_TERMINAL3270', if_true: files('terminal3270.c'))
-specific_ss.add(when: 'CONFIG_VIRTIO', if_true: files('virtio-serial-bus.c'))
 specific_ss.add(when: 'CONFIG_PSERIES', if_true: files('spapr_vty.c'))
 
 specific_ss.add(when: 'CONFIG_GOLDFISH_TTY', if_true: files('goldfish_tty.c'))
diff --git a/hw/net/meson.build b/hw/net/meson.build
index ebac261542..8035aaa560 100644
--- a/hw/net/meson.build
+++ b/hw/net/meson.build
@@ -44,7 +44,7 @@ specific_ss.add(when: 'CONFIG_PSERIES', if_true: 
files('spapr_llan.c'))
 specific_ss.add(when: 'CONFIG_XILINX_ETHLITE', if_true: 
files('xilinx_ethlite.c'))
 
 softmmu_ss.add(when: 'CONFIG_VIRTIO_NET', if_true: files('net_rx_pkt.c'))
-specific_ss.add(when: 'CONFIG_VIRTIO_NET', if_true: files('virtio-net.c'))
+softmmu_ss.add(when: 'CONFIG_VIRTIO_NET', if_true: files('virtio-net.c'))
 
 if have_vhost_net
   softmmu_ss.add(when: 'CONFIG_VIRTIO_NET', if_true: files('vhost_net.c'), 
if_false: files('vhost_net-stub.c'))
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index bf7e35fa8a..23be895c8e 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -1,38 +1,38 @@
 softmmu_virtio_ss = ss.source_set()
-softmmu_virtio_ss.add(files('virtio-bus.c'))
+softmmu_virtio_ss.add(files('virtio.c', 'virtio-bus.c'))
 softmmu_virtio_ss.add(when: 'CONFIG_VIRTIO_PCI', if_true: 
files('virtio-pci.c'))
 softmmu_virtio_ss.add(when: 'CONFIG_VIRTIO_MMIO', if_true: 
files('virtio-mmio.c'))
 
 specific_virtio_ss = ss.source_set()
-specific_virtio_ss.add(files('virtio.c'))
 specific_virtio_ss.add(files('virtio-config.c', 'virtio-qmp.c'))
+specific_virtio_ss.add(when: 'CONFIG_V

[PATCH-for-8.0 08/10] hw/virtio: Un-inline virtio_access_is_big_endian()

[Philippe Mathieu-Daudé]

In order to avoid target-specific code in VirtIO headers,
move this particular function -- which is only called once
in virtio_init() -- in its own unit.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/virtio/virtio-config.c | 20 
 include/hw/virtio/virtio-access.h | 19 +--
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/hw/virtio/virtio-config.c b/hw/virtio/virtio-config.c
index ad78e0b9bc..aca6ef5e1b 100644
--- a/hw/virtio/virtio-config.c
+++ b/hw/virtio/virtio-config.c
@@ -11,8 +11,28 @@
 
 #include "qemu/osdep.h"
 #include "hw/virtio/virtio.h"
+#include "hw/virtio/virtio-access.h"
 #include "cpu.h"
 
+#if defined(TARGET_PPC64) || defined(TARGET_ARM)
+#define LEGACY_VIRTIO_IS_BIENDIAN 1
+#endif
+
+bool virtio_access_is_big_endian(VirtIODevice *vdev)
+{
+#if defined(LEGACY_VIRTIO_IS_BIENDIAN)
+return virtio_is_big_endian(vdev);
+#elif TARGET_BIG_ENDIAN
+if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+/* Devices conforming to VIRTIO 1.0 or later are always LE. */
+return false;
+}
+return true;
+#else
+return false;
+#endif
+}
+
 uint32_t virtio_config_readb(VirtIODevice *vdev, uint32_t addr)
 {
 VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
diff --git a/include/hw/virtio/virtio-access.h 
b/include/hw/virtio/virtio-access.h
index 985f39fe16..7229088b7c 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -20,24 +20,7 @@
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-bus.h"
 
-#if defined(TARGET_PPC64) || defined(TARGET_ARM)
-#define LEGACY_VIRTIO_IS_BIENDIAN 1
-#endif
-
-static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
-{
-#if defined(LEGACY_VIRTIO_IS_BIENDIAN)
-return virtio_is_big_endian(vdev);
-#elif TARGET_BIG_ENDIAN
-if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
-/* Devices conforming to VIRTIO 1.0 or later are always LE. */
-return false;
-}
-return true;
-#else
-return false;
-#endif
-}
+bool virtio_access_is_big_endian(VirtIODevice *vdev);
 
 static inline uint16_t virtio_lduw_phys(VirtIODevice *vdev, hwaddr pa)
 {
-- 
2.38.1

[RFC PATCH-for-8.0 06/10] hw/virtio: Cache access_is_big_endian value in VirtIODevice state

[Philippe Mathieu-Daudé]

The device endianness doesn't change during runtime.
Cache it in the VirtIODevice state.

Signed-off-by: Philippe Mathieu-Daudé 
---
RFC: I'm not sure virtio_init() is the correct place to add this
 check. We want to initialize this field once the features are
 negociated.
---
 hw/virtio/virtio.c | 1 +
 include/hw/virtio/virtio.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 09b1a0e3d9..dbb1fe33f7 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3193,6 +3193,7 @@ void virtio_init(VirtIODevice *vdev, uint16_t device_id, 
size_t config_size)
 vdev->vmstate = qdev_add_vm_change_state_handler(DEVICE(vdev),
 virtio_vmstate_change, vdev);
 vdev->device_endian = virtio_default_endian();
+vdev->access_is_big_endian = virtio_access_is_big_endian(vdev);
 vdev->use_guest_notifier_mask = true;
 }
 
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index acfd4df125..5f28e51e93 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -130,6 +130,7 @@ struct VirtIODevice
 bool vhost_started;
 VMChangeStateEntry *vmstate;
 char *bus_name;
+bool access_is_big_endian;
 uint8_t device_endian;
 bool use_guest_notifier_mask;
 AddressSpace *dma_as;
-- 
2.38.1

[RFC PATCH-for-8.0 07/10] hw/virtio: Directly access cached VirtIODevice::access_is_big_endian

[Philippe Mathieu-Daudé]

Since the device endianness doesn't change at runtime,
use the cached value instead of evaluating it on each call.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/virtio/virtio-access.h | 44 +++
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/include/hw/virtio/virtio-access.h 
b/include/hw/virtio/virtio-access.h
index 07aae69042..985f39fe16 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -43,7 +43,7 @@ static inline uint16_t virtio_lduw_phys(VirtIODevice *vdev, 
hwaddr pa)
 {
 AddressSpace *dma_as = vdev->dma_as;
 
-if (virtio_access_is_big_endian(vdev)) {
+if (vdev->access_is_big_endian) {
 return lduw_be_phys(dma_as, pa);
 }
 return lduw_le_phys(dma_as, pa);
@@ -53,7 +53,7 @@ static inline uint32_t virtio_ldl_phys(VirtIODevice *vdev, 
hwaddr pa)
 {
 AddressSpace *dma_as = vdev->dma_as;
 
-if (virtio_access_is_big_endian(vdev)) {
+if (vdev->access_is_big_endian) {
 return ldl_be_phys(dma_as, pa);
 }
 return ldl_le_phys(dma_as, pa);
@@ -63,7 +63,7 @@ static inline uint64_t virtio_ldq_phys(VirtIODevice *vdev, 
hwaddr pa)
 {
 AddressSpace *dma_as = vdev->dma_as;
 
-if (virtio_access_is_big_endian(vdev)) {
+if (vdev->access_is_big_endian) {
 return ldq_be_phys(dma_as, pa);
 }
 return ldq_le_phys(dma_as, pa);
@@ -74,7 +74,7 @@ static inline void virtio_stw_phys(VirtIODevice *vdev, hwaddr 
pa,
 {
 AddressSpace *dma_as = vdev->dma_as;
 
-if (virtio_access_is_big_endian(vdev)) {
+if (vdev->access_is_big_endian) {
 stw_be_phys(dma_as, pa, value);
 } else {
 stw_le_phys(dma_as, pa, value);
@@ -86,7 +86,7 @@ static inline void virtio_stl_phys(VirtIODevice *vdev, hwaddr 
pa,
 {
 AddressSpace *dma_as = vdev->dma_as;
 
-if (virtio_access_is_big_endian(vdev)) {
+if (vdev->access_is_big_endian) {
 stl_be_phys(dma_as, pa, value);
 } else {
 stl_le_phys(dma_as, pa, value);
@@ -95,7 +95,7 @@ static inline void virtio_stl_phys(VirtIODevice *vdev, hwaddr 
pa,
 
 static inline void virtio_stw_p(VirtIODevice *vdev, void *ptr, uint16_t v)
 {
-if (virtio_access_is_big_endian(vdev)) {
+if (vdev->access_is_big_endian) {
 stw_be_p(ptr, v);
 } else {
 stw_le_p(ptr, v);
@@ -104,7 +104,7 @@ static inline void virtio_stw_p(VirtIODevice *vdev, void 
*ptr, uint16_t v)
 
 static inline void virtio_stl_p(VirtIODevice *vdev, void *ptr, uint32_t v)
 {
-if (virtio_access_is_big_endian(vdev)) {
+if (vdev->access_is_big_endian) {
 stl_be_p(ptr, v);
 } else {
 stl_le_p(ptr, v);
@@ -113,7 +113,7 @@ static inline void virtio_stl_p(VirtIODevice *vdev, void 
*ptr, uint32_t v)
 
 static inline void virtio_stq_p(VirtIODevice *vdev, void *ptr, uint64_t v)
 {
-if (virtio_access_is_big_endian(vdev)) {
+if (vdev->access_is_big_endian) {
 stq_be_p(ptr, v);
 } else {
 stq_le_p(ptr, v);
@@ -122,7 +122,7 @@ static inline void virtio_stq_p(VirtIODevice *vdev, void 
*ptr, uint64_t v)
 
 static inline int virtio_lduw_p(VirtIODevice *vdev, const void *ptr)
 {
-if (virtio_access_is_big_endian(vdev)) {
+if (vdev->access_is_big_endian) {
 return lduw_be_p(ptr);
 } else {
 return lduw_le_p(ptr);
@@ -131,7 +131,7 @@ static inline int virtio_lduw_p(VirtIODevice *vdev, const 
void *ptr)
 
 static inline int virtio_ldl_p(VirtIODevice *vdev, const void *ptr)
 {
-if (virtio_access_is_big_endian(vdev)) {
+if (vdev->access_is_big_endian) {
 return ldl_be_p(ptr);
 } else {
 return ldl_le_p(ptr);
@@ -140,7 +140,7 @@ static inline int virtio_ldl_p(VirtIODevice *vdev, const 
void *ptr)
 
 static inline uint64_t virtio_ldq_p(VirtIODevice *vdev, const void *ptr)
 {
-if (virtio_access_is_big_endian(vdev)) {
+if (vdev->access_is_big_endian) {
 return ldq_be_p(ptr);
 } else {
 return ldq_le_p(ptr);
@@ -150,9 +150,9 @@ static inline uint64_t virtio_ldq_p(VirtIODevice *vdev, 
const void *ptr)
 static inline uint16_t virtio_tswap16(VirtIODevice *vdev, uint16_t s)
 {
 #if HOST_BIG_ENDIAN
-return virtio_access_is_big_endian(vdev) ? s : bswap16(s);
+return vdev->access_is_big_endian ? s : bswap16(s);
 #else
-return virtio_access_is_big_endian(vdev) ? bswap16(s) : s;
+return vdev->access_is_big_endian ? bswap16(s) : s;
 #endif
 }
 
@@ -160,7 +160,7 @@ static inline uint16_t virtio_lduw_phys_cached(VirtIODevice 
*vdev,
MemoryRegionCache *cache,
hwaddr pa)
 {
-if (virtio_access_is_big_endian(vdev)) {
+if (vdev->access_is_big_endian) {
 return lduw_be_phys_cached(cache, pa);
 }
 return lduw_le_phys_cached(cache, pa);
@@ -170,7 +170,7 @@ static inline uint32_t virtio_ldl_phys_cached(VirtIODevice 
*vdev,
   MemoryReg

[PATCH-for-8.0 05/10] hw/virtio: Extract QMP related code virtio-qmp.c

[Philippe Mathieu-Daudé]

The monitor decoders are the only functions using the CONFIG_xxx
definitions declared in the target specific CONFIG_DEVICES header.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/virtio/meson.build  |   2 +-
 hw/virtio/virtio-qmp.c | 631 +
 hw/virtio/virtio-qmp.h |  20 ++
 hw/virtio/virtio.c | 607 +--
 4 files changed, 654 insertions(+), 606 deletions(-)
 create mode 100644 hw/virtio/virtio-qmp.c
 create mode 100644 hw/virtio/virtio-qmp.h

diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index 097220b4b5..eb7ee8ea92 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -5,7 +5,7 @@ softmmu_virtio_ss.add(when: 'CONFIG_VIRTIO_MMIO', if_true: 
files('virtio-mmio.c'
 
 specific_virtio_ss = ss.source_set()
 specific_virtio_ss.add(files('virtio.c'))
-specific_virtio_ss.add(files('virtio-config.c'))
+specific_virtio_ss.add(files('virtio-config.c', 'virtio-qmp.c'))
 
 if have_vhost
   specific_virtio_ss.add(files('vhost.c', 'vhost-backend.c', 
'vhost-iova-tree.c'))
diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
new file mode 100644
index 00..63d03cc6db
--- /dev/null
+++ b/hw/virtio/virtio-qmp.c
@@ -0,0 +1,631 @@
+/*
+ * Virtio QMP helpers
+ *
+ * Copyright IBM, Corp. 2007
+ *
+ * Authors:
+ *  Anthony Liguori   
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/virtio/virtio.h"
+#include "virtio-qmp.h"
+
+#include "standard-headers/linux/virtio_ids.h"
+#include "standard-headers/linux/vhost_types.h"
+#include "standard-headers/linux/virtio_blk.h"
+#include "standard-headers/linux/virtio_console.h"
+#include "standard-headers/linux/virtio_gpu.h"
+#include "standard-headers/linux/virtio_net.h"
+#include "standard-headers/linux/virtio_scsi.h"
+#include "standard-headers/linux/virtio_i2c.h"
+#include "standard-headers/linux/virtio_balloon.h"
+#include "standard-headers/linux/virtio_iommu.h"
+#include "standard-headers/linux/virtio_mem.h"
+#include "standard-headers/linux/virtio_vsock.h"
+
+#include CONFIG_DEVICES
+
+#define FEATURE_ENTRY(name, desc) (qmp_virtio_feature_map_t) \
+{ .virtio_bit = name, .feature_desc = desc }
+
+enum VhostUserProtocolFeature {
+VHOST_USER_PROTOCOL_F_MQ = 0,
+VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1,
+VHOST_USER_PROTOCOL_F_RARP = 2,
+VHOST_USER_PROTOCOL_F_REPLY_ACK = 3,
+VHOST_USER_PROTOCOL_F_NET_MTU = 4,
+VHOST_USER_PROTOCOL_F_SLAVE_REQ = 5,
+VHOST_USER_PROTOCOL_F_CROSS_ENDIAN = 6,
+VHOST_USER_PROTOCOL_F_CRYPTO_SESSION = 7,
+VHOST_USER_PROTOCOL_F_PAGEFAULT = 8,
+VHOST_USER_PROTOCOL_F_CONFIG = 9,
+VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD = 10,
+VHOST_USER_PROTOCOL_F_HOST_NOTIFIER = 11,
+VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD = 12,
+VHOST_USER_PROTOCOL_F_RESET_DEVICE = 13,
+VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS = 14,
+VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS = 15,
+VHOST_USER_PROTOCOL_F_MAX
+};
+
+/* Virtio transport features mapping */
+static const qmp_virtio_feature_map_t virtio_transport_map[] = {
+/* Virtio device transport features */
+#ifndef VIRTIO_CONFIG_NO_LEGACY
+FEATURE_ENTRY(VIRTIO_F_NOTIFY_ON_EMPTY, \
+"VIRTIO_F_NOTIFY_ON_EMPTY: Notify when device runs out of avail. "
+"descs. on VQ"),
+FEATURE_ENTRY(VIRTIO_F_ANY_LAYOUT, \
+"VIRTIO_F_ANY_LAYOUT: Device accepts arbitrary desc. layouts"),
+#endif /* !VIRTIO_CONFIG_NO_LEGACY */
+FEATURE_ENTRY(VIRTIO_F_VERSION_1, \
+"VIRTIO_F_VERSION_1: Device compliant for v1 spec (legacy)"),
+FEATURE_ENTRY(VIRTIO_F_IOMMU_PLATFORM, \
+"VIRTIO_F_IOMMU_PLATFORM: Device can be used on IOMMU platform"),
+FEATURE_ENTRY(VIRTIO_F_RING_PACKED, \
+"VIRTIO_F_RING_PACKED: Device supports packed VQ layout"),
+FEATURE_ENTRY(VIRTIO_F_IN_ORDER, \
+"VIRTIO_F_IN_ORDER: Device uses buffers in same order as made "
+"available by driver"),
+FEATURE_ENTRY(VIRTIO_F_ORDER_PLATFORM, \
+"VIRTIO_F_ORDER_PLATFORM: Memory accesses ordered by platform"),
+FEATURE_ENTRY(VIRTIO_F_SR_IOV, \
+"VIRTIO_F_SR_IOV: Device supports single root I/O virtualization"),
+/* Virtio ring transport features */
+FEATURE_ENTRY(VIRTIO_RING_F_INDIRECT_DESC, \
+"VIRTIO_RING_F_INDIRECT_DESC: Indirect descriptors supported"),
+FEATURE_ENTRY(VIRTIO_RING_F_EVENT_IDX, \
+"VIRTIO_RING_F_EVENT_IDX: Used & avail. event fields enabled"),
+{ -1, "" }
+};
+
+/* Vhost-user protocol features mapping */
+static const qmp_virtio_feature_map_t vhost_user_protocol_map[] = {
+FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_MQ, \
+"VHOST_USER_PROTOCOL_F_MQ: Multiqueue protocol supported"),
+FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_LOG_SHMFD, \
+"VHOST_USER_PROTOCOL_F_LOG_SHMFD: Shared log memory fd supported"),
+FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_RARP, \
+"VH

[PATCH-for-8.0 02/10] hw/virtio: Rename virtio_ss[] -> specific_virtio_ss[]

[Philippe Mathieu-Daudé]

Since virtio_ss[] is added to specific_ss[], rename it as
specific_virtio_ss[] to make it clearer.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/virtio/meson.build | 41 +
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index dfed1e7af5..23a980efaa 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -3,34 +3,34 @@ softmmu_virtio_ss.add(files('virtio-bus.c'))
 softmmu_virtio_ss.add(when: 'CONFIG_VIRTIO_PCI', if_true: 
files('virtio-pci.c'))
 softmmu_virtio_ss.add(when: 'CONFIG_VIRTIO_MMIO', if_true: 
files('virtio-mmio.c'))
 
-virtio_ss = ss.source_set()
-virtio_ss.add(files('virtio.c'))
+specific_virtio_ss = ss.source_set()
+specific_virtio_ss.add(files('virtio.c'))
 
 if have_vhost
-  virtio_ss.add(files('vhost.c', 'vhost-backend.c', 'vhost-iova-tree.c'))
+  specific_virtio_ss.add(files('vhost.c', 'vhost-backend.c', 
'vhost-iova-tree.c'))
   if have_vhost_user
-virtio_ss.add(files('vhost-user.c'))
+specific_virtio_ss.add(files('vhost-user.c'))
   endif
   if have_vhost_vdpa
-virtio_ss.add(files('vhost-vdpa.c', 'vhost-shadow-virtqueue.c'))
+specific_virtio_ss.add(files('vhost-vdpa.c', 'vhost-shadow-virtqueue.c'))
   endif
 else
   softmmu_virtio_ss.add(files('vhost-stub.c'))
 endif
 
-virtio_ss.add(when: 'CONFIG_VIRTIO_BALLOON', if_true: 
files('virtio-balloon.c'))
-virtio_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: files('virtio-crypto.c'))
-virtio_ss.add(when: 'CONFIG_VHOST_USER_FS', if_true: files('vhost-user-fs.c'))
-virtio_ss.add(when: 'CONFIG_VIRTIO_PMEM', if_true: files('virtio-pmem.c'))
-virtio_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock.c', 
'vhost-vsock-common.c'))
-virtio_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: 
files('vhost-user-vsock.c', 'vhost-vsock-common.c'))
-virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng.c'))
-virtio_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu.c'))
-virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem.c'))
-virtio_ss.add(when: 'CONFIG_VHOST_USER_I2C', if_true: 
files('vhost-user-i2c.c'))
-virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: 
files('vhost-user-rng.c'))
-virtio_ss.add(when: 'CONFIG_VHOST_USER_GPIO', if_true: 
files('vhost-user-gpio.c'))
-virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_GPIO'], if_true: 
files('vhost-user-gpio-pci.c'))
+specific_virtio_ss.add(when: 'CONFIG_VIRTIO_BALLOON', if_true: 
files('virtio-balloon.c'))
+specific_virtio_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: 
files('virtio-crypto.c'))
+specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_FS', if_true: 
files('vhost-user-fs.c'))
+specific_virtio_ss.add(when: 'CONFIG_VIRTIO_PMEM', if_true: 
files('virtio-pmem.c'))
+specific_virtio_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: 
files('vhost-vsock.c', 'vhost-vsock-common.c'))
+specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: 
files('vhost-user-vsock.c', 'vhost-vsock-common.c'))
+specific_virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: 
files('virtio-rng.c'))
+specific_virtio_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: 
files('virtio-iommu.c'))
+specific_virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: 
files('virtio-mem.c'))
+specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_I2C', if_true: 
files('vhost-user-i2c.c'))
+specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: 
files('vhost-user-rng.c'))
+specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_GPIO', if_true: 
files('vhost-user-gpio.c'))
+specific_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_GPIO'], 
if_true: files('vhost-user-gpio-pci.c'))
 
 virtio_pci_ss = ss.source_set()
 virtio_pci_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: 
files('vhost-vsock-pci.c'))
@@ -57,11 +57,12 @@ virtio_pci_ss.add(when: 'CONFIG_VIRTIO_PMEM', if_true: 
files('virtio-pmem-pci.c'
 virtio_pci_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: 
files('virtio-iommu-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: 
files('virtio-mem-pci.c'))
 
-virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)
+specific_virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)
 
-specific_ss.add_all(when: 'CONFIG_VIRTIO', if_true: virtio_ss)
 softmmu_ss.add_all(when: 'CONFIG_VIRTIO', if_true: softmmu_virtio_ss)
 softmmu_ss.add(when: 'CONFIG_VIRTIO', if_false: files('vhost-stub.c'))
 softmmu_ss.add(when: 'CONFIG_VIRTIO', if_false: files('virtio-stub.c'))
 softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('vhost-stub.c'))
 softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('virtio-stub.c'))
+
+specific_ss.add_all(when: 'CONFIG_VIRTIO', if_true: specific_virtio_ss)
-- 
2.38.1

[PATCH-for-8.0 03/10] hw/virtio: Constify qmp_virtio_feature_map_t[]

[Philippe Mathieu-Daudé]

These arrays are only accessed read-only, move them to .rodata.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/virtio/virtio.c | 34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 5817f4cbc9..f54cc23304 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -80,7 +80,7 @@ enum VhostUserProtocolFeature {
 };
 
 /* Virtio transport features mapping */
-static qmp_virtio_feature_map_t virtio_transport_map[] = {
+static const qmp_virtio_feature_map_t virtio_transport_map[] = {
 /* Virtio device transport features */
 #ifndef VIRTIO_CONFIG_NO_LEGACY
 FEATURE_ENTRY(VIRTIO_F_NOTIFY_ON_EMPTY, \
@@ -111,7 +111,7 @@ static qmp_virtio_feature_map_t virtio_transport_map[] = {
 };
 
 /* Vhost-user protocol features mapping */
-static qmp_virtio_feature_map_t vhost_user_protocol_map[] = {
+static const qmp_virtio_feature_map_t vhost_user_protocol_map[] = {
 FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_MQ, \
 "VHOST_USER_PROTOCOL_F_MQ: Multiqueue protocol supported"),
 FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_LOG_SHMFD, \
@@ -161,7 +161,7 @@ static qmp_virtio_feature_map_t vhost_user_protocol_map[] = 
{
 };
 
 /* virtio device configuration statuses */
-static qmp_virtio_feature_map_t virtio_config_status_map[] = {
+static const qmp_virtio_feature_map_t virtio_config_status_map[] = {
 FEATURE_ENTRY(VIRTIO_CONFIG_S_DRIVER_OK, \
 "VIRTIO_CONFIG_S_DRIVER_OK: Driver setup and ready"),
 FEATURE_ENTRY(VIRTIO_CONFIG_S_FEATURES_OK, \
@@ -179,7 +179,7 @@ static qmp_virtio_feature_map_t virtio_config_status_map[] 
= {
 };
 
 /* virtio-blk features mapping */
-qmp_virtio_feature_map_t virtio_blk_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_blk_feature_map[] = {
 FEATURE_ENTRY(VIRTIO_BLK_F_SIZE_MAX, \
 "VIRTIO_BLK_F_SIZE_MAX: Max segment size is size_max"),
 FEATURE_ENTRY(VIRTIO_BLK_F_SEG_MAX, \
@@ -218,7 +218,7 @@ qmp_virtio_feature_map_t virtio_blk_feature_map[] = {
 };
 
 /* virtio-serial features mapping */
-qmp_virtio_feature_map_t virtio_serial_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_serial_feature_map[] = {
 FEATURE_ENTRY(VIRTIO_CONSOLE_F_SIZE, \
 "VIRTIO_CONSOLE_F_SIZE: Host providing console size"),
 FEATURE_ENTRY(VIRTIO_CONSOLE_F_MULTIPORT, \
@@ -229,7 +229,7 @@ qmp_virtio_feature_map_t virtio_serial_feature_map[] = {
 };
 
 /* virtio-gpu features mapping */
-qmp_virtio_feature_map_t virtio_gpu_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_gpu_feature_map[] = {
 FEATURE_ENTRY(VIRTIO_GPU_F_VIRGL, \
 "VIRTIO_GPU_F_VIRGL: Virgl 3D mode supported"),
 FEATURE_ENTRY(VIRTIO_GPU_F_EDID, \
@@ -250,7 +250,7 @@ qmp_virtio_feature_map_t virtio_gpu_feature_map[] = {
 };
 
 /* virtio-input features mapping */
-qmp_virtio_feature_map_t virtio_input_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_input_feature_map[] = {
 FEATURE_ENTRY(VHOST_F_LOG_ALL, \
 "VHOST_F_LOG_ALL: Logging write descriptors supported"),
 FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
@@ -260,7 +260,7 @@ qmp_virtio_feature_map_t virtio_input_feature_map[] = {
 };
 
 /* virtio-net features mapping */
-qmp_virtio_feature_map_t virtio_net_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_net_feature_map[] = {
 FEATURE_ENTRY(VIRTIO_NET_F_CSUM, \
 "VIRTIO_NET_F_CSUM: Device handling packets with partial checksum "
 "supported"),
@@ -338,7 +338,7 @@ qmp_virtio_feature_map_t virtio_net_feature_map[] = {
 };
 
 /* virtio-scsi features mapping */
-qmp_virtio_feature_map_t virtio_scsi_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_scsi_feature_map[] = {
 FEATURE_ENTRY(VIRTIO_SCSI_F_INOUT, \
 "VIRTIO_SCSI_F_INOUT: Requests including read and writable data "
 "buffers suppoted"),
@@ -359,7 +359,7 @@ qmp_virtio_feature_map_t virtio_scsi_feature_map[] = {
 };
 
 /* virtio/vhost-user-fs features mapping */
-qmp_virtio_feature_map_t virtio_fs_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_fs_feature_map[] = {
 FEATURE_ENTRY(VHOST_F_LOG_ALL, \
 "VHOST_F_LOG_ALL: Logging write descriptors supported"),
 FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
@@ -369,7 +369,7 @@ qmp_virtio_feature_map_t virtio_fs_feature_map[] = {
 };
 
 /* virtio/vhost-user-i2c features mapping */
-qmp_virtio_feature_map_t virtio_i2c_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_i2c_feature_map[] = {
 FEATURE_ENTRY(VIRTIO_I2C_F_ZERO_LENGTH_REQUEST, \
 "VIRTIO_I2C_F_ZERO_LEGNTH_REQUEST: Zero length requests 
supported"),
 FEATURE_ENTRY(VHOST_F_LOG_ALL, \
@@ -381,7 +381,7 @@ qmp_virtio_feature_map_t virtio_i2c_feature_map[] = {
 };
 
 /* virtio/vhost-vsock features mapping */
-qmp_virtio_feature_map_t virtio_vsock_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_vsock_feature_map[] = {
 FEATURE_ENTRY(VIRT

[PATCH-for-8.0 04/10] hw/virtio: Extract config read/write accessors to virtio-config.c

[Philippe Mathieu-Daudé]

These config helpers use the target-dependent LD/ST API.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/virtio/meson.build |   1 +
 hw/virtio/virtio-config.c | 204 ++
 hw/virtio/virtio.c| 190 ---
 3 files changed, 205 insertions(+), 190 deletions(-)
 create mode 100644 hw/virtio/virtio-config.c

diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index 23a980efaa..097220b4b5 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -5,6 +5,7 @@ softmmu_virtio_ss.add(when: 'CONFIG_VIRTIO_MMIO', if_true: 
files('virtio-mmio.c'
 
 specific_virtio_ss = ss.source_set()
 specific_virtio_ss.add(files('virtio.c'))
+specific_virtio_ss.add(files('virtio-config.c'))
 
 if have_vhost
   specific_virtio_ss.add(files('vhost.c', 'vhost-backend.c', 
'vhost-iova-tree.c'))
diff --git a/hw/virtio/virtio-config.c b/hw/virtio/virtio-config.c
new file mode 100644
index 00..ad78e0b9bc
--- /dev/null
+++ b/hw/virtio/virtio-config.c
@@ -0,0 +1,204 @@
+/*
+ * Virtio Support
+ *
+ * Copyright IBM, Corp. 2007
+ *
+ * Authors:
+ *  Anthony Liguori   
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/virtio/virtio.h"
+#include "cpu.h"
+
+uint32_t virtio_config_readb(VirtIODevice *vdev, uint32_t addr)
+{
+VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+uint8_t val;
+
+if (addr + sizeof(val) > vdev->config_len) {
+return (uint32_t)-1;
+}
+
+k->get_config(vdev, vdev->config);
+
+val = ldub_p(vdev->config + addr);
+return val;
+}
+
+uint32_t virtio_config_readw(VirtIODevice *vdev, uint32_t addr)
+{
+VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+uint16_t val;
+
+if (addr + sizeof(val) > vdev->config_len) {
+return (uint32_t)-1;
+}
+
+k->get_config(vdev, vdev->config);
+
+val = lduw_p(vdev->config + addr);
+return val;
+}
+
+uint32_t virtio_config_readl(VirtIODevice *vdev, uint32_t addr)
+{
+VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+uint32_t val;
+
+if (addr + sizeof(val) > vdev->config_len) {
+return (uint32_t)-1;
+}
+
+k->get_config(vdev, vdev->config);
+
+val = ldl_p(vdev->config + addr);
+return val;
+}
+
+void virtio_config_writeb(VirtIODevice *vdev, uint32_t addr, uint32_t data)
+{
+VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+uint8_t val = data;
+
+if (addr + sizeof(val) > vdev->config_len) {
+return;
+}
+
+stb_p(vdev->config + addr, val);
+
+if (k->set_config) {
+k->set_config(vdev, vdev->config);
+}
+}
+
+void virtio_config_writew(VirtIODevice *vdev, uint32_t addr, uint32_t data)
+{
+VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+uint16_t val = data;
+
+if (addr + sizeof(val) > vdev->config_len) {
+return;
+}
+
+stw_p(vdev->config + addr, val);
+
+if (k->set_config) {
+k->set_config(vdev, vdev->config);
+}
+}
+
+void virtio_config_writel(VirtIODevice *vdev, uint32_t addr, uint32_t data)
+{
+VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+uint32_t val = data;
+
+if (addr + sizeof(val) > vdev->config_len) {
+return;
+}
+
+stl_p(vdev->config + addr, val);
+
+if (k->set_config) {
+k->set_config(vdev, vdev->config);
+}
+}
+
+uint32_t virtio_config_modern_readb(VirtIODevice *vdev, uint32_t addr)
+{
+VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+uint8_t val;
+
+if (addr + sizeof(val) > vdev->config_len) {
+return (uint32_t)-1;
+}
+
+k->get_config(vdev, vdev->config);
+
+val = ldub_p(vdev->config + addr);
+return val;
+}
+
+uint32_t virtio_config_modern_readw(VirtIODevice *vdev, uint32_t addr)
+{
+VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+uint16_t val;
+
+if (addr + sizeof(val) > vdev->config_len) {
+return (uint32_t)-1;
+}
+
+k->get_config(vdev, vdev->config);
+
+val = lduw_le_p(vdev->config + addr);
+return val;
+}
+
+uint32_t virtio_config_modern_readl(VirtIODevice *vdev, uint32_t addr)
+{
+VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+uint32_t val;
+
+if (addr + sizeof(val) > vdev->config_len) {
+return (uint32_t)-1;
+}
+
+k->get_config(vdev, vdev->config);
+
+val = ldl_le_p(vdev->config + addr);
+return val;
+}
+
+void virtio_config_modern_writeb(VirtIODevice *vdev,
+ uint32_t addr, uint32_t data)
+{
+VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+uint8_t val = data;
+
+if (addr + sizeof(val) > vdev->config_len) {
+return;
+}
+
+stb_p(vdev->config + addr, val);
+
+if (k->set_config) {
+k->set_config(vdev, vdev->config);
+}
+}
+
+void virtio_config_modern_writew(VirtIODevice *vdev,
+ uint32_t addr, uint32_t data)
+{
+VirtioDeviceClass *k = VIRTIO_D

[RFC PATCH-for-8.0 00/10] hw/virtio: Build most objects as target independent units

[Philippe Mathieu-Daudé]

Currently the inlined virtio_access_is_big_endian() function
"hw/virtio/virtio-access.h" which is used by all I/O accesses
force any virtio device to be built as target-dependent object.

This series isolates the few VirtIO target specific bits, trying
to not impact the performance (a function is un-inlined once
not in the hot path).

At the end only 6 files remain in the target specific source set,
all other files are built once.

On a Linux/x86_64 host when building QEMU configured with
--disable-user, before this series a "make clean all" builds
7713 objects, after it only build 6831... I don't think my maths
are correct, because that would save building a few hundreds objects.

RFC because only build-tested.

Regards,

Phil.

Philippe Mathieu-Daudé (10):
  hw/virtio: Add missing "hw/core/cpu.h" include
  hw/virtio: Rename virtio_ss[] -> specific_virtio_ss[]
  hw/virtio: Constify qmp_virtio_feature_map_t[]
  hw/virtio: Extract config read/write accessors to virtio-config.c
  hw/virtio: Extract QMP related code virtio-qmp.c
  hw/virtio: Cache access_is_big_endian value in VirtIODevice state
  hw/virtio: Directly access cached VirtIODevice::access_is_big_endian
  hw/virtio: Un-inline virtio_access_is_big_endian()
  hw/virtio: Extract vhost_user_ram_slots_max() to vhost-user-target.c
  hw/virtio: Make most of virtio devices target-independent

 hw/9pfs/meson.build   |   2 +-
 hw/block/dataplane/meson.build|   2 +-
 hw/block/meson.build  |   4 +-
 hw/char/meson.build   |   2 +-
 hw/net/meson.build|   2 +-
 hw/virtio/meson.build |  45 +-
 hw/virtio/vhost-user-target.c |  29 ++
 hw/virtio/vhost-user.c|  26 +-
 hw/virtio/virtio-config.c | 224 +
 hw/virtio/virtio-qmp.c| 631 +++
 hw/virtio/virtio-qmp.h|  20 +
 hw/virtio/virtio.c| 799 +-
 include/hw/virtio/vhost-user.h|   7 +
 include/hw/virtio/virtio-access.h |  63 +--
 include/hw/virtio/virtio.h|   1 +
 15 files changed, 974 insertions(+), 883 deletions(-)
 create mode 100644 hw/virtio/vhost-user-target.c
 create mode 100644 hw/virtio/virtio-config.c
 create mode 100644 hw/virtio/virtio-qmp.c
 create mode 100644 hw/virtio/virtio-qmp.h

-- 
2.38.1

Re: [PATCH v4 18/27] tcg/s390x: Tighten constraints for and_i64

[Ilya Leoshkevich]

On Thu, Dec 08, 2022 at 08:05:21PM -0600, Richard Henderson wrote:
> Let the register allocator handle such immediates by matching
> only what one insn can achieve.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Ilya Leoshkevich 

Re: [PATCH] tpm: add backend for mssim

[Stefan Berger]

On 12/12/22 17:27, James Bottomley wrote:
.


Swtpm currently isn't building for Leap:

https://build.opensuse.org/package/show/security/swtpm


Someone could have notified me...

Re: [PATCH v4 17/27] tcg/s390x: Tighten constraints for or_i64 and xor_i64

[Ilya Leoshkevich]

On Thu, Dec 08, 2022 at 08:05:20PM -0600, Richard Henderson wrote:
> Drop support for sequential OR and XOR, as the serial dependency is
> slower than loading the constant first.  Let the register allocator
> handle such immediates by matching only what one insn can achieve.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Ilya Leoshkevich 

Re: [PATCH v4 16/27] tcg/s390x: Issue XILF directly for xor_i32

[Ilya Leoshkevich]

On Thu, Dec 08, 2022 at 08:05:19PM -0600, Richard Henderson wrote:
> There is only one instruction that is applicable
> to a 32-bit immediate xor.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Ilya Leoshkevich 

Re: [PATCH v4 10/27] tcg/s390x: Remove DISTINCT_OPERANDS facility check

[Ilya Leoshkevich]

On Thu, Dec 08, 2022 at 08:05:13PM -0600, Richard Henderson wrote:
> The distinct-operands facility is bundled into facility 45,
> along with load-on-condition.  We are checking this at startup.
> Remove the a0 == a1 checks for 64-bit sub, and, or, xor, as there
> is no space savings for avoiding the distinct-operands insn.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Ilya Leoshkevich 

Re: [PATCH] tpm: add backend for mssim

[James Bottomley]

On Mon, 2022-12-12 at 17:02 -0500, Stefan Berger wrote:
> 
> 
> On 12/12/22 16:36, James Bottomley wrote:
> > On Mon, 2022-12-12 at 14:32 -0500, Stefan Berger wrote:
[...]
> > >   Either way, what is the latency that this introduces because I
> > > would expect that this slows down IMA since the PCR extensions &
> > > TPM 2 response now go back and forth across the network?
> > 
> > Most data centre protocols are now encrypted and networked (NVMeoF
> > would probably be the poster child) with no real ill effects.  In
> > terms of a TPM, the competition is an underpowered discrete chip
> > over a slow serial bus, so I think we'll actually improve the
> > latency not diminish it.
> 
> Compared to QEMU and swtpm talking over a local socket you probably
> have a decent amount of slow-down if this is over the network.

I can only repeat that doesn't happen with other much more volume and
latency bound networked protocols.

> I still fail to see the advantage over what we have at the moment.
> Also I don't see what advantage the mssim protocol brings over what
> swtpm provides.

I think I've said a couple of times now: The primary advantage is that
it talks to the reference implementation over its native protocol.

>  If you are willing to do a 'dnf -y install swtpm_setup' and start
> the VM via libvirt it really doesn't matter what protocol the TPM is
> running underneath since it's all transparent.

Swtpm currently isn't building for Leap:

https://build.opensuse.org/package/show/security/swtpm

And, as I said, this is primarily for testing, so I need the reference
implementation ... swtpm has started deviating from it.

James

Re: [PATCH v4 08/27] tcg/s390x: Check for load-on-condition facility at startup

[Ilya Leoshkevich]

On Thu, Dec 08, 2022 at 08:05:11PM -0600, Richard Henderson wrote:
> The general-instruction-extension facility was introduced in z196,
> which itself was end-of-life in 2021.  In addition, z196 is the
> minimum CPU supported by our set of supported operating systems:
> RHEL 7 (z196), SLES 12 (z196) and Ubuntu 16.04 (zEC12).
> 
> Check for facility number 45, which will be the consilidated check
> for several facilities.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Ilya Leoshkevich 

Re: [RFC PATCH v2 02/22] xen: add CONFIG_XENFV_MACHINE and CONFIG_XEN_EMU options for Xen emulation

[David Woodhouse]

On Mon, 2022-12-12 at 18:07 +0100, Paolo Bonzini wrote:
> On 12/9/22 10:55, David Woodhouse wrote:
> >   config KVM
> >   bool
> > +imply XEN_EMU if (I386 || X86_64)
> 
> No need for the "imply", just make it "default y" below and it will have 
> the same effect.
> 
> > 
> > diff --git a/target/Kconfig b/target/Kconfig
> > index 83da0bd293..e19c9d77b5 100644
> > --- a/target/Kconfig
> > +++ b/target/Kconfig
> > @@ -18,3 +18,7 @@ source sh4/Kconfig
> >  source sparc/Kconfig
> >  source tricore/Kconfig
> >  source xtensa/Kconfig
> > +
> > +config XEN_EMU
> > +bool
> > +depends on KVM && (I386 || X86_64)
> 
> Please place it in hw/xen/Kconfig.

Perhaps I misunderstand, but I'm not sure that is consistent with what
Philippe was asking for in  
https://lore.kernel.org/qemu-devel/d203e13d-e2f9-5816-030d-c1449bde3...@linaro.org/
specifically:

>> I rather have hw/ and target/ features disentangled, so I'd use
>> CONFIG_XEN_EMU under target/ and CONFIG_XENFV_MACHINE under hw/,
>> eventually having CONFIG_XEN_EMU depending on CONFIG_XENFV_MACHINE
>> and -- for now -- CONFIG_KVM.

The idea there seems to be that XEN_EMU is a *target* feature since it
covers the support in target/i386/kvm.

But yes, it *also* covers the devices I'm adding to hw/i386/kvm. Do I
want a *separate* config symbol for that? Or just make those depend on
XENFV_MACHINE && XEN_EMU ? 

I'll move XEN_EMU to hw/i386/Kconfig for now, thereby doing what
*neither* of you said (I don't think hw/xen/Kconfig is the best choice
when the *code* it enables is under hw/i386/kvm?)




smime.p7s
Description: S/MIME cryptographic signature

Re: [PATCH v4 07/27] tcg/s390x: Check for general-instruction-extension facility at startup

[Ilya Leoshkevich]

On Thu, Dec 08, 2022 at 08:05:10PM -0600, Richard Henderson wrote:
> The general-instruction-extension facility was introduced in z10,
> which itself was end-of-life in 2019.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/s390x/tcg-target.h |  10 ++--
>  tcg/s390x/tcg-target.c.inc | 100 -
>  2 files changed, 49 insertions(+), 61 deletions(-)

Reviewed-by: Ilya Leoshkevich 

Re: [PATCH 1/3] include/block: Untangle inclusion loops

[Paolo Bonzini]

On 12/8/22 15:39, Markus Armbruster wrote:

   * Global state (GS) API. These functions run under the BQL.
   *
   * See include/block/block-global-state.h for more information about
- * the GS API.
+ * the GS API.b
   */


One-character typo.

Paolo

Re: [PATCH v4 06/27] tcg/s390x: Check for extended-immediate facility at startup

[Ilya Leoshkevich]

On Thu, Dec 08, 2022 at 08:05:09PM -0600, Richard Henderson wrote:
> The extended-immediate facility was introduced in z9-109,
> which itself was end-of-life in 2017.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/s390x/tcg-target.h |   4 +-
>  tcg/s390x/tcg-target.c.inc | 231 +++--
>  2 files changed, 72 insertions(+), 163 deletions(-)

Reviewed-by: Ilya Leoshkevich 

Re: [PATCH v3 0/2] hw/usb: add configuration flags for emulated and passthru usb smartcard

[Philippe Mathieu-Daudé]

On 12/12/22 23:09, Jon Maloy wrote:

We add three new configuration flags, LIBCACARD, USB_SMARTCARD_PASSTHRU
and USB_SMARTCARD_EMULATED in order to improve configurability of these
functionalities.



Jon Maloy (2):
   hw/usb: add configuration flags for emulated and passthru usb
 smartcard
   hw/usb: add configuration flag for Common Access Card library code


Series:
Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH] linux-user: Add emulation for MADV_WIPEONFORK and MADV_KEEPONFORK in madvise()

[Ilya Leoshkevich]

On Mon, Dec 12, 2022 at 10:49:24PM +0100, Helge Deller wrote:
> On 12/12/22 22:16, Ilya Leoshkevich wrote:
> > On Mon, Dec 12, 2022 at 08:00:45AM +0100, Helge Deller wrote:
> > > Both parameters have a different value on the parisc platform, so first
> > > translate the target value into a host value for usage in the native
> > > madvise() syscall.
> > > 
> > > Those parameters are often used by security sensitive applications (e.g.
> > > tor browser, boringssl, ...) which expect the call to return a proper
> > > return code on failure, so return -EINVAL if qemu fails to forward the
> > > syscall to the host OS.
> > > 
> > > Tested with testcase of tor browser when running hppa-linux guest on
> > > x86-64 host.
> > > 
> > > Signed-off-by: Helge Deller 
> > > 
> > > diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> > > index 10f5079331..c75342108c 100644
> > > --- a/linux-user/mmap.c
> > > +++ b/linux-user/mmap.c
> > > @@ -901,11 +901,25 @@ abi_long target_madvise(abi_ulong start, abi_ulong 
> > > len_in, int advice)
> > >   return -TARGET_EINVAL;
> > >   }
> > > 
> > > +/* Translate for some architectures which have different MADV_xxx 
> > > values */
> > > +switch (advice) {
> > > +case TARGET_MADV_DONTNEED:  /* alpha */
> > > +advice = MADV_DONTNEED;
> > > +break;
> > > +case TARGET_MADV_WIPEONFORK:/* parisc */
> > > +advice = MADV_WIPEONFORK;
> > > +break;
> > > +case TARGET_MADV_KEEPONFORK:/* parisc */
> > > +advice = MADV_KEEPONFORK;
> > > +break;
> > > +/* we do not care about the other MADV_xxx values yet */
> > > +}
> > > +
> > >   /*
> > >* A straight passthrough may not be safe because qemu sometimes 
> > > turns
> > >* private file-backed mappings into anonymous mappings.
> > >*
> > > - * This is a hint, so ignoring and returning success is ok.
> > > + * For MADV_DONTNEED, which is a hint, ignoring and returning 
> > > success is ok.
> > 
> > Actually, MADV_DONTNEED is one of the few values, which is not always a
> > hint - it can be used to e.g. zero out pages.
> 
> Right, it _should_ zero out pages and return 0, or otherwise return failure.
> I think the problem is that some userspace apps will then sadly break if we
> change the current behaviour
> 
> Anyway, in this patch I didn't wanted to touch MAD_DONTNEED.
> 
> > As the next paragraph states, strictly speaking, MADV_DONTNEED is
> > currently broken, because it can indeed be ignored without indication
> > in some cases, but it's still arguably better than not honoring it at
> > all.
> 
> Yep.
> 
> > >*
> > >* This breaks MADV_DONTNEED, completely implementing which is quite
> > >* complicated. However, there is one low-hanging fruit: mappings 
> > > that are
> > > @@ -913,11 +927,17 @@ abi_long target_madvise(abi_ulong start, abi_ulong 
> > > len_in, int advice)
> > >* passthrough is safe, so do it.
> > >*/
> > >   mmap_lock();
> > > -if (advice == TARGET_MADV_DONTNEED &&
> > > -can_passthrough_madv_dontneed(start, end)) {
> > > -ret = get_errno(madvise(g2h_untagged(start), len, 
> > > MADV_DONTNEED));
> > > -if (ret == 0) {
> > > -page_reset_target_data(start, start + len);
> > > +switch (advice) {
> > > +case MADV_WIPEONFORK:
> > > +case MADV_KEEPONFORK:
> > > +ret = -EINVAL;
> > > +/* fall through */
> > > +case MADV_DONTNEED:
> > > +if (can_passthrough_madv_dontneed(start, end)) {
> > > +ret = get_errno(madvise(g2h_untagged(start), len, advice));
> > > +if ((advice == MADV_DONTNEED) && (ret == 0)) {
> > > +page_reset_target_data(start, start + len);
> > > +}
> > >   }
> > >   }
> > >   mmap_unlock();
> > > 
> > 
> > Nit: maybe rename can_passthrough_madv_dontneed() to can_passthrough(),
> > since now it's used not only for MADV_DONTNEED?
> 
> Maybe can_passthrough_madvise() is better?

Sounds good to me as well. The idea with PAGE_PASSTHROUGH was that we
should be able to passthrough anything; but with this patch we still
use it only for madvise(), and indicating it in the name makes sense.

> > > With the MADV_DONTNEED comment change:
> 
> Just for me to understand correctly:
> You propose that I shouldn't touch that comment in my followup-patch, right?
> That's ok.

Either that, or maybe make it more precise - strictly speaking, it's
not correct at the moment anyway. Maybe something like this?

Most advice values are hints, so ignoring and returning success is
ok.

However, this would break MADV_DONTNEED, MADV_WIPEONFORK and
MADV_KEEPONFORK ...

> > Acked-by: Ilya Leoshkevich 
> 
> Thanks!
> Helge

[PATCH v3 1/2] hw/usb: add configuration flags for emulated and passthru usb smartcard

[Jon Maloy]

We add two new configuration flags, USB_SMARTCARD_PASSTHRU and
USB_SMARTCARD_EMULATED in order to improve configurability of these
functionalities.

Signed-off-by: Jon Maloy 
---
 hw/usb/Kconfig | 12 
 hw/usb/meson.build |  4 ++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index ce4f433976..6b29e91593 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -109,6 +109,18 @@ config USB_SMARTCARD
 default y
 depends on USB
 
+config USB_SMARTCARD_PASSTHRU
+bool
+default y
+depends on USB
+select USB_SMARTCARD
+
+config USB_SMARTCARD_EMULATED
+bool
+default y
+depends on USB
+select USB_SMARTCARD
+
 config USB_STORAGE_MTP
 bool
 default y
diff --git a/hw/usb/meson.build b/hw/usb/meson.build
index 793df42e21..353006fb6c 100644
--- a/hw/usb/meson.build
+++ b/hw/usb/meson.build
@@ -51,8 +51,8 @@ softmmu_ss.add(when: 'CONFIG_USB_SMARTCARD', if_true: 
files('dev-smartcard-reade
 
 if cacard.found()
   usbsmartcard_ss = ss.source_set()
-  usbsmartcard_ss.add(when: 'CONFIG_USB_SMARTCARD',
-  if_true: [cacard, files('ccid-card-emulated.c', 
'ccid-card-passthru.c')])
+  usbsmartcard_ss.add(when: 'CONFIG_USB_SMARTCARD_EMULATED', if_true: [cacard, 
files('ccid-card-emulated.c')])
+  usbsmartcard_ss.add(when: 'CONFIG_USB_SMARTCARD_PASSTHRU', if_true: [cacard, 
files('ccid-card-passthru.c')])
   hw_usb_modules += {'smartcard': usbsmartcard_ss}
 endif
 
-- 
2.35.3

[PATCH v3 2/2] hw/usb: add configuration flag for Common Access Card library code

[Jon Maloy]

We add a new configuration flag, LIBCACARD, indicating availability of the
libcacard code for building. This way, we can eliminate the explicit test
for cacard.found() when configuring 
USB_SMARTCARD_EMULATED/USB_SMARTCARD_PASSTHRU
in hw/usb/meson.build.

Signed-off-by: Jon Maloy 
---
 Kconfig.host   |  3 +++
 hw/usb/Kconfig |  2 ++
 hw/usb/meson.build | 11 ---
 meson.build|  1 +
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/Kconfig.host b/Kconfig.host
index d763d89269..d7fd4a2203 100644
--- a/Kconfig.host
+++ b/Kconfig.host
@@ -39,6 +39,9 @@ config MULTIPROCESS_ALLOWED
 bool
 imply MULTIPROCESS
 
+config LIBCACARD
+bool
+
 config FUZZ
 bool
 select SPARSE_MEM
diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index 6b29e91593..5c3da7c34d 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -113,12 +113,14 @@ config USB_SMARTCARD_PASSTHRU
 bool
 default y
 depends on USB
+depends on LIBCACARD
 select USB_SMARTCARD
 
 config USB_SMARTCARD_EMULATED
 bool
 default y
 depends on USB
+depends on LIBCACARD
 select USB_SMARTCARD
 
 config USB_STORAGE_MTP
diff --git a/hw/usb/meson.build b/hw/usb/meson.build
index 353006fb6c..499be1122c 100644
--- a/hw/usb/meson.build
+++ b/hw/usb/meson.build
@@ -48,13 +48,10 @@ softmmu_ss.add(when: ['CONFIG_POSIX', 
'CONFIG_USB_STORAGE_MTP'], if_true: files(
 
 # smartcard
 softmmu_ss.add(when: 'CONFIG_USB_SMARTCARD', if_true: 
files('dev-smartcard-reader.c'))
-
-if cacard.found()
-  usbsmartcard_ss = ss.source_set()
-  usbsmartcard_ss.add(when: 'CONFIG_USB_SMARTCARD_EMULATED', if_true: [cacard, 
files('ccid-card-emulated.c')])
-  usbsmartcard_ss.add(when: 'CONFIG_USB_SMARTCARD_PASSTHRU', if_true: [cacard, 
files('ccid-card-passthru.c')])
-  hw_usb_modules += {'smartcard': usbsmartcard_ss}
-endif
+usbsmartcard_ss = ss.source_set()
+usbsmartcard_ss.add(when: 'CONFIG_USB_SMARTCARD_EMULATED', if_true: [cacard, 
files('ccid-card-emulated.c')])
+usbsmartcard_ss.add(when: 'CONFIG_USB_SMARTCARD_PASSTHRU', if_true: [cacard, 
files('ccid-card-passthru.c')])
+hw_usb_modules += {'smartcard': usbsmartcard_ss}
 
 # U2F
 softmmu_ss.add(when: 'CONFIG_USB_U2F', if_true: files('u2f.c'))
diff --git a/meson.build b/meson.build
index 5c6b5a1c75..10e9b77ec1 100644
--- a/meson.build
+++ b/meson.build
@@ -2493,6 +2493,7 @@ have_ivshmem = config_host_data.get('CONFIG_EVENTFD')
 host_kconfig = \
   (get_option('fuzzing') ? ['CONFIG_FUZZ=y'] : []) + \
   (have_tpm ? ['CONFIG_TPM=y'] : []) + \
+  (cacard.found() ? ['CONFIG_LIBCACARD=y'] : []) + \
   (spice.found() ? ['CONFIG_SPICE=y'] : []) + \
   (have_ivshmem ? ['CONFIG_IVSHMEM=y'] : []) + \
   (opengl.found() ? ['CONFIG_OPENGL=y'] : []) + \
-- 
2.35.3

[PATCH v3 0/2] hw/usb: add configuration flags for emulated and passthru usb smartcard

[Jon Maloy]

We add three new configuration flags, LIBCACARD, USB_SMARTCARD_PASSTHRU
and USB_SMARTCARD_EMULATED in order to improve configurability of these
functionalities.

Signed-off-by: Jon Maloy 

---
v2: Added a LIBACARD flag, plus reversed 'select' clauses, as suggested
by Paolo Bonzini and Marc-André Lureau.

v3: Split in two commits, so that LIBCACARD is added separately, as suggested
by Philippe Mathieu-Daudé.


Jon Maloy (2):
  hw/usb: add configuration flags for emulated and passthru usb
smartcard
  hw/usb: add configuration flag for Common Access Card library code

 Kconfig.host   |  3 +++
 hw/usb/Kconfig | 14 ++
 hw/usb/meson.build | 11 ---
 meson.build|  1 +
 4 files changed, 22 insertions(+), 7 deletions(-)

-- 
2.35.3

Re: [PATCH v4 09/27] tcg/s390x: Remove FAST_BCR_SER facility check

[Philippe Mathieu-Daudé]

On 9/12/22 03:05, Richard Henderson wrote:

The fast-bcr-serialization facility is bundled into facility 45,
along with load-on-condition.  We are checking this at startup.

Signed-off-by: Richard Henderson 
---
  tcg/s390x/tcg-target.h | 1 -
  tcg/s390x/tcg-target.c.inc | 3 ++-
  2 files changed, 2 insertions(+), 2 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [RFC PATCH v2 05/22] xen-platform-pci: allow its creation with XEN_EMULATE mode

[David Woodhouse]

On Mon, 2022-12-12 at 13:24 +, Paul Durrant wrote:
> On 09/12/2022 09:55, David Woodhouse wrote:
> > --- a/hw/i386/xen/xen_platform.c
> > +++ b/hw/i386/xen/xen_platform.c
> > @@ -271,7 +271,10 @@ static void platform_fixed_ioport_writeb(void *opaque, 
> > uint32_t addr, uint32_t v
> >case 0: /* Platform flags */ {
> >hvmmem_type_t mem_type = (val & PFFLAG_ROM_LOCK) ?
> >HVMMEM_ram_ro : HVMMEM_ram_rw;
> > -if (xen_set_mem_type(xen_domid, mem_type, 0xc0, 0x40)) {
> > +if (xen_mode == XEN_EMULATE) {
> > +/* XXX */
> > +s->flags = val & PFFLAG_ROM_LOCK;
> > +} else if (xen_set_mem_type(xen_domid, mem_type, 0xc0, 0x40)) {
> >DPRINTF("unable to change ro/rw state of ROM memory 
> > area!\n");
> >} else {
> >s->flags = val & PFFLAG_ROM_LOCK;
> 
> 
> 
> Surely this would cleaner as:
> 
> 
> 
> if (xen_mode != XEN_EMULATE && xen_set_mem_type(xen_domid, mem_type, 0xc0, 
> 0x40))
>  DPRINTF("unable to change ro/rw state of ROM memory area!\n");
> else
>  s->flags = val & PFFLAG_ROM_LOCK;

Or maybe it should actually call into the PIIX code for frobbing the
read-only state of the UMBs? Do we even implement that in qemu? But
again, this part is just the necessary evil to make the thing testable
with -M xenfv for now.

I'm going to take a closer look at Paolo's suggestion which should
reduce the amount of such noise before we get to the real parts.



smime.p7s
Description: S/MIME cryptographic signature

Re: [PATCH] tpm: add backend for mssim

[James Bottomley]

On Mon, 2022-12-12 at 15:47 +, Daniel P. Berrangé wrote:
> Copy'ing Markus for QAPI design feedback.
> 
> On Sat, Dec 10, 2022 at 12:10:18PM -0500, James Bottomley wrote:
[...]
> > +##
> > +# @TPMmssimOptions:
> > +#
> > +# Information for the mssim emulator connection
> > +#
> > +# @host: host name or IP address to connect to
> > +# @port: port for the standard TPM commands
> > +# @ctrl: control port for TPM state changes
> > +#
> > +# Since: 7.2.0
> > +##
> > +{ 'struct': 'TPMmssimOptions',
> > +  'data': {
> > +  'host': 'str',
> > +  'port': 'str',
> > +  'ctrl': 'str' },
> > +  'if': 'CONFIG_TPM' }
> 
> We don't want to be adding new code using plain host/port combos,
> as that misses extra functionality for controlling IPv4 vs IPv6
> usage.
> 
> The existing 'emulator' backend references a chardev, but I'm
> not especially in favour of using the chardev indirection either,
> when all we should really need is a SocketAddress
> 
> IOW, from a QAPI design POV, IMHO the best practice would be
> 
>  { 'struct': 'TPMmssimOptions',
>    'data': {
>    'command': 'SocketAddress',
>    'control': 'SocketAddress' },
>    'if': 'CONFIG_TPM' }
> 
> 
> The main wrinkle with this is that exprssing nested struct fields
> with QemuOpts is a disaster zone, and -tpmdev doesn't yet support
> JSON syntax.
> 
> IMHO we should just fix the latter problem, as I don't think it
> ought to be too hard. Probably a cut+paste / search/replace job
> on the chanmge we did for -device in:
> 
>   commit 5dacda5167560b3af8eadbce5814f60ba44b467e
>   Author: Kevin Wolf 
>   Date:   Fri Oct 8 15:34:42 2021 +0200
> 
>     vl: Enable JSON syntax for -device
> 
> This would mean we could use plain -tpmdev for a local instance
> 
>    -tpmdev mssim,id=tpm0 \
>     -device tpm-crb,tpmdev=tpm0 \
> 
> but to use a remote emulator we would use
> 
>     -tpmdev "{'backend': 'mssim', 'id': 'tpm0',
>   'command': {
>  'type': 'inet',
>  'host': 'remote',
>  'port': '4455'
>    },
>   'control': {
>  'type': 'inet',
>  'host': 'remote',
>  'port': '4456'
>    }}"
> 
> (without the whitepace/newlines, which i just used for sake of
> clarity)

Just on this, might it not be easier for the commandline to do what
gluster does?  just use the '.' as a separator and subqdict extraction,
so you'd specify

-tpmdev 
mssim,id=tpm0,command.type=inet,command.host=remote,command.port=4455,control.type=inet,control.host=remote,control.port=4456

With the added bonus that X.type could be defaulted to inet and
control.host could follow command.host and so on?

James

Re: [PATCH] tpm: add backend for mssim

[Stefan Berger]

On 12/12/22 16:36, James Bottomley wrote:

On Mon, 2022-12-12 at 14:32 -0500, Stefan Berger wrote:



On 12/12/22 14:12, James Bottomley wrote:

On Mon, 2022-12-12 at 13:58 -0500, Stefan Berger wrote:

On 12/12/22 13:48, James Bottomley wrote:

On Mon, 2022-12-12 at 11:59 -0500, Stefan Berger wrote:

On 12/12/22 11:38, James Bottomley wrote:

[...]

the kernel use of the TPM, but I'm trying to fix that.  The
standard mssim server is too simplistic to do transport
layer
security, but like everything that does this (or rather
doesn't
do this), you can front it with stunnel4.


And who or what is going to set this up?


I'm not sure I understand the question.  Stunnel4 is mostly
used to
convert unencrypted proxies like imap on 143 or smtp on 25 to
the
secure version.  Most people who run servers are fairly
familiar
with using it.  It's what IBM used for encrypted migration
initially.  You can run stunnel on both ends, or the qemu side
could be built in using the qemu tls-creds way of doing things
but
anything running the standard MS server would have to front it
with
stunnel still.


So it's up to libvirt to setup stunnel to support a completely
different setup than what it has for swtpm already?


I don't think so, no.  Libvirt doesn't usually help with server
setup (witness the complexity of setting up a server side vtpm
proxy) so in the case tls-creds were built in, it would just work
if the object is


I see, so you are extending the TPM emulator with TLS on the client
side so you don't need another tool to setup a TLS connection from
the QEMU/client side.


I didn't say I would do this, just that it's an easy possibility with
the current qemu framework.  I actually need to fiddle with the TPM
externally to do some of my testing (like platform reset injection) so
I won't use TLS anyway.


Is the server side across the network or on the same host?


It can be either.


For the remote TPM you'll need some sort of management stack (who is building 
this?) that does the port assignments (allocations and freeing, starting of TPM 
instances etc) for the possibly many TPMs you would run on a remote machine and 
then create the libvirt XML or QEMU command line with the port assignments. I 
am not sure I see the advantage of this versus what we have at the moment with 
a single management stack . Also, if you did this you'd have a single point of 
failure for many VMs whose TPM is presumably running on some dedicated 
machine(s).




  Either way, what is the latency that this introduces because I would
expect that this slows down IMA since the PCR extensions & TPM 2
response now go back and forth across the network?


Most data centre protocols are now encrypted and networked (NVMeoF
would probably be the poster child) with no real ill effects.  In terms
of a TPM, the competition is an underpowered discrete chip over a slow
serial bus, so I think we'll actually improve the latency not diminish
it.


Compared to QEMU and swtpm talking over a local socket you probably have a 
decent amount of slow-down if this is over the network.
I still fail to see the advantage over what we have at the moment. Also I don't 
see what advantage the mssim protocol brings over what swtpm provides. If you 
are willing to do a 'dnf -y install swtpm_setup' and start the VM via libvirt 
it really doesn't matter what protocol the TPM is running underneath since it's 
all transparent.

   Stefan



James

Re: [PATCH] target/tricore: Fix gdbstub write to address registers

[Philippe Mathieu-Daudé]

On 12/12/22 21:49, Richard Henderson wrote:

Typo had double-writes to data registers.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1363
Signed-off-by: Richard Henderson 
---
  target/tricore/gdbstub.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH] include: Don't include qemu/osdep.h

[Alistair Francis]

On Mon, Dec 12, 2022 at 5:05 PM Markus Armbruster  wrote:
>
> docs/devel/style.rst mandates:
>
> The "qemu/osdep.h" header contains preprocessor macros that affect
> the behavior of core system headers like .  It must be
> the first include so that core system headers included by external
> libraries get the preprocessor macros that QEMU depends on.
>
> Do not include "qemu/osdep.h" from header files since the .c file
> will have already included it.
>
> A few violations have crept in.  Fix them.
>
> Signed-off-by: Markus Armbruster 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  bsd-user/qemu.h | 1 -
>  crypto/block-luks-priv.h| 1 -
>  include/hw/cxl/cxl_host.h   | 1 -
>  include/hw/input/pl050.h| 1 -
>  include/hw/tricore/triboard.h   | 1 -
>  include/qemu/userfaultfd.h  | 1 -
>  net/vmnet_int.h | 1 -
>  qga/cutils.h| 1 -
>  target/hexagon/hex_arch_types.h | 1 -
>  target/hexagon/mmvec/macros.h   | 1 -
>  target/riscv/pmu.h  | 1 -
>  qga/cutils.c| 3 ++-
>  12 files changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> index be6105385e..0ceecfb6df 100644
> --- a/bsd-user/qemu.h
> +++ b/bsd-user/qemu.h
> @@ -17,7 +17,6 @@
>  #ifndef QEMU_H
>  #define QEMU_H
>
> -#include "qemu/osdep.h"
>  #include "cpu.h"
>  #include "qemu/units.h"
>  #include "exec/cpu_ldst.h"
> diff --git a/crypto/block-luks-priv.h b/crypto/block-luks-priv.h
> index 90a20d432b..1066df0307 100644
> --- a/crypto/block-luks-priv.h
> +++ b/crypto/block-luks-priv.h
> @@ -18,7 +18,6 @@
>   *
>   */
>
> -#include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "qemu/bswap.h"
>
> diff --git a/include/hw/cxl/cxl_host.h b/include/hw/cxl/cxl_host.h
> index a1b662ce40..c9bc9c7c50 100644
> --- a/include/hw/cxl/cxl_host.h
> +++ b/include/hw/cxl/cxl_host.h
> @@ -7,7 +7,6 @@
>   * COPYING file in the top-level directory.
>   */
>
> -#include "qemu/osdep.h"
>  #include "hw/cxl/cxl.h"
>  #include "hw/boards.h"
>
> diff --git a/include/hw/input/pl050.h b/include/hw/input/pl050.h
> index 89ec4fafc9..4cb8985f31 100644
> --- a/include/hw/input/pl050.h
> +++ b/include/hw/input/pl050.h
> @@ -10,7 +10,6 @@
>  #ifndef HW_PL050_H
>  #define HW_PL050_H
>
> -#include "qemu/osdep.h"
>  #include "hw/sysbus.h"
>  #include "migration/vmstate.h"
>  #include "hw/input/ps2.h"
> diff --git a/include/hw/tricore/triboard.h b/include/hw/tricore/triboard.h
> index 094c8bd563..4fdd2d7d97 100644
> --- a/include/hw/tricore/triboard.h
> +++ b/include/hw/tricore/triboard.h
> @@ -18,7 +18,6 @@
>   * License along with this library; if not, see 
> .
>   */
>
> -#include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "hw/boards.h"
>  #include "sysemu/sysemu.h"
> diff --git a/include/qemu/userfaultfd.h b/include/qemu/userfaultfd.h
> index 6b74f92792..55c95998e8 100644
> --- a/include/qemu/userfaultfd.h
> +++ b/include/qemu/userfaultfd.h
> @@ -13,7 +13,6 @@
>  #ifndef USERFAULTFD_H
>  #define USERFAULTFD_H
>
> -#include "qemu/osdep.h"
>  #include "exec/hwaddr.h"
>  #include 
>
> diff --git a/net/vmnet_int.h b/net/vmnet_int.h
> index adf6e8c20d..d0b90594f2 100644
> --- a/net/vmnet_int.h
> +++ b/net/vmnet_int.h
> @@ -10,7 +10,6 @@
>  #ifndef VMNET_INT_H
>  #define VMNET_INT_H
>
> -#include "qemu/osdep.h"
>  #include "vmnet_int.h"
>  #include "clients.h"
>
> diff --git a/qga/cutils.h b/qga/cutils.h
> index f0f30a7d28..2bfaf554a8 100644
> --- a/qga/cutils.h
> +++ b/qga/cutils.h
> @@ -1,7 +1,6 @@
>  #ifndef CUTILS_H_
>  #define CUTILS_H_
>
> -#include "qemu/osdep.h"
>
>  int qga_open_cloexec(const char *name, int flags, mode_t mode);
>
> diff --git a/target/hexagon/hex_arch_types.h b/target/hexagon/hex_arch_types.h
> index 885f68f760..52a7f2b2f3 100644
> --- a/target/hexagon/hex_arch_types.h
> +++ b/target/hexagon/hex_arch_types.h
> @@ -18,7 +18,6 @@
>  #ifndef HEXAGON_HEX_ARCH_TYPES_H
>  #define HEXAGON_HEX_ARCH_TYPES_H
>
> -#include "qemu/osdep.h"
>  #include "mmvec/mmvec.h"
>  #include "qemu/int128.h"
>
> diff --git a/target/hexagon/mmvec/macros.h b/target/hexagon/mmvec/macros.h
> index 8345753580..6a463a7db3 100644
> --- a/target/hexagon/mmvec/macros.h
> +++ b/target/hexagon/mmvec/macros.h
> @@ -18,7 +18,6 @@
>  #ifndef HEXAGON_MMVEC_MACROS_H
>  #define HEXAGON_MMVEC_MACROS_H
>
> -#include "qemu/osdep.h"
>  #include "qemu/host-utils.h"
>  #include "arch.h"
>  #include "mmvec/system_ext_mmvec.h"
> diff --git a/target/riscv/pmu.h b/target/riscv/pmu.h
> index 3004ce37b6..0c819ca983 100644
> --- a/target/riscv/pmu.h
> +++ b/target/riscv/pmu.h
> @@ -16,7 +16,6 @@
>   * this program.  If not, see .
>   */
>
> -#include "qemu/osdep.h"
>  #include "qemu/log.h"
>  #include "cpu.h"
>  #include "qemu/main-loop.h"
> diff --git a/qga/cutils.c b/qga/cutils.c
> index b8e142ef64..b21bcf3683 100644
> --- a/qga/cutils.c
> +++ b/qga/cutils.c
> @@ -2,8 +2,9 @@
>

Re: [PATCH v4 05/27] tcg/s390x: Check for long-displacement facility at startup

[Ilya Leoshkevich]

On Thu, Dec 08, 2022 at 08:05:08PM -0600, Richard Henderson wrote:
> We are already assuming the existance of long-displacement, but were
> not being explicit about it.  This has been present since z990.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/s390x/tcg-target.h |  6 --
>  tcg/s390x/tcg-target.c.inc | 15 +++
>  2 files changed, 19 insertions(+), 2 deletions(-)

Reviewed-by: Ilya Leoshkevich 

Re: [PATCH v4 04/27] tcg/s390x: Remove USE_LONG_BRANCHES

[Ilya Leoshkevich]

On Thu, Dec 08, 2022 at 08:05:07PM -0600, Richard Henderson wrote:
> The size of a compiled TB is limited by the uint16_t used by
> gen_insn_end_off[] -- there is no need for a 32-bit branch.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/s390x/tcg-target.c.inc | 9 -
>  1 file changed, 9 deletions(-)

Reviewed-by: Ilya Leoshkevich 

Re: [PATCH v4 03/27] tcg/s390x: Always set TCG_TARGET_HAS_direct_jump

[Ilya Leoshkevich]

On Thu, Dec 08, 2022 at 08:05:06PM -0600, Richard Henderson wrote:
> Since USE_REG_TB is removed, there is no need to load the
> target TB address into a register.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/s390x/tcg-target.h |  2 +-
>  tcg/s390x/tcg-target.c.inc | 48 +++---
>  2 files changed, 10 insertions(+), 40 deletions(-)

Reviewed-by: Ilya Leoshkevich 

Re: [PATCH] linux-user: Add emulation for MADV_WIPEONFORK and MADV_KEEPONFORK in madvise()

[Helge Deller]

On 12/12/22 22:16, Ilya Leoshkevich wrote:

On Mon, Dec 12, 2022 at 08:00:45AM +0100, Helge Deller wrote:

Both parameters have a different value on the parisc platform, so first
translate the target value into a host value for usage in the native
madvise() syscall.

Those parameters are often used by security sensitive applications (e.g.
tor browser, boringssl, ...) which expect the call to return a proper
return code on failure, so return -EINVAL if qemu fails to forward the
syscall to the host OS.

Tested with testcase of tor browser when running hppa-linux guest on
x86-64 host.

Signed-off-by: Helge Deller 

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 10f5079331..c75342108c 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -901,11 +901,25 @@ abi_long target_madvise(abi_ulong start, abi_ulong 
len_in, int advice)
  return -TARGET_EINVAL;
  }

+/* Translate for some architectures which have different MADV_xxx values */
+switch (advice) {
+case TARGET_MADV_DONTNEED:  /* alpha */
+advice = MADV_DONTNEED;
+break;
+case TARGET_MADV_WIPEONFORK:/* parisc */
+advice = MADV_WIPEONFORK;
+break;
+case TARGET_MADV_KEEPONFORK:/* parisc */
+advice = MADV_KEEPONFORK;
+break;
+/* we do not care about the other MADV_xxx values yet */
+}
+
  /*
   * A straight passthrough may not be safe because qemu sometimes turns
   * private file-backed mappings into anonymous mappings.
   *
- * This is a hint, so ignoring and returning success is ok.
+ * For MADV_DONTNEED, which is a hint, ignoring and returning success is 
ok.


Actually, MADV_DONTNEED is one of the few values, which is not always a
hint - it can be used to e.g. zero out pages.


Right, it _should_ zero out pages and return 0, or otherwise return failure.
I think the problem is that some userspace apps will then sadly break if we
change the current behaviour

Anyway, in this patch I didn't wanted to touch MAD_DONTNEED.


As the next paragraph states, strictly speaking, MADV_DONTNEED is
currently broken, because it can indeed be ignored without indication
in some cases, but it's still arguably better than not honoring it at
all.


Yep.


   *
   * This breaks MADV_DONTNEED, completely implementing which is quite
   * complicated. However, there is one low-hanging fruit: mappings that are
@@ -913,11 +927,17 @@ abi_long target_madvise(abi_ulong start, abi_ulong 
len_in, int advice)
   * passthrough is safe, so do it.
   */
  mmap_lock();
-if (advice == TARGET_MADV_DONTNEED &&
-can_passthrough_madv_dontneed(start, end)) {
-ret = get_errno(madvise(g2h_untagged(start), len, MADV_DONTNEED));
-if (ret == 0) {
-page_reset_target_data(start, start + len);
+switch (advice) {
+case MADV_WIPEONFORK:
+case MADV_KEEPONFORK:
+ret = -EINVAL;
+/* fall through */
+case MADV_DONTNEED:
+if (can_passthrough_madv_dontneed(start, end)) {
+ret = get_errno(madvise(g2h_untagged(start), len, advice));
+if ((advice == MADV_DONTNEED) && (ret == 0)) {
+page_reset_target_data(start, start + len);
+}
  }
  }
  mmap_unlock();



Nit: maybe rename can_passthrough_madv_dontneed() to can_passthrough(),
since now it's used not only for MADV_DONTNEED?


Maybe can_passthrough_madvise() is better?


With the MADV_DONTNEED comment change:


Just for me to understand correctly:
You propose that I shouldn't touch that comment in my followup-patch, right?
That's ok.


Acked-by: Ilya Leoshkevich 


Thanks!
Helge

Re: [PATCH RESEND v3 07/10] migration: Refactor auto-converge capability logic

[Peter Xu]

On Sun, Dec 04, 2022 at 01:09:10AM +0800, huang...@chinatelecom.cn wrote:
> From: Hyman Huang(黄勇) 
> 
> Check if block migration is running before throttling
> guest down in auto-converge way.
> 
> Note that this modification is kind of like code clean,
> because block migration does not depend on auto-converge
> capability, so the order of checks can be adjusted.
> 
> Signed-off-by: Hyman Huang(黄勇) 

Acked-by: Peter Xu 

-- 
Peter Xu

Re: [PATCH] tpm: add backend for mssim

[James Bottomley]

On Mon, 2022-12-12 at 14:32 -0500, Stefan Berger wrote:
> 
> 
> On 12/12/22 14:12, James Bottomley wrote:
> > On Mon, 2022-12-12 at 13:58 -0500, Stefan Berger wrote:
> > > On 12/12/22 13:48, James Bottomley wrote:
> > > > On Mon, 2022-12-12 at 11:59 -0500, Stefan Berger wrote:
> > > > > On 12/12/22 11:38, James Bottomley wrote:
> > [...]
> > > > > > the kernel use of the TPM, but I'm trying to fix that.  The
> > > > > > standard mssim server is too simplistic to do transport
> > > > > > layer
> > > > > > security, but like everything that does this (or rather
> > > > > > doesn't
> > > > > > do this), you can front it with stunnel4.
> > > > > 
> > > > > And who or what is going to set this up?
> > > > 
> > > > I'm not sure I understand the question.  Stunnel4 is mostly
> > > > used to
> > > > convert unencrypted proxies like imap on 143 or smtp on 25 to
> > > > the
> > > > secure version.  Most people who run servers are fairly
> > > > familiar
> > > > with using it.  It's what IBM used for encrypted migration
> > > > initially.  You can run stunnel on both ends, or the qemu side
> > > > could be built in using the qemu tls-creds way of doing things
> > > > but
> > > > anything running the standard MS server would have to front it
> > > > with
> > > > stunnel still.
> > > 
> > > So it's up to libvirt to setup stunnel to support a completely
> > > different setup than what it has for swtpm already?
> > 
> > I don't think so, no.  Libvirt doesn't usually help with server
> > setup (witness the complexity of setting up a server side vtpm
> > proxy) so in the case tls-creds were built in, it would just work
> > if the object is
> 
> I see, so you are extending the TPM emulator with TLS on the client
> side so you don't need another tool to setup a TLS connection from
> the QEMU/client side.

I didn't say I would do this, just that it's an easy possibility with
the current qemu framework.  I actually need to fiddle with the TPM
externally to do some of my testing (like platform reset injection) so
I won't use TLS anyway.

> Is the server side across the network or on the same host?

It can be either.

>  Either way, what is the latency that this introduces because I would
> expect that this slows down IMA since the PCR extensions & TPM 2
> response now go back and forth across the network?

Most data centre protocols are now encrypted and networked (NVMeoF
would probably be the poster child) with no real ill effects.  In terms
of a TPM, the competition is an underpowered discrete chip over a slow
serial bus, so I think we'll actually improve the latency not diminish
it.

James

Re: [PATCH 30/30] meson: always log qemu-iotests verbosely

[Paolo Bonzini]

On 12/12/22 17:55, Peter Maydell wrote:

On Fri, 9 Dec 2022 at 11:44, Paolo Bonzini  wrote:


---
  tests/qemu-iotests/meson.build | 1 +
  1 file changed, 1 insertion(+)

diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
index 583468c5b9b3..3d8637c8f2b6 100644
--- a/tests/qemu-iotests/meson.build
+++ b/tests/qemu-iotests/meson.build
@@ -43,5 +43,6 @@ foreach format, speed: qemu_iotests_formats
 protocol: 'tap',
 suite: suites,
 timeout: 0,
+   verbose: true,
 is_parallel: false)
  endforeach


How much does this increase the size of a build-and-test logfile by?
I know the gitlab CI has a size limit on that these days, so if this
noticeably increases the total log size we might want to check how
close we are to the limit...


~300 lines, so probably around 20-30 kilobytes.

Paolo

Re: [PATCH v1 06/24] vfio-user: Define type vfio_user_pci_dev_info

[John Johnson]

> On Dec 12, 2022, at 1:01 AM, Cédric Le Goater  wrote:
> 
> On 11/9/22 00:13, John Johnson wrote:
>> 
>> +
>> +static Property vfio_user_pci_dev_properties[] = {
>> +DEFINE_PROP_STRING("socket", VFIOUserPCIDevice, sock_name),
> 
> This looks like a good candidate for using a chardev. It could only
> support AF_UNIX to start with if fd passing is the required feature.
> But at least, the model would be using a well known backend. I think
> vhost has the same kind of constraints.
> 

It should be a SocketAddress, but there is no command line
parser for that.  The next best option was to name the struct members
individually, so a string for addr.u.q_unix.path

JJ

Re: [PATCH-for-8.0 1/7] hw/mips/Kconfig: Introduce CONFIG_GT64120 to select gt64xxx_pci.c

[Bernhard Beschow]

Am 12. Dezember 2022 08:02:26 UTC schrieb "Philippe Mathieu-Daudé" 
:
>On 12/12/22 01:13, Bernhard Beschow wrote:
>> 
>> 
>> Am 9. Dezember 2022 15:15:27 UTC schrieb "Philippe Mathieu-Daudé" 
>> :
>>> From: Philippe Mathieu-Daudé 
>>> 
>>> Signed-off-by: Philippe Mathieu-Daudé 
>>> ---
>>> hw/mips/Kconfig | 6 ++
>>> hw/mips/meson.build | 3 ++-
>>> 2 files changed, 8 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
>>> index 725525358d..d6bbbe7069 100644
>>> --- a/hw/mips/Kconfig
>>> +++ b/hw/mips/Kconfig
>>> @@ -1,5 +1,6 @@
>>> config MALTA
>>>  bool
>>> +select GT64120
>>>  select ISA_SUPERIO
>>> 
>>> config MIPSSIM
>>> @@ -59,3 +60,8 @@ config MIPS_BOSTON
>>> 
>>> config FW_CFG_MIPS
>>>  bool
>>> +
>>> +config GT64120
>>> +bool
>>> +select PCI
>>> +select I8259
>> 
>> s/select I8259/depends on I8259/ since the model needs but doesn't provide 
>> I8259? Then just take my mail regarding the last patch as a reminder.
>
>I try to remember the 'depends on' directive as "depends on BUS".
>If there is no BUS, then the option is pointless. Here "select PCI"
>means 'provide/expose a PCI bus on the machine'.
>
>I8259 must be available for GT64120 to be working. If you need a
>GT64120, its 'select' directive will select the minimum options
>required, so it will implicitly select a I8259.
>
>See docs/devel/kconfig.rst:
>
>**dependencies**: ``depends on ``
>
>  This defines a dependency for this configurable element. Dependencies
>  evaluate an expression and force the value of the variable to false
>  if the expression is false.
>
>**reverse dependencies**: ``select  [if ]``
>
>  While ``depends on`` can force a symbol to false, reverse dependencies
>  can be used to force another symbol to true.
>
>**devices**
>
>  Example::
>
>config MEGASAS_SCSI_PCI
>  bool
>  default y if PCI_DEVICES
>  depends on PCI
>  select SCSI
>
>  Devices are the most complex of the five.  They can have a variety
>  of directives that cooperate so that a default configuration includes
>  all the devices that can be accessed from QEMU.
>
>  Devices *depend on* the bus that they lie on, for example a PCI
>  device would specify ``depends on PCI``.  An MMIO device will likely
>  have no ``depends on`` directive.

Yes, I agree that the patch follows this wording, so I should have given my R-b 
regardless (fixed below).

I was more contemplating aloud whether above wording could be interpretet as: 
I8259 is a device which provides an "interrupt bus" and GT64120 consumes that 
bus but doesn't provide I8259, hence "GT64120 depends on I8259".

Of course, a classical PIC doesn't provide a bus in the usual sense while an 
APIC actually does, though this isn't modelled in QEMU that way. In a more 
abstract view one could see devices as providing and consuming communication 
channels (via PCI, MMIO, ...), or even implementing and consuming interfaces, 
in which case above conclusion might make more sense. After all, it seems to me 
that today implementation details decide whether something should be "select" 
or "depends on".

Anyway, this is getting off-topic - just some food for thought. Sorry for the 
noise...

>  Devices also *select* the buses
>  that the device provides, for example a SCSI adapter would specify
>  ``select SCSI``.  Finally, devices are usually ``default y`` if and
>  only if they have at least one ``depends on``; the default could be
>  conditional on a device group.
>
>> Otherwise:
>> Reviewed-by: Bernhard Beschow 

That should actually be:
Anyway,
Reviewed-by: Bernhard Beschow 

>
>Thanks!

Re: [PATCH] linux-user: Add emulation for MADV_WIPEONFORK and MADV_KEEPONFORK in madvise()

[Ilya Leoshkevich]

On Mon, Dec 12, 2022 at 08:00:45AM +0100, Helge Deller wrote:
> Both parameters have a different value on the parisc platform, so first
> translate the target value into a host value for usage in the native
> madvise() syscall.
> 
> Those parameters are often used by security sensitive applications (e.g.
> tor browser, boringssl, ...) which expect the call to return a proper
> return code on failure, so return -EINVAL if qemu fails to forward the
> syscall to the host OS.
> 
> Tested with testcase of tor browser when running hppa-linux guest on
> x86-64 host.
> 
> Signed-off-by: Helge Deller 
> 
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index 10f5079331..c75342108c 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -901,11 +901,25 @@ abi_long target_madvise(abi_ulong start, abi_ulong 
> len_in, int advice)
>  return -TARGET_EINVAL;
>  }
> 
> +/* Translate for some architectures which have different MADV_xxx values 
> */
> +switch (advice) {
> +case TARGET_MADV_DONTNEED:  /* alpha */
> +advice = MADV_DONTNEED;
> +break;
> +case TARGET_MADV_WIPEONFORK:/* parisc */
> +advice = MADV_WIPEONFORK;
> +break;
> +case TARGET_MADV_KEEPONFORK:/* parisc */
> +advice = MADV_KEEPONFORK;
> +break;
> +/* we do not care about the other MADV_xxx values yet */
> +}
> +
>  /*
>   * A straight passthrough may not be safe because qemu sometimes turns
>   * private file-backed mappings into anonymous mappings.
>   *
> - * This is a hint, so ignoring and returning success is ok.
> + * For MADV_DONTNEED, which is a hint, ignoring and returning success is 
> ok.

Actually, MADV_DONTNEED is one of the few values, which is not always a
hint - it can be used to e.g. zero out pages.

As the next paragraph states, strictly speaking, MADV_DONTNEED is
currently broken, because it can indeed be ignored without indication
in some cases, but it's still arguably better than not honoring it at
all.

>   *
>   * This breaks MADV_DONTNEED, completely implementing which is quite
>   * complicated. However, there is one low-hanging fruit: mappings that 
> are
> @@ -913,11 +927,17 @@ abi_long target_madvise(abi_ulong start, abi_ulong 
> len_in, int advice)
>   * passthrough is safe, so do it.
>   */
>  mmap_lock();
> -if (advice == TARGET_MADV_DONTNEED &&
> -can_passthrough_madv_dontneed(start, end)) {
> -ret = get_errno(madvise(g2h_untagged(start), len, MADV_DONTNEED));
> -if (ret == 0) {
> -page_reset_target_data(start, start + len);
> +switch (advice) {
> +case MADV_WIPEONFORK:
> +case MADV_KEEPONFORK:
> +ret = -EINVAL;
> +/* fall through */
> +case MADV_DONTNEED:
> +if (can_passthrough_madv_dontneed(start, end)) {
> +ret = get_errno(madvise(g2h_untagged(start), len, advice));
> +if ((advice == MADV_DONTNEED) && (ret == 0)) {
> +page_reset_target_data(start, start + len);
> +}
>  }
>  }
>  mmap_unlock();
> 

Nit: maybe rename can_passthrough_madv_dontneed() to can_passthrough(),
since now it's used not only for MADV_DONTNEED?



With the MADV_DONTNEED comment change:

Acked-by: Ilya Leoshkevich 

[PATCH] target/tricore: Fix gdbstub write to address registers

[Richard Henderson]

Typo had double-writes to data registers.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1363
Signed-off-by: Richard Henderson 
---
 target/tricore/gdbstub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/tricore/gdbstub.c b/target/tricore/gdbstub.c
index ebf32defde..3a27a7e65d 100644
--- a/target/tricore/gdbstub.c
+++ b/target/tricore/gdbstub.c
@@ -130,7 +130,7 @@ int tricore_cpu_gdb_write_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 if (n < 16) { /* data registers */
 env->gpr_d[n] = tmp;
 } else if (n < 32) { /* address registers */
-env->gpr_d[n - 16] = tmp;
+env->gpr_a[n - 16] = tmp;
 } else {
 tricore_cpu_gdb_write_csfr(env, n, tmp);
 }
-- 
2.34.1

Re: [PATCH v1 06/24] vfio-user: Define type vfio_user_pci_dev_info

[John Johnson]

> On Dec 12, 2022, at 3:46 AM, Philippe Mathieu-Daudé  wrote:
> 
> On 12/12/22 12:03, John Levon wrote:
>> On Mon, Dec 12, 2022 at 10:01:33AM +0100, Cédric Le Goater wrote:
 diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
 index 80b03a2..dc19869 100644
 --- a/hw/vfio/pci.c
 +++ b/hw/vfio/pci.c
 @@ -19,6 +19,7 @@
*/
   #include "qemu/osdep.h"
 +#include CONFIG_DEVICES
   #include 
   #include 
 @@ -3421,3 +3422,91 @@ static void register_vfio_pci_dev_type(void)
   }
   type_init(register_vfio_pci_dev_type)
 +
 +
 +#ifdef CONFIG_VFIO_USER_PCI
>>> 
>>> Why not introduce a new file hw/vfio/user.c file ? It would be
>>> cleaner.
>> user.c is in this series, and holds the vfio-user implementation - it's not a
>> PCI specific thing. So it would have to be hw/vfio/user_pci.c or something
> 
> Or hw/vfio/pci-user.c


The vfio_user_* routines use a lot of hw/vfio/pci.c routines, so we’d
need to make a lot of those routines non-static if we made a new file.

JJ

Re: [PATCH 21/30] build: move sanitizer tests to meson

[Paolo Bonzini]

Il lun 12 dic 2022, 13:16 Marc-André Lureau  ha
scritto:

> +if get_option('sanitizers')
> +  if cc.has_argument('-fsanitize=address')
> +qemu_cflags = ['-fsanitize=address'] + qemu_cflags
> +qemu_ldflags = ['-fsanitize=address'] + qemu_ldflags

why not the += syntax? same below


Configure puts it at the beginning of QEMU_CFLAGS and I didn't want to
change it.

Paolo

Re: [PATCH v1 19/24] vfio-user: secure DMA support

[John Johnson]

> On Dec 9, 2022, at 10:01 AM, John Levon  wrote:
> 
> On Tue, Nov 08, 2022 at 03:13:41PM -0800, John Johnson wrote:
> 
>> Secure DMA forces the remote process to use DMA r/w messages
>> instead of directly mapping guest memeory.
> 
> I don't really get why this is called "secure" - shouldn't have an option name
> more closely resembling what it actually does?
> 

The option was added to address a security concern if the server has
mmap() access to guest memory.  I can re-name it.

JJ

Re: [PATCH v1 02/24] vfio-user: add VFIO base abstract class

[John Johnson]

I can look into using orderfile

JJ


> On Dec 9, 2022, at 8:04 AM, Cédric Le Goater  wrote:
> 
> Hello John,
> 
> On 11/9/22 00:13, John Johnson wrote:
>> Add an abstract base class both the kernel driver
>> and user socket implementations can use to share code.
>> Signed-off-by: John G Johnson 
>> Signed-off-by: Elena Ufimtseva 
>> Signed-off-by: Jagannathan Raman 
> 
> I would help the reader if the header files came first. You could use :
> 
>  [diff]
>   orderFile = /path/to/qemu/scripts/git.orderfile
> 
> 
> I would simply use 'Kernel' instead of 'Kern' in the type definition.
> A part from that,
> 
> Reviewed-by: Cédric Le Goater 
> 
> Thanks,
> 
> C.
> 
>> ---
>>  hw/vfio/pci.c | 106 
>> +++---
>>  hw/vfio/pci.h |  16 +++--
>>  2 files changed, 78 insertions(+), 44 deletions(-)
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index 939dcc3..60acde5 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -235,7 +235,7 @@ static void vfio_intx_update(VFIOPCIDevice *vdev, 
>> PCIINTxRoute *route)
>>static void vfio_intx_routing_notifier(PCIDevice *pdev)
>>  {
>> -VFIOPCIDevice *vdev = VFIO_PCI(pdev);
>> +VFIOPCIDevice *vdev = VFIO_PCI_BASE(pdev);
>>  PCIINTxRoute route;
>>if (vdev->interrupt != VFIO_INT_INTx) {
>> @@ -467,7 +467,7 @@ static void vfio_update_kvm_msi_virq(VFIOMSIVector 
>> *vector, MSIMessage msg,
>>  static int vfio_msix_vector_do_use(PCIDevice *pdev, unsigned int nr,
>> MSIMessage *msg, IOHandler *handler)
>>  {
>> -VFIOPCIDevice *vdev = VFIO_PCI(pdev);
>> +VFIOPCIDevice *vdev = VFIO_PCI_BASE(pdev);
>>  VFIOMSIVector *vector;
>>  int ret;
>>  @@ -561,7 +561,7 @@ static int vfio_msix_vector_use(PCIDevice *pdev,
>>static void vfio_msix_vector_release(PCIDevice *pdev, unsigned int nr)
>>  {
>> -VFIOPCIDevice *vdev = VFIO_PCI(pdev);
>> +VFIOPCIDevice *vdev = VFIO_PCI_BASE(pdev);
>>  VFIOMSIVector *vector = &vdev->msi_vectors[nr];
>>trace_vfio_msix_vector_release(vdev->vbasedev.name, nr);
>> @@ -1109,7 +1109,7 @@ static const MemoryRegionOps vfio_vga_ops = {
>>   */
>>  static void vfio_sub_page_bar_update_mapping(PCIDevice *pdev, int bar)
>>  {
>> -VFIOPCIDevice *vdev = VFIO_PCI(pdev);
>> +VFIOPCIDevice *vdev = VFIO_PCI_BASE(pdev);
>>  VFIORegion *region = &vdev->bars[bar].region;
>>  MemoryRegion *mmap_mr, *region_mr, *base_mr;
>>  PCIIORegion *r;
>> @@ -1155,7 +1155,7 @@ static void vfio_sub_page_bar_update_mapping(PCIDevice 
>> *pdev, int bar)
>>   */
>>  uint32_t vfio_pci_read_config(PCIDevice *pdev, uint32_t addr, int len)
>>  {
>> -VFIOPCIDevice *vdev = VFIO_PCI(pdev);
>> +VFIOPCIDevice *vdev = VFIO_PCI_BASE(pdev);
>>  uint32_t emu_bits = 0, emu_val = 0, phys_val = 0, val;
>>memcpy(&emu_bits, vdev->emulated_config_bits + addr, len);
>> @@ -1188,7 +1188,7 @@ uint32_t vfio_pci_read_config(PCIDevice *pdev, 
>> uint32_t addr, int len)
>>  void vfio_pci_write_config(PCIDevice *pdev,
>> uint32_t addr, uint32_t val, int len)
>>  {
>> -VFIOPCIDevice *vdev = VFIO_PCI(pdev);
>> +VFIOPCIDevice *vdev = VFIO_PCI_BASE(pdev);
>>  uint32_t val_le = cpu_to_le32(val);
>>trace_vfio_pci_write_config(vdev->vbasedev.name, addr, val, len);
>> @@ -2845,7 +2845,7 @@ static void vfio_unregister_req_notifier(VFIOPCIDevice 
>> *vdev)
>>static void vfio_realize(PCIDevice *pdev, Error **errp)
>>  {
>> -VFIOPCIDevice *vdev = VFIO_PCI(pdev);
>> +VFIOPCIDevice *vdev = VFIO_PCI_BASE(pdev);
>>  VFIODevice *vbasedev = &vdev->vbasedev;
>>  VFIODevice *vbasedev_iter;
>>  VFIOGroup *group;
>> @@ -3169,7 +3169,7 @@ error:
>>static void vfio_instance_finalize(Object *obj)
>>  {
>> -VFIOPCIDevice *vdev = VFIO_PCI(obj);
>> +VFIOPCIDevice *vdev = VFIO_PCI_BASE(obj);
>>  VFIOGroup *group = vdev->vbasedev.group;
>>vfio_display_finalize(vdev);
>> @@ -3189,7 +3189,7 @@ static void vfio_instance_finalize(Object *obj)
>>static void vfio_exitfn(PCIDevice *pdev)
>>  {
>> -VFIOPCIDevice *vdev = VFIO_PCI(pdev);
>> +VFIOPCIDevice *vdev = VFIO_PCI_BASE(pdev);
>>vfio_unregister_req_notifier(vdev);
>>  vfio_unregister_err_notifier(vdev);
>> @@ -3208,7 +3208,7 @@ static void vfio_exitfn(PCIDevice *pdev)
>>static void vfio_pci_reset(DeviceState *dev)
>>  {
>> -VFIOPCIDevice *vdev = VFIO_PCI(dev);
>> +VFIOPCIDevice *vdev = VFIO_PCI_BASE(dev);
>>trace_vfio_pci_reset(vdev->vbasedev.name);
>>  @@ -3248,7 +3248,7 @@ post_reset:
>>  static void vfio_instance_init(Object *obj)
>>  {
>>  PCIDevice *pci_dev = PCI_DEVICE(obj);
>> -VFIOPCIDevice *vdev = VFIO_PCI(obj);
>> +VFIOPCIDevice *vdev = VFIO_PCI_BASE(obj);
>>device_add_bootindex_property(obj, &vdev->bootindex,
>>"bootindex", NULL,
>> @@ -3265,24 +3265,12 @@ static

Re: [PATCH v1 14/24] vfio-user: get and set IRQs

[John Johnson]

> On Dec 9, 2022, at 9:29 AM, John Levon  wrote:
> 
> On Tue, Nov 08, 2022 at 03:13:36PM -0800, John Johnson wrote:
> 
>> +static int vfio_user_io_get_irq_info(VFIODevice *vbasedev,
>> + struct vfio_irq_info *irq)
>> +{
>> +int ret;
>> +
>> +ret = vfio_user_get_irq_info(vbasedev->proxy, irq);
>> +if (ret) {
>> +return ret;
>> +}
>> +
>> +if (irq->index > vbasedev->num_irqs) {
>> +return -EINVAL;
>> +}
> 
> Why are we validating ->index *after* requesting the info? Seems a bit weird?
> 

That check is to validate the server return content (to the extent we 
can).

JJ

Re: [PATCH v1 10/24] vfio-user: get device info

[John Johnson]

> On Dec 9, 2022, at 7:57 AM, John Levon  wrote:
> 
> On Tue, Nov 08, 2022 at 03:13:32PM -0800, John Johnson wrote:
> 
>> +/*
>> + * VFIO_USER_DEVICE_GET_INFO
>> + * imported from struct_device_info
>> + */
>> +typedef struct {
>> +VFIOUserHdr hdr;
>> +uint32_t argsz;
>> +uint32_t flags;
>> +uint32_t num_regions;
>> +uint32_t num_irqs;
>> +uint32_t cap_offset;
>> +} VFIOUserDeviceInfo;
> 
> The server doesn't have or populate cap_offset - and this isn't in the 
> protocol
> either.
> 

I can just delete it from VFIOUserDeviceInfo

JJ

Re: [RFC v2 0/3] migration: reduce time of loading non-iterable vmstate

[Peter Xu]

On Tue, Dec 13, 2022 at 12:49:39AM +0800, Chuang Xu wrote:
> 
> Hi!

Chuang,

> 
> In this version:
> 
> - rebase to latest upstream.
> - add sanity check to address_space_to_flatview().
> - postpone the init of the vring cache until migration's loading completes. 

Since there'll be other changes besides migration, please consider also
copy the relevant maintainers too on either memory and virtio in your next
post:

$ ./scripts/get_maintainer.pl -f softmmu/memory.c -f hw/virtio/virtio.c
Paolo Bonzini  (supporter:Memory API)
Peter Xu  (supporter:Memory API)
David Hildenbrand  (supporter:Memory API)
"Philippe Mathieu-Daudé"  (reviewer:Memory API)
"Michael S. Tsirkin"  (supporter:virtio)
qemu-devel@nongnu.org (open list:All patches CC here)

-- 
Peter Xu

Re: [PATCH] tpm: add backend for mssim

[Stefan Berger]

On 12/12/22 14:32, Stefan Berger wrote:



On 12/12/22 14:12, James Bottomley wrote:

On Mon, 2022-12-12 at 13:58 -0500, Stefan Berger wrote:

On 12/12/22 13:48, James Bottomley wrote:

On Mon, 2022-12-12 at 11:59 -0500, Stefan Berger wrote:

On 12/12/22 11:38, James Bottomley wrote:

[...]

the kernel use of the TPM, but I'm trying to fix that.  The
standard mssim server is too simplistic to do transport layer
security, but like everything that does this (or rather doesn't
do this), you can front it with stunnel4.


And who or what is going to set this up?


I'm not sure I understand the question.  Stunnel4 is mostly used to
convert unencrypted proxies like imap on 143 or smtp on 25 to the
secure version.  Most people who run servers are fairly familiar
with using it.  It's what IBM used for encrypted migration
initially.  You can run stunnel on both ends, or the qemu side
could be built in using the qemu tls-creds way of doing things but
anything running the standard MS server would have to front it with
stunnel still.


So it's up to libvirt to setup stunnel to support a completely
different setup than what it has for swtpm already?


I don't think so, no.  Libvirt doesn't usually help with server setup
(witness the complexity of setting up a server side vtpm proxy) so in
the case tls-creds were built in, it would just work if the object is


I see, so you are extending the TPM emulator with TLS on the client side so you 
don't need another tool to setup a TLS connection from the QEMU/client side.


s/TPM emulator/TPM backend/



Is the server side across the network or on the same host? Either way, what is the 
latency that this introduces because I would expect that this slows down IMA since 
the PCR extensions & TPM 2 response now go back and forth across the network?

     Stefan


specified.  The complexity is all on the server side to front it with
stunnel.

James

Re: [RFC v2 2/3] virtio: support delay of checks in virtio_load()

[Peter Xu]

On Tue, Dec 13, 2022 at 12:49:41AM +0800, Chuang Xu wrote:
> +bool migration_enable_load_check_delay;

I'm just afraid this is still too hacky.

One thing is because this variable itself to be only set at specific phase
during migration to cover that commit().  The other thing is I'm not sure
we can always rely on the commit() being happen 100% - what if there's no
memory layout changes throughout the whole process of vm load?  That'll be
skipped if memory_region_update_pending==false as I said.

So far the best I can come up with is we allow each virtio device to
register a vm state change handler (during virtio_load) to do the rest,
then in the handler it unregisters itself so it only runs once right before
the VM starts.  But I'm not sure whether the virtio developers will be
happy with it.  Maybe worth a try.

Feel free to have a look at like kvmvapic_vm_state_change() if you think
that idea worth exploring.

-- 
Peter Xu

Re: [PATCH] tpm: add backend for mssim

[Stefan Berger]

On 12/12/22 14:12, James Bottomley wrote:

On Mon, 2022-12-12 at 13:58 -0500, Stefan Berger wrote:

On 12/12/22 13:48, James Bottomley wrote:

On Mon, 2022-12-12 at 11:59 -0500, Stefan Berger wrote:

On 12/12/22 11:38, James Bottomley wrote:

[...]

the kernel use of the TPM, but I'm trying to fix that.  The
standard mssim server is too simplistic to do transport layer
security, but like everything that does this (or rather doesn't
do this), you can front it with stunnel4.


And who or what is going to set this up?


I'm not sure I understand the question.  Stunnel4 is mostly used to
convert unencrypted proxies like imap on 143 or smtp on 25 to the
secure version.  Most people who run servers are fairly familiar
with using it.  It's what IBM used for encrypted migration
initially.  You can run stunnel on both ends, or the qemu side
could be built in using the qemu tls-creds way of doing things but
anything running the standard MS server would have to front it with
stunnel still.


So it's up to libvirt to setup stunnel to support a completely
different setup than what it has for swtpm already?


I don't think so, no.  Libvirt doesn't usually help with server setup
(witness the complexity of setting up a server side vtpm proxy) so in
the case tls-creds were built in, it would just work if the object is


I see, so you are extending the TPM emulator with TLS on the client side so you 
don't need another tool to setup a TLS connection from the QEMU/client side.

Is the server side across the network or on the same host? Either way, what is the 
latency that this introduces because I would expect that this slows down IMA since 
the PCR extensions & TPM 2 response now go back and forth across the network?

Stefan


specified.  The complexity is all on the server side to front it with
stunnel.

James

Re: [PATCH] tpm: add backend for mssim

[James Bottomley]

On Mon, 2022-12-12 at 13:58 -0500, Stefan Berger wrote:
> On 12/12/22 13:48, James Bottomley wrote:
> > On Mon, 2022-12-12 at 11:59 -0500, Stefan Berger wrote:
> > > On 12/12/22 11:38, James Bottomley wrote:
[...]
> > > > the kernel use of the TPM, but I'm trying to fix that.  The
> > > > standard mssim server is too simplistic to do transport layer
> > > > security, but like everything that does this (or rather doesn't
> > > > do this), you can front it with stunnel4.
> > > 
> > > And who or what is going to set this up?
> > 
> > I'm not sure I understand the question.  Stunnel4 is mostly used to
> > convert unencrypted proxies like imap on 143 or smtp on 25 to the
> > secure version.  Most people who run servers are fairly familiar
> > with using it.  It's what IBM used for encrypted migration
> > initially.  You can run stunnel on both ends, or the qemu side
> > could be built in using the qemu tls-creds way of doing things but
> > anything running the standard MS server would have to front it with
> > stunnel still.
> 
> So it's up to libvirt to setup stunnel to support a completely
> different setup than what it has for swtpm already?

I don't think so, no.  Libvirt doesn't usually help with server setup
(witness the complexity of setting up a server side vtpm proxy) so in
the case tls-creds were built in, it would just work if the object is
specified.  The complexity is all on the server side to front it with
stunnel.

James

Re: [PATCH] tpm: add backend for mssim

[Stefan Berger]

On 12/12/22 13:48, James Bottomley wrote:

On Mon, 2022-12-12 at 11:59 -0500, Stefan Berger wrote:



On 12/12/22 11:38, James Bottomley wrote:

On Mon, 2022-12-12 at 15:47 +, Daniel P. Berrangé wrote:

Copy'ing Markus for QAPI design feedback.

On Sat, Dec 10, 2022 at 12:10:18PM -0500, James Bottomley wrote:

The Microsoft Simulator (mssim) is the reference emulation
platform for the TCG TPM 2.0 specification.

https://github.com/Microsoft/ms-tpm-20-ref.git

It exports a fairly simple network socket baset protocol on two
sockets, one for command (default 2321) and one for control
(default 2322).  This patch adds a simple backend that can
speak the mssim protocol over the network.  It also allows the
host, and two ports to be specified on the qemu command line.
The benefits are twofold: firstly it gives us a backend that
actually speaks a standard TPM emulation protocol instead of
the linux specific TPM driver format of the current emulated
TPM backend and secondly, using the microsoft protocol, the end
point of the emulator can be anywhere on the network,
facilitating the cloud use case where a central TPM service can
be used over a control network.


What's the story with security for this ?  The patch isn't using
TLS, so talking to any emulator over anything other than
localhost looks insecure, unless I'm missing something.


Pretty much every TPM application fears interposers and should thus
be using the TPM transport security anyway. *If* this is the case,
then the transport is secure.  Note that this currently isn't the
case for


What about all the older kernels that are out there?


No current kernel uses transport security.  In the event the patch
eventually gets upstream, the kernel be secure against interposer
attacks going forwards.  I would imagine there might be pressure to
backport the patch given the current level of worry about interposers.


the kernel use of the TPM, but I'm trying to fix that.  The
standard mssim server is too simplistic to do transport layer
security, but like everything that does this (or rather doesn't do
this), you can front it with stunnel4.


And who or what is going to set this up?


I'm not sure I understand the question.  Stunnel4 is mostly used to
convert unencrypted proxies like imap on 143 or smtp on 25 to the
secure version.  Most people who run servers are fairly familiar with
using it.  It's what IBM used for encrypted migration initially.  You
can run stunnel on both ends, or the qemu side could be built in using
the qemu tls-creds way of doing things but anything running the
standard MS server would have to front it with stunnel still.



So it's up to libvirt to setup stunnel to support a completely different setup 
than what it has for swtpm already?

   Stefan



James

Re: [PATCH] tpm: add backend for mssim

[James Bottomley]

On Mon, 2022-12-12 at 11:59 -0500, Stefan Berger wrote:
> 
> 
> On 12/12/22 11:38, James Bottomley wrote:
> > On Mon, 2022-12-12 at 15:47 +, Daniel P. Berrangé wrote:
> > > Copy'ing Markus for QAPI design feedback.
> > > 
> > > On Sat, Dec 10, 2022 at 12:10:18PM -0500, James Bottomley wrote:
> > > > The Microsoft Simulator (mssim) is the reference emulation
> > > > platform for the TCG TPM 2.0 specification.
> > > > 
> > > > https://github.com/Microsoft/ms-tpm-20-ref.git
> > > > 
> > > > It exports a fairly simple network socket baset protocol on two
> > > > sockets, one for command (default 2321) and one for control
> > > > (default 2322).  This patch adds a simple backend that can
> > > > speak the mssim protocol over the network.  It also allows the
> > > > host, and two ports to be specified on the qemu command line. 
> > > > The benefits are twofold: firstly it gives us a backend that
> > > > actually speaks a standard TPM emulation protocol instead of
> > > > the linux specific TPM driver format of the current emulated
> > > > TPM backend and secondly, using the microsoft protocol, the end
> > > > point of the emulator can be anywhere on the network,
> > > > facilitating the cloud use case where a central TPM service can
> > > > be used over a control network.
> > > 
> > > What's the story with security for this ?  The patch isn't using
> > > TLS, so talking to any emulator over anything other than
> > > localhost looks insecure, unless I'm missing something.
> > 
> > Pretty much every TPM application fears interposers and should thus
> > be using the TPM transport security anyway. *If* this is the case,
> > then the transport is secure.  Note that this currently isn't the
> > case for
> 
> What about all the older kernels that are out there?

No current kernel uses transport security.  In the event the patch
eventually gets upstream, the kernel be secure against interposer
attacks going forwards.  I would imagine there might be pressure to
backport the patch given the current level of worry about interposers.

> > the kernel use of the TPM, but I'm trying to fix that.  The
> > standard mssim server is too simplistic to do transport layer
> > security, but like everything that does this (or rather doesn't do
> > this), you can front it with stunnel4.
> 
> And who or what is going to set this up?

I'm not sure I understand the question.  Stunnel4 is mostly used to
convert unencrypted proxies like imap on 143 or smtp on 25 to the
secure version.  Most people who run servers are fairly familiar with
using it.  It's what IBM used for encrypted migration initially.  You
can run stunnel on both ends, or the qemu side could be built in using
the qemu tls-creds way of doing things but anything running the
standard MS server would have to front it with stunnel still.

James

Re: [PATCH v3 7/8] accel/tcg: Move PageDesc tree into tb-maint.c for system

[Philippe Mathieu-Daudé]

On 12/12/22 16:28, Richard Henderson wrote:

On 12/9/22 01:22, Philippe Mathieu-Daudé wrote:

On 9/12/22 06:19, Richard Henderson wrote:

Now that PageDesc is not used for user-only, and for system
it is only used for tb maintenance, move the implementation
into tb-main.c appropriately ifdefed.

We have not yet eliminated all references to PageDesc for
user-only, so retain a typedef to the structure without definition.

Signed-off-by: Richard Henderson 
---
  accel/tcg/internal.h  |  49 +++---
  accel/tcg/tb-maint.c  | 130 --
  accel/tcg/translate-all.c |  95 
  3 files changed, 134 insertions(+), 140 deletions(-)




-/*
- * In system mode we want L1_MAP to be based on ram offsets,
- * while in user mode we want it to be based on virtual addresses.
- *
- * TODO: For user mode, see the caveat re host vs guest virtual
- * address spaces near GUEST_ADDR_MAX.
- */
-#if !defined(CONFIG_USER_ONLY)
-#if HOST_LONG_BITS < TARGET_PHYS_ADDR_SPACE_BITS
-# define L1_MAP_ADDR_SPACE_BITS  HOST_LONG_BITS
-#else
-# define L1_MAP_ADDR_SPACE_BITS  TARGET_PHYS_ADDR_SPACE_BITS
-#endif
-#else
-# define L1_MAP_ADDR_SPACE_BITS  MIN(HOST_LONG_BITS, TARGET_ABI_BITS)
-#endif




diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
index 20e86c813d..9b996bbeb2 100644
--- a/accel/tcg/tb-maint.c
+++ b/accel/tcg/tb-maint.c
@@ -127,6 +127,121 @@ static PageForEachNext 
foreach_tb_next(PageForEachNext tb,

  }
  #else
+/*
+ * In system mode we want L1_MAP to be based on ram offsets.
+ */
+#if HOST_LONG_BITS < TARGET_PHYS_ADDR_SPACE_BITS
+# define L1_MAP_ADDR_SPACE_BITS  HOST_LONG_BITS
+#else
+# define L1_MAP_ADDR_SPACE_BITS  TARGET_PHYS_ADDR_SPACE_BITS
+#endif

So you removed L1_MAP_ADDR_SPACE_BITS in this patch. If you ever respin,
I'd rather have it cleaned in the previous patch, along with the comment
updated and TODO removed.


I don't agree.  I move all of the PageDesc symbols together in this 
patch.  I think that it would get in the way of the main point of the 
previous patch.


OK then :)

Re: [PATCH] MAINTAINERS: Add MIPS-related docs and configs to the MIPS architecture section

[Philippe Mathieu-Daudé]

On 12/12/22 18:12, Thomas Huth wrote:

docs/system/target-mips.rst and configs/targets/mips* are not covered
in our MAINTAINERS file yet, so let's add them now.

Signed-off-by: Thomas Huth 
---
  MAINTAINERS | 2 ++
  1 file changed, 2 insertions(+)


Thanks!

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH] MAINTAINERS: Add documentation files to the corresponding sections

[Philippe Mathieu-Daudé]

On 12/12/22 18:48, Thomas Huth wrote:

A lot of files in the docs directory do not have a maintainer according to
our MAINTAINERS file, though they can be clearly associated with one of the
sections in there. Add the files now so that our scripts/get_maintainer.pl
script can output the right maintainer for them.

Signed-off-by: Thomas Huth 
---
  MAINTAINERS | 20 +---
  1 file changed, 17 insertions(+), 3 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH] include: Don't include qemu/osdep.h

[Daniel P . Berrangé]

On Mon, Dec 12, 2022 at 04:55:55PM +, Taylor Simpson wrote:
> 
> 
> > -Original Message-
> > From: Markus Armbruster 
> > Sent: Monday, December 12, 2022 1:05 AM
> > To: qemu-devel@nongnu.org
> > Cc: i...@bsdimp.com; kev...@freebsd.org; berra...@redhat.com;
> > jonathan.came...@huawei.com; kbast...@mail.uni-paderborn.de;
> > jasow...@redhat.com; michael.r...@amd.com; kkost...@redhat.com;
> > Taylor Simpson ; pal...@dabbelt.com;
> > alistair.fran...@wdc.com; bin.m...@windriver.com; qemu-
> > ri...@nongnu.org
> > Subject: [PATCH] include: Don't include qemu/osdep.h
> > 
> > docs/devel/style.rst mandates:
> > 
> > The "qemu/osdep.h" header contains preprocessor macros that affect
> > the behavior of core system headers like .  It must be
> > the first include so that core system headers included by external
> > libraries get the preprocessor macros that QEMU depends on.
> > 
> > Do not include "qemu/osdep.h" from header files since the .c file
> > will have already included it.
> > 
> > A few violations have crept in.  Fix them.
> > 
> > Signed-off-by: Markus Armbruster 
> > ---
> >  bsd-user/qemu.h | 1 -
> >  crypto/block-luks-priv.h| 1 -
> >  include/hw/cxl/cxl_host.h   | 1 -
> >  include/hw/input/pl050.h| 1 -
> >  include/hw/tricore/triboard.h   | 1 -
> >  include/qemu/userfaultfd.h  | 1 -
> >  net/vmnet_int.h | 1 -
> >  qga/cutils.h| 1 -
> >  target/hexagon/hex_arch_types.h | 1 -
> >  target/hexagon/mmvec/macros.h   | 1 -
> >  target/riscv/pmu.h  | 1 -
> >  qga/cutils.c| 3 ++-
> >  12 files changed, 2 insertions(+), 12 deletions(-)
> > 
> > diff --git a/target/hexagon/hex_arch_types.h
> > b/target/hexagon/hex_arch_types.h index 885f68f760..52a7f2b2f3 100644
> > --- a/target/hexagon/hex_arch_types.h
> > +++ b/target/hexagon/hex_arch_types.h
> > @@ -18,7 +18,6 @@
> >  #ifndef HEXAGON_HEX_ARCH_TYPES_H
> >  #define HEXAGON_HEX_ARCH_TYPES_H
> > 
> > -#include "qemu/osdep.h"
> >  #include "mmvec/mmvec.h"
> >  #include "qemu/int128.h"
> 
> Please change the copyright year in this file from "2019-2021" to "2019-2022".

No, that would be inappropriate.

The Copyright line is attributed to Qualcomm, and Markus doesn't
work for, nor assign copyright to, Qualcomm, so he must not change
the Qualcomm copyright line. Further, merely deleting a line of
code is not a significant change from POV of claiming copyright.

> Otherwise
> Reviewed-by: Taylor Simpson 

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[PATCH v2] linux-user: Enhance strace output for various syscalls

[Helge Deller]

Add appropriate strace printf formats for various Linux syscalls.

Signed-off-by: Helge Deller 
Reviewed-by: Philippe Mathieu-Daudé 

---
v2: Fixed a few entries based on review by Philippe Mathieu-Daudé

diff --git a/linux-user/strace.list b/linux-user/strace.list
index 3a898e2532..a75101fca1 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -337,7 +337,7 @@
 { TARGET_NR_getpagesize, "getpagesize" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_getpeername
-{ TARGET_NR_getpeername, "getpeername" , NULL, NULL, NULL },
+{ TARGET_NR_getpeername, "getpeername" , "%s(%d,%p,%p)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_getpgid
 { TARGET_NR_getpgid, "getpgid" , "%s(%u)", NULL, NULL },
@@ -361,19 +361,19 @@
 { TARGET_NR_getrandom, "getrandom", "%s(%p,%u,%u)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_getresgid
-{ TARGET_NR_getresgid, "getresgid" , NULL, NULL, NULL },
+{ TARGET_NR_getresgid, "getresgid" , "%s(%p,%p,%p)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_getresgid32
 { TARGET_NR_getresgid32, "getresgid32" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_getresuid
-{ TARGET_NR_getresuid, "getresuid" , NULL, NULL, NULL },
+{ TARGET_NR_getresuid, "getresuid" , "%s(%p,%p,%p)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_getresuid32
 { TARGET_NR_getresuid32, "getresuid32" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_getrlimit
-{ TARGET_NR_getrlimit, "getrlimit" , NULL, NULL, NULL },
+{ TARGET_NR_getrlimit, "getrlimit" , "%s(%d,%p)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_get_robust_list
 { TARGET_NR_get_robust_list, "get_robust_list" , NULL, NULL, NULL },
@@ -385,10 +385,10 @@
 { TARGET_NR_getsid, "getsid" , "%s(%d)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_getsockname
-{ TARGET_NR_getsockname, "getsockname" , NULL, NULL, NULL },
+{ TARGET_NR_getsockname, "getsockname" , "%s(%d,%p,%p)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_getsockopt
-{ TARGET_NR_getsockopt, "getsockopt" , NULL, NULL, NULL },
+{ TARGET_NR_getsockopt, "getsockopt" , "%s(%d,%d,%d,%p,%p)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_get_thread_area
 #if defined(TARGET_I386) && defined(TARGET_ABI32)
@@ -1052,10 +1052,10 @@
 { TARGET_NR_pivot_root, "pivot_root" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_poll
-{ TARGET_NR_poll, "poll" , NULL, NULL, NULL },
+{ TARGET_NR_poll, "poll" , "%s(%p,%u,%d)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_ppoll
-{ TARGET_NR_ppoll, "ppoll" , NULL, NULL, NULL },
+{ TARGET_NR_ppoll, "ppoll" , "%s(%p,%u,%p,%p)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_prctl
 { TARGET_NR_prctl, "prctl" , NULL, NULL, NULL },
@@ -1124,7 +1124,7 @@
 { TARGET_NR_reboot, "reboot" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_recv
-{ TARGET_NR_recv, "recv" , NULL, NULL, NULL },
+{ TARGET_NR_recv, "recv" , "%s(%d,%p,%u,%d)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_recvfrom
 { TARGET_NR_recvfrom, "recvfrom" , NULL, NULL, NULL },
@@ -1184,7 +1184,7 @@
 { TARGET_NR_rt_sigqueueinfo, "rt_sigqueueinfo" , NULL, print_rt_sigqueueinfo, 
NULL },
 #endif
 #ifdef TARGET_NR_rt_sigreturn
-{ TARGET_NR_rt_sigreturn, "rt_sigreturn" , NULL, NULL, NULL },
+{ TARGET_NR_rt_sigreturn, "rt_sigreturn" , "%s(%p)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_rt_sigsuspend
 { TARGET_NR_rt_sigsuspend, "rt_sigsuspend" , NULL, NULL, NULL },
@@ -1196,16 +1196,19 @@
 { TARGET_NR_rt_tgsigqueueinfo, "rt_tgsigqueueinfo" , NULL, 
print_rt_tgsigqueueinfo, NULL },
 #endif
 #ifdef TARGET_NR_sched_getaffinity
-{ TARGET_NR_sched_getaffinity, "sched_getaffinity" , NULL, NULL, NULL },
+{ TARGET_NR_sched_getaffinity, "sched_getaffinity" , "%s(%d,%u,%p)", NULL, 
NULL },
 #endif
 #ifdef TARGET_NR_sched_get_affinity
 { TARGET_NR_sched_get_affinity, "sched_get_affinity" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_sched_getattr
-{ TARGET_NR_sched_getattr, "sched_getattr" , NULL, NULL, NULL },
+{ TARGET_NR_sched_getattr, "sched_getattr" , "%s(%d,%p,%u,%u)", NULL, NULL },
+#endif
+#ifdef TARGET_NR_sched_setattr
+{ TARGET_NR_sched_setattr, "sched_setattr" , "%s(%p,%p)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_sched_getparam
-{ TARGET_NR_sched_getparam, "sched_getparam" , NULL, NULL, NULL },
+{ TARGET_NR_sched_getparam, "sched_getparam" , "%s(%d,%p)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_sched_get_priority_max
 { TARGET_NR_sched_get_priority_max, "sched_get_priority_max" , NULL, NULL, 
NULL },
@@ -1220,7 +1223,7 @@
 { TARGET_NR_sched_rr_get_interval, "sched_rr_get_interval" , NULL, NULL, NULL 
},
 #endif
 #ifdef TARGET_NR_sched_setaffinity
-{ TARGET_NR_sched_setaffinity, "sched_setaffinity" , NULL, NULL, NULL },
+{ TARGET_NR_sched_setaffinity, "sched_setaffinity" , "%s(%d,%u,%p)", NULL, 
NULL },
 #endif
 #ifdef TARGET_NR_sched_setatt
 { TARGET_NR_sched_setatt, "sched_setatt" , NULL, NULL, NULL },
@@ -1353,23 +1356,23 @@
 { TARGET_NR_setreuid32, "setreuid32" , "%s(%u,%u)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_setrlimit
-{ TARGET_NR_setrlimit, "setrlimit" , NULL, NULL, NULL },
+{ TARGET_NR_setrlimit, "setrlimit" , "%s(%d,%p)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_set_robu

Re: [RFC PATCH v2 03/22] i386/xen: Add xen-version machine property and init KVM Xen support

[Paul Durrant]

On 12/12/2022 17:30, Paolo Bonzini wrote:
[snip]


The platform device can be created either in mc->kvm_type or manually 
(not sure if it makes sense to have a "XenVMMXenVMM" CPUID + emulated 
hypercalls but no platform device---would it still use pvclock for 
example?).




Not sure it's wise but the platform device is certainly optional in 
xl.cfg so you can easily bring up a VM without it.


  Paul

[PATCH] MAINTAINERS: Add documentation files to the corresponding sections

[Thomas Huth]

A lot of files in the docs directory do not have a maintainer according to
our MAINTAINERS file, though they can be clearly associated with one of the
sections in there. Add the files now so that our scripts/get_maintainer.pl
script can output the right maintainer for them.

Signed-off-by: Thomas Huth 
---
 MAINTAINERS | 20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4e0e2ba36f..a0d8e9b086 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -78,6 +78,7 @@ M: Laurent Vivier 
 S: Maintained
 L: qemu-triv...@nongnu.org
 K: ^Subject:.*(?i)trivial
+F: docs/devel/trivial-patches.rst
 T: git git://git.corpit.ru/qemu.git trivial-patches
 T: git https://github.com/vivier/qemu.git trivial-patches
 
@@ -131,6 +132,7 @@ F: util/cacheinfo.c
 F: util/cacheflush.c
 F: scripts/decodetree.py
 F: docs/devel/decodetree.rst
+F: docs/devel/tcg*
 F: include/exec/cpu*.h
 F: include/exec/exec-all.h
 F: include/exec/helper*.h
@@ -256,6 +258,7 @@ F: 
tests/docker/dockerfiles/debian-nios2-cross.d/build-toolchain.sh
 OpenRISC TCG CPUs
 M: Stafford Horne 
 S: Odd Fixes
+F: docs/system/openrisc/cpu-features.rst
 F: target/openrisc/
 F: hw/openrisc/
 F: tests/tcg/openrisc/
@@ -334,6 +337,7 @@ F: target/i386/tcg/
 F: tests/tcg/i386/
 F: tests/tcg/x86_64/
 F: hw/i386/
+F: docs/system/i386/cpu.rst
 F: docs/system/cpu-models-x86*
 T: git https://gitlab.com/ehabkost/qemu.git x86-next
 
@@ -875,6 +879,7 @@ M: Peter Maydell 
 R: Jean-Christophe Dubois 
 L: qemu-...@nongnu.org
 S: Odd Fixes
+F: docs/system/arm/sabrelite.rst
 F: hw/arm/sabrelite.c
 F: hw/arm/fsl-imx6.c
 F: hw/misc/imx6_*.c
@@ -1275,6 +1280,7 @@ OpenRISC Machines
 or1k-sim
 M: Jia Liu 
 S: Maintained
+F: docs/system/openrisc/or1k-sim.rst
 F: hw/openrisc/openrisc_sim.c
 
 PowerPC Machines
@@ -2018,6 +2024,7 @@ F: hw/virtio/trace-events
 F: qapi/virtio.json
 F: net/vhost-user.c
 F: include/hw/virtio/
+F: docs/devel/virtio*
 
 virtio-balloon
 M: Michael S. Tsirkin 
@@ -2110,7 +2117,7 @@ F: tests/qtest/virtio-rng-test.c
 vhost-user-rng
 M: Mathieu Poirier 
 S: Supported
-F: docs/tools/vhost-user-rng.rst
+F: docs/system/devices/vhost-user-rng.rst
 F: hw/virtio/vhost-user-rng.c
 F: hw/virtio/vhost-user-rng-pci.c
 F: include/hw/virtio/vhost-user-rng.h
@@ -2148,7 +2155,7 @@ S: Supported
 F: hw/nvme/*
 F: include/block/nvme.h
 F: tests/qtest/nvme-test.c
-F: docs/system/nvme.rst
+F: docs/system/devices/nvme.rst
 T: git git://git.infradead.org/qemu-nvme.git nvme-next
 
 megasas
@@ -2698,6 +2705,7 @@ GDB stub
 M: Alex Bennée 
 R: Philippe Mathieu-Daudé 
 S: Maintained
+F: docs/system/gdb.rst
 F: gdbstub/*
 F: include/exec/gdbstub.h
 F: gdb-xml/
@@ -2755,6 +2763,7 @@ F: ui/
 F: include/ui/
 F: qapi/ui.json
 F: util/drm.c
+F: docs/devel/ui.rst
 
 Cocoa graphics
 M: Peter Maydell 
@@ -2932,6 +2941,7 @@ M: Paolo Bonzini 
 R: Daniel P. Berrange 
 R: Eduardo Habkost 
 S: Supported
+F: docs/devel/qom.rst
 F: docs/qdev-device-use.txt
 F: hw/core/qdev*
 F: hw/core/bus.c
@@ -2980,6 +2990,7 @@ F: softmmu/qtest.c
 F: accel/qtest/
 F: tests/qtest/
 F: docs/devel/qgraph.rst
+F: docs/devel/qtest.rst
 X: tests/qtest/bios-tables-test*
 
 Device Fuzzing
@@ -3046,6 +3057,7 @@ F: include/sysemu/tpm*
 F: qapi/tpm.json
 F: backends/tpm/
 F: tests/qtest/*tpm*
+F: docs/specs/tpm.rst
 T: git https://github.com/stefanberger/qemu-tpm.git tpm-next
 
 Checkpatch
@@ -3197,7 +3209,8 @@ F: replay/*
 F: block/blkreplay.c
 F: net/filter-replay.c
 F: include/sysemu/replay.h
-F: docs/replay.txt
+F: docs/devel/replay.rst
+F: docs/system/replay.rst
 F: stubs/replay.c
 F: tests/avocado/replay_kernel.py
 F: tests/avocado/replay_linux.py
@@ -3721,6 +3734,7 @@ F: tests/docker/
 F: tests/vm/
 F: tests/lcitool/
 F: scripts/archive-source.sh
+F: docs/devel/testing.rst
 W: https://gitlab.com/qemu-project/qemu/pipelines
 W: https://travis-ci.org/qemu/qemu
 
-- 
2.31.1

[PATCH 2/2] linux-user: Allow sendmsg() without IOV

[Helge Deller]

Applications do call sendmsg() without any IOV, e.g.:
 sendmsg(4, {msg_name=NULL, msg_namelen=0, msg_iov=NULL, msg_iovlen=0,
msg_control=[{cmsg_len=36, cmsg_level=SOL_ALG, cmsg_type=0x2}],
msg_controllen=40, msg_flags=0}, MSG_MORE) = 0
 sendmsg(4, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="The quick brown 
fox jumps over t"..., iov_len=183}],
msg_iovlen=1, msg_control=[{cmsg_len=20, cmsg_level=SOL_ALG, 
cmsg_type=0x3}],
msg_controllen=24, msg_flags=0}, 0) = 183

The function do_sendrecvmsg_locked() is used for sndmsg() and recvmsg()
and calls lock_iovec() to lock the IOV into memory. For the first
sendmsg() above it returns NULL and thus wrongly skips the call the host
sendmsg() syscall, which will break the calling application.

Fix this issue by:
- allowing sendmsg() even with empty IOV
- skip recvmsg() if IOV is NULL
- skip both if the return code of do_sendrecvmsg_locked() != 0, which
  indicates some failure like EFAULT on the IOV

Tested with the debian "ell" package with hppa guest on x86_64 host.

Signed-off-by: Helge Deller 
---
 linux-user/syscall.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index a365903a3a..9e2c0a18fc 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3330,7 +3330,10 @@ static abi_long do_sendrecvmsg_locked(int fd, struct 
target_msghdr *msgp,
  target_vec, count, send);
 if (vec == NULL) {
 ret = -host_to_target_errno(errno);
-goto out2;
+/* allow sending packet without any iov, e.g. with MSG_MORE flag */
+if (!send || ret) {
+goto out2;
+}
 }
 msg.msg_iovlen = count;
 msg.msg_iov = vec;
@@ -3382,7 +3385,9 @@ static abi_long do_sendrecvmsg_locked(int fd, struct 
target_msghdr *msgp,
 }

 out:
-unlock_iovec(vec, target_vec, count, !send);
+if (vec) {
+unlock_iovec(vec, target_vec, count, !send);
+}
 out2:
 return ret;
 }
--
2.38.1

[PATCH 1/2] linux-user: Implement SOL_ALG encryption support

[Helge Deller]

Add suport to handle SOL_ALG packets via sendmsg() and recvmsg().
This allows emulated userspace to use encryption functionality.

Tested with the debian ell package with hppa guest on x86_64 host.

Signed-off-by: Helge Deller 
---
 linux-user/syscall.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 71ae867024..a365903a3a 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1871,6 +1871,14 @@ static inline abi_long target_to_host_cmsg(struct msghdr 
*msgh,
 __get_user(cred->pid, &target_cred->pid);
 __get_user(cred->uid, &target_cred->uid);
 __get_user(cred->gid, &target_cred->gid);
+} else if (cmsg->cmsg_level == SOL_ALG) {
+uint32_t *dst = (uint32_t *)data;
+
+memcpy(dst, target_data, len);
+/* fix endianess of first 32-bit word */
+if (len >= sizeof(uint32_t)) {
+*dst = tswap32(*dst);
+}
 } else {
 qemu_log_mask(LOG_UNIMP, "Unsupported ancillary data: %d/%d\n",
   cmsg->cmsg_level, cmsg->cmsg_type);
--
2.38.1

Re: [PATCH v2 14/15] block: Don't poll in bdrv_replace_child_noperm()

[Paolo Bonzini]

On 12/12/22 16:57, Kevin Wolf wrote:

I looks to me like this is a problem with the test case rather than the
change per se. It seems to be fixed with this patch that is already
posted as part of the next series:

[PATCH 09/18] test-bdrv-drain: Fix incorrrect drain assumptions
https://lists.gnu.org/archive/html/qemu-block/2022-12/msg00165.html


Cool, thanks.  I'll retest once the series hits block-next.  In the 
meanwhile you can ignore the first three patches of 
http://patchew.org/QEMU/20221212125920.248567-1-pbonz...@redhat.com.


Paolo

Re: [PATCH 21/30] build: move sanitizer tests to meson

[Paolo Bonzini]

On 12/12/22 13:16, Marc-André Lureau wrote:

No, it compiles successfully with clang (Fedora 15.0.4-1.fc37) and
glibc-2.36-8.fc37.x86_64 at least.

I guess we need someone to check on macos with glibc 2.35
(https://formulae.brew.sh/formula/glibc#default)


That's a Linux-only formula, so no need to check with macos.  I'll leave 
this patch as is, we can remove it later.


Paolo

Re: [RFC PATCH v2 03/22] i386/xen: Add xen-version machine property and init KVM Xen support

[Paolo Bonzini]

On 12/9/22 10:55, David Woodhouse wrote:

-m->default_machine_opts = "accel=xen,suppress-vmdesc=on";
+if (xen_enabled())
+m->default_machine_opts = "accel=xen,suppress-vmdesc=on";
+else
+m->default_machine_opts = "accel=kvm,xen-version=0x30001";


Please do not modify pc_xen_hvm_init().

"-M xenfv" should be the same as "-M pc-i440fx-...,suppress-vmdesc=on 
-accel xen -device xen-platform".  It must work *without* "-accel xen", 
while here you're basically requiring it.  For now, please make 
KVM-emulated Xen use "-M pc -device xen-platform".  We can figure out 
"-M xenfv" later.


You can instead have:

- a check in xen_init() that checks that xen_mode is XEN_ATTACH.  If 
not, fail.


- an extra enum value for xen_mode, XEN_DISABLED, which is the default 
instead of XEN_EMULATE;


- an accelerator property "-accel kvm,xen-version=...", added in 
kvm_accel_class_init() instead of the machine property.  The property, 
when set to a nonzero value, flips xen_mode from XEN_DISABLED to 
XEN_EMULATE.


The Xen overlay device can be created using the mc->kvm_type function 
(which you can set in DEFINE_PC_MACHINE); at that point, xen_mode has 
switched from XEN_DISABLED to XEN_EMULATE.  Those xen_enabled() checks 
that apply to KVM then become xen_mode != XEN_DISABLED, as long as they 
run during mc->kvm_type or afterwards.


The platform device can be created either in mc->kvm_type or manually 
(not sure if it makes sense to have a "XenVMMXenVMM" CPUID + emulated 
hypercalls but no platform device---would it still use pvclock for 
example?).


Paolo

Re: [PATCH 00/18] block: Introduce a block graph rwlock

[Kevin Wolf]

Am 07.12.2022 um 15:12 hat Emanuele Giuseppe Esposito geschrieben:
> Am 07/12/2022 um 14:18 schrieb Kevin Wolf:
> > This series supersedes the first half of Emanuele's "Protect the block
> > layer with a rwlock: part 1". It introduces the basic infrastructure for
> > protecting the block graph (specifically parent/child links) with a
> > rwlock. Actually taking the reader lock in all necessary places is left
> > for future series.
> > 
> > Compared to Emanuele's series, this one adds patches to make use of
> > clang's Thread Safety Analysis (TSA) feature in order to statically
> > check at compile time that the places where we assert that we hold the
> > lock actually do hold it. Once we cover all relevant places, the check
> > can be extended to verify that all accesses of bs->children and
> > bs->parents hold the lock.
> > 
> > For reference, here is the more detailed version of our plan in
> > Emanuele's words from his series:
> > 
> > The aim is to replace the current AioContext lock with much
> > fine-grained locks, aimed to protect only specific data. Currently
> > the AioContext lock is used pretty much everywhere, and it's not
> > even clear what it is protecting exactly.
> > 
> > The aim of the rwlock is to cover graph modifications: more
> > precisely, when a BlockDriverState parent or child list is modified
> > or read, since it can be concurrently accessed by the main loop and
> > iothreads.
> > 
> > The main assumption is that the main loop is the only one allowed to
> > perform graph modifications, and so far this has always been held by
> > the current code.
> > 
> > The rwlock is inspired from cpus-common.c implementation, and aims
> > to reduce cacheline bouncing by having per-aiocontext counter of
> > readers.  All details and implementation of the lock are in patch 2.
> > 
> > We distinguish between writer (main loop, under BQL) that modifies
> > the graph, and readers (all other coroutines running in various
> > AioContext), that go through the graph edges, reading ->parents
> > and->children.  The writer (main loop)  has an "exclusive" access,
> > so it first waits for current read to finish, and then prevents
> > incoming ones from entering while it has the exclusive access.  The
> > readers (coroutines in multiple AioContext) are free to access the
> > graph as long the writer is not modifying the graph.  In case it is,
> > they go in a CoQueue and sleep until the writer is done.
> > 
> > In this and following series, we try to follow the following locking
> > pattern:
> > 
> > - bdrv_co_* functions that call BlockDriver callbacks always expect
> >   the lock to be taken, therefore they assert.
> > 
> > - blk_co_* functions are called from external code outside the block
> >   layer, which should not have to care about the block layer's
> >   internal locking. Usually they also call blk_wait_while_drained().
> >   Therefore they take the lock internally.
> > 
> > The long term goal of this series is to eventually replace the
> > AioContext lock, so that we can get rid of it once and for all.
> > 
> > Emanuele Giuseppe Esposito (7):
> >   graph-lock: Implement guard macros
> >   async: Register/unregister aiocontext in graph lock list
> >   block: wrlock in bdrv_replace_child_noperm
> >   block: remove unnecessary assert_bdrv_graph_writable()
> >   block: assert that graph read and writes are performed correctly
> >   block-coroutine-wrapper.py: introduce annotations that take the graph
> > rdlock
> >   block: use co_wrapper_mixed_bdrv_rdlock in functions taking the rdlock
> > 
> > Kevin Wolf (10):
> >   block: Factor out bdrv_drain_all_begin_nopoll()
> >   Import clang-tsa.h
> >   clang-tsa: Add TSA_ASSERT() macro
> >   clang-tsa: Add macros for shared locks
> >   configure: Enable -Wthread-safety if present
> >   test-bdrv-drain: Fix incorrrect drain assumptions
> >   block: Fix locking in external_snapshot_prepare()
> >   graph-lock: TSA annotations for lock/unlock functions
> >   Mark assert_bdrv_graph_readable/writable() GRAPH_RD/WRLOCK
> >   block: GRAPH_RDLOCK for functions only called by co_wrappers
> > 
> > Paolo Bonzini (1):
> >   graph-lock: Introduce a lock to protect block graph operations
> > 
> Reviewed-by: Emanuele Giuseppe Esposito 

Thanks, applied to block-next.

Kevin

Re: [RFC PATCH v2 12/22] hw/xen: Add xen_overlay device for emulating shared xenheap pages

[Paolo Bonzini]

On 12/9/22 10:56, David Woodhouse wrote:

Expecting some heckling at the use of xen_overlay_singleton. What is
the best way to do that? Using qemu_find_recursive() every time seemed
a bit wrong. But I suppose mapping it into the*guest*  isn't a fast
path, and if the actual grant table code is allowed to just stash the
pointer it gets from xen_overlay_page_ptr() for later use then that
isn't a fast path for device I/O either.


Nah, it's fine.  However I'll reply to patch 2 as to how to handle Xen 
vs. KVM mode.


Paolo

[PATCH] MAINTAINERS: Add MIPS-related docs and configs to the MIPS architecture section

[Thomas Huth]

docs/system/target-mips.rst and configs/targets/mips* are not covered
in our MAINTAINERS file yet, so let's add them now.

Signed-off-by: Thomas Huth 
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6966490c94..4e0e2ba36f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -112,6 +112,8 @@ M: Philippe Mathieu-Daudé 
 R: Jiaxun Yang 
 S: Odd Fixes
 K: ^Subject:.*(?i)mips
+F: docs/system/target-mips.rst
+F: configs/targets/mips*
 
 Guest CPU cores (TCG)
 -
-- 
2.31.1

Re: [RFC PATCH v2 02/22] xen: add CONFIG_XENFV_MACHINE and CONFIG_XEN_EMU options for Xen emulation

[Paolo Bonzini]

On 12/9/22 10:55, David Woodhouse wrote:

  config KVM
  bool
+imply XEN_EMU if (I386 || X86_64)


No need for the "imply", just make it "default y" below and it will have 
the same effect.




diff --git a/target/Kconfig b/target/Kconfig
index 83da0bd293..e19c9d77b5 100644
--- a/target/Kconfig
+++ b/target/Kconfig
@@ -18,3 +18,7 @@ source sh4/Kconfig
 source sparc/Kconfig
 source tricore/Kconfig
 source xtensa/Kconfig
+
+config XEN_EMU
+bool
+depends on KVM && (I386 || X86_64)


Please place it in hw/xen/Kconfig.

Paolo

Re: [RFC PATCH v2 10/22] i386/xen: handle guest hypercalls

[Paolo Bonzini]

On 12/9/22 10:56, David Woodhouse wrote:

+static bool __kvm_xen_handle_exit(X86CPU *cpu, struct kvm_xen_exit *exit)


No double underscores in userspace.


+{
+uint16_t code = exit->u.hcall.input;
+
+if (exit->u.hcall.cpl > 0) {
+exit->u.hcall.result = -EPERM;
+return true;
+}
+
+switch (code) {
+default:
+return false;
+}
+}
+
+int kvm_xen_handle_exit(X86CPU *cpu, struct kvm_xen_exit *exit)


BTW, please name this file either target/i386/xen-emu.c or 
target/i386/kvm/xen-emu.c (probably the latter is better, since XEN_EMU 
depends on KVM.)


Paolo

Re: [PATCH] tpm: add backend for mssim

[Stefan Berger]

On 12/12/22 11:38, James Bottomley wrote:

On Mon, 2022-12-12 at 15:47 +, Daniel P. Berrangé wrote:

Copy'ing Markus for QAPI design feedback.

On Sat, Dec 10, 2022 at 12:10:18PM -0500, James Bottomley wrote:

The Microsoft Simulator (mssim) is the reference emulation platform
for the TCG TPM 2.0 specification.

https://github.com/Microsoft/ms-tpm-20-ref.git

It exports a fairly simple network socket baset protocol on two
sockets, one for command (default 2321) and one for control
(default 2322).  This patch adds a simple backend that can speak
the mssim protocol over the network.  It also allows the host, and
two ports to be specified on the qemu command line.  The benefits
are twofold: firstly it gives us a backend that actually speaks a
standard TPM emulation protocol instead of the linux specific TPM
driver format of the current emulated TPM backend and secondly,
using the microsoft protocol, the end point of the emulator can be
anywhere on the network, facilitating the cloud use case where a
central TPM service can be used over a control network.


What's the story with security for this ?  The patch isn't using
TLS, so talking to any emulator over anything other than localhost
looks insecure, unless I'm missing something.


Pretty much every TPM application fears interposers and should thus be
using the TPM transport security anyway. *If* this is the case, then
the transport is secure.  Note that this currently isn't the case for


What about all the older kernels that are out there?


the kernel use of the TPM, but I'm trying to fix that.  The standard
mssim server is too simplistic to do transport layer security, but like
everything that does this (or rather doesn't do this), you can front it
with stunnel4.


And who or what is going to set this up?

   Stefan

RE: [PATCH] include: Don't include qemu/osdep.h

[Taylor Simpson]

> -Original Message-
> From: Markus Armbruster 
> Sent: Monday, December 12, 2022 1:05 AM
> To: qemu-devel@nongnu.org
> Cc: i...@bsdimp.com; kev...@freebsd.org; berra...@redhat.com;
> jonathan.came...@huawei.com; kbast...@mail.uni-paderborn.de;
> jasow...@redhat.com; michael.r...@amd.com; kkost...@redhat.com;
> Taylor Simpson ; pal...@dabbelt.com;
> alistair.fran...@wdc.com; bin.m...@windriver.com; qemu-
> ri...@nongnu.org
> Subject: [PATCH] include: Don't include qemu/osdep.h
> 
> docs/devel/style.rst mandates:
> 
> The "qemu/osdep.h" header contains preprocessor macros that affect
> the behavior of core system headers like .  It must be
> the first include so that core system headers included by external
> libraries get the preprocessor macros that QEMU depends on.
> 
> Do not include "qemu/osdep.h" from header files since the .c file
> will have already included it.
> 
> A few violations have crept in.  Fix them.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  bsd-user/qemu.h | 1 -
>  crypto/block-luks-priv.h| 1 -
>  include/hw/cxl/cxl_host.h   | 1 -
>  include/hw/input/pl050.h| 1 -
>  include/hw/tricore/triboard.h   | 1 -
>  include/qemu/userfaultfd.h  | 1 -
>  net/vmnet_int.h | 1 -
>  qga/cutils.h| 1 -
>  target/hexagon/hex_arch_types.h | 1 -
>  target/hexagon/mmvec/macros.h   | 1 -
>  target/riscv/pmu.h  | 1 -
>  qga/cutils.c| 3 ++-
>  12 files changed, 2 insertions(+), 12 deletions(-)
> 
> diff --git a/target/hexagon/hex_arch_types.h
> b/target/hexagon/hex_arch_types.h index 885f68f760..52a7f2b2f3 100644
> --- a/target/hexagon/hex_arch_types.h
> +++ b/target/hexagon/hex_arch_types.h
> @@ -18,7 +18,6 @@
>  #ifndef HEXAGON_HEX_ARCH_TYPES_H
>  #define HEXAGON_HEX_ARCH_TYPES_H
> 
> -#include "qemu/osdep.h"
>  #include "mmvec/mmvec.h"
>  #include "qemu/int128.h"

Please change the copyright year in this file from "2019-2021" to "2019-2022".

Otherwise
Reviewed-by: Taylor Simpson 

Re: [PATCH 30/30] meson: always log qemu-iotests verbosely

[Peter Maydell]

On Fri, 9 Dec 2022 at 11:44, Paolo Bonzini  wrote:
>
> ---
>  tests/qemu-iotests/meson.build | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
> index 583468c5b9b3..3d8637c8f2b6 100644
> --- a/tests/qemu-iotests/meson.build
> +++ b/tests/qemu-iotests/meson.build
> @@ -43,5 +43,6 @@ foreach format, speed: qemu_iotests_formats
> protocol: 'tap',
> suite: suites,
> timeout: 0,
> +   verbose: true,
> is_parallel: false)
>  endforeach

How much does this increase the size of a build-and-test logfile by?
I know the gitlab CI has a size limit on that these days, so if this
noticeably increases the total log size we might want to check how
close we are to the limit...

thanks
-- PMM

Re: [PATCH 2/3] qapi, audio: respect build time conditions in audio schema

[Thomas Huth]

On 02/03/2021 18.55, Daniel P. Berrangé wrote:

Currently the -audiodev accepts any audiodev type regardless of what is
built in to QEMU. An error only occurs later at runtime when a sound
device tries to use the audio backend.

With this change QEMU will immediately reject -audiodev args that are
not compiled into the binary. The QMP schema will also be introspectable
to identify what is compiled in.

Signed-off-by: Daniel P. Berrangé 
---
  audio/audio.c  | 16 +++
  audio/audio_legacy.c   | 41 ++-
  audio/audio_template.h | 16 +++
  qapi/audio.json| 44 --
  4 files changed, 106 insertions(+), 11 deletions(-)


 Hi Daniel!

Would you have time to respin this patch for QEMU 8.0 ?

 Thomas



diff --git a/audio/audio.c b/audio/audio.c
index 40a4bbd7ce..53434fc674 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1989,14 +1989,30 @@ void audio_create_pdos(Audiodev *dev)
  break
  
  CASE(NONE, none, );

+#ifdef CONFIG_AUDIO_ALSA
  CASE(ALSA, alsa, Alsa);
+#endif
+#ifdef CONFIG_AUDIO_COREAUDIO
  CASE(COREAUDIO, coreaudio, Coreaudio);
+#endif
+#ifdef CONFIG_AUDIO_DSOUND
  CASE(DSOUND, dsound, );
+#endif
+#ifdef CONFIG_AUDIO_JACK
  CASE(JACK, jack, Jack);
+#endif
+#ifdef CONFIG_AUDIO_OSS
  CASE(OSS, oss, Oss);
+#endif
+#ifdef CONFIG_AUDIO_PA
  CASE(PA, pa, Pa);
+#endif
+#ifdef CONFIG_AUDIO_SDL
  CASE(SDL, sdl, Sdl);
+#endif
+#ifdef CONFIG_SPICE
  CASE(SPICE, spice, );
+#endif
  CASE(WAV, wav, );
  
  case AUDIODEV_DRIVER__MAX:

diff --git a/audio/audio_legacy.c b/audio/audio_legacy.c
index 0fe827b057..bb2268f2b2 100644
--- a/audio/audio_legacy.c
+++ b/audio/audio_legacy.c
@@ -93,6 +93,7 @@ static void get_fmt(const char *env, AudioFormat *dst, bool 
*has_dst)
  }
  
  
+#if defined(CONFIG_AUDIO_ALSA) || defined(CONFIG_AUDIO_DSOUND)

  static void get_millis_to_usecs(const char *env, uint32_t *dst, bool *has_dst)
  {
  const char *val = getenv(env);
@@ -101,15 +102,20 @@ static void get_millis_to_usecs(const char *env, uint32_t 
*dst, bool *has_dst)
  *has_dst = true;
  }
  }
+#endif
  
+#if defined(CONFIG_AUDIO_ALSA) || defined(CONFIG_AUDIO_COREAUDIO) || \

+defined(CONFIG_AUDIO_PA) || defined(CONFIG_AUDIO_SDL) || \
+defined(CONFIG_AUDIO_DSOUND) || defined(CONFIG_AUDIO_OSS)
  static uint32_t frames_to_usecs(uint32_t frames,
  AudiodevPerDirectionOptions *pdo)
  {
  uint32_t freq = pdo->has_frequency ? pdo->frequency : 44100;
  return (frames * 100 + freq / 2) / freq;
  }
+#endif
  
-

+#ifdef CONFIG_AUDIO_COREAUDIO
  static void get_frames_to_usecs(const char *env, uint32_t *dst, bool *has_dst,
  AudiodevPerDirectionOptions *pdo)
  {
@@ -119,14 +125,19 @@ static void get_frames_to_usecs(const char *env, uint32_t 
*dst, bool *has_dst,
  *has_dst = true;
  }
  }
+#endif
  
+#if defined(CONFIG_AUDIO_PA) || defined(CONFIG_AUDIO_SDL) || \

+defined(CONFIG_AUDIO_DSOUND) || defined(CONFIG_AUDIO_OSS)
  static uint32_t samples_to_usecs(uint32_t samples,
   AudiodevPerDirectionOptions *pdo)
  {
  uint32_t channels = pdo->has_channels ? pdo->channels : 2;
  return frames_to_usecs(samples / channels, pdo);
  }
+#endif
  
+#if defined(CONFIG_AUDIO_PA) || defined(CONFIG_AUDIO_SDL)

  static void get_samples_to_usecs(const char *env, uint32_t *dst, bool 
*has_dst,
   AudiodevPerDirectionOptions *pdo)
  {
@@ -136,7 +147,9 @@ static void get_samples_to_usecs(const char *env, uint32_t 
*dst, bool *has_dst,
  *has_dst = true;
  }
  }
+#endif
  
+#if defined(CONFIG_AUDIO_DSOUND) || defined(CONFIG_AUDIO_OSS)

  static uint32_t bytes_to_usecs(uint32_t bytes, AudiodevPerDirectionOptions 
*pdo)
  {
  AudioFormat fmt = pdo->has_format ? pdo->format : AUDIO_FORMAT_S16;
@@ -153,8 +166,11 @@ static void get_bytes_to_usecs(const char *env, uint32_t 
*dst, bool *has_dst,
  *has_dst = true;
  }
  }
+#endif
  
  /* backend specific functions */

+
+#ifdef CONFIG_AUDIO_ALSA
  /* ALSA */
  static void handle_alsa_per_direction(
  AudiodevAlsaPerDirectionOptions *apdo, const char *prefix)
@@ -200,7 +216,9 @@ static void handle_alsa(Audiodev *dev)
  get_millis_to_usecs("QEMU_ALSA_THRESHOLD",
  &aopt->threshold, &aopt->has_threshold);
  }
+#endif
  
+#ifdef CONFIG_AUDIO_COREAUDIO

  /* coreaudio */
  static void handle_coreaudio(Audiodev *dev)
  {
@@ -213,7 +231,9 @@ static void handle_coreaudio(Audiodev *dev)
  &dev->u.coreaudio.out->buffer_count,
  &dev->u.coreaudio.out->has_buffer_count);
  }
+#endif
  
+#ifdef CONFIG_AUDIO_DSOUND

  /* dsound */
  static void handle_dsound(Audiodev *dev)
  {
@@ -228,7 +248,9 @@ static void handle_dsound(Audiodev *dev)
   

[RFC v2 0/3] migration: reduce time of loading non-iterable vmstate

[Chuang Xu]

Hi!

In this version:

- rebase to latest upstream.
- add sanity check to address_space_to_flatview().
- postpone the init of the vring cache until migration's loading completes. 

Please review, Chuang.

[v1]

The duration of loading non-iterable vmstate accounts for a significant
portion of downtime (starting with the timestamp of source qemu stop and
ending with the timestamp of target qemu start). Most of the time is spent
committing memory region changes repeatedly.

This patch packs all the changes to memory region during the period of
loading non-iterable vmstate in a single memory transaction. With the
increase of devices, this patch will greatly improve the performance.

Here are the test results:
test vm info:
- 32 CPUs 128GB RAM
- 8 16-queue vhost-net device
- 16 4-queue vhost-user-blk device.

time of loading non-iterable vmstate
before  about 210 ms
after   about 40 ms

[RFC v2 3/3] migration: reduce time of loading non-iterable vmstate

[Chuang Xu]

The duration of loading non-iterable vmstate accounts for a significant
portion of downtime (starting with the timestamp of source qemu stop and
ending with the timestamp of target qemu start). Most of the time is spent
committing memory region changes repeatedly.

This patch packs all the changes to memory region during the period of
loading non-iterable vmstate in a single memory transaction. With the
increase of devices, this patch will greatly improve the performance.

Here are the test results:
test vm info:
- 32 CPUs 128GB RAM
- 8 16-queue vhost-net device
- 16 4-queue vhost-user-blk device.

time of loading non-iterable vmstate
before  about 210 ms
after   about 40 ms

Signed-off-by: Chuang Xu 
---
 migration/savevm.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/migration/savevm.c b/migration/savevm.c
index a0cdb714f7..68a7a99b79 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2617,6 +2617,9 @@ int qemu_loadvm_state_main(QEMUFile *f, 
MigrationIncomingState *mis)
 uint8_t section_type;
 int ret = 0;
 
+/* call memory_region_transaction_begin() before loading vmstate */
+memory_region_transaction_begin();
+
 retry:
 while (true) {
 section_type = qemu_get_byte(f);
@@ -2684,6 +2687,16 @@ out:
 goto retry;
 }
 }
+
+/*
+ * call memory_region_transaction_commit() after loading non-iterable
+ * vmstate, make sure the migration_enable_load_check_delay flag is
+ * true during commit.
+ */
+migration_enable_load_check_delay = true;
+memory_region_transaction_commit();
+migration_enable_load_check_delay = false;
+
 return ret;
 }
 
-- 
2.20.1

[RFC v2 1/3] memory: add depth assert in address_space_to_flatview

[Chuang Xu]

Before using any flatview, sanity check we're not during a memory
region transaction or the map can be invalid.

Signed-off-by: Chuang Xu 
---
 include/exec/memory.h | 9 +
 softmmu/memory.c  | 1 -
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 91f8a2395a..b43cd46084 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1069,8 +1069,17 @@ struct FlatView {
 MemoryRegion *root;
 };
 
+static unsigned memory_region_transaction_depth;
+
 static inline FlatView *address_space_to_flatview(AddressSpace *as)
 {
+/*
+ * Before using any flatview, sanity check we're not during a memory
+ * region transaction or the map can be invalid.  Note that this can
+ * also be called during commit phase of memory transaction, but that
+ * should also only happen when the depth decreases to 0 first.
+ */
+assert(memory_region_transaction_depth == 0);
 return qatomic_rcu_read(&as->current_map);
 }
 
diff --git a/softmmu/memory.c b/softmmu/memory.c
index bc0be3f62c..f177c40cd8 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -37,7 +37,6 @@
 
 //#define DEBUG_UNASSIGNED
 
-static unsigned memory_region_transaction_depth;
 static bool memory_region_update_pending;
 static bool ioeventfd_update_pending;
 unsigned int global_dirty_tracking;
-- 
2.20.1

[RFC v2 2/3] virtio: support delay of checks in virtio_load()

[Chuang Xu]

Delay checks in virtio_load() to avoid possible address_space_to_flatview() call
during memory region's begin/commit.

Signed-off-by: Chuang Xu 
---
 hw/virtio/virtio.c  | 33 ++---
 include/sysemu/sysemu.h |  1 +
 softmmu/globals.c   |  3 +++
 3 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index eb6347ab5d..3e3fa2a89d 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -33,6 +33,7 @@
 #include "hw/virtio/virtio-access.h"
 #include "sysemu/dma.h"
 #include "sysemu/runstate.h"
+#include "sysemu/sysemu.h"
 #include "standard-headers/linux/virtio_ids.h"
 #include "standard-headers/linux/vhost_types.h"
 #include "standard-headers/linux/virtio_blk.h"
@@ -3642,8 +3643,20 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int 
version_id)
 vdev->start_on_kick = true;
 }
 
+if (vdc->post_load) {
+ret = vdc->post_load(vdev);
+if (ret) {
+return ret;
+}
+}
+
+return 0;
+}
+
+static void virtio_load_check_delay(VirtIODevice *vdev)
+{
 RCU_READ_LOCK_GUARD();
-for (i = 0; i < num; i++) {
+for (int i = 0; i < VIRTIO_QUEUE_MAX; i++) {
 if (vdev->vq[i].vring.desc) {
 uint16_t nheads;
 
@@ -3696,19 +3709,12 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int 
version_id)
  i, vdev->vq[i].vring.num,
  vdev->vq[i].last_avail_idx,
  vdev->vq[i].used_idx);
-return -1;
+abort();
 }
 }
 }
 
-if (vdc->post_load) {
-ret = vdc->post_load(vdev);
-if (ret) {
-return ret;
-}
-}
-
-return 0;
+return;
 }
 
 void virtio_cleanup(VirtIODevice *vdev)
@@ -4158,7 +4164,12 @@ static void virtio_memory_listener_commit(MemoryListener 
*listener)
 if (vdev->vq[i].vring.num == 0) {
 break;
 }
-virtio_init_region_cache(vdev, i);
+
+if (migration_enable_load_check_delay) {
+virtio_load_check_delay(vdev);
+} else {
+virtio_init_region_cache(vdev, i);
+}
 }
 }
 
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 6a7a31e64d..0523091445 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -12,6 +12,7 @@ extern int only_migratable;
 extern const char *qemu_name;
 extern QemuUUID qemu_uuid;
 extern bool qemu_uuid_set;
+extern bool migration_enable_load_check_delay;
 
 const char *qemu_get_vm_name(void);
 
diff --git a/softmmu/globals.c b/softmmu/globals.c
index 527edbefdd..1bd8f6c978 100644
--- a/softmmu/globals.c
+++ b/softmmu/globals.c
@@ -65,3 +65,6 @@ bool qemu_uuid_set;
 uint32_t xen_domid;
 enum xen_mode xen_mode = XEN_EMULATE;
 bool xen_domid_restrict;
+
+bool migration_enable_load_check_delay;
+
-- 
2.20.1

Re: [PATCH 30/30] meson: always log qemu-iotests verbosely

[Paolo Bonzini]

On 12/12/22 15:02, Marc-André Lureau wrote:

Sounds fine, but I like silence too. Could you give a bit of motivation details?
thanks!


One qemu-iotests TAP testcase is comparable to a test() invocation 
elsewhere (in terms of both runtime and kind of test).  This makes it 
useful to see the last logged test.


Paolo

Re: [RFC PATCH v2 20/22] i386/xen: HVMOP_set_param / HVM_PARAM_CALLBACK_IRQ

[Paul Durrant]

On 12/12/2022 16:26, David Woodhouse wrote:

On Mon, 2022-12-12 at 16:16 +, Paul Durrant wrote:

On 09/12/2022 09:56, David Woodhouse wrote:

From: Ankur Arora 
The HVM_PARAM_CALLBACK_IRQ parameter controls the system-wide event
channel upcall method.  The vector support is handled by KVM internally,
when the evtchn_upcall_pending field in the vcpu_info is set.
The GSI and PCI_INTX delivery methods are not supported. yet; those
need to simulate a level-triggered event on the I/OAPIC.


That's gonna be somewhat limiting if anyone runs a Windows guest with
upcall vector support turned off... which is an option at:

https://xenbits.xen.org/gitweb/?p=pvdrivers/win/xenbus.git;a=blob;f=src/xenbus/evtchn.c;;hb=HEAD#l1928


Sure. And as you know, I also added the 'xen_no_vector_callback' option
to the Linux command line to allow for that mode to be tested with
Linux too: https://git.kernel.org/torvalds/c/b36b0fe96a

The GSI and PCI_INTX modes will be added in time, but not yet.


Ok, but maybe worth calling out the limitation in the commit comment for 
those wishing to kick the tyres.


  Paul

Re: [PATCH] tpm: add backend for mssim

[James Bottomley]

On Mon, 2022-12-12 at 15:47 +, Daniel P. Berrangé wrote:
> Copy'ing Markus for QAPI design feedback.
> 
> On Sat, Dec 10, 2022 at 12:10:18PM -0500, James Bottomley wrote:
> > The Microsoft Simulator (mssim) is the reference emulation platform
> > for the TCG TPM 2.0 specification.
> > 
> > https://github.com/Microsoft/ms-tpm-20-ref.git
> > 
> > It exports a fairly simple network socket baset protocol on two
> > sockets, one for command (default 2321) and one for control
> > (default 2322).  This patch adds a simple backend that can speak
> > the mssim protocol over the network.  It also allows the host, and
> > two ports to be specified on the qemu command line.  The benefits
> > are twofold: firstly it gives us a backend that actually speaks a
> > standard TPM emulation protocol instead of the linux specific TPM
> > driver format of the current emulated TPM backend and secondly,
> > using the microsoft protocol, the end point of the emulator can be
> > anywhere on the network, facilitating the cloud use case where a
> > central TPM service can be used over a control network.
> 
> What's the story with security for this ?  The patch isn't using
> TLS, so talking to any emulator over anything other than localhost
> looks insecure, unless I'm missing something.

Pretty much every TPM application fears interposers and should thus be
using the TPM transport security anyway. *If* this is the case, then
the transport is secure.  Note that this currently isn't the case for
the kernel use of the TPM, but I'm trying to fix that.  The standard
mssim server is too simplistic to do transport layer security, but like
everything that does this (or rather doesn't do this), you can front it
with stunnel4.

> > diff --git a/backends/tpm/tpm_mssim.c b/backends/tpm/tpm_mssim.c
> > new file mode 100644
> > index 00..6864b1fbc0
> > --- /dev/null
> > +++ b/backends/tpm/tpm_mssim.c
> > @@ -0,0 +1,266 @@
> > +/*
> > + * Emulator TPM driver which connects over the mssim protocol
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + *
> > + * Copyright (c) 2022
> 
> Copyright by whom ?  Presumably this line should have "IBM" present
> if we're going to have it at all.

It can either be me or IBM, we're joint owners, that's why I thought
just author.

> > + * Author: James Bottomley 
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/error-report.h"
> > +#include "qemu/sockets.h"
> > +
> > +#include "qapi/clone-visitor.h"
> > +#include "qapi/qapi-visit-tpm.h"
> > +
> > +#include "io/channel-socket.h"
> > +
> > +#include "sysemu/tpm_backend.h"
> > +#include "sysemu/tpm_util.h"
> > +
> > +#include "qom/object.h"
> > +
> > +#include "tpm_int.h"
> > +#include "tpm_mssim.h"
> > +
> 
> > +static TPMBackend *tpm_mssim_create(QemuOpts *opts)
> > +{
> > +    TPMBackend *be = TPM_BACKEND(object_new(TYPE_TPM_MSSIM));
> > +    TPMmssim *t = TPM_MSSIM(be);
> > +    InetSocketAddress cmd_s, ctl_s;
> > +    int sock;
> > +    const char *host, *port, *ctrl;
> > +    Error *errp = NULL;
> > +
> > +    host = qemu_opt_get(opts, "host");
> > +    if (!host)
> > +    host = "localhost";
> > +    t->opts.host = g_strdup(host);
> > +
> > +    port = qemu_opt_get(opts, "port");
> > +    if (!port)
> > +    port = "2321";
> > +    t->opts.port = g_strdup(port);
> > +
> > +    ctrl = qemu_opt_get(opts, "ctrl");
> > +    if (!ctrl)
> > +    ctrl = "2322";
> > +    t->opts.ctrl = g_strdup(ctrl);
> > +
> > +    cmd_s.host = (char *)host;
> > +    cmd_s.port = (char *)port;
> > +
> > +    ctl_s.host = (char *)host;
> > +    ctl_s.port = (char *)ctrl;
> > +
> > +    sock = inet_connect_saddr(&cmd_s, &errp);
> > +    if (sock < 0)
> > +    goto fail;
> > +    t->cmd_qc = QIO_CHANNEL(qio_channel_socket_new_fd(sock,
> > &errp));
> > +    if (errp)
> > +    goto fail;
> > +    sock = inet_connect_saddr(&ctl_s, &errp);
> > +    if (sock < 0)
> > +    goto fail_unref_cmd;
> > +    t->ctrl_qc = QIO_CHANNEL(qio_channel_socket_new_fd(sock,
> > &errp));
> > +    if (errp)
> > +    goto fail_unref_cmd;
> 
> We don't want to be using inet_connect_saddr, that's a legacy
> API. All new code should be using the qio_channel_socket_connect*
> family of APIs. This is trivial if the QAPI design uses SocketAddress
> structs directly.

Heh, well I just copied this from the ssh block backend ...

I can easily find something more modern to update it.

[...]
> >  
> > +##
> > +# @TPMmssimOptions:
> > +#
> > +# Information for the mssim emulator connection
> > +#
> > +# @host: host name or IP address to connect to
> > +# @port: port for the standard TPM commands
> > +# @ctrl: control port for TPM state changes
> > +#
> > +# Since: 7.2.0
> > +##
> > +{ 'struct': 'TPMmssimOptions',
> > +  'data': {
> > +  'host': 'str',
> > +  'port': 'str',
> > +  'ctrl': 'str' },
> > +  'if': 'CONFIG_TPM' }
> 
> We don't want to be adding new code using plain host/port combos,
> as that misses extra functionality for controlling IPv4 vs

Re: [RFC PATCH v2 22/22] i386/xen: implement HYPERVISOR_sched_op

[Paul Durrant]

On 09/12/2022 09:56, David Woodhouse wrote:

From: Joao Martins 

It allows to shutdown itself via hypercall with any of the 3 reasons:
   1) self-reboot
   2) shutdown
   3) crash

Implementing SCHEDOP_shutdown sub op let us handle crashes gracefully rather
than leading to triple faults if it remains unimplemented.

Signed-off-by: Joao Martins 
Signed-off-by: David Woodhouse 
---
  target/i386/xen.c | 45 +
  1 file changed, 45 insertions(+)

diff --git a/target/i386/xen.c b/target/i386/xen.c
index f102c40f04..5f3b91450d 100644
--- a/target/i386/xen.c
+++ b/target/i386/xen.c
@@ -17,6 +17,7 @@
  #include "trace.h"
  #include "hw/i386/kvm/xen_overlay.h"
  #include "hw/i386/kvm/xen_evtchn.h"
+#include "sysemu/runstate.h"
  
  #define __XEN_INTERFACE_VERSION__ 0x00040400
  
@@ -25,6 +26,7 @@

  #include "standard-headers/xen/hvm/hvm_op.h"
  #include "standard-headers/xen/hvm/params.h"
  #include "standard-headers/xen/vcpu.h"
+#include "standard-headers/xen/sched.h"
  #include "standard-headers/xen/event_channel.h"
  
  static bool kvm_gva_to_gpa(CPUState *cs, uint64_t gva, uint64_t *gpa,

@@ -491,6 +493,45 @@ static bool kvm_xen_hcall_evtchn_op(struct kvm_xen_exit 
*exit,
  return true;
  }
  
+static int schedop_shutdown(CPUState *cs, uint64_t arg)

+{
+struct sched_shutdown shutdown;
+
+if (kvm_copy_from_gva(cs, arg, &shutdown, sizeof(shutdown))) {
+return -EFAULT;
+}
+
+if (shutdown.reason == SHUTDOWN_crash) {
+cpu_dump_state(cs, stderr, CPU_DUMP_CODE);
+qemu_system_guest_panicked(NULL);
+} else if (shutdown.reason == SHUTDOWN_reboot) {
+qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
+} else if (shutdown.reason == SHUTDOWN_poweroff) {
+qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+}


Why not a switch() statement? I think it would look neater and be more 
consistent with other code.

With that change...

Reviewed-by: Paul Durrant 

Re: [PATCH v3] hw/rtc/mc146818rtc: Make this rtc device target independent

[Mark Cave-Ayland]

On 12/12/2022 13:48, Thomas Huth wrote:


On 12/12/2022 14.39, Mark Cave-Ayland wrote:

On 12/12/2022 07:56, Thomas Huth wrote:


The only reason for this code being target dependent is the apic-related
code in rtc_policy_slew_deliver_irq(). Since these apic functions are rather
simple, we can easily move them into a new, separate file (apic_irqcount.c)
which will always be compiled and linked if either APIC or the mc146818 device
are required. This way we can get rid of the #ifdef TARGET_I386 switches in
mc146818rtc.c and declare it in the softmmu_ss instead of specific_ss, so
that the code only gets compiled once for all targets.

Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Mark Cave-Ayland 
Signed-off-by: Thomas Huth 
---
  v3: Move TYPE_APIC_COMMON from apic_internal.h to apic.h and use it

  include/hw/i386/apic.h  |  2 ++
  include/hw/i386/apic_internal.h |  2 --
  include/hw/rtc/mc146818rtc.h    |  1 +
  hw/intc/apic_common.c   | 27 -
  hw/intc/apic_irqcount.c | 53 +
  hw/rtc/mc146818rtc.c    | 25 +---
  hw/intc/meson.build |  6 +++-
  hw/rtc/meson.build  |  3 +-
  8 files changed, 69 insertions(+), 50 deletions(-)
  create mode 100644 hw/intc/apic_irqcount.c

diff --git a/include/hw/i386/apic.h b/include/hw/i386/apic.h
index da1d2fe155..24069fb961 100644
--- a/include/hw/i386/apic.h
+++ b/include/hw/i386/apic.h
@@ -1,6 +1,7 @@
  #ifndef APIC_H
  #define APIC_H
+#define TYPE_APIC_COMMON "apic-common"


Ah sorry, I should have been more specific here: what I was suggesting was to move 
the entire QOM type information into apic.h as per the normal convention, as 
opposed to just the #define. At first glance that would involve lines 128-190 in 
apic_internal.h which would also bring in APICCommonClass and APICCommonState - 
possibly the change may warrant its own commit.


At least APICCommonState is target specific since it uses "X86CPU" ... so moving that 
to apic.h would be very counterproductive here.


Ah okay I see now - sorry I didn't spot that earlier.

Anyway, moving those structs is certainly way more than what is required for this 
patch, so if we decide to move anything else related to the APIC, it should be done 
in a separate patch later.


Agreed, it makes sense to fix the build issue first and then sort out the headers 
later given that it seems to be less than straightforward :/



ATB,

Mark.