Re: [Qemu-devel] [Qemu-block] [PULL 0/9] Block patches
On Tue, Feb 12, 2019 at 11:51:18AM +0800, Stefan Hajnoczi wrote:
> On Mon, Feb 11, 2019 at 11:42:14AM +, Peter Maydell wrote:
> >
> > Hi; this fails to pass "make check" (all platforms):
> >
> > MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
> > QTEST_QEMU_BINARY=arm-softmmu/qemu-system-arm QTEST_QEMU_IMG=qemu-img
> > tests/virtio-blk-test -m
> > =quick -k --tap < /dev/null | ./scripts/tap-driver.pl
> > --test-name="virtio-blk-test"
> > **
> > ERROR:/home/pm215/qemu/tests/virtio-blk-test.c:272:test_basic:
> > assertion failed (status == 0): (1 == 0)
> > ERROR - too few tests run (expected 1, got 0)
>
> Stefano, I'm dropping the virtio-blk DISCARD/WRITE_ZEROES series.
> Please debug this failure.
>
> I cannot reproduce it on a Linux host so maybe it's related to macOS.
> First I thought it's because write zeroes is not supported on macOS, but
> QEMU should fall back to writing a bounce buffer of zeroes.
>
Hi Stefan,
I'll debug it! I also have a macOS here, so I'll try on it.
Thanks,
Stefano
Re: [Qemu-devel] [PATCH v3 1/2] pcie: Add a simple PCIe ACS (Access Control Services) helper function
On Mon, 2019-02-11 at 16:09 -0700, Alex Williamson wrote:
> On Sun, 10 Feb 2019 07:52:59 +0100
> Knut Omang wrote:
>
> > Add a helper function to add PCIe capability for Access Control Services
> > (ACS)
> > ACS support in the associated root port is a prerequisite to be able to do
> > passthrough of individual functions of a device with VFIO
> > without Alex Williamson's pcie_acs_override kernel patch or similar
> > in the guest.
>
> This is still incorrect, the ACS override patch is only required for
> separating multifunction endpoints or multifunction root ports. Single
> function endpoints are assignable without ACS simply by placing them
> downstream of a single function root port or directly on the root
> complex.
Hmm - that was the intended meaning of the comment, but I'll see if I can make
it more clear by saying it explicitly.
> > Endpoints may also implement an ACS capability, but with
> > limited features.
> >
> > Signed-off-by: Knut Omang
> > ---
> > hw/pci/pcie.c | 29 +
> > include/hw/pci/pcie.h | 6 ++
> > include/hw/pci/pcie_regs.h | 4
> > 3 files changed, 39 insertions(+)
> >
> > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > index 230478f..509632f 100644
> > --- a/hw/pci/pcie.c
> > +++ b/hw/pci/pcie.c
> > @@ -742,6 +742,14 @@ bool pcie_cap_is_arifwd_enabled(const PCIDevice *dev)
> > PCI_EXP_DEVCTL2_ARI;
> > }
> >
> > +/* Access Control Services (ACS) */
> > +void pcie_cap_acs_reset(PCIDevice *dev)
> > +{
> > +if (dev->exp.acs_cap) {
> > +pci_set_word(dev->config + dev->exp.acs_cap + PCI_ACS_CTRL, 0);
> > +}
> > +}
> > +
> > /**
> > * pci express extended capability list management functions
> > * uint16_t ext_cap_id (16 bit)
> > @@ -906,3 +914,24 @@ void pcie_ats_init(PCIDevice *dev, uint16_t offset)
> >
> > pci_set_word(dev->wmask + dev->exp.ats_cap + PCI_ATS_CTRL, 0x800f);
> > }
> > +
> > +/* ACS (Access Control Services) */
> > +void pcie_acs_init(PCIDevice *dev, uint16_t offset)
> > +{
> > +bool is_pcie_slot = !!object_dynamic_cast(OBJECT(dev), TYPE_PCIE_SLOT);
> > +uint16_t cap_bits = 0;
> > +
> > +pcie_add_capability(dev, PCI_EXT_CAP_ID_ACS, PCI_ACS_VER, offset,
> > +PCI_ACS_SIZEOF);
> > +dev->exp.acs_cap = offset;
> > +
> > +if (is_pcie_slot) {
> > +/* Endpoints may also implement ACS, but these capabilities are */
> > +/* only valid for slots: */
>
> Not quite, SV, TB, and UF must not be implemented by endpoints, but RR
> and CR must be implemented by multifunction endpoints that support p2p
> if they provide an ACS capability.
Hmm - are you ok with setting 0 here as I have done, just amending your
description to the comment? Then any future emulation that do support p2p
would have to set the needed bits after calling the init function.
After your previous comments on this, I had a look at Mellanox CX4 and CX5 which
are the only devices I could find in the lab that are endpoints and implement an
ACS capability, and neither seems to implement any extra capabilities:
Capabilities: [230 v1] Access Control Services
ACSCap: SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd-
EgressCtrl- DirectTrans-
ACSCtl: SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd-
EgressCtrl- DirectTrans-
that was the reason for my choice of value here - after skimming through the
spec (with my still very limited understanding of the details)
> Linux therefore infers that if ACS
> is supported by an endpoint and RR and CR are not implemented, the
> device does not support p2p.
Interesting - I thought the CX5 supported p2p, but I have not kept up with what
happens on the RDMA list on that front.
> We could just say that nothing supports
> p2p yet, but single function endpoints (except those implementing
> SR-IOV) must not implement an ACS capability per the spec, which could
> be difficult to exclude since the multifunction bit is handled
> separately from the device model.
Hmm - the older SR/IOV devices I know of, some Intel Ethernet devices and any of
the older Mellanox devices, and our own cancelled Infiniband device, all seem
not to implement ACS?
> Also comment style:
> https://git.qemu.org/?p=qemu.git;a=blob;f=CODING_STYLE;#l127
I see - will fix,
> > +cap_bits =
> > +PCI_ACS_SV | PCI_ACS_TB | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF;
> > +}
> > +
> > +pci_set_word(dev->config + offset + PCI_ACS_CAP, cap_bits);
> > +pci_set_word(dev->wmask + offset + PCI_ACS_CTRL, cap_bits);
> > +}
> > diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> > index 5b82a0d..4c40711 100644
> > --- a/include/hw/pci/pcie.h
> > +++ b/include/hw/pci/pcie.h
> > @@ -79,6 +79,9 @@ struct PCIExpressDevice {
> >
> > /* Offset of ATS capability in config space */
> > uint16_t ats_cap;
Re: [Qemu-devel] [PATCH v2 1/2] linux-user: Add ELF_PLATFORM for arm
On 12/02/2019 08:48, Richard Henderson wrote:
> The 32-bit kernel has strings for v4, v5, v6, v7, v7m.
> The 64-bit kernel, in compat mode, has strings for v8.
>
> Fixes: https://bugs.launchpad.net/bugs/1813034
> Signed-off-by: Richard Henderson
> ---
> v2: Include v8.
> ---
> linux-user/elfload.c | 31 +++
> 1 file changed, 31 insertions(+)
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 775a36ccdd..d796d51ec6 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -500,6 +500,37 @@ static uint32_t get_elf_hwcap2(void)
> #undef GET_FEATURE
> #undef GET_FEATURE_ID
>
> +#define ELF_PLATFORM get_elf_platform()
> +
> +static const char *get_elf_platform(void)
> +{
> +CPUARMState *env = thread_cpu->env_ptr;
> +
> +#ifdef TARGET_WORDS_BIGENDIAN
> +# define END "b"
> +#else
> +# define END "l"
> +#endif
> +
> +if (arm_feature(env, ARM_FEATURE_V8)) {
> +return "v8" END;
> +} else if (arm_feature(env, ARM_FEATURE_V7)) {
> +if (arm_feature(env, ARM_FEATURE_M)) {
> +return "v7m" END;
> +} else {
> +return "v7" END;
> +}
> +} else if (arm_feature(env, ARM_FEATURE_V6)) {
> +return "v6" END;
> +} else if (arm_feature(env, ARM_FEATURE_V5)) {
> +return "v5" END;
> +} else {
> +return "v4" END;
> +}
I know nothing about ARM, but in kernel we have also a "v5t"
(cpu_elf_name) and in QEMU we have a ARM_FEATURE_V4T which is set with
ARM_FEATURE_V5. Is it related?
Reviewed-by: Laurent Vivier
Thanks,
Laurent
Re: [Qemu-devel] [PATCH v2 2/2] linux-user: Fix ELF_PLATFORM for aarch64_be-linux-user
On 12/02/2019 08:48, Richard Henderson wrote: > Signed-off-by: Richard Henderson > --- > linux-user/elfload.c | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/linux-user/elfload.c b/linux-user/elfload.c > index d796d51ec6..5aa0628ae9 100644 > --- a/linux-user/elfload.c > +++ b/linux-user/elfload.c > @@ -537,7 +537,11 @@ static const char *get_elf_platform(void) > > #define ELF_ARCHEM_AARCH64 > #define ELF_CLASS ELFCLASS64 > -#define ELF_PLATFORM"aarch64" > +#ifdef TARGET_WORDS_BIGENDIAN > +# define ELF_PLATFORM"aarch64_be" > +#else > +# define ELF_PLATFORM"aarch64" > +#endif > > static inline void init_thread(struct target_pt_regs *regs, > struct image_info *infop) > Reviewed-by: Laurent Vivier
Re: [Qemu-devel] [PATCH 03/19] xive: extend the XiveRouter get_tctx() method with the page offset
On 2/12/19 5:34 AM, David Gibson wrote:
> On Mon, Jan 28, 2019 at 10:46:09AM +0100, Cédric Le Goater wrote:
>> The PowerNV machine can perform indirect loads and stores on the TIMA
>> on behalf of another CPU. The PIR of the CPU is controlled by a set of
>> 4 registers, one per TIMA page. To know which page is being accessed,
>> we need to inform the controller model of the operation offset.
>
> This doesn't seem like the right place to handle this. Shouldn't you
> instead be passing a different CPUState in for "foreign" TCTX
> accesses?
This patch only adds the page offset required to pick the correct
register from the XIVE IC. May be I should have introduced this
page offset extension in the baremetal patch. I might reorganize
the patchset in the next round.
Anyhow, we pass a different XiveTCTX instead. See the routines in
PnvXive, xive_tm_indirect_read() and xive_tm_indirect_write().
We don't pass a CPUState because we depend on the machine data to
retrieve the XiveTCTX and that would be problematic to do in the
common XIVE routines.
C.
>
>>
>> Signed-off-by: Cédric Le Goater
>> ---
>> include/hw/ppc/xive.h | 4 ++--
>> hw/intc/spapr_xive.c | 3 ++-
>> hw/intc/xive.c| 12 +++-
>> 3 files changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>> index 04d54e8315f7..a1f5ea2d9143 100644
>> --- a/include/hw/ppc/xive.h
>> +++ b/include/hw/ppc/xive.h
>> @@ -352,7 +352,7 @@ typedef struct XiveRouterClass {
>> XiveNVT *nvt);
>> int (*write_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>> XiveNVT *nvt, uint8_t word_number);
>> -XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs);
>> +XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs, hwaddr offset);
>> } XiveRouterClass;
>>
>> void xive_eas_pic_print_info(XiveEAS *eas, uint32_t lisn, Monitor *mon);
>> @@ -367,7 +367,7 @@ int xive_router_get_nvt(XiveRouter *xrtr, uint8_t
>> nvt_blk, uint32_t nvt_idx,
>> XiveNVT *nvt);
>> int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t
>> nvt_idx,
>>XiveNVT *nvt, uint8_t word_number);
>> -XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs);
>> +XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs, hwaddr
>> offset);
>>
>> /*
>> * XIVE END ESBs
>> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
>> index a0f5ff929447..c41ee96c4c84 100644
>> --- a/hw/intc/spapr_xive.c
>> +++ b/hw/intc/spapr_xive.c
>> @@ -391,7 +391,8 @@ static int spapr_xive_write_nvt(XiveRouter *xrtr,
>> uint8_t nvt_blk,
>> g_assert_not_reached();
>> }
>>
>> -static XiveTCTX *spapr_xive_get_tctx(XiveRouter *xrtr, CPUState *cs)
>> +static XiveTCTX *spapr_xive_get_tctx(XiveRouter *xrtr, CPUState *cs,
>> + hwaddr offset)
>> {
>> PowerPCCPU *cpu = POWERPC_CPU(cs);
>>
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index f5642f2338de..39dff557fadc 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -320,7 +320,8 @@ static const XiveTmOp *xive_tm_find_op(hwaddr offset,
>> unsigned size, bool write)
>> static void xive_tm_write(void *opaque, hwaddr offset,
>>uint64_t value, unsigned size)
>> {
>> -XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu);
>> +XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu,
>> + offset);
>> const XiveTmOp *xto;
>>
>> /*
>> @@ -358,7 +359,8 @@ static void xive_tm_write(void *opaque, hwaddr offset,
>>
>> static uint64_t xive_tm_read(void *opaque, hwaddr offset, unsigned size)
>> {
>> -XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu);
>> +XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu,
>> + offset);
>> const XiveTmOp *xto;
>>
>> /*
>> @@ -1134,11 +1136,11 @@ int xive_router_write_nvt(XiveRouter *xrtr, uint8_t
>> nvt_blk, uint32_t nvt_idx,
>> return xrc->write_nvt(xrtr, nvt_blk, nvt_idx, nvt, word_number);
>> }
>>
>> -XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs)
>> +XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs, hwaddr
>> offset)
>> {
>> XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr);
>>
>> -return xrc->get_tctx(xrtr, cs);
>> +return xrc->get_tctx(xrtr, cs, offset);
>> }
>>
>> /*
>> @@ -1234,7 +1236,7 @@ static bool xive_presenter_match(XiveRouter *xrtr,
>> uint8_t format,
>> */
>>
>> CPU_FOREACH(cs) {
>> -XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs);
>> +XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs, 0);
>> int ring;
>>
>> /*
>
Re: [Qemu-devel] [PATCH] hostmem-file: reject invalid pmem file sizes
Hi Stefan,
>
> Guests started with NVDIMMs larger than the underlying host file produce
> confusing errors inside the guest. This happens because the guest
> accesses pages beyond the end of the file.
>
> Check the pmem file size on startup and print a clear error message if
> the size is invalid.
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1669053
> Cc: Haozhong Zhang
> Cc: Zhang Yi
> Cc: Eduardo Habkost
> Cc: Igor Mammedov
> Signed-off-by: Stefan Hajnoczi
> ---
> include/qemu/osdep.h| 13 ++
> backends/hostmem-file.c | 16 +
> util/oslib-posix.c | 53 +
> util/oslib-win32.c | 5
> 4 files changed, 87 insertions(+)
>
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 840af09cb0..303d315c5d 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -570,6 +570,19 @@ void qemu_set_tty_echo(int fd, bool echo);
> void os_mem_prealloc(int fd, char *area, size_t sz, int smp_cpus,
> Error **errp);
>
> +/**
> + * qemu_get_pmem_size:
> + * @filename: path to a pmem file
> + * @errp: pointer to a NULL-initialized error object
> + *
> + * Determine the size of a persistent memory file. Besides supporting files
> on
> + * DAX file systems, this function also supports Linux devdax character
> + * devices.
> + *
> + * Returns: the size or 0 on failure
> + */
> +uint64_t qemu_get_pmem_size(const char *filename, Error **errp);
> +
> /**
> * qemu_get_pid_name:
> * @pid: pid of a process
> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> index ba601ce940..325ab4aad9 100644
> --- a/backends/hostmem-file.c
> +++ b/backends/hostmem-file.c
> @@ -46,6 +46,22 @@ file_backend_memory_alloc(HostMemoryBackend *backend,
> Error **errp)
> gchar *name;
> #endif
>
> +/*
> + * Verify pmem file size since starting a guest with an incorrect size
> + * leads to confusing failures inside the guest.
> + */
> +if (fb->is_pmem && fb->mem_path) {
> +uint64_t size;
> +
> +size = qemu_get_pmem_size(fb->mem_path, NULL);
> +if (size && backend->size > size) {
> +error_setg(errp, "size property %" PRIu64 " is larger than "
> + "pmem file \"%s\" size %" PRIu64, backend->size,
> + fb->mem_path, size);
> +return;
> +}
> +}
> +
> if (!backend->size) {
> error_setg(errp, "can't create backend with size 0");
> return;
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 37c5854b9c..10d90d1783 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -500,6 +500,59 @@ void os_mem_prealloc(int fd, char *area, size_t memory,
> int smp_cpus,
> }
> }
>
> +uint64_t qemu_get_pmem_size(const char *filename, Error **errp)
> +{
> +struct stat st;
> +
> +if (stat(filename, &st) < 0) {
> +error_setg(errp, "unable to stat pmem file \"%s\"", filename);
> +return 0;
> +}
> +
> +#if defined(__linux__)
> +/* Special handling for devdax character devices */
> +if (S_ISCHR(st.st_mode)) {
> +char *subsystem_path = NULL;
> +char *subsystem = NULL;
> +char *size_path = NULL;
> +char *size_str = NULL;
> +uint64_t ret = 0;
> +
> +subsystem_path = g_strdup_printf("/sys/dev/char/%d:%d/subsystem",
> + major(st.st_rdev),
> minor(st.st_rdev));
> +subsystem = g_file_read_link(subsystem_path, NULL);
> +if (!subsystem) {
> +error_setg(errp, "unable to read subsystem for pmem file
> \"%s\"",
> + filename);
> +goto devdax_err;
> +}
> +
> +if (!g_str_has_suffix(subsystem, "/dax")) {
> +error_setg(errp, "pmem file \"%s\" is not a dax device",
> filename);
> +goto devdax_err;
> +}
> +
> +size_path = g_strdup_printf("/sys/dev/char/%d:%d/size",
> +major(st.st_rdev), minor(st.st_rdev));
> +if (!g_file_get_contents(size_path, &size_str, NULL, NULL)) {
> +error_setg(errp, "unable to read size for pmem file \"%s\"",
> + size_path);
> +goto devdax_err;
> +}
> +
> +ret = g_ascii_strtoull(size_str, NULL, 0);
> +
> +devdax_err:
> +g_free(size_str);
> +g_free(size_path);
> +g_free(subsystem);
> +g_free(subsystem_path);
> +return ret;
> +}
> +#endif /* defined(__linux__) */
> +
> +return st.st_size;
> +}
>
> char *qemu_get_pid_name(pid_t pid)
> {
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index b4c17f5dfa..bd633afab6 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -560,6 +560,11 @@ void os_mem_prealloc(int fd, char *area, size_t memory,
> int smp_cpus,
> }
> }
>
> +uint64_t qemu_get_pmem_size(const char *filename, Error
Re: [Qemu-devel] [PATCH] Kconfig: add documentation
On 2019-02-11 17:38, Paolo Bonzini wrote: > Signed-off-by: Paolo Bonzini > --- > docs/devel/kconfig.rst | 284 + > 1 file changed, 284 insertions(+) > create mode 100644 docs/devel/kconfig.rst Thanks for writing this up - that's a really helpful text! > diff --git a/docs/devel/kconfig.rst b/docs/devel/kconfig.rst > new file mode 100644 > index 00..b653c43b12 > --- /dev/null > +++ b/docs/devel/kconfig.rst > @@ -0,0 +1,284 @@ [...] > +Unlike Linux, there is no user interface to edit the configuration, which is > instead > +specified in per-target files under the ``default-configs/`` directory of the > +QEMU source tree. This is because, unlike Linux, configuration and > dependencies can be > +treated as a black box when building QEMU; the default configuration that > QEMU > +ships with should be okay in almost all cases. I'd like to suggest to replace "Linux" with "Linux-Kconfig". [...] > +**reverse dependencies**: ``select [if ]`` > + > + While ``depends on`` forces a symbol to false, reverse dependencies can be > + used to force another symbol to true. In the following example, > + ``CONFIG_BAZ`` will be true whenever ``CONFIG_FOO`` is true:: > + > +config FOO > + select BAZ > + > + The optional expression will prevent ``select`` from having any effect > + unless it is true. > + > + Note that unlike Linux, QEMU will detect contradictions between ``depends > on`` and dito Thomas
Re: [Qemu-devel] [PATCH v3 1/2] qapi: move to QOM path for x-block-latency-histogram-set
Am 11.02.2019 um 19:30 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 11.02.2019 20:54, Kevin Wolf wrote:
> > Am 21.12.2018 um 17:53 hat Vladimir Sementsov-Ogievskiy geschrieben:
> >> Move to way of device selecting, however fall back to device name if
> >> path is not found.
> >>
> >> Signed-off-by: Vladimir Sementsov-Ogievskiy
> >> ---
> >> qapi/block-core.json | 4 ++--
> >> blockdev.c | 22 +++---
> >> 2 files changed, 17 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/qapi/block-core.json b/qapi/block-core.json
> >> index 762000f31f..bb70c51a57 100644
> >> --- a/qapi/block-core.json
> >> +++ b/qapi/block-core.json
> >> @@ -489,7 +489,7 @@
> >> # If only @device parameter is specified, remove all present latency
> >> histograms
> >> # for the device. Otherwise, add/reset some of (or all) latency
> >> histograms.
> >> #
> >> -# @device: device name to set latency histogram for.
> >> +# @id: The QOM path or name of the guest device.
> >> #
> >> # @boundaries: list of interval boundary values (see description in
> >> # BlockLatencyHistogramInfo definition). If specified, all
> >> @@ -547,7 +547,7 @@
> >> # <- { "return": {} }
> >> ##
> >> { 'command': 'x-block-latency-histogram-set',
> >> - 'data': {'device': 'str',
> >> + 'data': {'id': 'str',
> >> '*boundaries': ['uint64'],
> >> '*boundaries-read': ['uint64'],
> >> '*boundaries-write': ['uint64'],
> >> diff --git a/blockdev.c b/blockdev.c
> >> index a6f71f9d83..ff0d8ded5e 100644
> >> --- a/blockdev.c
> >> +++ b/blockdev.c
> >> @@ -4397,21 +4397,29 @@ void qmp_x_blockdev_set_iothread(const char
> >> *node_name, StrOrNull *iothread,
> >> }
> >>
> >> void qmp_x_block_latency_histogram_set(
> >> -const char *device,
> >> +const char *id,
> >> bool has_boundaries, uint64List *boundaries,
> >> bool has_boundaries_read, uint64List *boundaries_read,
> >> bool has_boundaries_write, uint64List *boundaries_write,
> >> bool has_boundaries_flush, uint64List *boundaries_flush,
> >> Error **errp)
> >> {
> >> -BlockBackend *blk = blk_by_name(device);
> >> BlockAcctStats *stats;
> >> int ret;
> >> +Error *local_err = NULL;
> >> +BlockBackend *blk = blk_by_qdev_id(id, &local_err);
> >>
> >> if (!blk) {
> >> -error_setg(errp, "Device '%s' not found", device);
> >> -return;
> >> +blk = blk_by_name(id);
> >> +if (!blk) {
> >> +error_propagate(errp, local_err);
> >> +return;
> >> +} else {
> >> +error_free(local_err);
> >> +local_err = NULL;
> >> +}
> >> }
> >
> > Why don't you use qmp_get_blk() here?
> >
>
> qmp_get_blk is used for cases when we have both @device and @id parameters,
> where @device is deprecated.
>
> And one of previous iteration was a try to do it in this way (have both @id
> and @device). But it was
> considered to be bad idea for a new command, which don't need any backward
> compatibility. And we decided
> to merge two variants in @id.
Ah, right I missed that you dropped @device and just noticed that this
looks different from other commands. Calling blk_by_qdev_id() directly
makes sense then.
> So, do you suggest to drop blk_by_name() variant at all, and go through
> qmp_get_blk(NULL, id), which
> will call blk_by_qdev_id() and fail if not found?
Yes, we don't need the blk_by_name() path, it's only for the legacy
@device options.
In any case, you can't use the same option for blk_by_name() and
blk_by_qdev_id(). They are separate namespaces and the name could exist
in both of them. This is why the other commands have both @id and
@device instead of just a single option.
Kevin
Re: [Qemu-devel] [PATCH v3 0/2] target/s390x: Implement STCK et al for CONFIG_USER_ONLY
On Mon, 11 Feb 2019 21:30:42 -0800 Richard Henderson wrote: > Changes since v2: > * Fix botched subject line, include r-b tags. > > Changes since v1: > * Move more of hw/s390x/tod.h to a new header. > * Use time2tod. > > > r~ > > > Richard Henderson (2): > target/s390x: Split out s390-tod.h > target/s390x: Implement STCK et al for CONFIG_USER_ONLY > > include/hw/s390x/tod.h | 16 +--- > target/s390x/helper.h | 2 +- > target/s390x/s390-tod.h| 29 + > target/s390x/misc_helper.c | 34 ++ > target/s390x/translate.c | 2 ++ > target/s390x/insn-data.def | 11 ++- > 6 files changed, 61 insertions(+), 33 deletions(-) > create mode 100644 target/s390x/s390-tod.h > Thanks, applied.
Re: [Qemu-devel] [PATCH] Kconfig: add documentation
Paolo Bonzini writes: > Signed-off-by: Paolo Bonzini > --- > docs/devel/kconfig.rst | 284 + > 1 file changed, 284 insertions(+) > create mode 100644 docs/devel/kconfig.rst > > diff --git a/docs/devel/kconfig.rst b/docs/devel/kconfig.rst > new file mode 100644 > index 00..b653c43b12 > --- /dev/null > +++ b/docs/devel/kconfig.rst > @@ -0,0 +1,284 @@ > +Introduction > + > + > +QEMU is a very versatile emulator; it can be built for a variety of targets, > where > +each target can emulate various boards and at the same time different > targets can > +share large amounts of code. For example, a POWER and an x86 boards can run > the > +same code to emulate a PCI network card, even though the boards use > different PCI > +host bridges, and they can run the same code to emulate a SCSI disk while > using > +different SCSI adapters. ARM, s390 and x86 boards can both present a > virtio-blk > +disk to their guests, but with three different virtio guest interfaces. > + Please wrap your lines at column 70 or so. Humans tend to have trouble following long lines with their eyes (I sure do). Typographic manuals suggest to limit columns to roughly 60 characters for exactly that reason[*]. > +Each QEMU target enables a subset of the boards, devices and buses that are > included > +in QEMU's source code. As a result, each QEMU executable only links a small > subset Really? Hmm... $ size aarch64-softmmu/qemu-system-aarch64 textdata bss dec hex filename 191832167200124 592732 2697607219b9f48 aarch64-softmmu/qemu-system-aarch64 $ size -t `find -name \*.o `| grep TOT 92713559186522271183961440 1295327226 4d351ffa (TOTALS) Yep, really. > +of the files that form QEMU's source code; anything that is not needed to > support > +a particular target is culled. > + > +QEMU uses a simple domain-specific language to describe the dependencies > between > +components. This is useful for two reasons: > + > +* new targets and boards can be added without knowing in detail the > architecture of > + the hardware emulation subsystems. Boards only have to list the > components they > + need, and the compiled executable will include all the required > dependencies and > + all the devices that the user can add to that board. > + > +* users can easily build reduced versions of QEMU that support only a subset > of > + boards or devices. For example, by default most targets will include all > emulated > + PCI devices that QEMU supports, but the build process is configurable and > it is easy > + to drop unnecessary (or otherwise unwanted) code to make a leaner binary; > + > +This domain-specific language is based on the Kconfig language that > originated in the > +Linux kernel, though it was heavily simplified and the handling of > dependencies is > +stricter in QEMU. > + > +Unlike Linux, there is no user interface to edit the configuration, which is > instead > +specified in per-target files under the ``default-configs/`` directory of the > +QEMU source tree. This is because, unlike Linux, configuration and > dependencies can be > +treated as a black box when building QEMU; the default configuration that > QEMU > +ships with should be okay in almost all cases. > + > +The Kconfig language > + > + > +Kconfig defines configurable components in files named ``hw/*/Kconfig``. > +Note that configurable components are _not_ visible in C code as > preprocessor symbols; > +they are only visible in the Makefile. Each configurable components > +defines a Makefile variable whose name starts with ``CONFIG_``. > + > +All elements have boolean (true/false) type. They are defined in a Kconfig > +stanza like the following:: > + > + config ARM_VIRT > + bool > + imply PCI_DEVICES > + imply VFIO_AMD_XGBE > + imply VFIO_XGMAC > + select A15MPCORE > + select ACPI > + select ARM_SMMUV3 > + > +The ``config`` keyword introduces a new configuration element. In the > example above, > +Makefiles will have access to a variable named ``CONFIG_ARM_VIRT``, with > value ``y`` or > +``n`` (respectively for boolean true and false). > + > +The ``bool`` data type declaration is optional, but it is suggested to > include it for > +clarity and future-proofing. After ``bool`` the following directives can be > included: > + > +**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. > + > + can be an arbitrary Boolean expression. The ``&&``, ``||`` and > ``!`` > + operators are supported, respectively for conjunction (AND), disjunction > + (OR) and negation (NOT). > + > +**reverse dependencies**: ``select [if ]`` > + > + While ``depends on`` forces a symbol to
Re: [Qemu-devel] [PATCH v2 2/3] s390x/cpumodel: default enable mepoch for z14 and later
On 12.02.19 02:16, Collin Walling wrote:
> Latest systems and host kernels support mepoch, which is a
> feature that was meant to be supported for z14 GA1 from the
> get-go. Let's copy it to the z14 GA1 default CPU model.
>
> Machines s390-ccw-virtio-3.1 and older will retain the old CPU
> models and will not provide this bit nor the extended PTFF
> functions in the default model.
>
> Signed-off-by: Collin Walling
> ---
> @Christian, @David: I elected to not add your r-b's to this
> patch since I've added some code to disable feature groups.
> I'd like to know what you both think :)
Looks good to me!
Reviewed-by: David Hildenbrand
> ---
> hw/s390x/s390-virtio-ccw.c | 2 ++
> target/s390x/cpu_models.c | 24
> target/s390x/cpu_models.h | 2 ++
> target/s390x/gen-features.c | 2 ++
> 4 files changed, 30 insertions(+)
>
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index fd9d0b0542..643265f0b4 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -757,6 +757,8 @@ DEFINE_CCW_MACHINE(4_0, "4.0", true);
> static void ccw_machine_3_1_instance_options(MachineState *machine)
> {
> ccw_machine_4_0_instance_options(machine);
> +s390_cpudef_featoff_greater(14, 1, S390_FEAT_MULTIPLE_EPOCH);
> +s390_cpudef_group_featoff_greater(14, 1,
> S390_FEAT_GROUP_MULTIPLE_EPOCH_PTFF);
> }
>
> static void ccw_machine_3_1_class_options(MachineClass *mc)
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index b42b5fd327..d75a20e49d 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -117,6 +117,30 @@ void s390_cpudef_featoff_greater(uint8_t gen, uint8_t
> ec_ga, S390Feat feat)
> }
> }
>
> +void s390_cpudef_group_featoff_greater(uint8_t gen, uint8_t ec_ga,
> + S390FeatGroup group)
> +{
> +const S390FeatGroupDef *group_def = s390_feat_group_def(group);
> +S390FeatBitmap group_def_off;
> +int i;
> +
> +bitmap_complement(group_def_off, group_def->feat, S390_FEAT_MAX);
> +
> +for (i = 0; i < ARRAY_SIZE(s390_cpu_defs); i++) {
> +const S390CPUDef *cpu_def = &s390_cpu_defs[i];
> +
> +if (cpu_def->gen < gen) {
> +continue;
> +}
> +if (cpu_def->gen == gen && cpu_def->ec_ga < ec_ga) {
> +continue;
> +}
> +
> +bitmap_and((unsigned long *)&cpu_def->default_feat,
> + cpu_def->default_feat, group_def_off, S390_FEAT_MAX);
> +}
> +}
> +
> uint32_t s390_get_hmfai(void)
> {
> static S390CPU *cpu;
> diff --git a/target/s390x/cpu_models.h b/target/s390x/cpu_models.h
> index 11cf5386fb..174a99e561 100644
> --- a/target/s390x/cpu_models.h
> +++ b/target/s390x/cpu_models.h
> @@ -75,6 +75,8 @@ struct S390CPUModel {
>
> void s390_cpudef_featoff(uint8_t gen, uint8_t ec_ga, S390Feat feat);
> void s390_cpudef_featoff_greater(uint8_t gen, uint8_t ec_ga, S390Feat feat);
> +void s390_cpudef_group_featoff_greater(uint8_t gen, uint8_t ec_ga,
> + S390FeatGroup group);
> uint32_t s390_get_hmfai(void);
> uint8_t s390_get_mha_pow(void);
> uint32_t s390_get_ibc_val(void);
> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
> index 70015eaaf5..4770f00105 100644
> --- a/target/s390x/gen-features.c
> +++ b/target/s390x/gen-features.c
> @@ -550,6 +550,8 @@ static uint16_t default_GEN14_GA1[] = {
> S390_FEAT_GROUP_MSA_EXT_6,
> S390_FEAT_GROUP_MSA_EXT_7,
> S390_FEAT_GROUP_MSA_EXT_8,
> +S390_FEAT_MULTIPLE_EPOCH,
> +S390_FEAT_GROUP_MULTIPLE_EPOCH_PTFF,
> };
>
> /* QEMU (CPU model) features */
>
--
Thanks,
David / dhildenb
Re: [Qemu-devel] [PATCH v12 00/25] Fixing record/replay and adding reverse debugging
dovgaluk writes: > Markus Armbruster писал 2019-02-12 10:14: >> "Pavel Dovgalyuk" writes: >> >>> Ping? >> >> Are you pinging for more review, or for someone to merge this? > > From my point of view this patch set is ready. MAINTAINERS Record/replay M: Pavel Dovgalyuk R: Paolo Bonzini makes me think either Paolo should pick it up, or he should tell you to post a pull request yourself.
[Qemu-devel] [Bug 1815143] Re: qemu-system-s390x fails when running without kvm: fatal: EXECUTE on instruction prefix 0x7f4 not implemented
This should be the commit that fixed this issue: https://git.qemu.org/?p=qemu.git;a=commitdiff;h=303c681a8f50eb88fbafc ** Changed in: qemu Status: Incomplete => Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1815143 Title: qemu-system-s390x fails when running without kvm: fatal: EXECUTE on instruction prefix 0x7f4 not implemented Status in QEMU: Fix Released Bug description: just wondering if TCG implements instruction prefix 0x7f4 server3:~ # zcat /boot/vmlinux-4.4.162-94.72-default.gz > /tmp/kernel --> starting qemu with kvm enabled works fine server3:~ # qemu-system-s390x -nographic -kernel /tmp/kernel -initrd /boot/initrd -enable-kvm Initializing cgroup subsys cpuset Initializing cgroup subsys cpu Initializing cgroup subsys cpuacct Linux version 4.4.162-94.72-default (geeko@buildhost) (gcc version 4.8.5 (SUSE Linux) ) #1 SMP Mon Nov 12 18:57:45 UTC 2018 (9de753f) setup.289988: Linux is running under KVM in 64-bit mode setup.b050d0: The maximum memory size is 128MB numa.196305: NUMA mode: plain Write protected kernel read-only data: 8692k [...] --> but starting qemu without kvm enabled works fails server3:~ # qemu-system-s390x -nographic -kernel /tmp/kernel -initrd /boot/initrd qemu: fatal: EXECUTE on instruction prefix 0x7f4 not implemented PSW=mask 00018000 addr 0067ed6e cc 00 R00=8000 R01=0067ed76 R02= R03= R04=00111548 R05= R06= R07= R08=000100f6 R09= R10= R11= R12=00ae2000 R13=00681978 R14=00111548 R15=bef0 F00= F01= F02= F03= F04= F05= F06= F07= F08= F09= F10= F11= F12= F13= F14= F15= V00= V01= V02= V03= V04= V05= V06= V07= V08= V09= V10= V11= V12= V13= V14= V15= V16= V17= V18= V19= V20= V21= V22= V23= V24= V25= V26= V27= V28= V29= V30= V31= C00= C01= C02= C03= C04= C05= C06= C07= C08= C09= C10= C11= C12= C13= C14= C15= Aborted (core dumped) server3:~ # lscpu Architecture: s390x CPU op-mode(s):32-bit, 64-bit Byte Order:Big Endian CPU(s):2 On-line CPU(s) list: 0,1 Thread(s) per core:1 Core(s) per socket:1 Socket(s) per book:1 Book(s) per drawer:1 Drawer(s): 2 NUMA node(s): 1 Vendor ID: IBM/S390 Machine type: 2964 BogoMIPS: 20325.00 Hypervisor:z/VM 6.4.0 Hypervisor vendor: IBM Virtualization type: full Dispatching mode: horizontal L1d cache: 128K L1i cache: 96K L2d cache: 2048K L2i cache: 2048K L3 cache: 65536K L4 cache: 491520K NUMA node0 CPU(s): 0-63 Flags: esan3 zarch stfle msa ldisp eimm dfp edat etf3eh highgprs te vx sie server3:~ # uname -a Linux server3 4.4.126-94.22-default #1 SMP Wed Apr 11 07:45:03 UTC 2018 (9649989) s390x s390x s390x GNU/Linux server3:~ # To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1815143/+subscriptions
[Qemu-devel] [PATCH v1] s390x: add zPCI feature to "qemu" CPU model
As we now always have PCI support, let's add it to the "qemu" CPU model,
taking care of backwards compatibility.
Signed-off-by: David Hildenbrand
---
hw/s390x/s390-virtio-ccw.c | 3 +++
target/s390x/gen-features.c | 8 ++--
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 3fb5c88af9..7f0d394fbe 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -657,7 +657,10 @@ DEFINE_CCW_MACHINE(4_0, "4.0", true);
static void ccw_machine_3_1_instance_options(MachineState *machine)
{
+static const S390FeatInit qemu_cpu_feat = { S390_FEAT_LIST_QEMU_V3_1 };
ccw_machine_4_0_instance_options(machine);
+
+s390_set_qemu_cpu_model(0x2064, 7, 1, qemu_cpu_feat);
}
static void ccw_machine_3_1_class_options(MachineClass *mc)
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index b0e9b271e2..19bb8ac7f8 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -560,7 +560,7 @@ static uint16_t qemu_V2_11[] = {
S390_FEAT_ZARCH,
};
-static uint16_t qemu_LATEST[] = {
+static uint16_t qemu_V3_1[] = {
S390_FEAT_DAT_ENH,
S390_FEAT_IDTE_SEGMENT,
S390_FEAT_STFLE,
@@ -592,13 +592,16 @@ static uint16_t qemu_LATEST[] = {
S390_FEAT_MSA_EXT_4,
};
+static uint16_t qemu_LATEST[] = {
+S390_FEAT_ZPCI,
+};
+
/* add all new definitions before this point */
static uint16_t qemu_MAX[] = {
/* z13+ features */
S390_FEAT_STFLE_53,
/* generates a dependency warning, leave it out for now */
S390_FEAT_MSA_EXT_5,
-S390_FEAT_ZPCI,
};
/** END FEATURE DEFS **/
@@ -708,6 +711,7 @@ static FeatGroupDefSpec FeatGroupDef[] = {
***/
static FeatGroupDefSpec QemuFeatDef[] = {
QEMU_FEAT_INITIALIZER(V2_11),
+QEMU_FEAT_INITIALIZER(V3_1),
QEMU_FEAT_INITIALIZER(LATEST),
QEMU_FEAT_INITIALIZER(MAX),
};
--
2.17.2
Re: [Qemu-devel] [PATCH 05/15] s390-bios: Factor finding boot device out of virtio code path
On Mon, 11 Feb 2019 12:57:36 -0500 "Jason J. Herne" wrote: > On 2/4/19 5:45 AM, Cornelia Huck wrote: > > On Tue, 29 Jan 2019 08:29:12 -0500 > > "Jason J. Herne" wrote: > >> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c > >> index 7e3f65e..2457752 100644 > >> --- a/pc-bios/s390-ccw/main.c > >> +++ b/pc-bios/s390-ccw/main.c > >> @@ -55,17 +55,18 @@ unsigned int get_loadparm_index(void) > >>* NOTE: The global variable blk_schid is updated to contain the > >> subchannel > >>* information. > >>*/ > >> -static bool find_dev(Schib *schib, int dev_no) > >> +static bool find_subch(int dev_no) > > > > I'm wondering why you drop passing in the schib here? But OTOH, the > > usage of global variables or not is a bit confused in the bios anyway... > > > > I dropped it as an argument because the schib was never used outside of > find_dev. Seems to > make sense to make it a local variable in this case. > Fair enough.
Re: [Qemu-devel] [PATCH 2/4] multifd: Drop x-multifd-page-count parameter
Daniel P. Berrangé wrote:
> On Wed, Feb 06, 2019 at 02:23:29PM +0100, Juan Quintela wrote:
>> Libvirt don't want to expose (and explain it). And testing looks like
>> 128 is good for all use cases, so just drop it.
>
> One significant concern inline...
>
>>
>> Signed-off-by: Juan Quintela
>> ---
>> hmp.c | 7 ---
>> migration/migration.c | 30 --
>> migration/migration.h | 1 -
>> migration/ram.c | 13 -
>> qapi/migration.json | 13 +
>> 5 files changed, 9 insertions(+), 55 deletions(-)
>
>
>> @@ -718,7 +721,7 @@ static void multifd_send_fill_packet(MultiFDSendParams
>> *p)
>> packet->magic = cpu_to_be32(MULTIFD_MAGIC);
>> packet->version = cpu_to_be32(MULTIFD_VERSION);
>> packet->flags = cpu_to_be32(p->flags);
>> -packet->size = cpu_to_be32(migrate_multifd_page_count());
>> +packet->size = cpu_to_be32(MULTIFD_PAGE_COUNT);
>> packet->used = cpu_to_be32(p->pages->used);
>> packet->packet_num = cpu_to_be64(p->packet_num);
>>
>
> Here the source QEMU sends the page size - which is now
> a hardcoded constant - to the target QEMU.
>
>> @@ -756,10 +759,10 @@ static int
>> multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
>> p->flags = be32_to_cpu(packet->flags);
>>
>> packet->size = be32_to_cpu(packet->size);
>> -if (packet->size > migrate_multifd_page_count()) {
>> +if (packet->size > MULTIFD_PAGE_COUNT) {
>> error_setg(errp, "multifd: received packet "
>> "with size %d and expected maximum size %d",
>> - packet->size, migrate_multifd_page_count()) ;
>> + packet->size, MULTIFD_PAGE_COUNT) ;
>> return -1;
>> }
>>
>
> Here the dest QEMU receives the page size that the source QEMU used, and
> checks that it is not larger than its constant.
>
> IIUC, the implication here is that if we ever increase the size of this
> constant in future QEMU, we will break live migration from new to old
> QEMU due to this check. In fact your previous patch in this series has
> done exactly that, so this appears to mean QEMU 4.0 -> QEMU 3.2
> multifd migration is broken now.
>
> Alternatively if we decrease the size of the constant in future
> QEMU, we will break live migration from old QEMU to new QEMU which
> is even worse.
>
> This problem existed before this patch, if the management app was
> not explicitly using migrate-set-parameters to set the page count
> on both sides of QEMU. So we're already broken, but at least the
> feature was marked experimental.
>
> What is the purpose of this packet size check ? Is it something
> we can safely remove, so that we can increase or decrease the
> size at will without breaking migration compat.
We have a "dinamyc" array of pages of that size. What we check is that
the array fits into the part that we have assigned.
We "could" wait until this moment to create the arrays, I need to look
into that. Notice that what the check does is making sure that whatewer
we receive is not bigger than the space that we have allocated.
At this point, that check can only fail if we are "being" attacked and
we have a malformed string. We check during negotiation that this value
is ok.
We should check this *also* in the initial packet, and then this check
should never be true.
>From a management point of view, what do you preffer here?
Later, Juan.
Re: [Qemu-devel] [PATCH v3 1/2] pcie: Add a simple PCIe ACS (Access Control Services) helper function
On Tue, 2019-02-12 at 09:07 +0100, Knut Omang wrote:
> On Mon, 2019-02-11 at 16:09 -0700, Alex Williamson wrote:
> > On Sun, 10 Feb 2019 07:52:59 +0100
> > Knut Omang wrote:
> >
> > > Add a helper function to add PCIe capability for Access Control Services
> > > (ACS)
> > > ACS support in the associated root port is a prerequisite to be able to do
> > > passthrough of individual functions of a device with VFIO
> > > without Alex Williamson's pcie_acs_override kernel patch or similar
> > > in the guest.
> >
> > This is still incorrect, the ACS override patch is only required for
> > separating multifunction endpoints or multifunction root ports. Single
> > function endpoints are assignable without ACS simply by placing them
> > downstream of a single function root port or directly on the root
> > complex.
>
> Hmm - that was the intended meaning of the comment, but I'll see if I can make
> it more clear by saying it explicitly.
>
> > > Endpoints may also implement an ACS capability, but with
> > > limited features.
> > >
> > > Signed-off-by: Knut Omang
> > > ---
> > > hw/pci/pcie.c | 29 +
> > > include/hw/pci/pcie.h | 6 ++
> > > include/hw/pci/pcie_regs.h | 4
> > > 3 files changed, 39 insertions(+)
> > >
> > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > > index 230478f..509632f 100644
> > > --- a/hw/pci/pcie.c
> > > +++ b/hw/pci/pcie.c
> > > @@ -742,6 +742,14 @@ bool pcie_cap_is_arifwd_enabled(const PCIDevice *dev)
> > > PCI_EXP_DEVCTL2_ARI;
> > > }
> > >
> > > +/* Access Control Services (ACS) */
> > > +void pcie_cap_acs_reset(PCIDevice *dev)
> > > +{
> > > +if (dev->exp.acs_cap) {
> > > +pci_set_word(dev->config + dev->exp.acs_cap + PCI_ACS_CTRL, 0);
> > > +}
> > > +}
> > > +
> > > /
> > > **
> > > * pci express extended capability list management functions
> > > * uint16_t ext_cap_id (16 bit)
> > > @@ -906,3 +914,24 @@ void pcie_ats_init(PCIDevice *dev, uint16_t offset)
> > >
> > > pci_set_word(dev->wmask + dev->exp.ats_cap + PCI_ATS_CTRL, 0x800f);
> > > }
> > > +
> > > +/* ACS (Access Control Services) */
> > > +void pcie_acs_init(PCIDevice *dev, uint16_t offset)
> > > +{
> > > +bool is_pcie_slot = !!object_dynamic_cast(OBJECT(dev),
> > > TYPE_PCIE_SLOT);
> > > +uint16_t cap_bits = 0;
> > > +
> > > +pcie_add_capability(dev, PCI_EXT_CAP_ID_ACS, PCI_ACS_VER, offset,
> > > +PCI_ACS_SIZEOF);
> > > +dev->exp.acs_cap = offset;
> > > +
> > > +if (is_pcie_slot) {
> > > +/* Endpoints may also implement ACS, but these capabilities are
> > > */
> > > +/* only valid for slots: */
> >
> > Not quite, SV, TB, and UF must not be implemented by endpoints, but RR
> > and CR must be implemented by multifunction endpoints that support p2p
> > if they provide an ACS capability.
>
> Hmm - are you ok with setting 0 here as I have done, just amending your
> description to the comment? Then any future emulation that do support p2p
> would have to set the needed bits after calling the init function.
>
> After your previous comments on this, I had a look at Mellanox CX4 and CX5
> which
> are the only devices I could find in the lab that are endpoints and implement
> an
> ACS capability, and neither seems to implement any extra capabilities:
>
> Capabilities: [230 v1] Access Control Services
> ACSCap: SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd-
> EgressCtrl- DirectTrans-
> ACSCtl: SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd-
> EgressCtrl- DirectTrans-
>
> that was the reason for my choice of value here - after skimming through the
> spec (with my still very limited understanding of the details)
>
> > Linux therefore infers that if ACS
> > is supported by an endpoint and RR and CR are not implemented, the
> > device does not support p2p.
>
> Interesting - I thought the CX5 supported p2p, but I have not kept up with
> what
> happens on the RDMA list on that front.
>
> > We could just say that nothing supports
> > p2p yet, but single function endpoints (except those implementing
> > SR-IOV) must not implement an ACS capability per the spec, which could
> > be difficult to exclude since the multifunction bit is handled
> > separately from the device model.
>
> Hmm - the older SR/IOV devices I know of, some Intel Ethernet devices and any
> of
> the older Mellanox devices, and our own cancelled Infiniband device, all seem
> not to implement ACS?
>
> > Also comment style:
> > https://git.qemu.org/?p=qemu.git;a=blob;f=CODING_STYLE;#l127
>
> I see - will fix,
>
> > > +cap_bits =
> > > +PCI_ACS_SV | PCI_ACS_TB | PCI_ACS_RR | PCI_ACS_CR |
> > > PCI_ACS_UF;
> > > +}
> > > +
> > > +pci_set_word(dev->config + offset + PCI_ACS_CAP, cap_bits);
> > > +pci_set_word(dev->wmask + offset + PCI_ACS_CTRL
Re: [Qemu-devel] [Qemu-block] [PULL 0/9] Block patches
On Tue, 12 Feb 2019 at 03:51, Stefan Hajnoczi wrote:
>
> On Mon, Feb 11, 2019 at 11:42:14AM +, Peter Maydell wrote:
> > Hi; this fails to pass "make check" (all platforms):
> >
> > MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
> > QTEST_QEMU_BINARY=arm-softmmu/qemu-system-arm QTEST_QEMU_IMG=qemu-img
> > tests/virtio-blk-test -m
> > =quick -k --tap < /dev/null | ./scripts/tap-driver.pl
> > --test-name="virtio-blk-test"
> > **
> > ERROR:/home/pm215/qemu/tests/virtio-blk-test.c:272:test_basic:
> > assertion failed (status == 0): (1 == 0)
> > ERROR - too few tests run (expected 1, got 0)
>
> Stefano, I'm dropping the virtio-blk DISCARD/WRITE_ZEROES series.
> Please debug this failure.
>
> I cannot reproduce it on a Linux host so maybe it's related to macOS.
> First I thought it's because write zeroes is not supported on macOS, but
> QEMU should fall back to writing a bounce buffer of zeroes.
The test failed everywhere, including the Linux platforms, not just OSX.
thanks
-- PMM
Re: [Qemu-devel] [PATCH v3 1/2] qapi: move to QOM path for x-block-latency-histogram-set
12.02.2019 12:07, Kevin Wolf wrote:
> Am 11.02.2019 um 19:30 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> 11.02.2019 20:54, Kevin Wolf wrote:
>>> Am 21.12.2018 um 17:53 hat Vladimir Sementsov-Ogievskiy geschrieben:
Move to way of device selecting, however fall back to device name if
path is not found.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
qapi/block-core.json | 4 ++--
blockdev.c | 22 +++---
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 762000f31f..bb70c51a57 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -489,7 +489,7 @@
# If only @device parameter is specified, remove all present latency
histograms
# for the device. Otherwise, add/reset some of (or all) latency
histograms.
#
-# @device: device name to set latency histogram for.
+# @id: The QOM path or name of the guest device.
#
# @boundaries: list of interval boundary values (see description in
# BlockLatencyHistogramInfo definition). If specified, all
@@ -547,7 +547,7 @@
# <- { "return": {} }
##
{ 'command': 'x-block-latency-histogram-set',
- 'data': {'device': 'str',
+ 'data': {'id': 'str',
'*boundaries': ['uint64'],
'*boundaries-read': ['uint64'],
'*boundaries-write': ['uint64'],
diff --git a/blockdev.c b/blockdev.c
index a6f71f9d83..ff0d8ded5e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -4397,21 +4397,29 @@ void qmp_x_blockdev_set_iothread(const char
*node_name, StrOrNull *iothread,
}
void qmp_x_block_latency_histogram_set(
-const char *device,
+const char *id,
bool has_boundaries, uint64List *boundaries,
bool has_boundaries_read, uint64List *boundaries_read,
bool has_boundaries_write, uint64List *boundaries_write,
bool has_boundaries_flush, uint64List *boundaries_flush,
Error **errp)
{
-BlockBackend *blk = blk_by_name(device);
BlockAcctStats *stats;
int ret;
+Error *local_err = NULL;
+BlockBackend *blk = blk_by_qdev_id(id, &local_err);
if (!blk) {
-error_setg(errp, "Device '%s' not found", device);
-return;
+blk = blk_by_name(id);
+if (!blk) {
+error_propagate(errp, local_err);
+return;
+} else {
+error_free(local_err);
+local_err = NULL;
+}
}
>>>
>>> Why don't you use qmp_get_blk() here?
>>>
>>
>> qmp_get_blk is used for cases when we have both @device and @id parameters,
>> where @device is deprecated.
>>
>> And one of previous iteration was a try to do it in this way (have both @id
>> and @device). But it was
>> considered to be bad idea for a new command, which don't need any backward
>> compatibility. And we decided
>> to merge two variants in @id.
>
> Ah, right I missed that you dropped @device and just noticed that this
> looks different from other commands. Calling blk_by_qdev_id() directly
> makes sense then.
>
>> So, do you suggest to drop blk_by_name() variant at all, and go through
>> qmp_get_blk(NULL, id), which
>> will call blk_by_qdev_id() and fail if not found?
>
> Yes, we don't need the blk_by_name() path, it's only for the legacy
> @device options.
>
> In any case, you can't use the same option for blk_by_name() and
> blk_by_qdev_id(). They are separate namespaces and the name could exist
> in both of them. This is why the other commands have both @id and
> @device instead of just a single option.
>
Ok, thank you for explanation. I'll resend today.
--
Best regards,
Vladimir
Re: [Qemu-devel] [Qemu-block] [RFC PATCH] coroutines: generate wrapper code
12.02.2019 6:22, Stefan Hajnoczi wrote: > On Mon, Feb 11, 2019 at 09:38:37AM +, Vladimir Sementsov-Ogievskiy wrote: >> 11.02.2019 6:42, Stefan Hajnoczi wrote: >>> On Fri, Feb 08, 2019 at 05:11:22PM +0300, Vladimir Sementsov-Ogievskiy >>> wrote: Hi all! We have a very frequent pattern of wrapping a coroutine_fn function to be called from non-coroutine context: - create structure to pack parameters - create function to call original function taking parameters from struct - create wrapper, which in case of non-coroutine context will create a coroutine, enter it and start poll-loop. Here is a draft of template code + example how it can be used to drop a lot of similar code. Hope someone like it except me) >>> >>> My 2 cents. Cons: >>> >>>* Synchronous poll loops are an anti-pattern. They block all of QEMU >>> with the big mutex held. Making them easier to write is >>> questionable because we should aim to have as few of these as >>> possible. >> >> Understand. Do we have a concept or a kind of target for a future to get rid >> of >> these a lot of poll-loops? What is the right way? At least for block-layer? > > It's non-trivial. The nested event loop could be flattened if there was > a mechanism to stop further activity on a specific object only (e.g. > BlockDriverState). That way the event loop can continue processing > events for other objects and device emulation could continue for other > objects. > > Unfortunately there are interactions between objects like in block jobs > that act on multiple BDSes, so it becomes even tricky. > > A simple way of imagining this is to make each object an "actor" > coroutine. The coroutine processes a single message (request) at a time > and yields when it needs to wait. Callers send messages and expect > asynchronous responses. This model is bad for efficiency (parallelism hmm, and with all these loops, where is parallelism? > is necessary) but at least it offers a sane way of thinking about > multiple asynchronous components coordinating together. (It's another > way of saying, let's put everything into coroutines.) > > The advantage of a flat event loop is that a hang in one object (e.g. > I/O getting stuck in one file) doesn't freeze the entire event loop. > >>> >>>* Code generation makes the code easier to write but harder to read. >>> Code is read more than written. In this case I think open coding >>> isn't too bad and I prefer it to reading a code generation script to >>> understand how it works. >> >> But you can read generated code in same way. You only need to read generator >> script if it generates something wrong, but should be rare. > > Generated code isn't visible unless the code has been built and indexed > (if you're using ctags). This makes it harder for people to navigate > the code. > >>> >>> If we were planning to add lots more of these then I agree code >>> generation would help. But in this case I'd rather not. >>> >> >> What do you think at least of generating code to create a coroutine from a >> function >> with multiple arguments? > > If it's easy to read without requiring one to figure out how the magic > works, then I like it. > Ok, I'll think about it. -- Best regards, Vladimir
Re: [Qemu-devel] [PATCH v2 1/3] s390x/cpumodel: mepochptff: warn when no mepoch and re-align group init
On Mon, 11 Feb 2019 20:16:55 -0500 Collin Walling wrote: > The extended PTFF features (qsie, qtoue, stoe, stoue) are dependent > on the multiple-epoch facility (mepoch). Let's print a warning if these > features are enabled without mepoch. > > While we're at it, let's move the FEAT_GROUP_INIT for mepochptff down > the s390_feature_groups list so it can be properly indexed with its > generated S390FeatGroup enum. > > Signed-off-by: Collin Walling > --- > target/s390x/cpu_features.c | 2 +- > target/s390x/cpu_models.c | 4 > 2 files changed, 5 insertions(+), 1 deletion(-) Not a comment for this patch (looks fine to me), but more general: Could you please add a cover letter if you send more than one patch? It's easy to miss that there are three patches and not two :)
[Qemu-devel] [PATCH v2] Kconfig: add documentation
Signed-off-by: Paolo Bonzini --- docs/devel/kconfig.rst | 305 + 1 file changed, 305 insertions(+) create mode 100644 docs/devel/kconfig.rst diff --git a/docs/devel/kconfig.rst b/docs/devel/kconfig.rst new file mode 100644 index 00..ff1ed3d1b2 --- /dev/null +++ b/docs/devel/kconfig.rst @@ -0,0 +1,305 @@ +Introduction + + +QEMU is a very versatile emulator; it can be built for a variety of +targets, where each target can emulate various boards and at the same +time different targets can share large amounts of code. For example, +a POWER and an x86 board can run the same code to emulate a PCI network +card, even though the boards use different PCI host bridges, and they +can run the same code to emulate a SCSI disk while using different +SCSI adapters. ARM, s390 and x86 boards can all present a virtio-blk +disk to their guests, but with three different virtio guest interfaces. + +Each QEMU target enables a subset of the boards, devices and buses that +are included in QEMU's source code. As a result, each QEMU executable +only links a small subset of the files that form QEMU's source code; +anything that is not needed to support a particular target is culled. + +QEMU uses a simple domain-specific language to describe the dependencies +between components. This is useful for two reasons: + +* new targets and boards can be added without knowing in detail the + architecture of the hardware emulation subsystems. Boards only have + to list the components they need, and the compiled executable will + include all the required dependencies and all the devices that the + user can add to that board; + +* users can easily build reduced versions of QEMU that support only a subset + of boards or devices. For example, by default most targets will include + all emulated PCI devices that QEMU supports, but the build process is + configurable and it is easy to drop unnecessary (or otherwise unwanted) + code to make a leaner binary. + +This domain-specific language is based on the Kconfig language that +originated in the Linux kernel, though it was heavily simplified and +the handling of dependencies is stricter in QEMU. + +Unlike Linux, there is no user interface to edit the configuration, which +is instead specified in per-target files under the ``default-configs/`` +directory of the QEMU source tree. This is because, unlike Linux, +configuration and dependencies can be treated as a black box when building +QEMU; the default configuration that QEMU ships with should be okay in +almost all cases. + +The Kconfig language + + +Kconfig defines configurable components in files named ``hw/*/Kconfig``. +Note that configurable components are _not_ visible in C code as preprocessor +symbols; they are only visible in the Makefile. Each configurable component +defines a Makefile variable whose name starts with ``CONFIG_``. + +All elements have boolean (true/false) type; truth is written as ``y``, while +falsehood is written ``n``. They are defined in a Kconfig +stanza like the following:: + + config ARM_VIRT + bool + imply PCI_DEVICES + imply VFIO_AMD_XGBE + imply VFIO_XGMAC + select A15MPCORE + select ACPI + select ARM_SMMUV3 + +The ``config`` keyword introduces a new configuration element. In the example +above, Makefiles will have access to a variable named ``CONFIG_ARM_VIRT``, +with value ``y`` or ``n`` (respectively for boolean true and false). + +Boolean expressions can be used within the language, whenever +is written in the remainder of this section. The ``&&``, ``||`` and +``!`` operators respectively denote conjunction (AND), disjunction (OR) +and negation (NOT). + +The ``bool`` data type declaration is optional, but it is suggested to +include it for clarity and future-proofing. After ``bool`` the following +directives can be included: + +**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. In the following example, + ``CONFIG_BAZ`` will be true whenever ``CONFIG_FOO`` is true:: + +config FOO + select BAZ + + The optional expression will prevent ``select`` from having any effect + unless it is true. + + Note that unlike Linux, QEMU will detect contradictions between + ``depends on`` and ``select`` statements and prevent you from building + such a configuration. + +**default value**: ``default [if ]`` + + Default values are assigned to the config symbol if no other value was + set by the user via ``default-configs/*.mak`` files, and only if + ``select`` or ``depends on`` directives do not force the value to true + or
Re: [Qemu-devel] [PATCH v2 1/3] s390x/cpumodel: mepochptff: warn when no mepoch and re-align group init
On 12.02.2019 02:16, Collin Walling wrote:
> The extended PTFF features (qsie, qtoue, stoe, stoue) are dependent
> on the multiple-epoch facility (mepoch). Let's print a warning if these
> features are enabled without mepoch.
>
> While we're at it, let's move the FEAT_GROUP_INIT for mepochptff down
> the s390_feature_groups list so it can be properly indexed with its
> generated S390FeatGroup enum.
>
> Signed-off-by: Collin Walling
Looks sane.
Reviewed-by: Christian Borntraeger
> ---
> target/s390x/cpu_features.c | 2 +-
> target/s390x/cpu_models.c | 4
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
> index 60cfeba48f..1843c84aaa 100644
> --- a/target/s390x/cpu_features.c
> +++ b/target/s390x/cpu_features.c
> @@ -456,7 +456,6 @@ static S390FeatGroupDef s390_feature_groups[] = {
> FEAT_GROUP_INIT("plo", PLO, "Perform-locked-operation facility"),
> FEAT_GROUP_INIT("tods", TOD_CLOCK_STEERING, "Tod-clock-steering
> facility"),
> FEAT_GROUP_INIT("gen13ptff", GEN13_PTFF, "PTFF enhancements introduced
> with z13"),
> -FEAT_GROUP_INIT("mepochptff", MULTIPLE_EPOCH_PTFF, "PTFF enhancements
> introduced with Multiple-epoch facility"),
> FEAT_GROUP_INIT("msa", MSA, "Message-security-assist facility"),
> FEAT_GROUP_INIT("msa1", MSA_EXT_1, "Message-security-assist-extension 1
> facility"),
> FEAT_GROUP_INIT("msa2", MSA_EXT_2, "Message-security-assist-extension 2
> facility"),
> @@ -466,6 +465,7 @@ static S390FeatGroupDef s390_feature_groups[] = {
> FEAT_GROUP_INIT("msa6", MSA_EXT_6, "Message-security-assist-extension 6
> facility"),
> FEAT_GROUP_INIT("msa7", MSA_EXT_7, "Message-security-assist-extension 7
> facility"),
> FEAT_GROUP_INIT("msa8", MSA_EXT_8, "Message-security-assist-extension 8
> facility"),
> +FEAT_GROUP_INIT("mepochptff", MULTIPLE_EPOCH_PTFF, "PTFF enhancements
> introduced with Multiple-epoch facility"),
> };
>
> const S390FeatGroupDef *s390_feat_group_def(S390FeatGroup group)
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 7c253ff308..b42b5fd327 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -788,6 +788,10 @@ static void check_consistency(const S390CPUModel *model)
> { S390_FEAT_SIE_KSS, S390_FEAT_SIE_F2 },
> { S390_FEAT_AP_QUERY_CONFIG_INFO, S390_FEAT_AP },
> { S390_FEAT_AP_FACILITIES_TEST, S390_FEAT_AP },
> +{ S390_FEAT_PTFF_QSIE, S390_FEAT_MULTIPLE_EPOCH },
> +{ S390_FEAT_PTFF_QTOUE, S390_FEAT_MULTIPLE_EPOCH },
> +{ S390_FEAT_PTFF_STOE, S390_FEAT_MULTIPLE_EPOCH },
> +{ S390_FEAT_PTFF_STOUE, S390_FEAT_MULTIPLE_EPOCH },
> };
> int i;
>
>
Re: [Qemu-devel] [PATCH v2] Kconfig: add documentation
On Tue, 12 Feb 2019 10:57:49 +0100 Paolo Bonzini wrote: > Signed-off-by: Paolo Bonzini > --- > docs/devel/kconfig.rst | 305 + > 1 file changed, 305 insertions(+) > create mode 100644 docs/devel/kconfig.rst Looks good! Reviewed-by: Cornelia Huck
Re: [Qemu-devel] [PATCH 2/4] multifd: Drop x-multifd-page-count parameter
On Tue, Feb 12, 2019 at 10:34:35AM +0100, Juan Quintela wrote:
> Daniel P. Berrangé wrote:
> > On Wed, Feb 06, 2019 at 02:23:29PM +0100, Juan Quintela wrote:
> >> Libvirt don't want to expose (and explain it). And testing looks like
> >> 128 is good for all use cases, so just drop it.
> >
> > One significant concern inline...
> >
> >>
> >> Signed-off-by: Juan Quintela
> >> ---
> >> hmp.c | 7 ---
> >> migration/migration.c | 30 --
> >> migration/migration.h | 1 -
> >> migration/ram.c | 13 -
> >> qapi/migration.json | 13 +
> >> 5 files changed, 9 insertions(+), 55 deletions(-)
> >
> >
> >> @@ -718,7 +721,7 @@ static void multifd_send_fill_packet(MultiFDSendParams
> >> *p)
> >> packet->magic = cpu_to_be32(MULTIFD_MAGIC);
> >> packet->version = cpu_to_be32(MULTIFD_VERSION);
> >> packet->flags = cpu_to_be32(p->flags);
> >> -packet->size = cpu_to_be32(migrate_multifd_page_count());
> >> +packet->size = cpu_to_be32(MULTIFD_PAGE_COUNT);
> >> packet->used = cpu_to_be32(p->pages->used);
> >> packet->packet_num = cpu_to_be64(p->packet_num);
> >>
> >
> > Here the source QEMU sends the page size - which is now
> > a hardcoded constant - to the target QEMU.
> >
> >> @@ -756,10 +759,10 @@ static int
> >> multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
> >> p->flags = be32_to_cpu(packet->flags);
> >>
> >> packet->size = be32_to_cpu(packet->size);
> >> -if (packet->size > migrate_multifd_page_count()) {
> >> +if (packet->size > MULTIFD_PAGE_COUNT) {
> >> error_setg(errp, "multifd: received packet "
> >> "with size %d and expected maximum size %d",
> >> - packet->size, migrate_multifd_page_count()) ;
> >> + packet->size, MULTIFD_PAGE_COUNT) ;
> >> return -1;
> >> }
> >>
> >
> > Here the dest QEMU receives the page size that the source QEMU used, and
> > checks that it is not larger than its constant.
> >
> > IIUC, the implication here is that if we ever increase the size of this
> > constant in future QEMU, we will break live migration from new to old
> > QEMU due to this check. In fact your previous patch in this series has
> > done exactly that, so this appears to mean QEMU 4.0 -> QEMU 3.2
> > multifd migration is broken now.
> >
> > Alternatively if we decrease the size of the constant in future
> > QEMU, we will break live migration from old QEMU to new QEMU which
> > is even worse.
> >
> > This problem existed before this patch, if the management app was
> > not explicitly using migrate-set-parameters to set the page count
> > on both sides of QEMU. So we're already broken, but at least the
> > feature was marked experimental.
> >
> > What is the purpose of this packet size check ? Is it something
> > we can safely remove, so that we can increase or decrease the
> > size at will without breaking migration compat.
>
> We have a "dinamyc" array of pages of that size. What we check is that
> the array fits into the part that we have assigned.
>
> We "could" wait until this moment to create the arrays, I need to look
> into that. Notice that what the check does is making sure that whatewer
> we receive is not bigger than the space that we have allocated.
>
> At this point, that check can only fail if we are "being" attacked and
> we have a malformed string. We check during negotiation that this value
> is ok.
>
> We should check this *also* in the initial packet, and then this check
> should never be true.
Right but checking earlier will still have the same problem I describe
with back compatibility if we ever change the page count in future QEMU,
as the limit checked by one QEMU may be smaller than what the other QEMU
is intentionally sending. I don't see where we have a bi-directional
channel that would allow the 2 QEMUs to negotiate a mutually acceptable
page count :-(
> From a management point of view, what do you preffer here?
The earlier we check requirements the better, as it means we get the
error reported sooner, before wasting time of data transmisions.
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 :|
Re: [Qemu-devel] [PATCH 1/1] RFC: net/slirp: link with libslirp
On Mon, Feb 11, 2019 at 11:22:27PM +0100, Samuel Thibault wrote: > Marc-André Lureau, le lun. 11 févr. 2019 12:34:47 +0100, a ecrit: > > On Mon, Feb 11, 2019 at 12:09 PM Daniel P. Berrangé > > wrote: > > > > > > On Fri, Feb 08, 2019 at 07:12:26PM +0100, Marc-André Lureau wrote: > > > > Once libslirp has received its first release, we can link with the > > > > external libslirp library. > > > > > > > > The migration data should be compatible with current and older qemu > > > > versions (same compatibility as today). See "slirp: add state > > > > saving/loading" patch. However, the content should be treated as a > > > > blob, as the format may change eventually in the future. > > > > > > How are we going to manage live migration compat if libslirp changes > > > the blob content ? > > > > > > Bear in mind that we need to support all existing QEMU releases live > > > migrating to effectively all future QEMU releases, with all future > > > libslirp releases, in *both* directions. ie arbitrarily newer > > > libslirp needs to be able to emit a blob format that can be read > > > by arbitrarily older slirp inside QEMU. > > > > Right, this is all supported currently with the proposed patch set, > > since it is effectively the same code. > > > > So register_savevm_live() get passed slirp_state_version() (currently == 4) > > > > & slirp_state_load() get the version_id from QEMU. > > Mmm, but do we guarantee that the current version of slirp will be able > to read blobs produced by future versions of slirp? > > Future extensions of the format would have to be so that the current > version could discard their content without issues. > > Perhaps qemu should actually explicitly pass 4 to > register_savevm_live(), for that function to only record that format > (and thus get compatibility of course) and only bump to greater values > when qemu is modified to make use of a functionality which requires > extending the blob format, which then makes it unreadable by older qemu > releases, but that is fine for qemu. If QEMU wants to make use of functionality that requires the new blob format, it would have to restrict that based on machine type. That way it can ensure full compatibility with old QEMU. 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 :|
Re: [Qemu-devel] [PULL v3 26/28] seccomp: Work-around GCC 4.x bug in gnu99 mode
On 17/01/2019 - 14:38:12, Thomas Huth wrote:
> We'd like to compile QEMU with -std=gnu99, but GCC 4.8 currently
> fails to compile qemu-seccomp.c in this mode:
>
> qemu-seccomp.c:45:1: error: initializer element is not constant
> };
> ^
> qemu-seccomp.c:45:1: error: (near initialization for
> ‘sched_setscheduler_arg[0]’)
>
> This is due to a compiler bug which has just been fixed in GCC 5.0:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63567
>
> Since we still want to support GCC 4.8 for a while and also want to use
> gnu99 mode, work-around the issue by expanding the macro manually.
>
> Reviewed-by: Daniel P. Berrangé
> Reviewed-by: Philippe Mathieu-Daudé
> Signed-off-by: Thomas Huth
> ---
> qemu-seccomp.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/qemu-seccomp.c b/qemu-seccomp.c
> index 5c73e6a..36d5829 100644
> --- a/qemu-seccomp.c
> +++ b/qemu-seccomp.c
> @@ -41,7 +41,8 @@ struct QemuSeccompSyscall {
> };
>
> const struct scmp_arg_cmp sched_setscheduler_arg[] = {
> -SCMP_A1(SCMP_CMP_NE, SCHED_IDLE)
> +/* was SCMP_A1(SCMP_CMP_NE, SCHED_IDLE), but expanded due to GCC 4.x bug
> */
> +{ .arg = 1, .op = SCMP_CMP_NE, .datum_a = SCHED_IDLE }
> };
>
> static const struct QemuSeccompSyscall blacklist[] = {
> --
> 1.8.3.1
>
>
Just noticed you already sent the pull request, sorry, my tree was outdated.
Please ignore my previous email :)
--
Eduardo Otubo
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH 1/4] io/channel: add qio_channel_get_attached_aio_context()
On Mon, Feb 11, 2019 at 03:55:58PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Expose attached aio context. It will be used in nbd code, to
> understand, in which aio context negotiation should be done.
I'm not especially objecting to the idea of adding the API to the
QIOChannel class, but I'm surprised that NBD needs this. Surely it
already knows what AIO context it assigned to the channel in the
first place. IOW, it feels like this is papering over a limitation
in NBD not keeping track of what AIO context it should be using.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> include/io/channel.h | 9 +
> io/channel.c | 5 +
> 2 files changed, 14 insertions(+)
>
> diff --git a/include/io/channel.h b/include/io/channel.h
> index da2f138200..1a1e4a01b0 100644
> --- a/include/io/channel.h
> +++ b/include/io/channel.h
> @@ -718,6 +718,15 @@ GSource *qio_channel_add_watch_source(QIOChannel *ioc,
> void qio_channel_attach_aio_context(QIOChannel *ioc,
> AioContext *ctx);
>
> +/*
> + * qio_channel_get_aio_context
> + * @ioc: the channel object
> + *
> + * Returns channel AioContext if any attached by
> + * qio_channel_attach_aio_context(), otherwise NULL.
> + */
> +AioContext *qio_channel_get_attached_aio_context(QIOChannel *ioc);
> +
> /**
> * qio_channel_detach_aio_context:
> * @ioc: the channel object
> diff --git a/io/channel.c b/io/channel.c
> index 8dd0684f5d..a1b937bb6b 100644
> --- a/io/channel.c
> +++ b/io/channel.c
> @@ -454,6 +454,11 @@ void qio_channel_detach_aio_context(QIOChannel *ioc)
> ioc->ctx = NULL;
> }
>
> +AioContext *qio_channel_get_attached_aio_context(QIOChannel *ioc)
> +{
> +return ioc->ctx;
> +}
> +
> void coroutine_fn qio_channel_yield(QIOChannel *ioc,
> GIOCondition condition)
> {
> --
> 2.18.0
>
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 :|
Re: [Qemu-devel] [PATCH] seccomp: Work-around GCC 4.x bug in gnu99 mode
On 23/01/2019 - 07:57:51, no-re...@patchew.org wrote: > Patchew URL: > https://patchew.org/QEMU/1547573562-32754-1-git-send-email-th...@redhat.com/ > > > > Hi, > > This series failed the docker-mingw@fedora build test. Please find the > testing commands and > their output below. If you have Docker installed, you can probably reproduce > it > locally. > > === TEST SCRIPT BEGIN === > #!/bin/bash > time make docker-test-mingw@fedora SHOW_ENV=1 J=14 > === TEST SCRIPT END === > > CC backends/rng-egd.o > CC backends/tpm.o > /tmp/qemu-test/src/block/sheepdog.c: In function 'find_vdi_name': > /tmp/qemu-test/src/block/sheepdog.c:1239:5: error: 'strncpy' specified bound > 256 equals destination size [-Werror=stringop-truncation] > strncpy(buf + SD_MAX_VDI_LEN, tag, SD_MAX_VDI_TAG_LEN); > ^~ > cc1: all warnings being treated as errors > > > The full log is available at > http://patchew.org/logs/1547573562-32754-1-git-send-email-th...@redhat.com/testing.docker-mingw@fedora/?type=message. > --- > Email generated automatically by Patchew [http://patchew.org/]. > Please send your feedback to patchew-de...@redhat.com This issue was not introduced by the patch itself. Will send a pull request anyways. -- Eduardo Otubo signature.asc Description: PGP signature
Re: [Qemu-devel] [PULL 00/25] pci, pc, virtio: fixes, cleanups, features
On 2/12/19 8:11 AM, Peter Xu wrote: > On Tue, Feb 05, 2019 at 11:06:42AM -0500, Michael S. Tsirkin wrote: >> >> OK I reverted the whole part dealing with vhost-user and reposted. > > I noticed that the merged pull request could possibly have squashed > the below two patches (in previous pull) into one super patch > (a56de056c91f87e1e): > > i386/kvm: ignore masked irqs when update msi routes > contrib/vhost-user-blk: fix the compilation issue > > Here, the first patch lost its commit message, and the last patch lost > its real author. ;) I suggest we revert a56de056c9 ASAP and reapply the both patches, this will ease cherry-picking/downstream workflow.
Re: [Qemu-devel] [PATCH 1/1] RFC: net/slirp: link with libslirp
* Daniel P. Berrangé (berra...@redhat.com) wrote: > On Mon, Feb 11, 2019 at 11:22:27PM +0100, Samuel Thibault wrote: > > Marc-André Lureau, le lun. 11 févr. 2019 12:34:47 +0100, a ecrit: > > > On Mon, Feb 11, 2019 at 12:09 PM Daniel P. Berrangé > > > wrote: > > > > > > > > On Fri, Feb 08, 2019 at 07:12:26PM +0100, Marc-André Lureau wrote: > > > > > Once libslirp has received its first release, we can link with the > > > > > external libslirp library. > > > > > > > > > > The migration data should be compatible with current and older qemu > > > > > versions (same compatibility as today). See "slirp: add state > > > > > saving/loading" patch. However, the content should be treated as a > > > > > blob, as the format may change eventually in the future. > > > > > > > > How are we going to manage live migration compat if libslirp changes > > > > the blob content ? > > > > > > > > Bear in mind that we need to support all existing QEMU releases live > > > > migrating to effectively all future QEMU releases, with all future > > > > libslirp releases, in *both* directions. ie arbitrarily newer > > > > libslirp needs to be able to emit a blob format that can be read > > > > by arbitrarily older slirp inside QEMU. > > > > > > Right, this is all supported currently with the proposed patch set, > > > since it is effectively the same code. > > > > > > So register_savevm_live() get passed slirp_state_version() (currently == > > > 4) > > > > > > & slirp_state_load() get the version_id from QEMU. > > > > Mmm, but do we guarantee that the current version of slirp will be able > > to read blobs produced by future versions of slirp? > > > > Future extensions of the format would have to be so that the current > > version could discard their content without issues. > > > > Perhaps qemu should actually explicitly pass 4 to > > register_savevm_live(), for that function to only record that format > > (and thus get compatibility of course) and only bump to greater values > > when qemu is modified to make use of a functionality which requires > > extending the blob format, which then makes it unreadable by older qemu > > releases, but that is fine for qemu. > > If QEMU wants to make use of functionality that requires the new blob > format, it would have to restrict that based on machine type. That way > it can ensure full compatibility with old QEMU. Library compatibility for migration blobs is messy; we've had the same problem with usbredir. It's very difficult to keep track of; for example lets say that libslirp gets used by 3 or 4 different projects, and the package maintainer for libslirp in your favorite distro updates it - it's not necessarily obvious to them that doing so could break qemu migration. The ideal way is to work in terms of features and not versions; so that a machine type can enable libslirp-feature-foo in the new machine type and only then will the migration contents change. Dave > 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 :| -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
[Qemu-devel] [PATCH] hw/arm/armsse: Fix miswiring of expansion IRQs
In commit 91c1e9fcbd7548db368 where we added dual-CPU support to
the ARMSSE, we set up the wiring of the expansion IRQs via nested
loops: the outer loop on 'i' loops for each CPU, and the inner loop
on 'j' loops for each interrupt. Fix a typo which meant we were
wiring every expansion IRQ line to external IRQ 0 on CPU 0 and
to external IRQ 1 on CPU 1.
Fixes: 91c1e9fcbd7548db368 ("hw/arm/armsse: Support dual-CPU configuration")
Signed-off-by: Peter Maydell
---
It turns out that the ARM-TFM image I was using to test that
I hadn't broken the mps2-an505 doesn't actually rely on any
interrupts from the external devices...
hw/arm/armsse.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index 5d53071a5a0..9a8c49547db 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -565,7 +565,7 @@ static void armsse_realize(DeviceState *dev, Error **errp)
/* Connect EXP_IRQ/EXP_CPUn_IRQ GPIOs to the NVIC's lines 32 and up */
s->exp_irqs[i] = g_new(qemu_irq, s->exp_numirq);
for (j = 0; j < s->exp_numirq; j++) {
-s->exp_irqs[i][j] = qdev_get_gpio_in(cpudev, i + 32);
+s->exp_irqs[i][j] = qdev_get_gpio_in(cpudev, j + 32);
}
if (i == 0) {
gpioname = g_strdup("EXP_IRQ");
--
2.20.1
Re: [Qemu-devel] [PULL 0/4] NBD patches for 2019-02-11
On Mon, 11 Feb 2019 at 20:53, Eric Blake wrote: > > The following changes since commit 22c5f446514a2a4bb0dbe1fea26713da92fc85fa: > > Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20190211' into > staging (2019-02-11 17:04:57 +) > > are available in the Git repository at: > > https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2019-02-11 > > for you to fetch changes up to ddd113beedd22908e676d53803843d2f85bf91ab: > > qcow2: list of bitmaps new test 242 (2019-02-11 14:35:43 -0600) > > > nbd patches for 2019-02-11 > > - Add qcow2 bitmap details to 'qemu-img info' > > > Andrey Shinkevich (3): > bdrv_query_image_info Error parameter added > qcow2: Add list of bitmaps to ImageInfoSpecificQCow2 > qcow2: list of bitmaps new test 242 > > Eric Blake (1): > nbd/server: Kill pointless shadowed variable Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0 for any user-visible changes. -- PMM
Re: [Qemu-devel] [Qemu-block] [RFC PATCH] coroutines: generate wrapper code
On Tue, Feb 12, 2019 at 10:03:19AM +, Vladimir Sementsov-Ogievskiy wrote: > 12.02.2019 6:22, Stefan Hajnoczi wrote: > > On Mon, Feb 11, 2019 at 09:38:37AM +, Vladimir Sementsov-Ogievskiy > > wrote: > >> 11.02.2019 6:42, Stefan Hajnoczi wrote: > >>> On Fri, Feb 08, 2019 at 05:11:22PM +0300, Vladimir Sementsov-Ogievskiy > >>> wrote: > Hi all! > > We have a very frequent pattern of wrapping a coroutine_fn function > to be called from non-coroutine context: > > - create structure to pack parameters > - create function to call original function taking parameters from > struct > - create wrapper, which in case of non-coroutine context will > create a coroutine, enter it and start poll-loop. > > Here is a draft of template code + example how it can be used to drop a > lot of similar code. > > Hope someone like it except me) > >>> > >>> My 2 cents. Cons: > >>> > >>>* Synchronous poll loops are an anti-pattern. They block all of QEMU > >>> with the big mutex held. Making them easier to write is > >>> questionable because we should aim to have as few of these as > >>> possible. > >> > >> Understand. Do we have a concept or a kind of target for a future to get > >> rid of > >> these a lot of poll-loops? What is the right way? At least for block-layer? > > > > It's non-trivial. The nested event loop could be flattened if there was > > a mechanism to stop further activity on a specific object only (e.g. > > BlockDriverState). That way the event loop can continue processing > > events for other objects and device emulation could continue for other > > objects. > > > > Unfortunately there are interactions between objects like in block jobs > > that act on multiple BDSes, so it becomes even tricky. > > > > A simple way of imagining this is to make each object an "actor" > > coroutine. The coroutine processes a single message (request) at a time > > and yields when it needs to wait. Callers send messages and expect > > asynchronous responses. This model is bad for efficiency (parallelism > > hmm, and with all these loops, where is parallelism? In this model all activity, including the async operations that work in parallel today, go through an actor coroutine. The loss of parallelism isn't in synchronous polling operations, it's in the async operations. But I'm just using this as a simple model for reasoning about a world where no synchronous polling loops are necessary. I don't think we would try to implement it - at least not without a lot more thought into how to make it efficient. Stefan signature.asc Description: PGP signature
Re: [Qemu-devel] [Bug 1814418] [NEW] persistent bitmap will be inconsistent when qemu crash,
12.02.2019 4:10, John Snow wrote:
>
>
> On 2/4/19 11:22 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 04.02.2019 17:55, Eric Blake wrote:
>>> On 2/2/19 11:52 PM, Cheng Chen wrote:
Public bug reported:
Follow these steps to reappear the bug:
1. start qemu
2. add persistent bitmap: '{ "execute": "block-dirty-bitmap-add",
"arguments": {"node": "drive-virtio-disk1","name": "bitmap0",
"persistent":true }}'
3. kill -9 qemu (simulate Host crash, eg. lose power)
4. restart qemu
Now, the '{ "execute": "query-block" }' can't find the bitmap0. I can
understand at this point, because the bitmap0 has not been synchronized
yet.
>>>
>>> This matches my observations here:
>>> https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg07700.html
>>>
>>> I'm of the opinion that updating the qcow2 headers any time a persistent
>>> bitmap is created or destroyed is worthwhile, even if the headers must
>>> still mark the bitmap as in-use. True, the crash will leave the bitmap
>>> as inconsistent, which is no different than if the bitmap is never
>>> written to the qcow2 header (when booting a new qemu, an inconsistent
>>> bitmap on disk has the same status as a missing bitmap - both imply that
>>> an incremental backup is not possible, and so a full backup is needed
>>> instead). But the creation of bitmaps is not a common occasion, and
>>> having the metadata track that a persistent bitmap has been requested
>>> seems like it would be useful information during a debugging session.
>>
>> Even if we store them, following query-block will not show them, as
>> in-use bitmaps are not loaded on open (or, we should load them too).
>>
>>>
>>>
But, when I try to add persistent bitmap0 again: '{ "execute": "block-
dirty-bitmap-add", "arguments": {"node": "drive-virtio-disk1","name":
"bitmap0", "persistent":true }}', It failed:
{"id":"libvirt-42","error":{"class":"GenericError","desc":"Can't make
bitmap 'bitmap0' persistent in 'drive-virtio-disk1': Bitmap with the
same name is already stored"}}
In other word, when qemu crash, the qcow2 image remain the incomplete
persistent bitmap.
>>
>> Yes (if it was stored at least once, may be on previous qemu run).
>>
>>>
>>> Does Andrey's proposed patch adding persistent bitmap details to
>>> 'qemu-img info' show anything after you first kill qemu?
>>>
>>> /me goes and tests...
>>>
>>> Oh weird - with Andrey's patch, we get semi-duplicated information
>>> during query-block (at least, after an initial clean shutdown prior to
>>> attempting an abrupt shutdown):
>>>
>>> {"return": [{"io-status": "ok", "device": "ide0-hd0", "locked": false,
>>> "removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off",
>>> "image": {"virtual-size": 104857600, "filename": "file5",
>>> "cluster-size": 65536, "format": "qcow2", "actual-size": 208896,
>>> "format-specific": {"type": "qcow2", "data": {"compat": "1.1",
>>> "lazy-refcounts": false, "bitmaps": [{"flags": ["in-use", "auto"],
>>> "name": "b2", "granularity": 65536}], "refcount-bits": 16, "corrupt":
>>> false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "node-name":
>>> "#block172", "backing_file_depth": 0, "drv": "qcow2", "iops": 0,
>>> "bps_wr": 0, "write_threshold": 0, "encrypted": false, "bps": 0,
>>> "bps_rd": 0, "cache": {"no-flush": false, "direct": false, "writeback":
>>> true}, "file": "file5", "encryption_key_missing": false}, "qdev":
>>> "/machine/unattached/device[18]", "dirty-bitmaps": [{"name": "b2",
>>> "status": "active", "granularity": 65536, "count": 0}, {"name": "b",
>>> "status": "active", "granularity": 65536, "count": 0}], "type": "unknown"}]}
>>>
>>> Note that the "format-specific" listing has a "bitmaps" entry resulting
>>> from Andrey's patch (which shows "auto" as one of the "flags" for any
>>> persistent bitmap) showing the state of the persistent bitmaps on disk;
>>> as well as the "dirty-bitmaps" entry that shows the state of the bitmaps
>>> in memory. Annoyingly, the "dirty-bitmaps" section does NOT state which
>>> bitmaps are persistent, and if a persistent bitmap has not yet been
>>> flushed to disk, then there is NO way to quickly determine which bitmaps
>>> are persistent and which are transient.
>>>
>>> At any rate, with qemu.git master + Andrey's patches for qemu-img info,
>>> I was able to reproduce a related oddity: attempting to
>>> block-dirty-bitmap-add a transient bitmap that has the same name as an
>>> existing in-use persistent bitmap succeeds; when it should have failed:
>>>
>>> $ qemu-img create -f qcow2 file5 100m
>>> $ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio
>>> file5
>>> {'execute':'qmp_capabilities'}
>>> {'execute':'query-block'} # learn the node name...
>>> {'execute':'block-dirty-bitmap-add','arguments':{'node':'#block111','name':'b1','persistent':true}}
>>> {'execute':'quit'}
>>> $ ./qemu-img info -U file5
>>> image: file5
>>> file forma
Re: [Qemu-devel] [PATCH] Kconfig: add documentation
On 12/02/19 10:08, Markus Armbruster wrote: > Please wrap your lines at column 70 or so. Humans tend to have trouble > following long lines with their eyes (I sure do). Typographic manuals > suggest to limit columns to roughly 60 characters for exactly that > reason[*]. Yup, fixed in v2. >> +Each QEMU target enables a subset of the boards, devices and buses that are >> included >> +in QEMU's source code. As a result, each QEMU executable only links a >> small subset > > Really? Hmm... > > $ size aarch64-softmmu/qemu-system-aarch64 >text data bss dec hex filename > 19183216 7200124 592732 2697607219b9f48 > aarch64-softmmu/qemu-system-aarch64 > $ size -t `find -name \*.o `| grep TOT > 92713559 186522271183961440 1295327226 4d351ffa > (TOTALS) > > Yep, really. Haha. :) >> + Optionally, a condition for applying the default value can be added with >> + ``if``. A config option can have any number of default values (usually, >> if more than >> + one default is present, they will have different conditions). If multiple >> + default values satisfy their condition, only the first defined one is >> active. > > Hmm. Is "multiple default values, first one wins" a healthy state? > How obvious is "first defined" to humans? It certainly helps that we never have more than one default. :) I could also be persuaded to remove "default n", so that multiple "default y" clauses are just an OR of the conditions and the ordering does not matter. Are multiple "default y" clauses useful? They were there in earlier versions of the patches, for now "imply" has removed the need everywhere. However, I'm not sure it's a good idea to remove them altogether before we try to extend Kconfig to more features. (A secondary effect of the documentation is to clarify the current scope of Kconfig). >> +**reverse default** (weak reverse dependency): ``imply [if >> ]`` > > If "reverse default" can be regarded as weak reverse dependency, could > "default value" be regarded as weak (forward) dependency? "default n if" could, but we never use it. This also shows how we use the language in a very limited way, according to the very simple rules in the second part of the document; but even Linux only has a handful of occurrences of "default n if". >> + >> + This is similar to ``select`` as it applies a lower limit of ``y`` to >> another >> + symbol. However, the lower limit is only a default and the "implied" >> symbol's >> + value may still be set to ``n`` from a ``default-configs/*.mak`` files. >> The > > I'm afraid I don't get "lower limit". What's the ordering relation? False < true, so "lower limit of y" means "tries to force to y". The difference is that a contradiction is ignored by "imply" (and then the symbol remains at n), while it causes a build error for "select". Paolo
Re: [Qemu-devel] [PATCH] Kconfig: add documentation
On 12/02/19 10:13, Thomas Huth wrote: >> +Unlike Linux, there is no user interface to edit the configuration, which >> is instead >> +specified in per-target files under the ``default-configs/`` directory of >> the >> +QEMU source tree. This is because, unlike Linux, configuration and >> dependencies can be >> +treated as a black box when building QEMU; the default configuration that >> QEMU >> +ships with should be okay in almost all cases. > I'd like to suggest to replace "Linux" with "Linux-Kconfig". Here I think it's really Linux vs. QEMU, not Linux-Kconfig versus minikconf. However... > [...] >> +**reverse dependencies**: ``select [if ]`` >> + >> + While ``depends on`` forces a symbol to false, reverse dependencies can be >> + used to force another symbol to true. In the following example, >> + ``CONFIG_BAZ`` will be true whenever ``CONFIG_FOO`` is true:: >> + >> +config FOO >> + select BAZ >> + >> + The optional expression will prevent ``select`` from having any effect >> + unless it is true. >> + >> + Note that unlike Linux, QEMU will detect contradictions between ``depends >> on`` and > dito ... I'll do the replacement you suggest here. Paolo
[Qemu-devel] [PATCH v1 00/15] s390x/tcg: Implement floating-point extension facility
This series is based on the patches currently on the list:
- softfloat: Implement float128_to_uint32
- softfloat: add float128_is_{normal,denormal}
- s390x: add zPCI feature to "qemu" CPU model
For the KVM folks, only the last patch is of interest.
The floating-point extension facility seems to be used often without
checking by user space. Vector instruction support seems to somewhat also
rely on it being available. Some parts are already implemented, others
not or are broken.
Fix some stuff in fpu code, add some FIXMEs to indicate what is definetly
missing and implement all BFP stuff needed for the floating-point extension
facility. Finally, enable it for TCG, adding it to the CPU model for the
4.0 compat machine.
This series also contains some refactorings needed for floating-point
vector instruction support.
David Hildenbrand (15):
s390x/tcg: Fix TEST DATA CLASS instructions
s390x/tcg: Fix rounding from float128 to uint64_t/uin32_t
s390x/tcg: Factor out conversion of softfloat exceptions
s390x/tcg: Fix parts of IEEE exception handling
s390x/tcg: Hide IEEE underflows in some scenarios
s390x/tcg: Refactor SET FPC AND SIGNAL handling
s390x/tcg: Fix simulated-IEEE exceptions
s390x/tcg: Handle SET FPC AND LOAD FPC 3-bit BFP rounding modes
s390x/tcg: Check for exceptions in SET BFP ROUNDING MODE
s390x/tcg: Refactor saving/restoring the bfp rounding mode
s390x/tcg: Prepare for IEEE-inexact-exception control (XxC)
s390x/tcg: Implement XxC and checks for most FP instructions
s390x/tcg: Implement rounding mode and XxC for LOAD ROUNDED
s390x/tcg: Handle all rounding modes overwritten by BFP instructions
s390x: Add floating-point extension facility to "qemu" cpu model
target/s390x/fpu_helper.c | 547 +++-
target/s390x/gen-features.c | 5 +
target/s390x/helper.h | 9 +-
target/s390x/insn-data.def | 12 +-
target/s390x/internal.h | 15 +
target/s390x/translate.c| 340 +++---
6 files changed, 628 insertions(+), 300 deletions(-)
--
2.17.2
Re: [Qemu-devel] [Qemu-ppc] [PATCH] cuda: decrease time delay before raising VIA SR interrupt
On Tue, 12 Feb 2019, Mark Cave-Ayland wrote:
On 11/02/2019 23:35, Philippe Mathieu-Daudé wrote:
Hi Mark,
On 2/10/19 6:44 PM, Mark Cave-Ayland wrote:
In order to handle a race condition in MacOS 9, a delay was introduced when
raising the VIA SR interrupt inspired by similar code in MacOnLinux.
During original testing of the MacOS 9 patches it was found that the 30us
delay used in MacOnLinux did not work reliably within QEMU, and a value of
300us was required to function correctly.
Recent experiments have shown that the previous reliability issues are no
longer present, and this value can be reduced down to 20us with no apparent
ill effects in my local tests. This has the benefit of considerably improving
the responsiveness of the ADB keyboard and mouse with the guest.
Signed-off-by: Mark Cave-Ayland
---
hw/misc/macio/cuda.c | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index c4f7a2f39b..3febacdd1e 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -97,17 +97,8 @@ static void cuda_set_sr_int(void *opaque)
static void cuda_delay_set_sr_int(CUDAState *s)
{
-MOS6522CUDAState *mcs = &s->mos6522_cuda;
-MOS6522State *ms = MOS6522(mcs);
-MOS6522DeviceClass *mdc = MOS6522_DEVICE_GET_CLASS(ms);
int64_t expire;
-if (ms->dirb == 0xff || s->sr_delay_ns == 0) {
-/* Disabled or not in Mac OS, fire the IRQ directly */
-mdc->set_sr_int(ms);
-return;
-}
The change of sr_delay_ns below is well explained, but I don't
understand why you remove the previous if().
IIRC it was a hack by Alex to try and restrict the delay on the interrupt just
to
MacOS instead of Linux, but with the reduced value it doesn't really matter any
more.
If this delay is to prevent a bug which only happens in MacOS then that's
the hack not the normal code path to run without the delay that you've
just removed. So maybe this should be kept if possible to avoid unecessary
delays for other guests. (Although if this only affects mac99,via=cuda but
not mac99,via=pmu then I don't care much as long as pmu works.)
As a plus it also prevents a guest OS from accidentally triggering the hack
whilst
programming the VIA port.
That may be a problem though. What's the issue exactly? Why is the delay
needed in the first place?
Regards,
BALATON Zoltan
[Qemu-devel] [PATCH v1] s390x: Add floating-point extension facility to "qemu" cpu model
The floating-point extension facility implemented certain changes to
BFP, HFP and DFP instructions.
As we don't implement HFP/DFP, we can ignore those completely. Related
to BFP, the changes include
- SET BFP ROUNDING MODE (SRNMB) instruction
- BFP-rounding-mode field in the FPC register is changed to 3 bits
- CONVERT FROM LOGICAL instructions
- CONVERT TO LOGICAL instructions
- Changes (rounding mode + XxC) added to
-- CONVERT TO FIXED
-- CONVERT FROM FIXED
-- LOAD FP INTEGER
-- LOAD ROUNDED
-- DIVIDE TO INTEGER
For TCG, we don't implement DIVIDE TO INTEGER, and it is harder to
implement, so skip that. Also, as we don't implement PFPO, we can skip
changes to that as well. The other parts are now implemented, we can
indicate the facility.
z14 PoP mentiones that "The floating-point extension facility is installed
in the z/Architecture architectural mode. When bit 37 is one, bit 42 is
also one.", meaning that the DFP (decimal-floating-point) facility also
has to be inidicated. We can ignore that for now.
Signed-off-by: David Hildenbrand
---
target/s390x/gen-features.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 19bb8ac7f8..02f6e9f475 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -593,6 +593,11 @@ static uint16_t qemu_V3_1[] = {
};
static uint16_t qemu_LATEST[] = {
+/*
+ * Only BFP bits are implemented (HFP, DFP, PFPO and DIVIDE TO INTEGER not
+ * implemented yet).
+ */
+S390_FEAT_FLOATING_POINT_EXT,
S390_FEAT_ZPCI,
};
--
2.17.2
[Qemu-devel] [PATCH v1 06/15] s390x/tcg: Refactor SET FPC AND SIGNAL handling
We can directly work on the uint64_t value, no need for a temporary
uint32_t value.
Also cleanup and shorten the comments.
Signed-off-by: David Hildenbrand
---
target/s390x/fpu_helper.c | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c
index 2e690073ea..7c39829352 100644
--- a/target/s390x/fpu_helper.c
+++ b/target/s390x/fpu_helper.c
@@ -820,21 +820,23 @@ void HELPER(sfpc)(CPUS390XState *env, uint64_t fpc)
}
/* set fpc and signal */
-void HELPER(sfas)(CPUS390XState *env, uint64_t val)
+void HELPER(sfas)(CPUS390XState *env, uint64_t fpc)
{
uint32_t signalling = env->fpc;
-uint32_t source = val;
uint32_t s390_exc;
-/* The contents of the source operand are placed in the FPC register;
- then the flags in the FPC register are set to the logical OR of the
- signalling flags and the source flags. */
-env->fpc = source | (signalling & 0x00ff);
-set_float_rounding_mode(fpc_to_rnd[source & 3], &env->fpu_status);
+/*
+ * FPC is set to the FPC operand with a bitwise OR of the signalling
+ * flags.
+ */
+env->fpc = fpc | (signalling & 0x00ff);
+set_float_rounding_mode(fpc_to_rnd[fpc & 3], &env->fpu_status);
-/* If any signalling flag is 1 and the corresponding source mask
- is also 1, a simulated-iee-exception trap occurs. */
-s390_exc = (signalling >> 16) & (source >> 24);
+/*
+ * If any signaling flag is enabled in the new FPC mask, a
+ * simulated-iee-exception exception occurs.
+ */
+s390_exc = (signalling >> 16) & (fpc >> 24);
if (s390_exc) {
tcg_s390_data_exception(env, s390_exc | 3, GETPC());
}
--
2.17.2
[Qemu-devel] [PATCH v1 02/15] s390x/tcg: Fix rounding from float128 to uint64_t/uin32_t
Let's use the proepr conversion functions now that we have them.
Signed-off-by: David Hildenbrand
---
target/s390x/fpu_helper.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c
index a3214bd546..f9ec519067 100644
--- a/target/s390x/fpu_helper.c
+++ b/target/s390x/fpu_helper.c
@@ -509,9 +509,7 @@ uint64_t HELPER(clgdb)(CPUS390XState *env, uint64_t v2,
uint32_t m3)
uint64_t HELPER(clgxb)(CPUS390XState *env, uint64_t h, uint64_t l, uint32_t m3)
{
int hold = swap_round_mode(env, m3);
-float128 v2 = make_float128(h, l);
-/* ??? Not 100% correct. */
-uint64_t ret = float128_to_int64(v2, &env->fpu_status);
+uint64_t ret = float128_to_uint64(make_float128(h, l), &env->fpu_status);
set_float_rounding_mode(hold, &env->fpu_status);
handle_exceptions(env, GETPC());
return ret;
@@ -541,9 +539,7 @@ uint64_t HELPER(clfdb)(CPUS390XState *env, uint64_t v2,
uint32_t m3)
uint64_t HELPER(clfxb)(CPUS390XState *env, uint64_t h, uint64_t l, uint32_t m3)
{
int hold = swap_round_mode(env, m3);
-float128 v2 = make_float128(h, l);
-/* Not 100% correct. */
-uint32_t ret = float128_to_int64(v2, &env->fpu_status);
+uint32_t ret = float128_to_uint32(make_float128(h, l), &env->fpu_status);
set_float_rounding_mode(hold, &env->fpu_status);
handle_exceptions(env, GETPC());
return ret;
--
2.17.2
[Qemu-devel] [PATCH v1 10/15] s390x/tcg: Refactor saving/restoring the bfp rounding mode
We want to reuse this in the context of vector instructions. So use
better matching names and introduce s390_restore_bfp_rounding_mode().
While at it, add proper newlines.
Signed-off-by: David Hildenbrand
---
target/s390x/fpu_helper.c | 113 +++---
target/s390x/internal.h | 2 +
2 files changed, 72 insertions(+), 43 deletions(-)
diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c
index a6c67527cd..518fb6365a 100644
--- a/target/s390x/fpu_helper.c
+++ b/target/s390x/fpu_helper.c
@@ -372,7 +372,7 @@ uint32_t HELPER(cxb)(CPUS390XState *env, uint64_t ah,
uint64_t al,
return float_comp_to_cc(env, cmp);
}
-static int swap_round_mode(CPUS390XState *env, int m3)
+int s390_swap_bfp_rounding_mode(CPUS390XState *env, int m3)
{
int ret = env->fpu_status.float_rounding_mode;
switch (m3) {
@@ -401,12 +401,18 @@ static int swap_round_mode(CPUS390XState *env, int m3)
return ret;
}
+void s390_restore_bfp_rounding_mode(CPUS390XState *env, int old_mode)
+{
+set_float_rounding_mode(old_mode, &env->fpu_status);
+}
+
/* convert 64-bit int to 32-bit float */
uint64_t HELPER(cegb)(CPUS390XState *env, int64_t v2, uint32_t m3)
{
-int hold = swap_round_mode(env, m3);
+int old_mode = s390_swap_bfp_rounding_mode(env, m3);
+
float32 ret = int64_to_float32(v2, &env->fpu_status);
-set_float_rounding_mode(hold, &env->fpu_status);
+s390_restore_bfp_rounding_mode(env, old_mode);
handle_exceptions(env, GETPC());
return ret;
}
@@ -414,9 +420,10 @@ uint64_t HELPER(cegb)(CPUS390XState *env, int64_t v2,
uint32_t m3)
/* convert 64-bit int to 64-bit float */
uint64_t HELPER(cdgb)(CPUS390XState *env, int64_t v2, uint32_t m3)
{
-int hold = swap_round_mode(env, m3);
+int old_mode = s390_swap_bfp_rounding_mode(env, m3);
+
float64 ret = int64_to_float64(v2, &env->fpu_status);
-set_float_rounding_mode(hold, &env->fpu_status);
+s390_restore_bfp_rounding_mode(env, old_mode);
handle_exceptions(env, GETPC());
return ret;
}
@@ -424,9 +431,10 @@ uint64_t HELPER(cdgb)(CPUS390XState *env, int64_t v2,
uint32_t m3)
/* convert 64-bit int to 128-bit float */
uint64_t HELPER(cxgb)(CPUS390XState *env, int64_t v2, uint32_t m3)
{
-int hold = swap_round_mode(env, m3);
+int old_mode = s390_swap_bfp_rounding_mode(env, m3);
+
float128 ret = int64_to_float128(v2, &env->fpu_status);
-set_float_rounding_mode(hold, &env->fpu_status);
+s390_restore_bfp_rounding_mode(env, old_mode);
handle_exceptions(env, GETPC());
return RET128(ret);
}
@@ -434,9 +442,10 @@ uint64_t HELPER(cxgb)(CPUS390XState *env, int64_t v2,
uint32_t m3)
/* convert 64-bit uint to 32-bit float */
uint64_t HELPER(celgb)(CPUS390XState *env, uint64_t v2, uint32_t m3)
{
-int hold = swap_round_mode(env, m3);
+int old_mode = s390_swap_bfp_rounding_mode(env, m3);
+
float32 ret = uint64_to_float32(v2, &env->fpu_status);
-set_float_rounding_mode(hold, &env->fpu_status);
+s390_restore_bfp_rounding_mode(env, old_mode);
handle_exceptions(env, GETPC());
return ret;
}
@@ -444,9 +453,10 @@ uint64_t HELPER(celgb)(CPUS390XState *env, uint64_t v2,
uint32_t m3)
/* convert 64-bit uint to 64-bit float */
uint64_t HELPER(cdlgb)(CPUS390XState *env, uint64_t v2, uint32_t m3)
{
-int hold = swap_round_mode(env, m3);
+int old_mode = s390_swap_bfp_rounding_mode(env, m3);
+
float64 ret = uint64_to_float64(v2, &env->fpu_status);
-set_float_rounding_mode(hold, &env->fpu_status);
+s390_restore_bfp_rounding_mode(env, old_mode);
handle_exceptions(env, GETPC());
return ret;
}
@@ -454,9 +464,10 @@ uint64_t HELPER(cdlgb)(CPUS390XState *env, uint64_t v2,
uint32_t m3)
/* convert 64-bit uint to 128-bit float */
uint64_t HELPER(cxlgb)(CPUS390XState *env, uint64_t v2, uint32_t m3)
{
-int hold = swap_round_mode(env, m3);
+int old_mode = s390_swap_bfp_rounding_mode(env, m3);
+
float128 ret = uint64_to_float128(v2, &env->fpu_status);
-set_float_rounding_mode(hold, &env->fpu_status);
+s390_restore_bfp_rounding_mode(env, old_mode);
handle_exceptions(env, GETPC());
return RET128(ret);
}
@@ -464,9 +475,10 @@ uint64_t HELPER(cxlgb)(CPUS390XState *env, uint64_t v2,
uint32_t m3)
/* convert 32-bit float to 64-bit int */
uint64_t HELPER(cgeb)(CPUS390XState *env, uint64_t v2, uint32_t m3)
{
-int hold = swap_round_mode(env, m3);
+int old_mode = s390_swap_bfp_rounding_mode(env, m3);
int64_t ret = float32_to_int64(v2, &env->fpu_status);
-set_float_rounding_mode(hold, &env->fpu_status);
+
+s390_restore_bfp_rounding_mode(env, old_mode);
handle_exceptions(env, GETPC());
return ret;
}
@@ -474,9 +486,10 @@ uint64_t HELPER(cgeb)(CPUS390XState *env, uint64_t v2,
uint32_t m3)
/* convert 64-bit float to 64-bit int */
uint64_t HELPER(cgdb)(CPUS390XState *env, uint64_t v2, uint32_t m3)
{
-int hold = swap_round_mode(env, m3);
+int old_
[Qemu-devel] [PATCH v1 04/15] s390x/tcg: Fix parts of IEEE exception handling
Many things are wrong and some parts cannot be fixed yet. Fix what we
can fix easily and add two FIXMEs:
The fpc flags are not updated in case an exception is actually injected.
Inexact exceptions have to be handled separately, as they are the only
exceptions that can coexist with underflows and overflows.
I reread the horribly complicated chapters in the PoP at least 5 times
and hope I got it right.
For references:
- z14 PoP, 9-18, "IEEE Exceptions"
- z14 PoP, 19-9, Figure 19-8
Signed-off-by: David Hildenbrand
---
target/s390x/fpu_helper.c | 38 --
1 file changed, 32 insertions(+), 6 deletions(-)
diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c
index 60f589608e..94680053f9 100644
--- a/target/s390x/fpu_helper.c
+++ b/target/s390x/fpu_helper.c
@@ -63,13 +63,39 @@ static void handle_exceptions(CPUS390XState *env, uintptr_t
retaddr)
env->fpu_status.float_exception_flags = 0;
s390_exc = s390_softfloat_exc_to_ieee(qemu_exc);
-/* Install the exceptions that we raised. */
-env->fpc |= s390_exc << 16;
+/*
+ * FIXME:
+ * 1. Right now, all inexact conditions are inidicated as
+ *"truncated" (0) and never as "incremented" (1) in the DXC.
+ * 2. Only traps due to invalid/divbyzero are suppressing. Other traps
+ *are completing, meaning the target register has to be written!
+ *This, however will mean that we have to write the register before
+ *triggering the trap - impossible right now.
+ */
+
+/*
+ * invalid/divbyzero cannot coexist with other conditions.
+ * overflow/underflow however can coexist with inexact, we have to
+ * handle it separatly.
+ */
+if (s390_exc & ~S390_IEEE_MASK_INEXACT) {
+if (s390_exc & ~S390_IEEE_MASK_INEXACT & env->fpc >> 24) {
+/* trap condition - inexact reported along */
+tcg_s390_data_exception(env, s390_exc, retaddr);
+}
+/* nontrap condition - inexact handled differently */
+env->fpc |= (s390_exc & ~S390_IEEE_MASK_INEXACT) << 16;
+}
-/* Send signals for enabled exceptions. */
-s390_exc &= env->fpc >> 24;
-if (s390_exc) {
-tcg_s390_data_exception(env, s390_exc, retaddr);
+/* inexact handling */
+if (s390_exc & S390_IEEE_MASK_INEXACT) {
+/* trap condition - overflow/underflow _not_ reported along */
+if (s390_exc & S390_IEEE_MASK_INEXACT & env->fpc >> 24) {
+tcg_s390_data_exception(env, s390_exc & S390_IEEE_MASK_INEXACT,
+retaddr);
+}
+/* nontrap condition */
+env->fpc |= (s390_exc & S390_IEEE_MASK_INEXACT) << 16;
}
}
--
2.17.2
[Qemu-devel] [PATCH v1 01/15] s390x/tcg: Fix TEST DATA CLASS instructions
Let's detect normal and denormal ("subnormal") numbers reliably. Also
test for quiet NaN's.
While at it, use a better check to test for the mask bits in the data
class mask. The data class mask has 12 bits, whereby bit 0 is the
leftmost bit and bit 11 the rightmost bit. In the PoP an easy to read
table with the numbers is provided for the VECTOR FP TEST DATA CLASS
IMMEDIATE instruction, the table for TEST DATA CLASS is more confusing
as it is based on 64 bit values.
Factor the checks out into separate functions, as they will also be
needed for floating point vector instructions.
Signed-off-by: David Hildenbrand
---
target/s390x/fpu_helper.c | 96 ++-
1 file changed, 65 insertions(+), 31 deletions(-)
diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c
index e921172bc4..a3214bd546 100644
--- a/target/s390x/fpu_helper.c
+++ b/target/s390x/fpu_helper.c
@@ -645,66 +645,100 @@ uint64_t HELPER(msdb)(CPUS390XState *env, uint64_t f1,
return ret;
}
+/*
+ * The rightmost 12 bits are the mask. The rightmost bit has the number 11.
+ */
+static inline bool test_dc_mask(uint16_t dc_mask, int bit, bool neg)
+{
+return dc_mask & (1 << (11 - bit - neg));
+}
+
+static bool s390_tdc32(CPUS390XState *env, float32 f1, uint16_t dc_mask)
+{
+const bool neg = float32_is_neg(f1);
+const bool zero = float32_is_zero(f1);
+const bool normal = float32_is_normal(f1);
+const bool denormal = float32_is_denormal(f1);
+const bool infinity = float32_is_infinity(f1);
+const bool quiet_nan = float32_is_quiet_nan(f1, &env->fpu_status);
+const bool sig_nan = float32_is_signaling_nan(f1, &env->fpu_status);
+
+return (zero && test_dc_mask(dc_mask, 0, neg)) ||
+ (normal && test_dc_mask(dc_mask, 2, neg)) ||
+ (denormal && test_dc_mask(dc_mask, 4, neg)) ||
+ (infinity && test_dc_mask(dc_mask, 6, neg)) ||
+ (quiet_nan && test_dc_mask(dc_mask, 8, neg)) ||
+ (sig_nan && test_dc_mask(dc_mask, 10, neg));
+}
+
/* test data class 32-bit */
uint32_t HELPER(tceb)(CPUS390XState *env, uint64_t f1, uint64_t m2)
{
-float32 v1 = f1;
-int neg = float32_is_neg(v1);
uint32_t cc = 0;
-if ((float32_is_zero(v1) && (m2 & (1 << (11-neg ||
-(float32_is_infinity(v1) && (m2 & (1 << (5-neg ||
-(float32_is_any_nan(v1) && (m2 & (1 << (3-neg ||
-(float32_is_signaling_nan(v1, &env->fpu_status) &&
- (m2 & (1 << (1-neg) {
-cc = 1;
-} else if (m2 & (1 << (9-neg))) {
-/* assume normalized number */
+if (s390_tdc32(env, f1, m2)) {
cc = 1;
}
-/* FIXME: denormalized? */
return cc;
}
+static bool s390_tdc64(CPUS390XState *env, float64 f1, uint16_t dc_mask)
+{
+const bool neg = float64_is_neg(f1);
+const bool zero = float64_is_zero(f1);
+const bool normal = float64_is_normal(f1);
+const bool denormal = float64_is_denormal(f1);
+const bool infinity = float64_is_infinity(f1);
+const bool quiet_nan = float64_is_quiet_nan(f1, &env->fpu_status);
+const bool sig_nan = float64_is_signaling_nan(f1, &env->fpu_status);
+
+return (zero && test_dc_mask(dc_mask, 0, neg)) ||
+ (normal && test_dc_mask(dc_mask, 2, neg)) ||
+ (denormal && test_dc_mask(dc_mask, 4, neg)) ||
+ (infinity && test_dc_mask(dc_mask, 6, neg)) ||
+ (quiet_nan && test_dc_mask(dc_mask, 8, neg)) ||
+ (sig_nan && test_dc_mask(dc_mask, 10, neg));
+}
+
/* test data class 64-bit */
uint32_t HELPER(tcdb)(CPUS390XState *env, uint64_t v1, uint64_t m2)
{
-int neg = float64_is_neg(v1);
uint32_t cc = 0;
-if ((float64_is_zero(v1) && (m2 & (1 << (11-neg ||
-(float64_is_infinity(v1) && (m2 & (1 << (5-neg ||
-(float64_is_any_nan(v1) && (m2 & (1 << (3-neg ||
-(float64_is_signaling_nan(v1, &env->fpu_status) &&
- (m2 & (1 << (1-neg) {
-cc = 1;
-} else if (m2 & (1 << (9-neg))) {
-/* assume normalized number */
+if (s390_tdc64(env, v1, m2)) {
cc = 1;
}
-/* FIXME: denormalized? */
return cc;
}
+static bool s390_tdc128(CPUS390XState *env, float128 f1, uint16_t dc_mask)
+{
+const bool neg = float128_is_neg(f1);
+const bool zero = float128_is_zero(f1);
+const bool normal = float128_is_normal(f1);
+const bool denormal = float128_is_denormal(f1);
+const bool infinity = float128_is_infinity(f1);
+const bool quiet_nan = float128_is_quiet_nan(f1, &env->fpu_status);
+const bool sig_nan = float128_is_signaling_nan(f1, &env->fpu_status);
+
+return (zero && test_dc_mask(dc_mask, 0, neg)) ||
+ (normal && test_dc_mask(dc_mask, 2, neg)) ||
+ (denormal && test_dc_mask(dc_mask, 4, neg)) ||
+ (infinity &&
[Qemu-devel] [PATCH v1 14/15] s390x/tcg: Handle all rounding modes overwritten by BFP instructions
PoP describes "Round to nearest with ties away from 0" as
"The candidate nearest to the input value is selected. In case of a tie,
the candidate selected is the one that is larger in magnitude."
While float_round_ties_away is according to the introducing commit
f9288a76f181 ("softfloat: Add support for ties-away rounding")
"roundTiesToAway: the floating-point number nearest to the infinitely
precise result shall be delivered; if the two nearest floating-point
numbers bracketing an unrepresentable infinitely precise result are
equally near, the one with larger magnitude shall be delivered."
So this could be it if we're lucky ;)
Handle "round to prepare for shorter precision" just as when setting it
via SET FLOATING POINT and friends.
As all instructions properly check for valid rounding modes in translate.c
we can add an assert. Fix one missing empty line.
Cc: Peter Maydell
Signed-off-by: David Hildenbrand
---
target/s390x/fpu_helper.c | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c
index 8ef3d2f0e4..748edcc0f0 100644
--- a/target/s390x/fpu_helper.c
+++ b/target/s390x/fpu_helper.c
@@ -385,14 +385,26 @@ uint32_t HELPER(cxb)(CPUS390XState *env, uint64_t ah,
uint64_t al,
int s390_swap_bfp_rounding_mode(CPUS390XState *env, int m3)
{
int ret = env->fpu_status.float_rounding_mode;
+
switch (m3) {
case 0:
/* current mode */
break;
case 1:
-/* biased round no nearest */
+/* round to nearest with ties away from 0 */
+set_float_rounding_mode(float_round_ties_away, &env->fpu_status);
+break;
+case 3:
+/*
+ * round to prepare for shorter precision
+ *
+ * FIXME: we actually want something like round_to_odd, but that
+ * does not support all data types yet.
+ */
+set_float_rounding_mode(float_round_to_zero, &env->fpu_status);
+break;
case 4:
-/* round to nearest */
+/* round to nearest with ties to even */
set_float_rounding_mode(float_round_nearest_even, &env->fpu_status);
break;
case 5:
@@ -407,6 +419,8 @@ int s390_swap_bfp_rounding_mode(CPUS390XState *env, int m3)
/* round to -inf */
set_float_rounding_mode(float_round_down, &env->fpu_status);
break;
+default:
+g_assert_not_reached();
}
return ret;
}
--
2.17.2
[Qemu-devel] [PATCH v1 03/15] s390x/tcg: Factor out conversion of softfloat exceptions
We want to reuse that function in vector instruction context. While at it,
cleanup the code, using defines for magic values and avoiding the
handcrafted bit conversion.
Signed-off-by: David Hildenbrand
---
target/s390x/fpu_helper.c | 25 +
target/s390x/internal.h | 7 +++
2 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c
index f9ec519067..60f589608e 100644
--- a/target/s390x/fpu_helper.c
+++ b/target/s390x/fpu_helper.c
@@ -36,10 +36,18 @@
#define RET128(F) (env->retxl = F.low, F.high)
-#define convert_bit(mask, from, to) \
-(to < from \
- ? (mask / (from / to)) & to\
- : (mask & from) * (to / from))
+uint8_t s390_softfloat_exc_to_ieee(unsigned int exc)
+{
+uint8_t s390_exc = 0;
+
+s390_exc |= (exc & float_flag_invalid) ? S390_IEEE_MASK_INVALID : 0;
+s390_exc |= (exc & float_flag_divbyzero) ? S390_IEEE_MASK_DIVBYZERO : 0;
+s390_exc |= (exc & float_flag_overflow) ? S390_IEEE_MASK_OVERFLOW : 0;
+s390_exc |= (exc & float_flag_underflow) ? S390_IEEE_MASK_UNDERFLOW : 0;
+s390_exc |= (exc & float_flag_inexact) ? S390_IEEE_MASK_INEXACT : 0;
+
+return s390_exc;
+}
/* Should be called after any operation that may raise IEEE exceptions. */
static void handle_exceptions(CPUS390XState *env, uintptr_t retaddr)
@@ -53,14 +61,7 @@ static void handle_exceptions(CPUS390XState *env, uintptr_t
retaddr)
return;
}
env->fpu_status.float_exception_flags = 0;
-
-/* Convert softfloat exception bits to s390 exception bits. */
-s390_exc = 0;
-s390_exc |= convert_bit(qemu_exc, float_flag_invalid, 0x80);
-s390_exc |= convert_bit(qemu_exc, float_flag_divbyzero, 0x40);
-s390_exc |= convert_bit(qemu_exc, float_flag_overflow, 0x20);
-s390_exc |= convert_bit(qemu_exc, float_flag_underflow, 0x10);
-s390_exc |= convert_bit(qemu_exc, float_flag_inexact, 0x08);
+s390_exc = s390_softfloat_exc_to_ieee(qemu_exc);
/* Install the exceptions that we raised. */
env->fpc |= s390_exc << 16;
diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index f2a771e2b4..e1c0b1bd3b 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -308,6 +308,13 @@ void s390x_cpu_do_unaligned_access(CPUState *cs, vaddr
addr,
uint32_t set_cc_nz_f32(float32 v);
uint32_t set_cc_nz_f64(float64 v);
uint32_t set_cc_nz_f128(float128 v);
+#define S390_IEEE_MASK_INVALID 0x80
+#define S390_IEEE_MASK_DIVBYZERO 0x40
+#define S390_IEEE_MASK_OVERFLOW 0x20
+#define S390_IEEE_MASK_UNDERFLOW 0x10
+#define S390_IEEE_MASK_INEXACT 0x08
+#define S390_IEEE_MASK_QUANTUM 0x04
+uint8_t s390_softfloat_exc_to_ieee(unsigned int exc);
/* gdbstub.c */
--
2.17.2
[Qemu-devel] [PATCH v1 12/15] s390x/tcg: Implement XxC and checks for most FP instructions
With the floating-point extension facility
- CONVERT FROM LOGICAL
- CONVERT TO LOGICAL
- CONVERT TO FIXED
- CONVERT FROM FIXED
- LOAD FP INTEGER
have both, a rounding mode specification and the inexact-exception control
(XxC). Other instructions will be handled separatly.
Check for valid rounding modes and forward also the XxC. To avoid a lot
of boilerplate code and changes to the helpers, combine both, the m3 and
m4 field in a combined 32 bit TCG variable. Mangle/unmangle on a 8bit
basis, so this can be reused for other fields later on.
Signed-off-by: David Hildenbrand
---
target/s390x/fpu_helper.c | 126 +--
target/s390x/internal.h | 6 +
target/s390x/translate.c | 254 --
3 files changed, 260 insertions(+), 126 deletions(-)
diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c
index 5136147da6..a0501b9ee5 100644
--- a/target/s390x/fpu_helper.c
+++ b/target/s390x/fpu_helper.c
@@ -407,239 +407,239 @@ void s390_restore_bfp_rounding_mode(CPUS390XState *env,
int old_mode)
}
/* convert 64-bit int to 32-bit float */
-uint64_t HELPER(cegb)(CPUS390XState *env, int64_t v2, uint32_t m3)
+uint64_t HELPER(cegb)(CPUS390XState *env, int64_t v2, uint32_t m)
{
-int old_mode = s390_swap_bfp_rounding_mode(env, m3);
+int old_mode = s390_swap_bfp_rounding_mode(env, m & 0xf);
float32 ret = int64_to_float32(v2, &env->fpu_status);
s390_restore_bfp_rounding_mode(env, old_mode);
-handle_exceptions(env, false, GETPC());
+handle_exceptions(env, (m >> 8) & 1, GETPC());
return ret;
}
/* convert 64-bit int to 64-bit float */
-uint64_t HELPER(cdgb)(CPUS390XState *env, int64_t v2, uint32_t m3)
+uint64_t HELPER(cdgb)(CPUS390XState *env, int64_t v2, uint32_t m)
{
-int old_mode = s390_swap_bfp_rounding_mode(env, m3);
+int old_mode = s390_swap_bfp_rounding_mode(env, m & 0xf);
float64 ret = int64_to_float64(v2, &env->fpu_status);
s390_restore_bfp_rounding_mode(env, old_mode);
-handle_exceptions(env, false, GETPC());
+handle_exceptions(env, (m >> 8) & 1, GETPC());
return ret;
}
/* convert 64-bit int to 128-bit float */
-uint64_t HELPER(cxgb)(CPUS390XState *env, int64_t v2, uint32_t m3)
+uint64_t HELPER(cxgb)(CPUS390XState *env, int64_t v2, uint32_t m)
{
-int old_mode = s390_swap_bfp_rounding_mode(env, m3);
+int old_mode = s390_swap_bfp_rounding_mode(env, m & 0xf);
float128 ret = int64_to_float128(v2, &env->fpu_status);
s390_restore_bfp_rounding_mode(env, old_mode);
-handle_exceptions(env, false, GETPC());
+handle_exceptions(env, (m >> 8) & 1, GETPC());
return RET128(ret);
}
/* convert 64-bit uint to 32-bit float */
-uint64_t HELPER(celgb)(CPUS390XState *env, uint64_t v2, uint32_t m3)
+uint64_t HELPER(celgb)(CPUS390XState *env, uint64_t v2, uint32_t m)
{
-int old_mode = s390_swap_bfp_rounding_mode(env, m3);
+int old_mode = s390_swap_bfp_rounding_mode(env, m & 0xf);
float32 ret = uint64_to_float32(v2, &env->fpu_status);
s390_restore_bfp_rounding_mode(env, old_mode);
-handle_exceptions(env, false, GETPC());
+handle_exceptions(env, (m >> 8) & 1, GETPC());
return ret;
}
/* convert 64-bit uint to 64-bit float */
-uint64_t HELPER(cdlgb)(CPUS390XState *env, uint64_t v2, uint32_t m3)
+uint64_t HELPER(cdlgb)(CPUS390XState *env, uint64_t v2, uint32_t m)
{
-int old_mode = s390_swap_bfp_rounding_mode(env, m3);
+int old_mode = s390_swap_bfp_rounding_mode(env, m & 0xf);
float64 ret = uint64_to_float64(v2, &env->fpu_status);
s390_restore_bfp_rounding_mode(env, old_mode);
-handle_exceptions(env, false, GETPC());
+handle_exceptions(env, (m >> 8) & 1, GETPC());
return ret;
}
/* convert 64-bit uint to 128-bit float */
-uint64_t HELPER(cxlgb)(CPUS390XState *env, uint64_t v2, uint32_t m3)
+uint64_t HELPER(cxlgb)(CPUS390XState *env, uint64_t v2, uint32_t m)
{
-int old_mode = s390_swap_bfp_rounding_mode(env, m3);
+int old_mode = s390_swap_bfp_rounding_mode(env, m & 0xf);
float128 ret = uint64_to_float128(v2, &env->fpu_status);
s390_restore_bfp_rounding_mode(env, old_mode);
-handle_exceptions(env, false, GETPC());
+handle_exceptions(env, (m >> 8) & 1, GETPC());
return RET128(ret);
}
/* convert 32-bit float to 64-bit int */
-uint64_t HELPER(cgeb)(CPUS390XState *env, uint64_t v2, uint32_t m3)
+uint64_t HELPER(cgeb)(CPUS390XState *env, uint64_t v2, uint32_t m)
{
-int old_mode = s390_swap_bfp_rounding_mode(env, m3);
+int old_mode = s390_swap_bfp_rounding_mode(env, m & 0xf);
int64_t ret = float32_to_int64(v2, &env->fpu_status);
s390_restore_bfp_rounding_mode(env, old_mode);
-handle_exceptions(env, false, GETPC());
+handle_exceptions(env, (m >> 8) & 1, GETPC());
return ret;
}
/* convert 64-bit float to 64-bit int */
-uint64_t HELPER(cgdb)(CPUS390XState *env, uint64_t v2, uint32_t m3)
+uint64_t HELPER(cgdb)(CPUS390XState *env, uint64
[Qemu-devel] [PATCH v1 09/15] s390x/tcg: Check for exceptions in SET BFP ROUNDING MODE
Let's split handling of BFP/DFP rounding mode configuration. Also,
let's not reuse the sfpc handler, use a separate handler so we can
properly check for specification exceptions for SRNMB.
Signed-off-by: David Hildenbrand
---
target/s390x/fpu_helper.c | 11
target/s390x/helper.h | 1 +
target/s390x/insn-data.def | 6 ++--
target/s390x/translate.c | 56 --
4 files changed, 39 insertions(+), 35 deletions(-)
diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c
index a578f849ad..a6c67527cd 100644
--- a/target/s390x/fpu_helper.c
+++ b/target/s390x/fpu_helper.c
@@ -870,3 +870,14 @@ void HELPER(sfas)(CPUS390XState *env, uint64_t fpc)
tcg_s390_data_exception(env, s390_exc | 3, GETPC());
}
}
+
+/* set bfp rounding mode */
+void HELPER(srnm)(CPUS390XState *env, uint64_t rnd)
+{
+if (rnd > 0x7 || fpc_to_rnd[rnd & 0x7] == -1) {
+s390_program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO, GETPC());
+}
+
+env->fpc = deposit32(env->fpc, 0, 3, rnd);
+set_float_rounding_mode(fpc_to_rnd[rnd & 0x7], &env->fpu_status);
+}
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 974f6bac2a..179c942fc8 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -106,6 +106,7 @@ DEF_HELPER_4(cksm, i64, env, i64, i64, i64)
DEF_HELPER_FLAGS_5(calc_cc, TCG_CALL_NO_RWG_SE, i32, env, i32, i64, i64, i64)
DEF_HELPER_FLAGS_2(sfpc, TCG_CALL_NO_WG, void, env, i64)
DEF_HELPER_FLAGS_2(sfas, TCG_CALL_NO_WG, void, env, i64)
+DEF_HELPER_FLAGS_2(srnm, TCG_CALL_NO_WG, void, env, i64)
DEF_HELPER_FLAGS_1(popcnt, TCG_CALL_NO_RWG_SE, i64, i64)
DEF_HELPER_2(stfle, i32, env, i64)
DEF_HELPER_FLAGS_2(lpq, TCG_CALL_NO_WG, i64, env, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index dab805fd90..7ef230029e 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -759,10 +759,10 @@
/* SET FPC AND SIGNAL */
F(0xb385, SFASR, RRE, I_SIM, 0, r1_o, 0, 0, sfas, 0, IF_DFP)
/* SET BFP ROUNDING MODE */
-F(0xb299, SRNM,S, Z, 0, 0, 0, 0, srnm, 0, IF_BFP)
-F(0xb2b8, SRNMB, S, FPE, 0, 0, 0, 0, srnm, 0, IF_BFP)
+F(0xb299, SRNM,S, Z, la2, 0, 0, 0, srnm, 0, IF_BFP)
+F(0xb2b8, SRNMB, S, FPE, la2, 0, 0, 0, srnmb, 0, IF_BFP)
/* SET DFP ROUNDING MODE */
-F(0xb2b9, SRNMT, S, DFPR, 0, 0, 0, 0, srnm, 0, IF_DFP)
+F(0xb2b9, SRNMT, S, DFPR, la2, 0, 0, 0, srnmt, 0, IF_DFP)
/* SET PROGRAM MASK */
C(0x0400, SPM, RR_a, Z, r1, 0, 0, 0, spm, 0)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 639084af07..f58155f20c 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3955,41 +3955,33 @@ static DisasJumpType op_sfas(DisasContext *s, DisasOps
*o)
static DisasJumpType op_srnm(DisasContext *s, DisasOps *o)
{
-int b2 = get_field(s->fields, b2);
-int d2 = get_field(s->fields, d2);
-TCGv_i64 t1 = tcg_temp_new_i64();
-TCGv_i64 t2 = tcg_temp_new_i64();
-int mask, pos, len;
+/* Bits other than 62 and 63 are ignored. Bit 29 is set to zero. */
+tcg_gen_andi_i64(o->addr1, o->addr1, 0x3ull);
+gen_helper_srnm(cpu_env, o->addr1);
+return DISAS_NEXT;
+}
-switch (s->fields->op2) {
-case 0x99: /* SRNM */
-pos = 0, len = 2;
-break;
-case 0xb8: /* SRNMB */
-pos = 0, len = 3;
-break;
-case 0xb9: /* SRNMT */
-pos = 4, len = 3;
-break;
-default:
-tcg_abort();
-}
-mask = (1 << len) - 1;
+static DisasJumpType op_srnmb(DisasContext *s, DisasOps *o)
+{
+/* Bits 0-55 are are ignored. */
+tcg_gen_andi_i64(o->addr1, o->addr1, 0xffull);
+gen_helper_srnm(cpu_env, o->addr1);
+return DISAS_NEXT;
+}
-/* Insert the value into the appropriate field of the FPC. */
-if (b2 == 0) {
-tcg_gen_movi_i64(t1, d2 & mask);
-} else {
-tcg_gen_addi_i64(t1, regs[b2], d2);
-tcg_gen_andi_i64(t1, t1, mask);
-}
-tcg_gen_ld32u_i64(t2, cpu_env, offsetof(CPUS390XState, fpc));
-tcg_gen_deposit_i64(t2, t2, t1, pos, len);
-tcg_temp_free_i64(t1);
+static DisasJumpType op_srnmt(DisasContext *s, DisasOps *o)
+{
+TCGv_i64 tmp = tcg_temp_new_i64();
-/* Then install the new FPC to set the rounding mode in fpu_status. */
-gen_helper_sfpc(cpu_env, t2);
-tcg_temp_free_i64(t2);
+/* Bits other than 61-63 are ignored. */
+tcg_gen_andi_i64(o->addr1, o->addr1, 0x7ull);
+
+/* No need to call a helper, we don't implement dfp */
+tcg_gen_ld32u_i64(tmp, cpu_env, offsetof(CPUS390XState, fpc));
+tcg_gen_deposit_i64(tmp, tmp, o->addr1, 4, 3);
+tcg_gen_st32_i64(tmp, cpu_env, offsetof(CPUS390XState, fpc));
+
+tcg_temp_free_i64(tmp);
return DISAS_NEXT;
}
--
2.17.2
[Qemu-devel] [PATCH v1 15/15] s390x: Add floating-point extension facility to "qemu" cpu model
The floating-point extension facility implemented certain changes to
BFP, HFP and DFP instructions.
As we don't implement HFP/DFP, we can ignore those completely. Related
to BFP, the changes include
- SET BFP ROUNDING MODE (SRNMB) instruction
- BFP-rounding-mode field in the FPC register is changed to 3 bits
- CONVERT FROM LOGICAL instructions
- CONVERT TO LOGICAL instructions
- Changes (rounding mode + XxC) added to
-- CONVERT TO FIXED
-- CONVERT FROM FIXED
-- LOAD FP INTEGER
-- LOAD ROUNDED
-- DIVIDE TO INTEGER
For TCG, we don't implement DIVIDE TO INTEGER, and it is harder to
implement, so skip that. Also, as we don't implement PFPO, we can skip
changes to that as well. The other parts are now implemented, we can
indicate the facility.
z14 PoP mentiones that "The floating-point extension facility is installed
in the z/Architecture architectural mode. When bit 37 is one, bit 42 is
also one.", meaning that the DFP (decimal-floating-point) facility also
has to be inidicated. We can ignore that for now.
Signed-off-by: David Hildenbrand
---
target/s390x/gen-features.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 19bb8ac7f8..02f6e9f475 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -593,6 +593,11 @@ static uint16_t qemu_V3_1[] = {
};
static uint16_t qemu_LATEST[] = {
+/*
+ * Only BFP bits are implemented (HFP, DFP, PFPO and DIVIDE TO INTEGER not
+ * implemented yet).
+ */
+S390_FEAT_FLOATING_POINT_EXT,
S390_FEAT_ZPCI,
};
--
2.17.2
[Qemu-devel] [PATCH v1 11/15] s390x/tcg: Prepare for IEEE-inexact-exception control (XxC)
Some instructions allow to suppress IEEE inexact exceptions.
z14 PoP, 9-23, "Suppression of Certain IEEE Exceptions"
IEEE-inexact-exception control (XxC): Bit 1 of
the M4 field is the XxC bit. If XxC is zero, recogni-
tion of IEEE-inexact exception is not suppressed;
if XxC is one, recognition of IEEE-inexact excep-
tion is suppressed.
Especially, handling for overflow/unerflow remains as is, inexact is
reported along
z14 PoP, 9-23, "Suppression of Certain IEEE Exceptions"
For example, the IEEE-inexact-exception control (XxC)
has no effect on the DXC; that is, the DXC for IEEE-
overflow or IEEE-underflow exceptions along with the
detail for exact, inexact and truncated, or inexact and
incremented, is reported according to the actual con-
dition.
Follow up patches will wire it correctly up for the applicable
instructions.
Signed-off-by: David Hildenbrand
---
target/s390x/fpu_helper.c | 112 +++---
1 file changed, 56 insertions(+), 56 deletions(-)
diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c
index 518fb6365a..5136147da6 100644
--- a/target/s390x/fpu_helper.c
+++ b/target/s390x/fpu_helper.c
@@ -50,7 +50,7 @@ uint8_t s390_softfloat_exc_to_ieee(unsigned int exc)
}
/* Should be called after any operation that may raise IEEE exceptions. */
-static void handle_exceptions(CPUS390XState *env, uintptr_t retaddr)
+static void handle_exceptions(CPUS390XState *env, bool XxC, uintptr_t retaddr)
{
unsigned s390_exc, qemu_exc;
@@ -101,7 +101,7 @@ static void handle_exceptions(CPUS390XState *env, uintptr_t
retaddr)
}
/* inexact handling */
-if (s390_exc & S390_IEEE_MASK_INEXACT) {
+if (s390_exc & S390_IEEE_MASK_INEXACT && !XxC) {
/* trap condition - overflow/underflow _not_ reported along */
if (s390_exc & S390_IEEE_MASK_INEXACT & env->fpc >> 24) {
tcg_s390_data_exception(env, s390_exc & S390_IEEE_MASK_INEXACT,
@@ -174,7 +174,7 @@ uint32_t set_cc_nz_f128(float128 v)
uint64_t HELPER(aeb)(CPUS390XState *env, uint64_t f1, uint64_t f2)
{
float32 ret = float32_add(f1, f2, &env->fpu_status);
-handle_exceptions(env, GETPC());
+handle_exceptions(env, false, GETPC());
return ret;
}
@@ -182,7 +182,7 @@ uint64_t HELPER(aeb)(CPUS390XState *env, uint64_t f1,
uint64_t f2)
uint64_t HELPER(adb)(CPUS390XState *env, uint64_t f1, uint64_t f2)
{
float64 ret = float64_add(f1, f2, &env->fpu_status);
-handle_exceptions(env, GETPC());
+handle_exceptions(env, false, GETPC());
return ret;
}
@@ -193,7 +193,7 @@ uint64_t HELPER(axb)(CPUS390XState *env, uint64_t ah,
uint64_t al,
float128 ret = float128_add(make_float128(ah, al),
make_float128(bh, bl),
&env->fpu_status);
-handle_exceptions(env, GETPC());
+handle_exceptions(env, false, GETPC());
return RET128(ret);
}
@@ -201,7 +201,7 @@ uint64_t HELPER(axb)(CPUS390XState *env, uint64_t ah,
uint64_t al,
uint64_t HELPER(seb)(CPUS390XState *env, uint64_t f1, uint64_t f2)
{
float32 ret = float32_sub(f1, f2, &env->fpu_status);
-handle_exceptions(env, GETPC());
+handle_exceptions(env, false, GETPC());
return ret;
}
@@ -209,7 +209,7 @@ uint64_t HELPER(seb)(CPUS390XState *env, uint64_t f1,
uint64_t f2)
uint64_t HELPER(sdb)(CPUS390XState *env, uint64_t f1, uint64_t f2)
{
float64 ret = float64_sub(f1, f2, &env->fpu_status);
-handle_exceptions(env, GETPC());
+handle_exceptions(env, false, GETPC());
return ret;
}
@@ -220,7 +220,7 @@ uint64_t HELPER(sxb)(CPUS390XState *env, uint64_t ah,
uint64_t al,
float128 ret = float128_sub(make_float128(ah, al),
make_float128(bh, bl),
&env->fpu_status);
-handle_exceptions(env, GETPC());
+handle_exceptions(env, false, GETPC());
return RET128(ret);
}
@@ -228,7 +228,7 @@ uint64_t HELPER(sxb)(CPUS390XState *env, uint64_t ah,
uint64_t al,
uint64_t HELPER(deb)(CPUS390XState *env, uint64_t f1, uint64_t f2)
{
float32 ret = float32_div(f1, f2, &env->fpu_status);
-handle_exceptions(env, GETPC());
+handle_exceptions(env, false, GETPC());
return ret;
}
@@ -236,7 +236,7 @@ uint64_t HELPER(deb)(CPUS390XState *env, uint64_t f1,
uint64_t f2)
uint64_t HELPER(ddb)(CPUS390XState *env, uint64_t f1, uint64_t f2)
{
float64 ret = float64_div(f1, f2, &env->fpu_status);
-handle_exceptions(env, GETPC());
+handle_exceptions(env, false, GETPC());
return ret;
}
@@ -247,7 +247,7 @@ uint64_t HELPER(dxb)(CPUS390XState *env, uint64_t ah,
uint64_t al,
float128 ret = float128_div(make_float128(ah, al),
make_float128(bh, bl),
&env->fpu_status);
-handle_exceptions(env, GETPC());
+handle_exceptions(env, false, GETPC());
return RET128(ret);
}
@@ -255,
[Qemu-devel] [PATCH v1 08/15] s390x/tcg: Handle SET FPC AND LOAD FPC 3-bit BFP rounding modes
We already forward the 3 bits correctly in the translation functions. We
also have to handle them properly and check for specification
exceptions.
Setting an invalid rounding mode (BFP only, all DFP rounding modes)
results in a specification exception. Setting unassigned bits in the
fpc, results in a specification exception.
This fixes LOAD FPC (AND SIGNAL), SET FPC (AND SIGNAL). Also for,
SET BFP ROUNDING MODE, 3-bit rounding mode is now explicitly checked.
Notes:
1. Use "float_round_to_zero" for now to handle "Round to prepare for
shorter precision". Looking at the PoP "Summary of Rounding and Range
Actions" for BFP. They differ when it comes to tiny values.
2. TCG_CALL_NO_WG is required for sfpc handler, as we now inject
exceptions.
We won't be modeling abscence of the "floating-point extension facility"
for now, not necessary as most take the facility for granted without
checking.
z14 PoP, 9-23, "LOAD FPC"
When the floating-point extension facility is
installed, bits 29-31 of the second operand must
specify a valid BFP rounding mode and bits 6-7,
14-15, 24, and 28 must be zero; otherwise, a
specification exception is recognized.
Signed-off-by: David Hildenbrand
---
target/s390x/fpu_helper.c | 24
target/s390x/helper.h | 2 +-
2 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c
index 15ede530d8..a578f849ad 100644
--- a/target/s390x/fpu_helper.c
+++ b/target/s390x/fpu_helper.c
@@ -802,21 +802,33 @@ uint64_t HELPER(sqxb)(CPUS390XState *env, uint64_t ah,
uint64_t al)
return RET128(ret);
}
-static const int fpc_to_rnd[4] = {
+static const int fpc_to_rnd[8] = {
float_round_nearest_even,
float_round_to_zero,
float_round_up,
-float_round_down
+float_round_down,
+-1,
+-1,
+-1,
+/*
+ * FIXME: we actually want something like round_to_odd, but that does not
+ * support all data types yet.
+ */
+float_round_to_zero,
};
/* set fpc */
void HELPER(sfpc)(CPUS390XState *env, uint64_t fpc)
{
+if (fpc_to_rnd[fpc & 0x7] == -1 || fpc & 0x03030088u) {
+s390_program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO, GETPC());
+}
+
/* Install everything in the main FPC. */
env->fpc = fpc;
/* Install the rounding mode in the shadow fpu_status. */
-set_float_rounding_mode(fpc_to_rnd[fpc & 3], &env->fpu_status);
+set_float_rounding_mode(fpc_to_rnd[fpc & 0x7], &env->fpu_status);
}
/* set fpc and signal */
@@ -825,12 +837,16 @@ void HELPER(sfas)(CPUS390XState *env, uint64_t fpc)
uint32_t signalling = env->fpc;
uint32_t s390_exc;
+if (fpc_to_rnd[fpc & 0x7] == -1 || fpc & 0x03030088u) {
+s390_program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO, GETPC());
+}
+
/*
* FPC is set to the FPC operand with a bitwise OR of the signalling
* flags.
*/
env->fpc = fpc | (signalling & 0x00ff);
-set_float_rounding_mode(fpc_to_rnd[fpc & 3], &env->fpu_status);
+set_float_rounding_mode(fpc_to_rnd[fpc & 0x7], &env->fpu_status);
/*
* If any signaling flag is enabled in the new FPC mask, a
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 018e9dd414..974f6bac2a 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -104,7 +104,7 @@ DEF_HELPER_4(trtr, i32, env, i32, i64, i64)
DEF_HELPER_5(trXX, i32, env, i32, i32, i32, i32)
DEF_HELPER_4(cksm, i64, env, i64, i64, i64)
DEF_HELPER_FLAGS_5(calc_cc, TCG_CALL_NO_RWG_SE, i32, env, i32, i64, i64, i64)
-DEF_HELPER_FLAGS_2(sfpc, TCG_CALL_NO_RWG, void, env, i64)
+DEF_HELPER_FLAGS_2(sfpc, TCG_CALL_NO_WG, void, env, i64)
DEF_HELPER_FLAGS_2(sfas, TCG_CALL_NO_WG, void, env, i64)
DEF_HELPER_FLAGS_1(popcnt, TCG_CALL_NO_RWG_SE, i64, i64)
DEF_HELPER_2(stfle, i32, env, i64)
--
2.17.2
Re: [Qemu-devel] [PATCH] hw/arm/armsse: Fix miswiring of expansion IRQs
On Tue, 12 Feb 2019 at 11:05, Philippe Mathieu-Daudé wrote:
>
> On 2/12/19 11:52 AM, Peter Maydell wrote:
> > In commit 91c1e9fcbd7548db368 where we added dual-CPU support to
> > the ARMSSE, we set up the wiring of the expansion IRQs via nested
> > loops: the outer loop on 'i' loops for each CPU, and the inner loop
> > on 'j' loops for each interrupt. Fix a typo which meant we were
> > wiring every expansion IRQ line to external IRQ 0 on CPU 0 and
> > to external IRQ 1 on CPU 1.
> >
> > Fixes: 91c1e9fcbd7548db368 ("hw/arm/armsse: Support dual-CPU configuration")
> >
> > Signed-off-by: Peter Maydell
>
> Reviewed-by: Philippe Mathieu-Daudé
>
> > ---
> > It turns out that the ARM-TFM image I was using to test that
> > I hadn't broken the mps2-an505 doesn't actually rely on any
> > interrupts from the external devices...
>
> How did you notice that, simply reviewing? Via 'info qtree'?
I'm working on a model of a different board (Musca) which also
uses the SSE-200, and the test code I had for that does happen
to care about the interrupts.
thanks
-- PMM
[Qemu-devel] [PATCH v1 05/15] s390x/tcg: Hide IEEE underflows in some scenarios
IEEE underflows are not reported when the mask bit is off and we don't
also have an inexact exception.
z14 PoP, 9-20, "IEEE Underflow":
An IEEE-underflow exception is recognized for an
IEEE target when the tininess condition exists and
either: (1) the IEEE-underflow mask bit in the FPC
register is zero and the result value is inexact, or (2)
the IEEE-underflow mask bit in the FPC register is
one.
Signed-off-by: David Hildenbrand
---
target/s390x/fpu_helper.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c
index 94680053f9..2e690073ea 100644
--- a/target/s390x/fpu_helper.c
+++ b/target/s390x/fpu_helper.c
@@ -63,6 +63,19 @@ static void handle_exceptions(CPUS390XState *env, uintptr_t
retaddr)
env->fpu_status.float_exception_flags = 0;
s390_exc = s390_softfloat_exc_to_ieee(qemu_exc);
+/*
+ * IEEE-Underflow exception recognition exists if a tininess condition
+ * (underflow) exists and
+ * - The mask bit in the FPC is zero and the result is inexact
+ * - The mask bit in the FPC is one
+ * So tininess conditions that are not inexact don't trigger any
+ * underflow action in case the mask bit is not one.
+ */
+if (!(s390_exc & S390_IEEE_MASK_INEXACT) &&
+!((env->fpc >> 24) & S390_IEEE_MASK_UNDERFLOW)) {
+s390_exc &= ~S390_IEEE_MASK_UNDERFLOW;
+}
+
/*
* FIXME:
* 1. Right now, all inexact conditions are inidicated as
--
2.17.2
Re: [Qemu-devel] [PATCH] hw/arm/armsse: Fix miswiring of expansion IRQs
On 2/12/19 11:52 AM, Peter Maydell wrote:
> In commit 91c1e9fcbd7548db368 where we added dual-CPU support to
> the ARMSSE, we set up the wiring of the expansion IRQs via nested
> loops: the outer loop on 'i' loops for each CPU, and the inner loop
> on 'j' loops for each interrupt. Fix a typo which meant we were
> wiring every expansion IRQ line to external IRQ 0 on CPU 0 and
> to external IRQ 1 on CPU 1.
>
> Fixes: 91c1e9fcbd7548db368 ("hw/arm/armsse: Support dual-CPU configuration")
>
> Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
> ---
> It turns out that the ARM-TFM image I was using to test that
> I hadn't broken the mps2-an505 doesn't actually rely on any
> interrupts from the external devices...
How did you notice that, simply reviewing? Via 'info qtree'?
>
> hw/arm/armsse.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
> index 5d53071a5a0..9a8c49547db 100644
> --- a/hw/arm/armsse.c
> +++ b/hw/arm/armsse.c
> @@ -565,7 +565,7 @@ static void armsse_realize(DeviceState *dev, Error **errp)
> /* Connect EXP_IRQ/EXP_CPUn_IRQ GPIOs to the NVIC's lines 32 and up
> */
> s->exp_irqs[i] = g_new(qemu_irq, s->exp_numirq);
> for (j = 0; j < s->exp_numirq; j++) {
> -s->exp_irqs[i][j] = qdev_get_gpio_in(cpudev, i + 32);
> +s->exp_irqs[i][j] = qdev_get_gpio_in(cpudev, j + 32);
> }
> if (i == 0) {
> gpioname = g_strdup("EXP_IRQ");
>
[Qemu-devel] [PATCH v1 07/15] s390x/tcg: Fix simulated-IEEE exceptions
The trap is triggered based on priority of the enabled signaling flags.
Only overflow and underflow allow a concurrent inexact exception.
z14 PoP, 9-33, Figure 9-21
Signed-off-by: David Hildenbrand
---
target/s390x/fpu_helper.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c
index 7c39829352..15ede530d8 100644
--- a/target/s390x/fpu_helper.c
+++ b/target/s390x/fpu_helper.c
@@ -838,6 +838,19 @@ void HELPER(sfas)(CPUS390XState *env, uint64_t fpc)
*/
s390_exc = (signalling >> 16) & (fpc >> 24);
if (s390_exc) {
+if (s390_exc & S390_IEEE_MASK_INVALID) {
+s390_exc = S390_IEEE_MASK_INVALID;
+} else if (s390_exc & S390_IEEE_MASK_DIVBYZERO) {
+s390_exc = S390_IEEE_MASK_DIVBYZERO;
+} else if (s390_exc & S390_IEEE_MASK_OVERFLOW) {
+s390_exc &= (S390_IEEE_MASK_OVERFLOW | S390_IEEE_MASK_INEXACT);
+} else if (s390_exc & S390_IEEE_MASK_UNDERFLOW) {
+s390_exc &= (S390_IEEE_MASK_UNDERFLOW | S390_IEEE_MASK_INEXACT);
+} else if (s390_exc & S390_IEEE_MASK_INEXACT) {
+s390_exc = S390_IEEE_MASK_INEXACT;
+} else if (s390_exc & S390_IEEE_MASK_QUANTUM) {
+s390_exc = S390_IEEE_MASK_QUANTUM;
+}
tcg_s390_data_exception(env, s390_exc | 3, GETPC());
}
}
--
2.17.2
Re: [Qemu-devel] [PATCH v1] s390x: add zPCI feature to "qemu" CPU model
On Tue, 12 Feb 2019 10:30:23 +0100
David Hildenbrand wrote:
> As we now always have PCI support, let's add it to the "qemu" CPU model,
> taking care of backwards compatibility.
>
> Signed-off-by: David Hildenbrand
> ---
> hw/s390x/s390-virtio-ccw.c | 3 +++
> target/s390x/gen-features.c | 8 ++--
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 3fb5c88af9..7f0d394fbe 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -657,7 +657,10 @@ DEFINE_CCW_MACHINE(4_0, "4.0", true);
>
> static void ccw_machine_3_1_instance_options(MachineState *machine)
> {
> +static const S390FeatInit qemu_cpu_feat = { S390_FEAT_LIST_QEMU_V3_1 };
> ccw_machine_4_0_instance_options(machine);
> +
> +s390_set_qemu_cpu_model(0x2064, 7, 1, qemu_cpu_feat);
Hm, don't we emulate a (stripped-down) z12 (2827)? Or am I
misunderstanding?
> }
>
> static void ccw_machine_3_1_class_options(MachineClass *mc)
Re: [Qemu-devel] [PATCH v2] Kconfig: add documentation
On 12/02/2019 09:57, Paolo Bonzini wrote: Signed-off-by: Paolo Bonzini Reviewed-by: Liam Merwick --- docs/devel/kconfig.rst | 305 + 1 file changed, 305 insertions(+) create mode 100644 docs/devel/kconfig.rst diff --git a/docs/devel/kconfig.rst b/docs/devel/kconfig.rst new file mode 100644 index 00..ff1ed3d1b2 --- /dev/null +++ b/docs/devel/kconfig.rst @@ -0,0 +1,305 @@ +Introduction + + +QEMU is a very versatile emulator; it can be built for a variety of +targets, where each target can emulate various boards and at the same +time different targets can share large amounts of code. For example, +a POWER and an x86 board can run the same code to emulate a PCI network +card, even though the boards use different PCI host bridges, and they +can run the same code to emulate a SCSI disk while using different +SCSI adapters. ARM, s390 and x86 boards can all present a virtio-blk +disk to their guests, but with three different virtio guest interfaces. + +Each QEMU target enables a subset of the boards, devices and buses that +are included in QEMU's source code. As a result, each QEMU executable +only links a small subset of the files that form QEMU's source code; +anything that is not needed to support a particular target is culled. + +QEMU uses a simple domain-specific language to describe the dependencies +between components. This is useful for two reasons: + +* new targets and boards can be added without knowing in detail the + architecture of the hardware emulation subsystems. Boards only have + to list the components they need, and the compiled executable will + include all the required dependencies and all the devices that the + user can add to that board; + +* users can easily build reduced versions of QEMU that support only a subset + of boards or devices. For example, by default most targets will include + all emulated PCI devices that QEMU supports, but the build process is + configurable and it is easy to drop unnecessary (or otherwise unwanted) + code to make a leaner binary. + +This domain-specific language is based on the Kconfig language that +originated in the Linux kernel, though it was heavily simplified and +the handling of dependencies is stricter in QEMU. + +Unlike Linux, there is no user interface to edit the configuration, which +is instead specified in per-target files under the ``default-configs/`` +directory of the QEMU source tree. This is because, unlike Linux, +configuration and dependencies can be treated as a black box when building +QEMU; the default configuration that QEMU ships with should be okay in +almost all cases. + +The Kconfig language + + +Kconfig defines configurable components in files named ``hw/*/Kconfig``. +Note that configurable components are _not_ visible in C code as preprocessor +symbols; they are only visible in the Makefile. Each configurable component +defines a Makefile variable whose name starts with ``CONFIG_``. + +All elements have boolean (true/false) type; truth is written as ``y``, while +falsehood is written ``n``. They are defined in a Kconfig +stanza like the following:: + + config ARM_VIRT + bool + imply PCI_DEVICES + imply VFIO_AMD_XGBE + imply VFIO_XGMAC + select A15MPCORE + select ACPI + select ARM_SMMUV3 + +The ``config`` keyword introduces a new configuration element. In the example +above, Makefiles will have access to a variable named ``CONFIG_ARM_VIRT``, +with value ``y`` or ``n`` (respectively for boolean true and false). + +Boolean expressions can be used within the language, whenever +is written in the remainder of this section. The ``&&``, ``||`` and +``!`` operators respectively denote conjunction (AND), disjunction (OR) +and negation (NOT). + +The ``bool`` data type declaration is optional, but it is suggested to +include it for clarity and future-proofing. After ``bool`` the following +directives can be included: + +**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. In the following example, + ``CONFIG_BAZ`` will be true whenever ``CONFIG_FOO`` is true:: + +config FOO + select BAZ + + The optional expression will prevent ``select`` from having any effect + unless it is true. + + Note that unlike Linux, QEMU will detect contradictions between + ``depends on`` and ``select`` statements and prevent you from building + such a configuration. + +**default value**: ``default [if ]`` + + Default values are assigned to the config symbol if no other value was + set by the user via ``default-configs/*.mak`` files, and only if +
[Qemu-devel] [PATCH v1 13/15] s390x/tcg: Implement rounding mode and XxC for LOAD ROUNDED
With the floating-point extension facility, LOAD ROUNDED has
a rounding mode specification and the inexact-exception control (XxC).
Handle them just like e.g. LOAD FP INTEGER.
Signed-off-by: David Hildenbrand
---
target/s390x/fpu_helper.c | 22 --
target/s390x/helper.h | 6 +++---
target/s390x/insn-data.def | 6 +++---
target/s390x/translate.c | 30 +++---
4 files changed, 49 insertions(+), 15 deletions(-)
diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c
index a0501b9ee5..8ef3d2f0e4 100644
--- a/target/s390x/fpu_helper.c
+++ b/target/s390x/fpu_helper.c
@@ -306,10 +306,14 @@ uint64_t HELPER(ldeb)(CPUS390XState *env, uint64_t f2)
}
/* convert 128-bit float to 64-bit float */
-uint64_t HELPER(ldxb)(CPUS390XState *env, uint64_t ah, uint64_t al)
+uint64_t HELPER(ldxb)(CPUS390XState *env, uint64_t ah, uint64_t al, uint32_t m)
{
+int old_mode = s390_swap_bfp_rounding_mode(env, m & 0xf);
+
+s390_restore_bfp_rounding_mode(env, old_mode);
float64 ret = float128_to_float64(make_float128(ah, al), &env->fpu_status);
-handle_exceptions(env, false, GETPC());
+s390_restore_bfp_rounding_mode(env, old_mode);
+handle_exceptions(env, (m >> 8) & 1, GETPC());
return ret;
}
@@ -330,18 +334,24 @@ uint64_t HELPER(lxeb)(CPUS390XState *env, uint64_t f2)
}
/* convert 64-bit float to 32-bit float */
-uint64_t HELPER(ledb)(CPUS390XState *env, uint64_t f2)
+uint64_t HELPER(ledb)(CPUS390XState *env, uint64_t f2, uint32_t m)
{
+int old_mode = s390_swap_bfp_rounding_mode(env, m & 0xf);
float32 ret = float64_to_float32(f2, &env->fpu_status);
-handle_exceptions(env, false, GETPC());
+
+s390_restore_bfp_rounding_mode(env, old_mode);
+handle_exceptions(env, (m >> 8) & 1, GETPC());
return ret;
}
/* convert 128-bit float to 32-bit float */
-uint64_t HELPER(lexb)(CPUS390XState *env, uint64_t ah, uint64_t al)
+uint64_t HELPER(lexb)(CPUS390XState *env, uint64_t ah, uint64_t al, uint32_t m)
{
+int old_mode = s390_swap_bfp_rounding_mode(env, m & 0xf);
float32 ret = float128_to_float32(make_float128(ah, al), &env->fpu_status);
-handle_exceptions(env, false, GETPC());
+
+s390_restore_bfp_rounding_mode(env, old_mode);
+handle_exceptions(env, (m >> 8) & 1, GETPC());
return ret;
}
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 179c942fc8..b92e5c5e40 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -53,11 +53,11 @@ DEF_HELPER_FLAGS_3(mdb, TCG_CALL_NO_WG, i64, env, i64, i64)
DEF_HELPER_FLAGS_5(mxb, TCG_CALL_NO_WG, i64, env, i64, i64, i64, i64)
DEF_HELPER_FLAGS_4(mxdb, TCG_CALL_NO_WG, i64, env, i64, i64, i64)
DEF_HELPER_FLAGS_2(ldeb, TCG_CALL_NO_WG, i64, env, i64)
-DEF_HELPER_FLAGS_3(ldxb, TCG_CALL_NO_WG, i64, env, i64, i64)
+DEF_HELPER_FLAGS_4(ldxb, TCG_CALL_NO_WG, i64, env, i64, i64, i32)
DEF_HELPER_FLAGS_2(lxdb, TCG_CALL_NO_WG, i64, env, i64)
DEF_HELPER_FLAGS_2(lxeb, TCG_CALL_NO_WG, i64, env, i64)
-DEF_HELPER_FLAGS_2(ledb, TCG_CALL_NO_WG, i64, env, i64)
-DEF_HELPER_FLAGS_3(lexb, TCG_CALL_NO_WG, i64, env, i64, i64)
+DEF_HELPER_FLAGS_3(ledb, TCG_CALL_NO_WG, i64, env, i64, i32)
+DEF_HELPER_FLAGS_4(lexb, TCG_CALL_NO_WG, i64, env, i64, i64, i32)
DEF_HELPER_FLAGS_3(ceb, TCG_CALL_NO_WG_SE, i32, env, i64, i64)
DEF_HELPER_FLAGS_3(cdb, TCG_CALL_NO_WG_SE, i32, env, i64, i64)
DEF_HELPER_FLAGS_5(cxb, TCG_CALL_NO_WG_SE, i32, env, i64, i64, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 7ef230029e..39117c0fd4 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -599,9 +599,9 @@
F(0xed05, LXDB,RXE, Z, 0, m2_64, new_P, x1, lxdb, 0, IF_BFP)
F(0xed06, LXEB,RXE, Z, 0, m2_32u, new_P, x1, lxeb, 0, IF_BFP)
/* LOAD ROUNDED */
-F(0xb344, LEDBR, RRE, Z, 0, f2, new, e1, ledb, 0, IF_BFP)
-F(0xb345, LDXBR, RRE, Z, x2h, x2l, new, f1, ldxb, 0, IF_BFP)
-F(0xb346, LEXBR, RRE, Z, x2h, x2l, new, e1, lexb, 0, IF_BFP)
+F(0xb344, LEDBR, RRF_e, Z, 0, f2, new, e1, ledb, 0, IF_BFP)
+F(0xb345, LDXBR, RRF_e, Z, x2h, x2l, new, f1, ldxb, 0, IF_BFP)
+F(0xb346, LEXBR, RRF_e, Z, x2h, x2l, new, e1, lexb, 0, IF_BFP)
/* LOAD MULTIPLE */
C(0x9800, LM, RS_a, Z, 0, a2, 0, 0, lm32, 0)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index e7cfc80131..d1fbd0cde1 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2806,19 +2806,43 @@ static DisasJumpType op_ldeb(DisasContext *s, DisasOps
*o)
static DisasJumpType op_ledb(DisasContext *s, DisasOps *o)
{
-gen_helper_ledb(o->out, cpu_env, o->in2);
+TCGv_i32 m;
+
+if (!valid_bfp_rounding_mode(get_field(s->fields, m3))) {
+gen_program_exception(s, PGM_SPECIFICATION);
+return DISAS_NORETURN;
+}
+m = tcg_const_i32(get_fields2(s->fields, m3, m4));
+gen_helper_ledb(o->out, cpu_env, o->in2, m);
+tcg_temp_free_i32(
[Qemu-devel] [PATCH v2] s390x: add zPCI feature to "qemu" CPU model
As we now always have PCI support, let's add it to the "qemu" CPU model,
taking care of backwards compatibility.
Signed-off-by: David Hildenbrand
---
v1 -> v2:
- Use correct model identifiction of the z12 we emulate
hw/s390x/s390-virtio-ccw.c | 3 +++
target/s390x/gen-features.c | 8 ++--
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 3fb5c88af9..1a3414b642 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -657,7 +657,10 @@ DEFINE_CCW_MACHINE(4_0, "4.0", true);
static void ccw_machine_3_1_instance_options(MachineState *machine)
{
+static const S390FeatInit qemu_cpu_feat = { S390_FEAT_LIST_QEMU_V3_1 };
ccw_machine_4_0_instance_options(machine);
+
+s390_set_qemu_cpu_model(0x2827, 12, 2, qemu_cpu_feat);
}
static void ccw_machine_3_1_class_options(MachineClass *mc)
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index b0e9b271e2..19bb8ac7f8 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -560,7 +560,7 @@ static uint16_t qemu_V2_11[] = {
S390_FEAT_ZARCH,
};
-static uint16_t qemu_LATEST[] = {
+static uint16_t qemu_V3_1[] = {
S390_FEAT_DAT_ENH,
S390_FEAT_IDTE_SEGMENT,
S390_FEAT_STFLE,
@@ -592,13 +592,16 @@ static uint16_t qemu_LATEST[] = {
S390_FEAT_MSA_EXT_4,
};
+static uint16_t qemu_LATEST[] = {
+S390_FEAT_ZPCI,
+};
+
/* add all new definitions before this point */
static uint16_t qemu_MAX[] = {
/* z13+ features */
S390_FEAT_STFLE_53,
/* generates a dependency warning, leave it out for now */
S390_FEAT_MSA_EXT_5,
-S390_FEAT_ZPCI,
};
/** END FEATURE DEFS **/
@@ -708,6 +711,7 @@ static FeatGroupDefSpec FeatGroupDef[] = {
***/
static FeatGroupDefSpec QemuFeatDef[] = {
QEMU_FEAT_INITIALIZER(V2_11),
+QEMU_FEAT_INITIALIZER(V3_1),
QEMU_FEAT_INITIALIZER(LATEST),
QEMU_FEAT_INITIALIZER(MAX),
};
--
2.17.2
Re: [Qemu-devel] [PATCH v1] s390x: Add floating-point extension facility to "qemu" cpu model
On 12.02.19 12:02, David Hildenbrand wrote: > The floating-point extension facility implemented certain changes to > BFP, HFP and DFP instructions. > > As we don't implement HFP/DFP, we can ignore those completely. Related > to BFP, the changes include > - SET BFP ROUNDING MODE (SRNMB) instruction > - BFP-rounding-mode field in the FPC register is changed to 3 bits > - CONVERT FROM LOGICAL instructions > - CONVERT TO LOGICAL instructions > - Changes (rounding mode + XxC) added to > -- CONVERT TO FIXED > -- CONVERT FROM FIXED > -- LOAD FP INTEGER > -- LOAD ROUNDED > -- DIVIDE TO INTEGER > > For TCG, we don't implement DIVIDE TO INTEGER, and it is harder to > implement, so skip that. Also, as we don't implement PFPO, we can skip > changes to that as well. The other parts are now implemented, we can > indicate the facility. > > z14 PoP mentiones that "The floating-point extension facility is installed > in the z/Architecture architectural mode. When bit 37 is one, bit 42 is > also one.", meaning that the DFP (decimal-floating-point) facility also > has to be inidicated. We can ignore that for now. > > Signed-off-by: David Hildenbrand This is the same patch as 15/15, refer to 0/15 as cover letter instead. -- Thanks, David / dhildenb
Re: [Qemu-devel] [PATCH v1] s390x: add zPCI feature to "qemu" CPU model
On 12.02.19 12:16, Cornelia Huck wrote:
> On Tue, 12 Feb 2019 10:30:23 +0100
> David Hildenbrand wrote:
>
>> As we now always have PCI support, let's add it to the "qemu" CPU model,
>> taking care of backwards compatibility.
>>
>> Signed-off-by: David Hildenbrand
>> ---
>> hw/s390x/s390-virtio-ccw.c | 3 +++
>> target/s390x/gen-features.c | 8 ++--
>> 2 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 3fb5c88af9..7f0d394fbe 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -657,7 +657,10 @@ DEFINE_CCW_MACHINE(4_0, "4.0", true);
>>
>> static void ccw_machine_3_1_instance_options(MachineState *machine)
>> {
>> +static const S390FeatInit qemu_cpu_feat = { S390_FEAT_LIST_QEMU_V3_1 };
>> ccw_machine_4_0_instance_options(machine);
>> +
>> +s390_set_qemu_cpu_model(0x2064, 7, 1, qemu_cpu_feat);
>
> Hm, don't we emulate a (stripped-down) z12 (2827)? Or am I
> misunderstanding?
Yes, stupid copy and past error,
0x2827, 12, 2 it is!
Thanks for spotting that!
>
>> }
>>
>> static void ccw_machine_3_1_class_options(MachineClass *mc)
--
Thanks,
David / dhildenb
[Qemu-devel] [PATCH] contrib/rdmacm-mux: Fix out-of-bounds risk
The function get_fd extract context from the received MAD message and
uses it as a key to fetch the destination fd from the mapping table.
A context can be dgid in case of CM request message or comm_id in case
of CM SIDR response message.
When MAD message with a smaller size as expected for the message type
received we are hitting out-of-bounds where we are looking for the
context out of message boundaries.
Fix it by validating the message size.
Reported-by Sam Smith
Signed-off-by: Yuval Shaia
---
contrib/rdmacm-mux/main.c | 35 +--
1 file changed, 33 insertions(+), 2 deletions(-)
diff --git a/contrib/rdmacm-mux/main.c b/contrib/rdmacm-mux/main.c
index ae88c77a1e..21cc804367 100644
--- a/contrib/rdmacm-mux/main.c
+++ b/contrib/rdmacm-mux/main.c
@@ -300,7 +300,7 @@ static void hash_tbl_remove_fd_ifid_pair(int fd)
pthread_rwlock_unlock(&server.lock);
}
-static int get_fd(const char *mad, int *fd, __be64 *gid_ifid)
+static int get_fd(const char *mad, int umad_len, int *fd, __be64 *gid_ifid)
{
struct umad_hdr *hdr = (struct umad_hdr *)mad;
char *data = (char *)hdr + sizeof(*hdr);
@@ -308,13 +308,35 @@ static int get_fd(const char *mad, int *fd, __be64
*gid_ifid)
uint16_t attr_id = be16toh(hdr->attr_id);
int rc = 0;
+if (umad_len <= sizeof(*hdr)) {
+rc = -EINVAL;
+syslog(LOG_DEBUG, "Ignoring MAD packets with header only\n");
+goto out;
+}
+
switch (attr_id) {
case UMAD_CM_ATTR_REQ:
+if (unlikely(umad_len < sizeof(*hdr) + CM_REQ_DGID_POS +
+sizeof(*gid_ifid))) {
+rc = -EINVAL;
+syslog(LOG_WARNING,
+ "Invalid MAD packet size (%d) for attr_id 0x%x\n", umad_len,
+attr_id);
+goto out;
+}
memcpy(gid_ifid, data + CM_REQ_DGID_POS, sizeof(*gid_ifid));
rc = hash_tbl_search_fd_by_ifid(fd, gid_ifid);
break;
case UMAD_CM_ATTR_SIDR_REQ:
+if (unlikely(umad_len < sizeof(*hdr) + CM_SIDR_REQ_DGID_POS +
+sizeof(*gid_ifid))) {
+rc = -EINVAL;
+syslog(LOG_WARNING,
+ "Invalid MAD packet size (%d) for attr_id 0x%x\n", umad_len,
+attr_id);
+goto out;
+}
memcpy(gid_ifid, data + CM_SIDR_REQ_DGID_POS, sizeof(*gid_ifid));
rc = hash_tbl_search_fd_by_ifid(fd, gid_ifid);
break;
@@ -331,6 +353,13 @@ static int get_fd(const char *mad, int *fd, __be64
*gid_ifid)
data += sizeof(comm_id);
/* Fall through */
case UMAD_CM_ATTR_SIDR_REP:
+if (unlikely(umad_len < sizeof(*hdr) + sizeof(comm_id))) {
+rc = -EINVAL;
+syslog(LOG_WARNING,
+ "Invalid MAD packet size (%d) for attr_id 0x%x\n", umad_len,
+ attr_id);
+goto out;
+}
memcpy(&comm_id, data, sizeof(comm_id));
if (comm_id) {
rc = hash_tbl_search_fd_by_comm_id(comm_id, fd, gid_ifid);
@@ -344,6 +373,7 @@ static int get_fd(const char *mad, int *fd, __be64
*gid_ifid)
syslog(LOG_DEBUG, "mad_to_vm: %d 0x%x 0x%x\n", *fd, attr_id, comm_id);
+out:
return rc;
}
@@ -372,7 +402,8 @@ static void *umad_recv_thread_func(void *args)
} while (rc && server.run);
if (server.run) {
-rc = get_fd(msg.umad.mad, &fd, &msg.hdr.sgid.global.interface_id);
+rc = get_fd(msg.umad.mad, msg.umad_len, &fd,
+&msg.hdr.sgid.global.interface_id);
if (rc) {
continue;
}
--
2.17.2
Re: [Qemu-devel] [PATCH] Kconfig: add documentation
On 11/02/19 18:17, Cornelia Huck wrote: >> + >> +This domain-specific language is based on the Kconfig language that >> originated in the >> +Linux kernel, though it was heavily simplified and the handling of >> dependencies is >> +stricter in QEMU. >> + >> +Unlike Linux, there is no user interface to edit the configuration, which >> is instead >> +specified in per-target files under the ``default-configs/`` directory of >> the >> +QEMU source tree. This is because, unlike Linux, configuration and >> dependencies can be >> +treated as a black box when building QEMU; the default configuration that >> QEMU >> +ships with should be okay in almost all cases. > So this is only about devices and friends, and not about cpu features > etc.? > For now it is... Ideas are welcome on extensions to the mechanism; for now it is only about what default-configs/ has always covered, just better. :) Paolo
Re: [Qemu-devel] [Qemu-block] [PATCH RFC 1/1] Stream block job involves copy-on-read filter
On Mon 11 Feb 2019 05:58:05 PM CET, Vladimir Sementsov-Ogievskiy wrote: >>> The problem is in the concept of "base" node. The code written in >>> manner that base is not changed during block job. However, job don't >>> own base and there is no guarantee that it will not change during >>> the job. >> >> But if that's the case then we have a problem already, because 'base' >> is a member of StreamBlockJob and is used in the existing >> stream_run() code. > > I think it should be possible to reproduce, using block-commit (which > already has filter) with block-stream in parallel, we'll try. It's not possible to run block-stream and block-commit in parallel on the same nodes. See iotest 030 for a few examples. So unless there's a bug it should be safe. >> So if there's a way to make 'base' disappear during the job (how?) >> then we could protect it with block_job_add_bdrv(). > > I'm not sure this is correct. What is the reason for stream to own > base? It's not really interested in it. stream does not need to write or modify base, but it does need to keep a reference to it in order to now where to stop copying data. As I said earlier base is a member of StreamBlockJob, so it should not disappear during the job. Berto
Re: [Qemu-devel] [PATCH] contrib/rdmacm-mux: Fix out-of-bounds risk
On 2/12/19 12:23 PM, Yuval Shaia wrote:
> The function get_fd extract context from the received MAD message and
> uses it as a key to fetch the destination fd from the mapping table.
> A context can be dgid in case of CM request message or comm_id in case
> of CM SIDR response message.
>
> When MAD message with a smaller size as expected for the message type
> received we are hitting out-of-bounds where we are looking for the
> context out of message boundaries.
>
> Fix it by validating the message size.
>
Cc: qemu-sta...@nongnu.org
> Reported-by Sam Smith
> Signed-off-by: Yuval Shaia
Reviewed-by: Philippe Mathieu-Daudé
> ---
> contrib/rdmacm-mux/main.c | 35 +--
> 1 file changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/rdmacm-mux/main.c b/contrib/rdmacm-mux/main.c
> index ae88c77a1e..21cc804367 100644
> --- a/contrib/rdmacm-mux/main.c
> +++ b/contrib/rdmacm-mux/main.c
> @@ -300,7 +300,7 @@ static void hash_tbl_remove_fd_ifid_pair(int fd)
> pthread_rwlock_unlock(&server.lock);
> }
>
> -static int get_fd(const char *mad, int *fd, __be64 *gid_ifid)
> +static int get_fd(const char *mad, int umad_len, int *fd, __be64 *gid_ifid)
> {
> struct umad_hdr *hdr = (struct umad_hdr *)mad;
> char *data = (char *)hdr + sizeof(*hdr);
> @@ -308,13 +308,35 @@ static int get_fd(const char *mad, int *fd, __be64
> *gid_ifid)
> uint16_t attr_id = be16toh(hdr->attr_id);
> int rc = 0;
>
> +if (umad_len <= sizeof(*hdr)) {
> +rc = -EINVAL;
> +syslog(LOG_DEBUG, "Ignoring MAD packets with header only\n");
> +goto out;
> +}
> +
> switch (attr_id) {
> case UMAD_CM_ATTR_REQ:
> +if (unlikely(umad_len < sizeof(*hdr) + CM_REQ_DGID_POS +
> +sizeof(*gid_ifid))) {
> +rc = -EINVAL;
> +syslog(LOG_WARNING,
> + "Invalid MAD packet size (%d) for attr_id 0x%x\n",
> umad_len,
> +attr_id);
> +goto out;
> +}
> memcpy(gid_ifid, data + CM_REQ_DGID_POS, sizeof(*gid_ifid));
> rc = hash_tbl_search_fd_by_ifid(fd, gid_ifid);
> break;
>
> case UMAD_CM_ATTR_SIDR_REQ:
> +if (unlikely(umad_len < sizeof(*hdr) + CM_SIDR_REQ_DGID_POS +
> +sizeof(*gid_ifid))) {
> +rc = -EINVAL;
> +syslog(LOG_WARNING,
> + "Invalid MAD packet size (%d) for attr_id 0x%x\n",
> umad_len,
> +attr_id);
> +goto out;
> +}
> memcpy(gid_ifid, data + CM_SIDR_REQ_DGID_POS, sizeof(*gid_ifid));
> rc = hash_tbl_search_fd_by_ifid(fd, gid_ifid);
> break;
> @@ -331,6 +353,13 @@ static int get_fd(const char *mad, int *fd, __be64
> *gid_ifid)
> data += sizeof(comm_id);
> /* Fall through */
> case UMAD_CM_ATTR_SIDR_REP:
> +if (unlikely(umad_len < sizeof(*hdr) + sizeof(comm_id))) {
> +rc = -EINVAL;
> +syslog(LOG_WARNING,
> + "Invalid MAD packet size (%d) for attr_id 0x%x\n",
> umad_len,
> + attr_id);
> +goto out;
> +}
> memcpy(&comm_id, data, sizeof(comm_id));
> if (comm_id) {
> rc = hash_tbl_search_fd_by_comm_id(comm_id, fd, gid_ifid);
> @@ -344,6 +373,7 @@ static int get_fd(const char *mad, int *fd, __be64
> *gid_ifid)
>
> syslog(LOG_DEBUG, "mad_to_vm: %d 0x%x 0x%x\n", *fd, attr_id, comm_id);
>
> +out:
> return rc;
> }
>
> @@ -372,7 +402,8 @@ static void *umad_recv_thread_func(void *args)
> } while (rc && server.run);
>
> if (server.run) {
> -rc = get_fd(msg.umad.mad, &fd,
> &msg.hdr.sgid.global.interface_id);
> +rc = get_fd(msg.umad.mad, msg.umad_len, &fd,
> +&msg.hdr.sgid.global.interface_id);
> if (rc) {
> continue;
> }
>
Re: [Qemu-devel] [PULL 1/7] audio: fix pc speaker init
On 24.01.19 14:20, Gerd Hoffmann wrote:
> Get rid of the pcspk_state global, allow pc speaker
> be added using "-device isa-pcspk".
>
> Signed-off-by: Gerd Hoffmann
> Reviewed-by: Philippe Mathieu-Daudé
> Message-id: 20190124110810.1040-1-kra...@redhat.com
> ---
> hw/audio/pcspk.c | 35 +++
> 1 file changed, 15 insertions(+), 20 deletions(-)
>
> diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
> index 908696d483..b80a62ce90 100644
> --- a/hw/audio/pcspk.c
> +++ b/hw/audio/pcspk.c
> @@ -57,7 +57,6 @@ typedef struct {
> } PCSpkState;
>
> static const char *s_spk = "pcspk";
> -static PCSpkState *pcspk_state;
>
> static inline void generate_samples(PCSpkState *s)
> {
> @@ -111,22 +110,6 @@ static void pcspk_callback(void *opaque, int free)
> }
> }
>
> -static int pcspk_audio_init(ISABus *bus)
> -{
> -PCSpkState *s = pcspk_state;
> -struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUD_FMT_U8, 0};
> -
> -AUD_register_card(s_spk, &s->card);
> -
> -s->voice = AUD_open_out(&s->card, s->voice, s_spk, s, pcspk_callback,
> &as);
> -if (!s->voice) {
> -AUD_log(s_spk, "Could not open voice\n");
> -return -1;
> -}
> -
> -return 0;
> -}
> -
> static uint64_t pcspk_io_read(void *opaque, hwaddr addr,
>unsigned size)
> {
> @@ -179,12 +162,20 @@ static void pcspk_initfn(Object *obj)
>
> static void pcspk_realizefn(DeviceState *dev, Error **errp)
> {
> +struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUD_FMT_U8, 0};
> ISADevice *isadev = ISA_DEVICE(dev);
> PCSpkState *s = PC_SPEAKER(dev);
>
> isa_register_ioport(isadev, &s->ioport, s->iobase);
>
> -pcspk_state = s;
> +AUD_register_card(s_spk, &s->card);
> +
> +s->voice = AUD_open_out(&s->card, s->voice, s_spk, s, pcspk_callback,
> &as);
> +if (!s->voice) {
> +error_setg(errp, "Initializing audio voice failed");
> +AUD_remove_card(&s->card);
> +return;
> +}
> }
>
> static bool migrate_needed(void *opaque)
> @@ -221,8 +212,6 @@ static void pcspk_class_initfn(ObjectClass *klass, void
> *data)
> set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
> dc->vmsd = &vmstate_spk;
> dc->props = pcspk_properties;
> -/* Reason: realize sets global pcspk_state */
> -dc->user_creatable = false;
> }
>
> static const TypeInfo pcspk_info = {
> @@ -233,6 +222,12 @@ static const TypeInfo pcspk_info = {
> .class_init = pcspk_class_initfn,
> };
>
> +static int pcspk_audio_init(ISABus *bus)
> +{
> +isa_create_simple(bus, TYPE_PC_SPEAKER);
> +return 0;
> +}
> +
> static void pcspk_register(void)
> {
> type_register_static(&pcspk_info);
>
I suddenly get (under fedora 28)
ALSA lib pulse.c:243:(pulse_connect) PulseAudio: Unable to connect:
Connection refused
alsa: Could not initialize DAC
alsa: Failed to open `default':
alsa: Reason: Connection refused
ALSA lib pulse.c:243:(pulse_connect) PulseAudio: Unable to connect:
Connection refused
alsa: Could not initialize DAC
alsa: Failed to open `default':
alsa: Reason: Connection refused
audio: Failed to create voice `pcspk'
qemu-system-x86_64: Initialization of device isa-pcspk failed:
Initializing audio voice failed
With
sudo x86_64-softmmu/qemu-system-x86_64 \
--enable-kvm \
-m 4G,maxmem=40G,slots=2 \
-smp sockets=2,cores=2 \
-numa node,nodeid=0,cpus=0-1 -numa node,nodeid=1,cpus=2-3 \
-kernel /boot/vmlinuz-4.19.6-200.fc28.x86_64 \
-append "console=ttyS0 rd.shell rd.luks=0 rd.lvm=0 rd.md=0 rd.dm=0" \
-initrd /boot/initramfs-4.19.6-200.fc28.x86_64.img \
-machine pc,nvdimm \
-nographic \
-nodefaults \
-chardev stdio,id=serial \
--trace events=to_trace \
-device isa-serial,chardev=serial \
-chardev socket,id=monitor,path=/var/tmp/monitor,server,nowait \
-mon chardev=monitor,mode=readline
Could this be related to this patch? (or is maybe something about my
system messed up? will try rebooting, but other audio - e.g. via firefox
- works fine)
--
Thanks,
David / dhildenb
[Qemu-devel] [PATCH v2] ppc: add host-serial and host-model machine attributes
From: Prasad J Pandit
On ppc hosts, hypervisor shares following system attributes
- /proc/device-tree/system-id
- /proc/device-tree/model
with a guest. This could lead to information leakage and misuse.[*]
Add machine attributes to control such system information exposure
to a guest.
[*] https://wiki.openstack.org/wiki/OSSN/OSSN-0028
Reported-by: Daniel P. Berrangé
Fix-suggested-by: Daniel P. Berrangé
Signed-off-by: Prasad J Pandit
---
hw/core/machine.c | 46 +
hw/ppc/spapr.c | 36 +--
include/hw/boards.h | 2 ++
qemu-options.hx | 10 +-
util/qemu-config.c | 8
5 files changed, 95 insertions(+), 7 deletions(-)
Update v2: add backward compatible properties
-> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00593.html
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 2629515363..2d5a52476a 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -476,6 +476,38 @@ static void machine_set_memory_encryption(Object *obj,
const char *value,
ms->memory_encryption = g_strdup(value);
}
+static char *machine_get_host_serial(Object *obj, Error **errp)
+{
+MachineState *ms = MACHINE(obj);
+
+return g_strdup(ms->host_serial);
+}
+
+static void machine_set_host_serial(Object *obj, const char *value,
+Error **errp)
+{
+MachineState *ms = MACHINE(obj);
+
+g_free(ms->host_serial);
+ms->host_serial = g_strdup(value);
+}
+
+static char *machine_get_host_model(Object *obj, Error **errp)
+{
+MachineState *ms = MACHINE(obj);
+
+return g_strdup(ms->host_model);
+}
+
+static void machine_set_host_model(Object *obj, const char *value,
+ Error **errp)
+{
+MachineState *ms = MACHINE(obj);
+
+g_free(ms->host_model);
+ms->host_model = g_strdup(value);
+}
+
void machine_class_allow_dynamic_sysbus_dev(MachineClass *mc, const char *type)
{
strList *item = g_new0(strList, 1);
@@ -760,6 +792,18 @@ static void machine_class_init(ObjectClass *oc, void *data)
&error_abort);
object_class_property_set_description(oc, "memory-encryption",
"Set memory encryption object to use", &error_abort);
+
+object_class_property_add_str(oc, "host-serial",
+machine_get_host_serial, machine_set_host_serial,
+&error_abort);
+object_class_property_set_description(oc, "host-serial",
+"Set host's system-id to use", &error_abort);
+
+object_class_property_add_str(oc, "host-model",
+machine_get_host_model, machine_set_host_model,
+&error_abort);
+object_class_property_set_description(oc, "host-model",
+"Set host's model-id to use", &error_abort);
}
static void machine_class_base_init(ObjectClass *oc, void *data)
@@ -785,6 +829,8 @@ static void machine_initfn(Object *obj)
ms->dump_guest_core = true;
ms->mem_merge = true;
ms->enable_graphics = true;
+ms->host_serial = NULL;
+ms->host_model = NULL;
/* Register notifier when init is done for sysbus sanity checks */
ms->sysbus_notifier.notify = machine_init_notify;
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0942f35bf8..a70667d72d 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1249,13 +1249,31 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
* Add info to guest to indentify which host is it being run on
* and what is the uuid of the guest
*/
-if (kvmppc_get_host_model(&buf)) {
-_FDT(fdt_setprop_string(fdt, 0, "host-model", buf));
-g_free(buf);
+if (machine->host_model && !g_str_equal(machine->host_model, "none")) {
+if (g_str_equal(machine->host_model, "passthrough")) {
+/* -M host-model=passthrough */
+if (kvmppc_get_host_model(&buf)) {
+_FDT(fdt_setprop_string(fdt, 0, "host-model", buf));
+g_free(buf);
+}
+} else {
+/* -M host-model= */
+_FDT(fdt_setprop_string(fdt, 0, "host-model",
machine->host_model));
+}
}
-if (kvmppc_get_host_serial(&buf)) {
-_FDT(fdt_setprop_string(fdt, 0, "host-serial", buf));
-g_free(buf);
+
+if (machine->host_serial && !g_str_equal(machine->host_serial, "none")) {
+if (g_str_equal(machine->host_serial, "passthrough")) {
+/* -M host-serial=passthrough */
+if (kvmppc_get_host_serial(&buf)) {
+_FDT(fdt_setprop_string(fdt, 0, "host-serial", buf));
+g_free(buf);
+}
+} else {
+/* -M host-serial= */
+_FDT(fdt_setprop_string(fdt, 0,
+"host-serial", machine->host_serial));
+}
}
buf = qemu_uuid_unparse_strdup(&qemu_uuid);
@@ -4080,9 +4098,15 @@ DEFINE_SPAPR_MACHINE(4_0, "4.0", true);
static void spapr_machine_3_1_class_options(MachineClass *mc)
{
Re: [Qemu-devel] [PATCH v2 3/3] tpm_tis: fix format string specifier in tpm_tis_show_buffer()
On 2/11/19 10:03 AM, Liam Merwick wrote:
cppcheck reports:
[hw/tpm/tpm_tis.c:113]: (warning) %d in format string (no. 2) requires 'int'
but the argument type is 'unsigned int'
Fix this by using %u instead of %d
Signed-off-by: Liam Merwick
Reviewed-by: Stefan Berger
---
hw/tpm/tpm_tis.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 860c2ace7d99..395500e8a61d 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -110,7 +110,7 @@ static void tpm_tis_show_buffer(const unsigned char *buffer,
uint32_t len, i;
len = MIN(tpm_cmd_get_size(buffer), buffer_size);
-printf("tpm_tis: %s length = %d\n", string, len);
+printf("tpm_tis: %s length = %u\n", string, len);
for (i = 0; i < len; i++) {
if (i && !(i % 16)) {
printf("\n");
Re: [Qemu-devel] [Qemu-block] [RFC PATCH] coroutines: generate wrapper code
Am 12.02.2019 um 04:22 hat Stefan Hajnoczi geschrieben: > On Mon, Feb 11, 2019 at 09:38:37AM +, Vladimir Sementsov-Ogievskiy wrote: > > 11.02.2019 6:42, Stefan Hajnoczi wrote: > > > On Fri, Feb 08, 2019 at 05:11:22PM +0300, Vladimir Sementsov-Ogievskiy > > > wrote: > > >> Hi all! > > >> > > >> We have a very frequent pattern of wrapping a coroutine_fn function > > >> to be called from non-coroutine context: > > >> > > >>- create structure to pack parameters > > >>- create function to call original function taking parameters from > > >> struct > > >>- create wrapper, which in case of non-coroutine context will > > >> create a coroutine, enter it and start poll-loop. > > >> > > >> Here is a draft of template code + example how it can be used to drop a > > >> lot of similar code. > > >> > > >> Hope someone like it except me) > > > > > > My 2 cents. Cons: > > > > > > * Synchronous poll loops are an anti-pattern. They block all of QEMU > > > with the big mutex held. Making them easier to write is > > > questionable because we should aim to have as few of these as > > > possible. > > > > Understand. Do we have a concept or a kind of target for a future to get > > rid of > > these a lot of poll-loops? What is the right way? At least for block-layer? > > It's non-trivial. The nested event loop could be flattened if there was > a mechanism to stop further activity on a specific object only (e.g. > BlockDriverState). That way the event loop can continue processing > events for other objects and device emulation could continue for other > objects. The mechanism to stop activity on BlockDriverStates is bdrv_drain(). But I don't see how this is related. Nested event loops aren't for stopping concurrent activity (events related to async operations started earlier are still processed in nested event loops), but for making progress on the operation we're waiting for. They happen when synchronous code calls into asynchronous code. The way to get rid of them is making their callers async. I think we would come a long way if we ran QMP command handlers (at least the block related ones) and qemu-img operations in coroutines instead of blocking while we wait for the result. > Unfortunately there are interactions between objects like in block jobs > that act on multiple BDSes, so it becomes even tricky. > > A simple way of imagining this is to make each object an "actor" > coroutine. The coroutine processes a single message (request) at a time > and yields when it needs to wait. Callers send messages and expect > asynchronous responses. This model is bad for efficiency (parallelism > is necessary) but at least it offers a sane way of thinking about > multiple asynchronous components coordinating together. (It's another > way of saying, let's put everything into coroutines.) > > The advantage of a flat event loop is that a hang in one object (e.g. > I/O getting stuck in one file) doesn't freeze the entire event loop. I think this one is more theoretical because you'll still have dependencies between the components. blk_drain_all() isn't hanging because the code is designed suboptimally, but because its semantics is to wait until all requests have completed. And it's called because this semantics is required. Kevin signature.asc Description: PGP signature
Re: [Qemu-devel] [PULL 1/7] audio: fix pc speaker init
Hi David,
On 2/12/19 12:47 PM, David Hildenbrand wrote:
> On 24.01.19 14:20, Gerd Hoffmann wrote:
>> Get rid of the pcspk_state global, allow pc speaker
>> be added using "-device isa-pcspk".
>>
>> Signed-off-by: Gerd Hoffmann
>> Reviewed-by: Philippe Mathieu-Daudé
>> Message-id: 20190124110810.1040-1-kra...@redhat.com
>> ---
>> hw/audio/pcspk.c | 35 +++
>> 1 file changed, 15 insertions(+), 20 deletions(-)
>>
>> diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
>> index 908696d483..b80a62ce90 100644
>> --- a/hw/audio/pcspk.c
>> +++ b/hw/audio/pcspk.c
>> @@ -57,7 +57,6 @@ typedef struct {
>> } PCSpkState;
>>
>> static const char *s_spk = "pcspk";
>> -static PCSpkState *pcspk_state;
>>
>> static inline void generate_samples(PCSpkState *s)
>> {
>> @@ -111,22 +110,6 @@ static void pcspk_callback(void *opaque, int free)
>> }
>> }
>>
>> -static int pcspk_audio_init(ISABus *bus)
>> -{
>> -PCSpkState *s = pcspk_state;
>> -struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUD_FMT_U8, 0};
>> -
>> -AUD_register_card(s_spk, &s->card);
>> -
>> -s->voice = AUD_open_out(&s->card, s->voice, s_spk, s, pcspk_callback,
>> &as);
>> -if (!s->voice) {
>> -AUD_log(s_spk, "Could not open voice\n");
>> -return -1;
>> -}
>> -
>> -return 0;
>> -}
>> -
>> static uint64_t pcspk_io_read(void *opaque, hwaddr addr,
>>unsigned size)
>> {
>> @@ -179,12 +162,20 @@ static void pcspk_initfn(Object *obj)
>>
>> static void pcspk_realizefn(DeviceState *dev, Error **errp)
>> {
>> +struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUD_FMT_U8, 0};
>> ISADevice *isadev = ISA_DEVICE(dev);
>> PCSpkState *s = PC_SPEAKER(dev);
>>
>> isa_register_ioport(isadev, &s->ioport, s->iobase);
>>
>> -pcspk_state = s;
>> +AUD_register_card(s_spk, &s->card);
>> +
>> +s->voice = AUD_open_out(&s->card, s->voice, s_spk, s, pcspk_callback,
>> &as);
>> +if (!s->voice) {
>> +error_setg(errp, "Initializing audio voice failed");
>> +AUD_remove_card(&s->card);
>> +return;
>> +}
>> }
>>
>> static bool migrate_needed(void *opaque)
>> @@ -221,8 +212,6 @@ static void pcspk_class_initfn(ObjectClass *klass, void
>> *data)
>> set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
>> dc->vmsd = &vmstate_spk;
>> dc->props = pcspk_properties;
>> -/* Reason: realize sets global pcspk_state */
>> -dc->user_creatable = false;
>> }
>>
>> static const TypeInfo pcspk_info = {
>> @@ -233,6 +222,12 @@ static const TypeInfo pcspk_info = {
>> .class_init = pcspk_class_initfn,
>> };
>>
>> +static int pcspk_audio_init(ISABus *bus)
>> +{
>> +isa_create_simple(bus, TYPE_PC_SPEAKER);
>> +return 0;
>> +}
>> +
>> static void pcspk_register(void)
>> {
>> type_register_static(&pcspk_info);
>>
>
> I suddenly get (under fedora 28)
>
> ALSA lib pulse.c:243:(pulse_connect) PulseAudio: Unable to connect:
> Connection refused
>
> alsa: Could not initialize DAC
> alsa: Failed to open `default':
> alsa: Reason: Connection refused
> ALSA lib pulse.c:243:(pulse_connect) PulseAudio: Unable to connect:
> Connection refused
>
> alsa: Could not initialize DAC
> alsa: Failed to open `default':
> alsa: Reason: Connection refused
This ALSA problem seems on your side.
> audio: Failed to create voice `pcspk'
> qemu-system-x86_64: Initialization of device isa-pcspk failed:
> Initializing audio voice failed
Previously the errors would be ignored and QEMU would start.
>
>
> With
>
> sudo x86_64-softmmu/qemu-system-x86_64 \
> --enable-kvm \
> -m 4G,maxmem=40G,slots=2 \
> -smp sockets=2,cores=2 \
> -numa node,nodeid=0,cpus=0-1 -numa node,nodeid=1,cpus=2-3 \
> -kernel /boot/vmlinuz-4.19.6-200.fc28.x86_64 \
> -append "console=ttyS0 rd.shell rd.luks=0 rd.lvm=0 rd.md=0 rd.dm=0" \
> -initrd /boot/initramfs-4.19.6-200.fc28.x86_64.img \
> -machine pc,nvdimm \
> -nographic \
> -nodefaults \
> -chardev stdio,id=serial \
> --trace events=to_trace \
> -device isa-serial,chardev=serial \
> -chardev socket,id=monitor,path=/var/tmp/monitor,server,nowait \
> -mon chardev=monitor,mode=readline
>
>
> Could this be related to this patch? (or is maybe something about my
> system messed up? will try rebooting, but other audio - e.g. via firefox
> - works fine)
Does your Firefox uses ALSA? The default install uses PulseAudio.
I think the behavior change you are experiencing comes from the patch 7
of this series "audio: probe audio drivers by default":
@@ -879,7 +879,7 @@
Linux)
- audio_drv_list="oss"
+ audio_drv_list="try-pa try-alsa try-sdl oss"
Previously you were using OSS, and how the ./configure found via
pkg-config that you have the ALSA libs installed, and use ALSA first.
Can you share the relevant part of the ./configure output?
Thanks,
Phil.
[Qemu-devel] [PATCH] ppc: fix crash during branch stepping
The PPC BRANCH exception could bubble up, but this is an QEMU internal exception
and QEMU then crased. Instead it should trigger TRACE exception, according to
PPC 2.07 book. It could happen only when using branch stepping, which is not
commonly used.
Change gen_prep_dbgex do do trigger TRACE. The excp, argument is now removed,
since the type of exception can be inferred from the singlestep_enabled flags.
removed the guards around gen_exception, since they are unnecessary.
Fixes: 0e3bf48909 ("ppc: add DBCR based debugging").
Signed-off-by: Roman Kapl
---
target/ppc/translate.c | 37 +++--
1 file changed, 15 insertions(+), 22 deletions(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index e169c43643..c22d1a69c7 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -287,26 +287,22 @@ static void gen_exception_nip(DisasContext *ctx, uint32_t
excp,
ctx->exception = (excp);
}
-/* Translates the EXCP_TRACE/BRANCH exceptions used on most PowerPCs to
- * EXCP_DEBUG, if we are running on cores using the debug enable bit (e.g.
- * BookE).
+/*
+ * Tells the caller what is the appropriate exception to generate and prepares
+ * SPR registers for this exception.
+ *
+ * The exception can be either POWERPC_EXCP_TRACE (on most PowerPCs) or
+ * POWERPC_EXCP_DEBUG (on BookE).
*/
-static uint32_t gen_prep_dbgex(DisasContext *ctx, uint32_t excp)
+static uint32_t gen_prep_dbgex(DisasContext *ctx)
{
-if ((ctx->singlestep_enabled & CPU_SINGLE_STEP)
-&& (excp == POWERPC_EXCP_BRANCH)) {
-/* Trace excpt. has priority */
-excp = POWERPC_EXCP_TRACE;
-}
if (ctx->flags & POWERPC_FLAG_DE) {
target_ulong dbsr = 0;
-switch (excp) {
-case POWERPC_EXCP_TRACE:
+if (ctx->singlestep_enabled & CPU_SINGLE_STEP) {
dbsr = DBCR0_ICMP;
-break;
-case POWERPC_EXCP_BRANCH:
+} else {
+/* Must have been branch */
dbsr = DBCR0_BRT;
-break;
}
TCGv t0 = tcg_temp_new();
gen_load_spr(t0, SPR_BOOKE_DBSR);
@@ -315,7 +311,7 @@ static uint32_t gen_prep_dbgex(DisasContext *ctx, uint32_t
excp)
tcg_temp_free(t0);
return POWERPC_EXCP_DEBUG;
} else {
-return excp;
+return POWERPC_EXCP_TRACE;
}
}
@@ -3652,10 +3648,8 @@ static void gen_lookup_and_goto_ptr(DisasContext *ctx)
if (sse & GDBSTUB_SINGLE_STEP) {
gen_debug_exception(ctx);
} else if (sse & (CPU_SINGLE_STEP | CPU_BRANCH_STEP)) {
-uint32_t excp = gen_prep_dbgex(ctx, POWERPC_EXCP_BRANCH);
-if (excp != POWERPC_EXCP_NONE) {
-gen_exception(ctx, excp);
-}
+uint32_t excp = gen_prep_dbgex(ctx);
+gen_exception(ctx, excp);
}
tcg_gen_exit_tb(NULL, 0);
} else {
@@ -7785,9 +7779,8 @@ static void ppc_tr_translate_insn(DisasContextBase
*dcbase, CPUState *cs)
ctx->exception != POWERPC_SYSCALL &&
ctx->exception != POWERPC_EXCP_TRAP &&
ctx->exception != POWERPC_EXCP_BRANCH)) {
-uint32_t excp = gen_prep_dbgex(ctx, POWERPC_EXCP_TRACE);
-if (excp != POWERPC_EXCP_NONE)
-gen_exception_nip(ctx, excp, ctx->base.pc_next);
+uint32_t excp = gen_prep_dbgex(ctx);
+gen_exception_nip(ctx, excp, ctx->base.pc_next);
}
if (tcg_check_temp_count()) {
--
2.20.1.390.gb5101f9297
Re: [Qemu-devel] [PATCH v2 00/11] hw: Remove "hw/devices.h"
On 2/5/19 12:18 AM, Philippe Mathieu-Daudé wrote: > Hi, > > As his first comment describes itself, the "hw/devices.h" contains > declarations for "Devices that have nowhere better to go." > This series remove it, creating new headers for devices covered there. > MAINTAINERS is updated. > I also included 2 cleanups while working on this, in "qemu/typedefs.h" > and "hw/net/ne2000-isa.h" header guard. > > v2: > - added R-b, T-b tags > - addressed many review comments from Thomas > > v1: https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg00611.html ping? Since it is mostly touch (old) ARM boards, Peter can you do a quick review and give your Ack-by, so it can go via the Trivial/Misc tree? Half of the series is already reviewed by Thomas. Thanks, Phil.
Re: [Qemu-devel] [qemu-s390x] [PATCH 06/15] s390-bios: Clean up cio.h
On 2019-02-04 11:48, Cornelia Huck wrote:
> On Tue, 29 Jan 2019 08:29:13 -0500
> "Jason J. Herne" wrote:
>
>> Add proper typedefs to all structs and modify all bit fields to use
>> consistent
>> formatting.
>>
>> Signed-off-by: Jason J. Herne
>> Reviewed-by: Collin Walling
>> ---
>> pc-bios/s390-ccw/cio.h | 86
>> ++---
>> pc-bios/s390-ccw/s390-ccw.h | 8 -
>> 2 files changed, 43 insertions(+), 51 deletions(-)
>>
>> diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
>> index 1a0795f..a48eee5 100644
>> --- a/pc-bios/s390-ccw/cio.h
>> +++ b/pc-bios/s390-ccw/cio.h
>> @@ -53,12 +53,12 @@ struct schib_config {
>> __u64 mba;
>> __u32 intparm;
>> __u16 mbi;
>> -__u32 isc:3;
>> -__u32 ena:1;
>> -__u32 mme:2;
>> -__u32 mp:1;
>> -__u32 csense:1;
>> -__u32 mbfc:1;
>> +__u32 isc: 3;
>> +__u32 ena: 1;
>> +__u32 mme: 2;
>> +__u32 mp : 1;
>> +__u32 csense : 1;
>> +__u32 mbfc : 1;
>> } __attribute__ ((packed));
>
> This seems to make checkpatch unhappy... maybe consolidate to the other
> formatting instead?
Yeah, you get lots of these errors this way:
ERROR: spaces prohibited around that ':' (ctx:WxW)
#141: FILE: pc-bios/s390-ccw/cio.h:56:
+__u32 isc: 3;
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#142: FILE: pc-bios/s390-ccw/cio.h:57:
+__u32 ena: 1;
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#143: FILE: pc-bios/s390-ccw/cio.h:58:
+__u32 mme: 2;
^
...
Not sure whether it's a checkpatch warning or really our official coding
style, but anyway, I'd rather prefer to keep checkpatch calm here and
thus no spaces around the ':' please.
Thomas
Re: [Qemu-devel] [PULL 1/7] audio: fix pc speaker init
On 12.02.19 13:20, David Hildenbrand wrote:
> On 12.02.19 13:08, Philippe Mathieu-Daudé wrote:
>> Hi David,
>>
>> On 2/12/19 12:47 PM, David Hildenbrand wrote:
>>> On 24.01.19 14:20, Gerd Hoffmann wrote:
Get rid of the pcspk_state global, allow pc speaker
be added using "-device isa-pcspk".
Signed-off-by: Gerd Hoffmann
Reviewed-by: Philippe Mathieu-Daudé
Message-id: 20190124110810.1040-1-kra...@redhat.com
---
hw/audio/pcspk.c | 35 +++
1 file changed, 15 insertions(+), 20 deletions(-)
diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
index 908696d483..b80a62ce90 100644
--- a/hw/audio/pcspk.c
+++ b/hw/audio/pcspk.c
@@ -57,7 +57,6 @@ typedef struct {
} PCSpkState;
static const char *s_spk = "pcspk";
-static PCSpkState *pcspk_state;
static inline void generate_samples(PCSpkState *s)
{
@@ -111,22 +110,6 @@ static void pcspk_callback(void *opaque, int free)
}
}
-static int pcspk_audio_init(ISABus *bus)
-{
-PCSpkState *s = pcspk_state;
-struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUD_FMT_U8, 0};
-
-AUD_register_card(s_spk, &s->card);
-
-s->voice = AUD_open_out(&s->card, s->voice, s_spk, s, pcspk_callback,
&as);
-if (!s->voice) {
-AUD_log(s_spk, "Could not open voice\n");
-return -1;
-}
-
-return 0;
-}
-
static uint64_t pcspk_io_read(void *opaque, hwaddr addr,
unsigned size)
{
@@ -179,12 +162,20 @@ static void pcspk_initfn(Object *obj)
static void pcspk_realizefn(DeviceState *dev, Error **errp)
{
+struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUD_FMT_U8, 0};
ISADevice *isadev = ISA_DEVICE(dev);
PCSpkState *s = PC_SPEAKER(dev);
isa_register_ioport(isadev, &s->ioport, s->iobase);
-pcspk_state = s;
+AUD_register_card(s_spk, &s->card);
+
+s->voice = AUD_open_out(&s->card, s->voice, s_spk, s, pcspk_callback,
&as);
+if (!s->voice) {
+error_setg(errp, "Initializing audio voice failed");
+AUD_remove_card(&s->card);
+return;
+}
}
static bool migrate_needed(void *opaque)
@@ -221,8 +212,6 @@ static void pcspk_class_initfn(ObjectClass *klass,
void *data)
set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
dc->vmsd = &vmstate_spk;
dc->props = pcspk_properties;
-/* Reason: realize sets global pcspk_state */
-dc->user_creatable = false;
}
static const TypeInfo pcspk_info = {
@@ -233,6 +222,12 @@ static const TypeInfo pcspk_info = {
.class_init = pcspk_class_initfn,
};
+static int pcspk_audio_init(ISABus *bus)
+{
+isa_create_simple(bus, TYPE_PC_SPEAKER);
+return 0;
+}
+
static void pcspk_register(void)
{
type_register_static(&pcspk_info);
>>>
>>> I suddenly get (under fedora 28)
>>>
>>> ALSA lib pulse.c:243:(pulse_connect) PulseAudio: Unable to connect:
>>> Connection refused
>>>
>>> alsa: Could not initialize DAC
>>> alsa: Failed to open `default':
>>> alsa: Reason: Connection refused
>>> ALSA lib pulse.c:243:(pulse_connect) PulseAudio: Unable to connect:
>>> Connection refused
>>>
>>> alsa: Could not initialize DAC
>>> alsa: Failed to open `default':
>>> alsa: Reason: Connection refused
>>
>> This ALSA problem seems on your side.
>>
>>> audio: Failed to create voice `pcspk'
>>> qemu-system-x86_64: Initialization of device isa-pcspk failed:
>>> Initializing audio voice failed
>>
>> Previously the errors would be ignored and QEMU would start.
>
> I just did a fedora 28 uupdate + reboot. Problem still exists. Would be
> strange if only I would be hitting this problem with stock alsa libraries.
>
>>
>>>
>>>
>>> With
>>>
>>> sudo x86_64-softmmu/qemu-system-x86_64 \
>>> --enable-kvm \
>>> -m 4G,maxmem=40G,slots=2 \
>>> -smp sockets=2,cores=2 \
>>> -numa node,nodeid=0,cpus=0-1 -numa node,nodeid=1,cpus=2-3 \
>>> -kernel /boot/vmlinuz-4.19.6-200.fc28.x86_64 \
>>> -append "console=ttyS0 rd.shell rd.luks=0 rd.lvm=0 rd.md=0 rd.dm=0" \
>>> -initrd /boot/initramfs-4.19.6-200.fc28.x86_64.img \
>>> -machine pc,nvdimm \
>>> -nographic \
>>> -nodefaults \
>>> -chardev stdio,id=serial \
>>> --trace events=to_trace \
>>> -device isa-serial,chardev=serial \
>>> -chardev socket,id=monitor,path=/var/tmp/monitor,server,nowait \
>>> -mon chardev=monitor,mode=readline
>>>
>>>
>>> Could this be related to this patch? (or is maybe something about my
>>> system messed up? will try rebooting, but other audio - e.g. via firefox
>>> - works fine)
>>
>> Does your Firefox uses ALSA? The de
Re: [Qemu-devel] [PULL 0/3] Block patches
On Tue, 12 Feb 2019 at 04:01, Stefan Hajnoczi wrote: > > The following changes since commit 22c5f446514a2a4bb0dbe1fea26713da92fc85fa: > > Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20190211' into > staging (2019-02-11 17:04:57 +) > > are available in the Git repository at: > > git://github.com/stefanha/qemu.git tags/block-pull-request > > for you to fetch changes up to 9a6719d572e99a4e79f589d0b73f7475b86f982d: > > virtio-blk: cleanup using VirtIOBlock *s and VirtIODevice *vdev (2019-02-12 > 11:49:17 +0800) > > > Pull request > > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0 for any user-visible changes. -- PMM
Re: [Qemu-devel] [PATCH v2 3/3] tpm_tis: fix format string specifier in tpm_tis_show_buffer()
On 2/11/19 10:13 PM, Stefan Berger wrote:
> On 2/11/19 3:09 PM, Liam Merwick wrote:
>> On 11/02/2019 19:56, Stefan Berger wrote:
>>> On 2/11/19 11:02 AM, Philippe Mathieu-Daudé wrote:
On 2/11/19 4:03 PM, Liam Merwick wrote:
[...]
> -printf("tpm_tis: %s length = %d\n", string, len);
> +printf("tpm_tis: %s length = %u\n", string, len);
So here the format is '%zu'.
However in code cleanup we try go get ride of printf() calls and
replace them with trace points.
>>>
>>>
>>> This code is only used for debugging if DEBUG_TIS has been #defined.
>>> No need to add tracing here.
>>
>> I'd come up the attached change (but that seems like overkill).
>
>
> I don't think we need tracing for this.
So if you think the code is mature enough, let's remove the DEBUG calls!
Else we prefer to convert DEBUG printf to trace events because (at least):
- no need to recompile to enable debugging
- when compiled with debugging, you don't mess with STDIO which can be
used as a chardev backend.
Thanks,
Phil.
Re: [Qemu-devel] [PULL 1/7] audio: fix pc speaker init
On 12.02.19 13:08, Philippe Mathieu-Daudé wrote:
> Hi David,
>
> On 2/12/19 12:47 PM, David Hildenbrand wrote:
>> On 24.01.19 14:20, Gerd Hoffmann wrote:
>>> Get rid of the pcspk_state global, allow pc speaker
>>> be added using "-device isa-pcspk".
>>>
>>> Signed-off-by: Gerd Hoffmann
>>> Reviewed-by: Philippe Mathieu-Daudé
>>> Message-id: 20190124110810.1040-1-kra...@redhat.com
>>> ---
>>> hw/audio/pcspk.c | 35 +++
>>> 1 file changed, 15 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
>>> index 908696d483..b80a62ce90 100644
>>> --- a/hw/audio/pcspk.c
>>> +++ b/hw/audio/pcspk.c
>>> @@ -57,7 +57,6 @@ typedef struct {
>>> } PCSpkState;
>>>
>>> static const char *s_spk = "pcspk";
>>> -static PCSpkState *pcspk_state;
>>>
>>> static inline void generate_samples(PCSpkState *s)
>>> {
>>> @@ -111,22 +110,6 @@ static void pcspk_callback(void *opaque, int free)
>>> }
>>> }
>>>
>>> -static int pcspk_audio_init(ISABus *bus)
>>> -{
>>> -PCSpkState *s = pcspk_state;
>>> -struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUD_FMT_U8, 0};
>>> -
>>> -AUD_register_card(s_spk, &s->card);
>>> -
>>> -s->voice = AUD_open_out(&s->card, s->voice, s_spk, s, pcspk_callback,
>>> &as);
>>> -if (!s->voice) {
>>> -AUD_log(s_spk, "Could not open voice\n");
>>> -return -1;
>>> -}
>>> -
>>> -return 0;
>>> -}
>>> -
>>> static uint64_t pcspk_io_read(void *opaque, hwaddr addr,
>>>unsigned size)
>>> {
>>> @@ -179,12 +162,20 @@ static void pcspk_initfn(Object *obj)
>>>
>>> static void pcspk_realizefn(DeviceState *dev, Error **errp)
>>> {
>>> +struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUD_FMT_U8, 0};
>>> ISADevice *isadev = ISA_DEVICE(dev);
>>> PCSpkState *s = PC_SPEAKER(dev);
>>>
>>> isa_register_ioport(isadev, &s->ioport, s->iobase);
>>>
>>> -pcspk_state = s;
>>> +AUD_register_card(s_spk, &s->card);
>>> +
>>> +s->voice = AUD_open_out(&s->card, s->voice, s_spk, s, pcspk_callback,
>>> &as);
>>> +if (!s->voice) {
>>> +error_setg(errp, "Initializing audio voice failed");
>>> +AUD_remove_card(&s->card);
>>> +return;
>>> +}
>>> }
>>>
>>> static bool migrate_needed(void *opaque)
>>> @@ -221,8 +212,6 @@ static void pcspk_class_initfn(ObjectClass *klass, void
>>> *data)
>>> set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
>>> dc->vmsd = &vmstate_spk;
>>> dc->props = pcspk_properties;
>>> -/* Reason: realize sets global pcspk_state */
>>> -dc->user_creatable = false;
>>> }
>>>
>>> static const TypeInfo pcspk_info = {
>>> @@ -233,6 +222,12 @@ static const TypeInfo pcspk_info = {
>>> .class_init = pcspk_class_initfn,
>>> };
>>>
>>> +static int pcspk_audio_init(ISABus *bus)
>>> +{
>>> +isa_create_simple(bus, TYPE_PC_SPEAKER);
>>> +return 0;
>>> +}
>>> +
>>> static void pcspk_register(void)
>>> {
>>> type_register_static(&pcspk_info);
>>>
>>
>> I suddenly get (under fedora 28)
>>
>> ALSA lib pulse.c:243:(pulse_connect) PulseAudio: Unable to connect:
>> Connection refused
>>
>> alsa: Could not initialize DAC
>> alsa: Failed to open `default':
>> alsa: Reason: Connection refused
>> ALSA lib pulse.c:243:(pulse_connect) PulseAudio: Unable to connect:
>> Connection refused
>>
>> alsa: Could not initialize DAC
>> alsa: Failed to open `default':
>> alsa: Reason: Connection refused
>
> This ALSA problem seems on your side.
>
>> audio: Failed to create voice `pcspk'
>> qemu-system-x86_64: Initialization of device isa-pcspk failed:
>> Initializing audio voice failed
>
> Previously the errors would be ignored and QEMU would start.
I just did a fedora 28 uupdate + reboot. Problem still exists. Would be
strange if only I would be hitting this problem with stock alsa libraries.
>
>>
>>
>> With
>>
>> sudo x86_64-softmmu/qemu-system-x86_64 \
>> --enable-kvm \
>> -m 4G,maxmem=40G,slots=2 \
>> -smp sockets=2,cores=2 \
>> -numa node,nodeid=0,cpus=0-1 -numa node,nodeid=1,cpus=2-3 \
>> -kernel /boot/vmlinuz-4.19.6-200.fc28.x86_64 \
>> -append "console=ttyS0 rd.shell rd.luks=0 rd.lvm=0 rd.md=0 rd.dm=0" \
>> -initrd /boot/initramfs-4.19.6-200.fc28.x86_64.img \
>> -machine pc,nvdimm \
>> -nographic \
>> -nodefaults \
>> -chardev stdio,id=serial \
>> --trace events=to_trace \
>> -device isa-serial,chardev=serial \
>> -chardev socket,id=monitor,path=/var/tmp/monitor,server,nowait \
>> -mon chardev=monitor,mode=readline
>>
>>
>> Could this be related to this patch? (or is maybe something about my
>> system messed up? will try rebooting, but other audio - e.g. via firefox
>> - works fine)
>
> Does your Firefox uses ALSA? The default install uses PulseAudio.
Yes, I think so. I will try to find another tool to test ALSA.
>
> I think the behavior change you are experiencing comes from the patch 7
> of this series "audio
Re: [Qemu-devel] [PATCH] hw/arm/armsse: Fix miswiring of expansion IRQs
On 2/12/19 12:06 PM, Peter Maydell wrote:
> On Tue, 12 Feb 2019 at 11:05, Philippe Mathieu-Daudé
> wrote:
>> On 2/12/19 11:52 AM, Peter Maydell wrote:
>>> In commit 91c1e9fcbd7548db368 where we added dual-CPU support to
>>> the ARMSSE, we set up the wiring of the expansion IRQs via nested
>>> loops: the outer loop on 'i' loops for each CPU, and the inner loop
>>> on 'j' loops for each interrupt. Fix a typo which meant we were
>>> wiring every expansion IRQ line to external IRQ 0 on CPU 0 and
>>> to external IRQ 1 on CPU 1.
>>>
>>> Fixes: 91c1e9fcbd7548db368 ("hw/arm/armsse: Support dual-CPU configuration")
>>>
>>> Signed-off-by: Peter Maydell
>>
>> Reviewed-by: Philippe Mathieu-Daudé
>>
>>> ---
>>> It turns out that the ARM-TFM image I was using to test that
>>> I hadn't broken the mps2-an505 doesn't actually rely on any
>>> interrupts from the external devices...
>>
>> How did you notice that, simply reviewing? Via 'info qtree'?
>
> I'm working on a model of a different board (Musca) which also
> uses the SSE-200, and the test code I had for that does happen
> to care about the interrupts.
OK, welcome Musca!
If you are testing Open Source code, please add a few notes on the wiki
or in a mail (or the cover when you introduce the Musca), so we can add
an acceptance test.
Thanks,
Phil.
[Qemu-devel] [PATCH] Discard old bitmap directories in QCOW2 image
Clean QCOW2 image from bitmap obsolete directory when a new one
is allocated and stored. It slows down the image growth a little bit.
The flag QCOW2_DISCARD_ALWAYS allows a call to raw_co_pdiscard()
that does the actual cleaning of the image on disk.
With the flag QCOW2_DISCARD_OTHER, a reference count of the cluster
is updated only.
Signed-off-by: Andrey Shinkevich
---
block/qcow2-bitmap.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index b946301..682cb09 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -202,7 +202,7 @@ static void clear_bitmap_table(BlockDriverState *bs,
uint64_t *bitmap_table,
continue;
}
-qcow2_free_clusters(bs, addr, s->cluster_size, QCOW2_DISCARD_OTHER);
+qcow2_free_clusters(bs, addr, s->cluster_size, QCOW2_DISCARD_ALWAYS);
bitmap_table[i] = 0;
}
}
@@ -257,7 +257,7 @@ static int free_bitmap_clusters(BlockDriverState *bs,
Qcow2BitmapTable *tb)
clear_bitmap_table(bs, bitmap_table, tb->size);
qcow2_free_clusters(bs, tb->offset, tb->size * sizeof(uint64_t),
-QCOW2_DISCARD_OTHER);
+QCOW2_DISCARD_ALWAYS);
g_free(bitmap_table);
tb->offset = 0;
@@ -801,7 +801,7 @@ fail:
g_free(dir);
if (!in_place && dir_offset > 0) {
-qcow2_free_clusters(bs, dir_offset, dir_size, QCOW2_DISCARD_OTHER);
+qcow2_free_clusters(bs, dir_offset, dir_size, QCOW2_DISCARD_ALWAYS);
}
return ret;
@@ -904,14 +904,14 @@ static int update_ext_header_and_dir(BlockDriverState *bs,
}
if (old_size > 0) {
-qcow2_free_clusters(bs, old_offset, old_size, QCOW2_DISCARD_OTHER);
+qcow2_free_clusters(bs, old_offset, old_size, QCOW2_DISCARD_ALWAYS);
}
return 0;
fail:
if (new_offset > 0) {
-qcow2_free_clusters(bs, new_offset, new_size, QCOW2_DISCARD_OTHER);
+qcow2_free_clusters(bs, new_offset, new_size, QCOW2_DISCARD_ALWAYS);
}
s->bitmap_directory_offset = old_offset;
@@ -1242,7 +1242,7 @@ fail:
if (tb_offset > 0) {
qcow2_free_clusters(bs, tb_offset, tb_size * sizeof(tb[0]),
-QCOW2_DISCARD_OTHER);
+QCOW2_DISCARD_ALWAYS);
}
g_free(tb);
--
1.8.3.1
Re: [Qemu-devel] [qemu-s390x] [PATCH 09/15] s390-bios: ptr2u32 and u32toptr
On 2019-01-29 14:29, Jason J. Herne wrote:
> Introduce inline functions to convert between pointers and unsigned 32-bit
> ints. These are used to hide the ugliness required to avoid compiler
> warnings.
>
> Signed-off-by: Jason J. Herne
> ---
> pc-bios/s390-ccw/libc.h | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/pc-bios/s390-ccw/libc.h b/pc-bios/s390-ccw/libc.h
> index 818517f..e198f0b 100644
> --- a/pc-bios/s390-ccw/libc.h
> +++ b/pc-bios/s390-ccw/libc.h
> @@ -19,6 +19,18 @@ typedef unsigned short uint16_t;
> typedef unsigned int uint32_t;
> typedef unsigned long long uint64_t;
>
> +/* Avoids compiler warnings when casting a pointer to a u32 */
> +static inline uint32_t ptr2u32(void *ptr)
> +{
> +return (uint32_t)(uint64_t)ptr;
> +}
> +
> +/* Avoids compiler warnings when casting a u32 to a pointer */
> +static inline void *u32toptr(uint32_t n)
> +{
> +return (void *)(uint64_t)n;
> +}
Could you please put this into another header? libc.h is for
standard-compliant libc functions, and these are no standard functions,
as far as I know.
Thanks,
Thomas
Re: [Qemu-devel] [qemu-s390x] [PATCH 08/15] s390-bios: Map low core memory
On 2019-01-29 14:29, Jason J. Herne wrote:
> Create a new header for basic architecture specific definitions and add a
> mapping of low core memory. This mapping will be used by the real dasd boot
> process.
>
> Signed-off-by: Jason J. Herne
> ---
> pc-bios/s390-ccw/main.c | 2 +
> pc-bios/s390-ccw/s390-arch.h | 100
> +++
> 2 files changed, 102 insertions(+)
> create mode 100644 pc-bios/s390-ccw/s390-arch.h
>
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index 1bc61b5..fa90aa3 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -9,6 +9,7 @@
> */
>
> #include "libc.h"
> +#include "s390-arch.h"
> #include "s390-ccw.h"
> #include "cio.h"
> #include "virtio.h"
> @@ -19,6 +20,7 @@ static char loadparm_str[LOADPARM_LEN + 1] = { 0, 0, 0, 0,
> 0, 0, 0, 0, 0 };
> QemuIplParameters qipl;
> IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
> static bool have_iplb;
> +const LowCore *lowcore; /* Yes, this *is* a pointer to address 0 */
>
> #define LOADPARM_PROMPT "PROMPT "
> #define LOADPARM_EMPTY ""
> diff --git a/pc-bios/s390-ccw/s390-arch.h b/pc-bios/s390-ccw/s390-arch.h
> new file mode 100644
> index 000..47eaa04
> --- /dev/null
> +++ b/pc-bios/s390-ccw/s390-arch.h
> @@ -0,0 +1,100 @@
> +/*
> + * S390 Basic Architecture
> + *
> + * Copyright (c) 2018 Jason J. Herne
2019 ?
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> + * your option) any later version. See the COPYING file in the top-level
> + * directory.
> + */
> +
> +#ifndef S390_ARCH_H
> +#define S390_ARCH_H
> +
> +typedef struct PSW {
> +uint64_t mask;
> +uint64_t addr;
> +} __attribute__ ((packed, aligned(8))) PSW;
We've seen quite some trouble with "packed" in the past... See
3b8afb41bc8e or 55281a2c53b884d0 for example ... This is only the
s390-ccw bios code here, so it is likely ok, but still, since this
structure is "naturally" packed, I'd rather go without that attribute
here (even if it's only to allow the compiler to generate better code in
some cases). You could still _Static_assert(sizeof(struct PSW) == 16)
afterwards, just to be sure.
> +
> +/* Older PSW format used by LPSW instruction */
> +typedef struct PSWLegacy {
> +uint32_t mask;
> +uint32_t addr;
> +} __attribute__ ((packed, aligned(8))) PSWLegacy;
dito, I'd drop the packed attribute here.
> +/* s390 psw bit masks */
> +#define PSW_MASK_IOINT 0x0200ULL
> +#define PSW_MASK_WAIT 0x0002ULL
> +#define PSW_MASK_EAMODE 0x0001ULL
> +#define PSW_MASK_BAMODE 0x8000ULL
> +#define PSW_MASK_ZMODE (PSW_MASK_EAMODE | PSW_MASK_BAMODE)
> +
> +/* Low core mapping */
> +typedef struct LowCore {
> +/* prefix area: defined by architecture */
> +uint64_tipl_psw; /* 0x000 */
No PSWLegacy here? ;-)
> +uint32_tccw1[2]; /* 0x008 */
> +uint32_tccw2[2]; /* 0x010 */
> +uint8_t pad1[0x80 - 0x18];/* 0x018 */
> +uint32_text_params; /* 0x080 */
> +uint16_tcpu_addr; /* 0x084 */
> +uint16_text_int_code; /* 0x086 */
> +uint16_tsvc_ilen; /* 0x088 */
> +uint16_tsvc_code; /* 0x08a */
> +uint16_tpgm_ilen; /* 0x08c */
> +uint16_tpgm_code; /* 0x08e */
> +uint32_tdata_exc_code;/* 0x090 */
> +uint16_tmon_class_num;/* 0x094 */
> +uint16_tper_perc_atmid; /* 0x096 */
> +uint64_tper_address; /* 0x098 */
> +uint8_t exc_access_id;/* 0x0a0 */
> +uint8_t per_access_id;/* 0x0a1 */
> +uint8_t op_access_id; /* 0x0a2 */
> +uint8_t ar_access_id; /* 0x0a3 */
> +uint8_t pad2[0xA8 - 0xA4];/* 0x0a4 */
> +uint64_ttrans_exc_code; /* 0x0a8 */
> +uint64_tmonitor_code; /* 0x0b0 */
> +uint16_tsubchannel_id;/* 0x0b8 */
> +uint16_tsubchannel_nr;/* 0x0ba */
> +uint32_tio_int_parm; /* 0x0bc */
> +uint32_tio_int_word; /* 0x0c0 */
> +uint8_t pad3[0xc8 - 0xc4];/* 0x0c4 */
> +uint32_tstfl_fac_list;/* 0x0c8 */
> +uint8_t pad4[0xe8 - 0xcc];/* 0x0cc */
> +uint64_tmcic; /* 0x0e8 */
> +uint8_t pad5[0xf4 - 0xf0];/* 0x0f0 */
> +uint32_texternal_damage_code; /* 0x0f4 */
> +uint64_tfailing_storage_address; /* 0x0f8 */
> +uint8_t pad6[0x110 - 0x100]; /* 0x100 */
> +uint64_tper_breaking_event_a
Re: [Qemu-devel] [PATCH v2 3/4] tests/migration-test: Add a test for ignore-shared capability
11.02.2019, 16:17, "Dr. David Alan Gilbert" :
> * Yury Kotov (yury-ko...@yandex-team.ru) wrote:
>> Signed-off-by: Yury Kotov
>> ---
>> tests/migration-test.c | 109 +
>> 1 file changed, 89 insertions(+), 20 deletions(-)
>>
>> diff --git a/tests/migration-test.c b/tests/migration-test.c
>> index 8352612364..485f42b2d2 100644
>> --- a/tests/migration-test.c
>> +++ b/tests/migration-test.c
>> @@ -332,6 +332,13 @@ static void cleanup(const char *filename)
>> g_free(path);
>> }
>>
>> +static char *get_shmem_opts(const char *mem_size, const char *shmem_path)
>> +{
>> + return g_strdup_printf("-object memory-backend-file,id=mem0,size=%s"
>> + ",mem-path=%s,share=on -numa node,memdev=mem0",
>> + mem_size, shmem_path);
>> +}
>> +
>> static void migrate_check_parameter(QTestState *who, const char *parameter,
>> long long value)
>> {
>> @@ -430,73 +437,91 @@ static void migrate_postcopy_start(QTestState *from,
>> QTestState *to)
>> }
>>
>> static int test_migrate_start(QTestState **from, QTestState **to,
>> - const char *uri, bool hide_stderr)
>> + const char *uri, bool hide_stderr,
>> + bool use_shmem)
>> {
>> gchar *cmd_src, *cmd_dst;
>> char *bootpath = g_strdup_printf("%s/bootsect", tmpfs);
>> + char *extra_opts = NULL;
>> + char *shmem_path = NULL;
>> const char *arch = qtest_get_arch();
>> const char *accel = "kvm:tcg";
>>
>> got_stop = false;
>>
>> + if (use_shmem) {
>> + shmem_path = g_strdup_printf("/dev/shm/qemu-%d", getpid());
>> + }
>
> I think /dev/shm is non-portable; so I think you'll need to have a way
> to skip the test on OSs that don't have it.
>
Ok, will fix in v3.
>> if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> init_bootfile(bootpath, x86_bootsect);
>> + extra_opts = use_shmem ? get_shmem_opts("150M", shmem_path) : NULL;
>> cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
>> " -name source,debug-threads=on"
>> " -serial file:%s/src_serial"
>> - " -drive file=%s,format=raw",
>> - accel, tmpfs, bootpath);
>> + " -drive file=%s,format=raw %s",
>> + accel, tmpfs, bootpath,
>> + extra_opts ? extra_opts : "");
>
> It's painful to have to use the ?: here as well as above where you set
> extra_opts, but I guess you need to, to allow you to free extra_opts
> below.
>
I'll think how to write it more clear. May be it's better to g_strdup("")
instead of NULL to eliminate some of ?:.
>> cmd_dst = g_strdup_printf("-machine accel=%s -m 150M"
>> " -name target,debug-threads=on"
>> " -serial file:%s/dest_serial"
>> " -drive file=%s,format=raw"
>> - " -incoming %s",
>> - accel, tmpfs, bootpath, uri);
>> + " -incoming %s %s",
>> + accel, tmpfs, bootpath, uri,
>> + extra_opts ? extra_opts : "");
>> start_address = X86_TEST_MEM_START;
>> end_address = X86_TEST_MEM_END;
>> } else if (g_str_equal(arch, "s390x")) {
>> init_bootfile_s390x(bootpath);
>> + extra_opts = use_shmem ? get_shmem_opts("128M", shmem_path) : NULL;
>> cmd_src = g_strdup_printf("-machine accel=%s -m 128M"
>> " -name source,debug-threads=on"
>> - " -serial file:%s/src_serial -bios %s",
>> - accel, tmpfs, bootpath);
>> + " -serial file:%s/src_serial -bios %s %s",
>> + accel, tmpfs, bootpath,
>> + extra_opts ? extra_opts : "");
>> cmd_dst = g_strdup_printf("-machine accel=%s -m 128M"
>> " -name target,debug-threads=on"
>> " -serial file:%s/dest_serial -bios %s"
>> - " -incoming %s",
>> - accel, tmpfs, bootpath, uri);
>> + " -incoming %s %s",
>> + accel, tmpfs, bootpath, uri,
>> + extra_opts ? extra_opts : "");
>> start_address = S390_TEST_MEM_START;
>> end_address = S390_TEST_MEM_END;
>> } else if (strcmp(arch, "ppc64") == 0) {
>> + extra_opts = use_shmem ? get_shmem_opts("256M", shmem_path) : NULL;
>> cmd_src = g_strdup_printf("-machine accel=%s -m 256M -nodefaults"
>> " -name source,debug-threads=on"
>> " -serial file:%s/src_serial"
>> " -prom-env 'use-nvramrc?=true'
>> -prom-env "
>> "'nvramrc=hex .\" _\" begin %x %x "
>> "do i c@ 1 + i c! 1000 +loop .\" B\" 0 "
>> - "until'", accel, tmpfs, end_address,
>> - start_address);
>> + "until' %s", accel, tmpfs, end_address,
>> + start_address, extra_opts ? extra_opts : "");
>> cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
>> " -name target,debug-threads=on"
>
[Qemu-devel] [Bug 1808928] Re: Bitmap Extra data is not supported
as far as i know nothing happened. it had worked normally while instance was running. For a reason, instance is shutdown, then it never open again. i have some backups, i tried to return previous backups. But they also gave same error. Thanks to replication i could get it back. i copied image from replication site... as i said "qemu-img" command completely unusable on that image. There should be another mechanism. It blocks everything. I try to edit header to remove extra data. But i could not find header information on website. thanks -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1808928 Title: Bitmap Extra data is not supported Status in QEMU: New Bug description: i am using dirty bitmaps and drive-backup. It works as aspected. Lately, i encounter a disastrous error. There is not any information about that situation. I cannot reach/open/attach/info or anything with a qcow2 file. virsh version Compiled against library: libvirt 4.10.0 Using library: libvirt 4.10.0 Using API: QEMU 4.10.0 Running hypervisor: QEMU 2.12.0 "qemu-img: Could not open '/var/lib/libvirt/images/test.qcow2': Bitmap extra data is not supported" what is that mean? what should i do? i cannot remove bitmap. i cannot open image or query. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1808928/+subscriptions
[Qemu-devel] Key repeat is no longer working on TTY and grub menu
I noticed that the key inputs are no longer repeating when using default setting on QEMU. For example, if I press "a" and keep it pressed it will print only one "a" instead of repeating them. Another example is when deleting text: instead of holding backspace pressed and the text is deleted, it's needed to press backspace once for each character. As this makes many tasks much slower, requiring many key presses for tasks that were done in a single press, I did a bisect to see when this change happened. The result is: 0c0d42737dfdcea872a987ecba6ef55047df8881 is the first bad commit commit 0c0d42737dfdcea872a987ecba6ef55047df8881 Author: Gerd Hoffmann Date: Tue Jan 22 10:28:11 2019 +0100 kbd-state: use state tracker for gtk Use the new keyboard state tracked for gtk. Allows to drop the gtk-specific modifier state tracking code. Signed-off-by: Gerd Hoffmann Reviewed-by: Daniel P. Berrangé Message-id: 20190122092814.14919-6-kra...@redhat.com :04 04 e882f144bafca90257a4604b5e62486a1ca038b2 0139afe1aee24256c3f711c080b0230ab104074a M include :04 04 c258efc06107b944b0627792b9f2377600033118 4fd01a25a3e0d1fccca72c35ec58b205aa4fd882 M ui I'm using the GTK interface and all guests without a Graphical User Interface (GUI) are affected by this. This happens on grub menu and on TTY of multiple Linux guests. Workarounds found for the key repeat issue: -Installing a GUI to server systems; -Adding a usb-kbd device, but then the key '/' is lost.
Re: [Qemu-devel] [PULL 00/25] pci, pc, virtio: fixes, cleanups, features
On Tue, Feb 12, 2019 at 11:39:21AM +0100, Philippe Mathieu-Daudé wrote: > On 2/12/19 8:11 AM, Peter Xu wrote: > > On Tue, Feb 05, 2019 at 11:06:42AM -0500, Michael S. Tsirkin wrote: > >> > >> OK I reverted the whole part dealing with vhost-user and reposted. > > > > I noticed that the merged pull request could possibly have squashed > > the below two patches (in previous pull) into one super patch > > (a56de056c91f87e1e): > > > > i386/kvm: ignore masked irqs when update msi routes > > contrib/vhost-user-blk: fix the compilation issue > > > > Here, the first patch lost its commit message, and the last patch lost > > its real author. ;) > > I suggest we revert a56de056c9 ASAP and reapply the both patches, this > will ease cherry-picking/downstream workflow. I don't see why does it help upstream. -- MST
Re: [Qemu-devel] [PATCH 10/15] s390-bios: Support for running format-0/1 channel programs
On Tue, 5 Feb 2019 11:18:38 +0100
Cornelia Huck wrote:
> On Mon, 4 Feb 2019 14:29:18 -0500
> Farhan Ali wrote:
>
> > On 02/04/2019 06:13 AM, Cornelia Huck wrote:
> > > On Thu, 31 Jan 2019 12:31:00 -0500
> > > Farhan Ali wrote:
> > >
> > >> On 01/29/2019 08:29 AM, Jason J. Herne wrote:
> > >>> Add struct for format-0 ccws. Support executing format-0 channel
> > >>> programs and waiting for their completion before continuing execution.
> > >>> This will be used for real dasd ipl.
> > >>>
> > >>> Add cu_type() to channel io library. This will be used to query control
> > >>> unit type which is used to determine if we are booting a virtio device
> > >>> or a
> > >>> real dasd device.
> > >>>
> > >>> Signed-off-by: Jason J. Herne
> > >>> ---
> > >>>pc-bios/s390-ccw/cio.c | 114
> > >>> +++
> > >>>pc-bios/s390-ccw/cio.h | 127
> > >>> ++--
> > >>>pc-bios/s390-ccw/s390-ccw.h | 1 +
> > >>>pc-bios/s390-ccw/start.S| 33 +++-
> > >>>4 files changed, 270 insertions(+), 5 deletions(-)
> > >
> > >>> +/*
> > >>> + * Executes a channel program at a given subchannel. The request to
> > >>> run the
> > >>> + * channel program is sent to the subchannel, we then wait for the
> > >>> interrupt
> > >>> + * signaling completion of the I/O operation(s) performed by the
> > >>> channel
> > >>> + * program. Lastly we verify that the i/o operation completed without
> > >>> error and
> > >>> + * that the interrupt we received was for the subchannel used to run
> > >>> the
> > >>> + * channel program.
> > >>> + *
> > >>> + * Note: This function assumes it is running in an environment where
> > >>> no other
> > >>> + * cpus are generating or receiving I/O interrupts. So either run it
> > >>> in a
> > >>> + * single-cpu environment or make sure all other cpus are not doing
> > >>> I/O and
> > >>> + * have I/O interrupts masked off.
> > >>> + */
> > >>> +int do_cio(SubChannelId schid, uint32_t ccw_addr, int fmt)
> > >>> +{
> > >>> +CmdOrb orb = {};
> > >>> +Irb irb = {};
> > >>> +sense_data_eckd_dasd sd;
> > >>> +int rc, retries = 0;
> > >>> +
> > >>> +IPL_assert(fmt == 0 || fmt == 1, "Invalid ccw format");
> > >>> +
> > >>> +/* ccw_addr must be <= 24 bits and point to at least one whole
> > >>> ccw. */
> > >>> +if (fmt == 0) {
> > >>> +IPL_assert(ccw_addr <= 0xFF - 8, "Invalid ccw address");
> > >>> +}
> > >>> +
> > >>> +orb.fmt = fmt ;
> > >>> +orb.pfch = 1; /* QEMU's cio implementation requires prefetch */
> > >>> +orb.c64 = 1; /* QEMU's cio implementation requires 64-bit idaws
> > >>> */
> > >>> +orb.lpm = 0xFF; /* All paths allowed */
> > >>> +orb.cpa = ccw_addr;
> > >>> +
> > >>> +while (true) {
> > >>> +rc = ssch(schid, &orb);
> > >>> +if (rc == 1) {
> > >>> +/* Status pending, not sure why. Let's eat the status and
> > >>> retry. */
> > >>> +tsch(schid, &irb);
> > >>> +retries++;
> > >>> +continue;
> > >>> +}
> > >>> +if (rc) {
> > >>> +print_int("ssch failed with rc=", rc);
> > >>> +break;
> > >>> +}
> > >>> +
> > >>> +consume_io_int();
> > >>> +
> > >>> +/* collect status */
> > >>> +rc = tsch(schid, &irb);
> > >>> +if (rc) {
> > >>> +print_int("tsch failed with rc=", rc);
> > >>> +break;
> > >>> +}
> > >>> +
> > >>> +if (!irb_error(&irb)) {
> > >>> +break;
> > >>> +}
> > >>> +
> > >>> +/*
> > >>> + * Unexpected unit check, or interface-control-check. Use
> > >>> sense to
> > >>> + * clear unit check then retry.
> > >>> + */
> > >>> +if ((unit_check(&irb) || iface_ctrl_check(&irb)) && retries <=
> > >>> 2) {
> > >>> +basic_sense(schid, &sd, sizeof(sd));
> > >>
> > >> We are using basic sense to clear any unit check or ifcc, but is it
> > >> possible for the basic sense to cause another unit check?
> > >>
> > >> The chapter on Basic Sense in the Common I/O Device Commands
> > >> (http://publibz.boulder.ibm.com/support/libraryserver/FRAMESET/dz9ar501/2.1?SHELF=&DT=19920409154647&CASE=)
> > >>says this:
> > >>
> > >> ""
> > >> The basic sense command initiates a sense operation at all devices
> > >> and cannot cause the command-reject, intervention-required,
> > >> data-check, or overrun bit to be set to one. If the control unit
> > >> detects an equipment malfunction or invalid checking-block code
> > >> (CBC) on the sense-command code, the equipment-check or bus-out-check
> > >> bit is set to one, and unit check is indicated in the device-status
> > >> byte.
> > >> ""
> > >>
> > >> If my understanding is correct, if there is an equipment malfunction
> > >> then the control unit can return a unit check even for a basic sense.
> > >> This c
Re: [Qemu-devel] [PATCH v2] s390x: add zPCI feature to "qemu" CPU model
On Tue, 12 Feb 2019 12:23:23 +0100 David Hildenbrand wrote: > As we now always have PCI support, let's add it to the "qemu" CPU model, > taking care of backwards compatibility. > > Signed-off-by: David Hildenbrand > --- > > v1 -> v2: > - Use correct model identifiction of the z12 we emulate > > hw/s390x/s390-virtio-ccw.c | 3 +++ > target/s390x/gen-features.c | 8 ++-- > 2 files changed, 9 insertions(+), 2 deletions(-) > Played around a bit with different machines and cpu models and got what I expected. Thanks, applied.
Re: [Qemu-devel] Key repeat is no longer working on TTY and grub menu
On Tue, Feb 12, 2019 at 01:02:42PM +, Leonardo Soares Müller wrote: > I noticed that the key inputs are no longer repeating when using default > setting on QEMU. For example, if I press "a" and keep it pressed it will > print only one "a" instead of repeating them. Another example is when > deleting text: instead of holding backspace pressed and the text is > deleted, it's needed to press backspace once for each character. > > As this makes many tasks much slower, requiring many key presses for > tasks that were done in a single press, I did a bisect to see when this > change happened. The result is: > > 0c0d42737dfdcea872a987ecba6ef55047df8881 is the first bad commit > commit 0c0d42737dfdcea872a987ecba6ef55047df8881 > Author: Gerd Hoffmann > Date: Tue Jan 22 10:28:11 2019 +0100 > > kbd-state: use state tracker for gtk > > Use the new keyboard state tracked for gtk. Allows to drop the > gtk-specific modifier state tracking code. > > Signed-off-by: Gerd Hoffmann > Reviewed-by: Daniel P. Berrangé > Message-id: 20190122092814.14919-6-kra...@redhat.com > > :04 04 e882f144bafca90257a4604b5e62486a1ca038b2 > 0139afe1aee24256c3f711c080b0230ab104074a Minclude > :04 04 c258efc06107b944b0627792b9f2377600033118 > 4fd01a25a3e0d1fccca72c35ec58b205aa4fd882 Mui > > I'm using the GTK interface and all guests without a Graphical User > Interface (GUI) are affected by this. This happens on grub menu and on > TTY of multiple Linux guests. Yes, this is another regression accidentally introduced by the keyboard state tracker. When GTK does key repeat it omits the Up event for repeated keys. IOW, you get Press (a) Press (a) Press (a) Release (a) Not Press (a) Release (a) Press (a) Release (a) Press (a) Release (a) The keyboard state tracker doesn't take this into account, so it is surpressing all except the first Press event. This might affect other frontends too if they use the same trick for key repeat 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 :|
Re: [Qemu-devel] [PATCH v2 3/3] tpm_tis: fix format string specifier in tpm_tis_show_buffer()
On 2/12/19 7:31 AM, Philippe Mathieu-Daudé wrote:
On 2/11/19 10:13 PM, Stefan Berger wrote:
On 2/11/19 3:09 PM, Liam Merwick wrote:
On 11/02/2019 19:56, Stefan Berger wrote:
On 2/11/19 11:02 AM, Philippe Mathieu-Daudé wrote:
On 2/11/19 4:03 PM, Liam Merwick wrote:
[...]
-printf("tpm_tis: %s length = %d\n", string, len);
+printf("tpm_tis: %s length = %u\n", string, len);
So here the format is '%zu'.
However in code cleanup we try go get ride of printf() calls and
replace them with trace points.
This code is only used for debugging if DEBUG_TIS has been #defined.
No need to add tracing here.
I'd come up the attached change (but that seems like overkill).
I don't think we need tracing for this.
So if you think the code is mature enough, let's remove the DEBUG calls!
Else we prefer to convert DEBUG printf to trace events because (at least):
- no need to recompile to enable debugging
- when compiled with debugging, you don't mess with STDIO which can be
used as a chardev backend.
Fine. Then I withdraw my reviewed-by.
Thanks,
Phil.
Re: [Qemu-devel] [PULL 00/24] target/hppa patch queue
On Tue, 12 Feb 2019 at 04:57, Richard Henderson wrote: > > The following changes since commit 22c5f446514a2a4bb0dbe1fea26713da92fc85fa: > > Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20190211' into > staging (2019-02-11 17:04:57 +) > > are available in the Git repository at: > > https://github.com/rth7680/qemu.git tags/pull-hppa-20190211 > > for you to fetch changes up to 23e76627deba013509b5f1a1e1c53e8e45aa: > > hw/hppa: forward requests to CPU HPA (2019-02-11 20:49:06 -0800) > > > Convert to decodetree. > Fix signed overflow conditions. > Fix dcor. > Add CPU MIE to PCI address space. Hi -- clang compiles fail with: In file included from /home/petmay01/linaro/qemu-for-merges/target/hppa/translate.c:337: target/hppa/decode.inc.c:471:16: error: redefinition of typedef 'arg_be' is a C11 feature [-Werror,-Wtypedef-redefinition] typedef arg_be arg_be; ^ target/hppa/decode.inc.c:9:3: note: previous definition is here } arg_be; ^ target/hppa/decode.inc.c:473:16: error: redefinition of typedef 'arg_bl' is a C11 feature [-Werror,-Wtypedef-redefinition] typedef arg_bl arg_bl; ^ target/hppa/decode.inc.c:15:3: note: previous definition is here } arg_bl; ^ thanks -- PMM
Re: [Qemu-devel] [PULL 00/25] pci, pc, virtio: fixes, cleanups, features
On Tue, Feb 12, 2019 at 02:15:36PM +0100, Philippe Mathieu-Daudé wrote: > On 2/12/19 2:04 PM, Michael S. Tsirkin wrote: > > On Tue, Feb 12, 2019 at 11:39:21AM +0100, Philippe Mathieu-Daudé wrote: > >> On 2/12/19 8:11 AM, Peter Xu wrote: > >>> On Tue, Feb 05, 2019 at 11:06:42AM -0500, Michael S. Tsirkin wrote: > > OK I reverted the whole part dealing with vhost-user and reposted. > >>> > >>> I noticed that the merged pull request could possibly have squashed > >>> the below two patches (in previous pull) into one super patch > >>> (a56de056c91f87e1e): > >>> > >>> i386/kvm: ignore masked irqs when update msi routes > >>> contrib/vhost-user-blk: fix the compilation issue > >>> > >>> Here, the first patch lost its commit message, and the last patch lost > >>> its real author. ;) > >> > >> I suggest we revert a56de056c9 ASAP and reapply the both patches, this > >> will ease cherry-picking/downstream workflow. > > > > I don't see why does it help upstream. > > I'd have suggested the same if I had no idea what 'downstream workflow' > mean, simply to keep the tree clear and avoid to have unrelated changes > squashed altogether. > Commit a56de056c9 really looks messy. MSI/MSIX changes described by "fix > vhost-user-blk compilation". > Hopefully it won't trigger any problem which requires bisecting to it, > then contact Changpeng Liu asking him what he intented to do with his > commit. > Your call anyway :) > > Regards, > > Phil. OK these are good points. I'm not sure what happened but it looks like I screwed up when resolving some conflicts. Care posting a patchset looking sane? -- MST
Re: [Qemu-devel] [PULL 00/25] pci, pc, virtio: fixes, cleanups, features
On 2/12/19 2:04 PM, Michael S. Tsirkin wrote: > On Tue, Feb 12, 2019 at 11:39:21AM +0100, Philippe Mathieu-Daudé wrote: >> On 2/12/19 8:11 AM, Peter Xu wrote: >>> On Tue, Feb 05, 2019 at 11:06:42AM -0500, Michael S. Tsirkin wrote: OK I reverted the whole part dealing with vhost-user and reposted. >>> >>> I noticed that the merged pull request could possibly have squashed >>> the below two patches (in previous pull) into one super patch >>> (a56de056c91f87e1e): >>> >>> i386/kvm: ignore masked irqs when update msi routes >>> contrib/vhost-user-blk: fix the compilation issue >>> >>> Here, the first patch lost its commit message, and the last patch lost >>> its real author. ;) >> >> I suggest we revert a56de056c9 ASAP and reapply the both patches, this >> will ease cherry-picking/downstream workflow. > > I don't see why does it help upstream. I'd have suggested the same if I had no idea what 'downstream workflow' mean, simply to keep the tree clear and avoid to have unrelated changes squashed altogether. Commit a56de056c9 really looks messy. MSI/MSIX changes described by "fix vhost-user-blk compilation". Hopefully it won't trigger any problem which requires bisecting to it, then contact Changpeng Liu asking him what he intented to do with his commit. Your call anyway :) Regards, Phil.
Re: [Qemu-devel] [PATCH v2] virtio-blk: set correct config size for the host driver
On Tue, 12 Feb 2019 14:01:44 +0800
Changpeng Liu wrote:
> Commit caa1ee43 "vhost-user-blk: add discard/write zeroes features support"
> introduced extra fields to existing struct virtio_blk_config, when
> migration was executed from older QEMU version to current head, it
A hint about the breakage is often a good practice, as it helps
people to find the fix if they hit the issue.
> will break the migration. While here, set the correct config size
"While here" usually means that the corresponding change isn't strictly
needed. I don't believe this is the case here since fixing the config
size is precisely the goal of this patch.
> when initializing the host driver, for now, discard/write zeroes
> are not supported by virtio-blk host driver, so set the config
As mentioned by Michael, there's no such thing as a "host driver".
Just say virtio-blk.
> size as before, users can change config size when adding the new
> feature bits support.
>
What about ?
Commit caa1ee43 "vhost-user-blk: add discard/write zeroes features
support" added fields to struct virtio_blk_config. This changes
the size of the config space and breaks migration from QEMU 3.1
and older:
qemu-system-ppc64: get_pci_config_device: Bad config data: i=0x10 read: 41
device: 1 cmask: ff wmask: 80 w1cmask:0
qemu-system-ppc64: Failed to load PCIDevice:config
qemu-system-ppc64: Failed to load virtio-blk:virtio
qemu-system-ppc64: error while loading state for instance 0x0 of device
'pci@8002000:01.0/virtio-blk'
qemu-system-ppc64: load of migration failed: Invalid argument
Since virtio-blk doesn't support the "discard" and "write zeroes"
features, it shouldn't even expose the associated fields in the
config space actually. Just include all fields up to num_queues to
match QEMU 3.1 and older.
> Signed-off-by: Changpeng Liu
> ---
LGTM
Reviewed-by: Greg Kurz
Tested-by: Greg Kurz
> hw/block/virtio-blk.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 9a87b3b..846b7b9 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -28,6 +28,9 @@
> #include "hw/virtio/virtio-bus.h"
> #include "hw/virtio/virtio-access.h"
>
> +#define VIRTIO_BLK_CFG_SIZE (offsetof(struct virtio_blk_config, num_queues)
> + \
> + sizeof_field(struct virtio_blk_config,
> num_queues))
> +
> static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq,
> VirtIOBlockReq *req)
> {
> @@ -761,7 +764,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev,
> uint8_t *config)
> blkcfg.alignment_offset = 0;
> blkcfg.wce = blk_enable_write_cache(s->blk);
> virtio_stw_p(vdev, &blkcfg.num_queues, s->conf.num_queues);
> -memcpy(config, &blkcfg, sizeof(struct virtio_blk_config));
> +memcpy(config, &blkcfg, VIRTIO_BLK_CFG_SIZE);
> }
>
> static void virtio_blk_set_config(VirtIODevice *vdev, const uint8_t *config)
> @@ -769,7 +772,7 @@ static void virtio_blk_set_config(VirtIODevice *vdev,
> const uint8_t *config)
> VirtIOBlock *s = VIRTIO_BLK(vdev);
> struct virtio_blk_config blkcfg;
>
> -memcpy(&blkcfg, config, sizeof(blkcfg));
> +memcpy(&blkcfg, config, VIRTIO_BLK_CFG_SIZE);
>
> aio_context_acquire(blk_get_aio_context(s->blk));
> blk_set_enable_write_cache(s->blk, blkcfg.wce != 0);
> @@ -952,8 +955,7 @@ static void virtio_blk_device_realize(DeviceState *dev,
> Error **errp)
> return;
> }
>
> -virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
> -sizeof(struct virtio_blk_config));
> +virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, VIRTIO_BLK_CFG_SIZE);
>
> s->blk = conf->conf.blk;
> s->rq = NULL;
Re: [Qemu-devel] [PATCH v2 3/3] tpm_tis: fix format string specifier in tpm_tis_show_buffer()
On 12/02/2019 13:27, Stefan Berger wrote:
On 2/12/19 7:31 AM, Philippe Mathieu-Daudé wrote:
On 2/11/19 10:13 PM, Stefan Berger wrote:
On 2/11/19 3:09 PM, Liam Merwick wrote:
On 11/02/2019 19:56, Stefan Berger wrote:
On 2/11/19 11:02 AM, Philippe Mathieu-Daudé wrote:
On 2/11/19 4:03 PM, Liam Merwick wrote:
[...]
- printf("tpm_tis: %s length = %d\n", string, len);
+ printf("tpm_tis: %s length = %u\n", string, len);
So here the format is '%zu'.
However in code cleanup we try go get ride of printf() calls and
replace them with trace points.
This code is only used for debugging if DEBUG_TIS has been #defined.
No need to add tracing here.
I'd come up the attached change (but that seems like overkill).
I don't think we need tracing for this.
So if you think the code is mature enough, let's remove the DEBUG calls!
Else we prefer to convert DEBUG printf to trace events because (at
least):
- no need to recompile to enable debugging
- when compiled with debugging, you don't mess with STDIO which can be
used as a chardev backend.
Fine. Then I withdraw my reviewed-by.
I don't see a way of removing the DEBUG calls without adding the
overhead of a call to tpm_tis_show_buffer() each time even if tracing is
not enabled (the 3 trace calls are interdependent and need a for loop).
Is there any example of a trace point that calls a function that then
does non-trivial printing?
I could send a v3 with the patch I attached previously with the 3 printf
calls changed to trace points but still wrapped in 'if (DEBUG_TIS)' and
optimised out in non-debug.
Regards,
Liam
[Qemu-devel] [PATCH 0/4] a few trivials
From: "Dr. David Alan Gilbert" Just cleaning out my trivial pile; a few more traces and a couple of error reporting tweaks. Dave Dr. David Alan Gilbert (4): pckbd: Convert DPRINTF->trace HMP: Prepend errors with 'Error:' kvm: Add kvm_set_ioeventfd* traces wavcapture: Convert to error_report accel/kvm/kvm-all.c| 3 +++ accel/kvm/trace-events | 2 ++ audio/wavcapture.c | 39 +-- hmp.c | 2 +- hw/input/pckbd.c | 19 ++- hw/input/trace-events | 7 +++ 6 files changed, 36 insertions(+), 36 deletions(-) -- 2.20.1
[Qemu-devel] [PATCH 1/4] pckbd: Convert DPRINTF->trace
From: "Dr. David Alan Gilbert"
Signed-off-by: Dr. David Alan Gilbert
---
hw/input/pckbd.c | 19 ++-
hw/input/trace-events | 7 +++
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
index 72e7d5f6cc..47a606f5e3 100644
--- a/hw/input/pckbd.c
+++ b/hw/input/pckbd.c
@@ -30,14 +30,7 @@
#include "hw/input/i8042.h"
#include "sysemu/sysemu.h"
-/* debug PC keyboard */
-//#define DEBUG_KBD
-#ifdef DEBUG_KBD
-#define DPRINTF(fmt, ...) \
-do { printf("KBD: " fmt , ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...)
-#endif
+#include "trace.h"
/* Keyboard Controller Commands */
#define KBD_CCMD_READ_MODE 0x20/* Read mode bits */
@@ -210,7 +203,7 @@ static uint64_t kbd_read_status(void *opaque, hwaddr addr,
KBDState *s = opaque;
int val;
val = s->status;
-DPRINTF("kbd: read status=0x%02x\n", val);
+trace_pckbd_kbd_read_status(val);
return val;
}
@@ -224,7 +217,7 @@ static void kbd_queue(KBDState *s, int b, int aux)
static void outport_write(KBDState *s, uint32_t val)
{
-DPRINTF("kbd: write outport=0x%02x\n", val);
+trace_pckbd_outport_write(val);
s->outport = val;
qemu_set_irq(s->a20_out, (val >> 1) & 1);
if (!(val & 1)) {
@@ -237,7 +230,7 @@ static void kbd_write_command(void *opaque, hwaddr addr,
{
KBDState *s = opaque;
-DPRINTF("kbd: write cmd=0x%02" PRIx64 "\n", val);
+trace_pckbd_kbd_write_command(val);
/* Bits 3-0 of the output port P2 of the keyboard controller may be pulsed
* low for approximately 6 micro seconds. Bits 3-0 of the KBD_CCMD_PULSE
@@ -326,7 +319,7 @@ static uint64_t kbd_read_data(void *opaque, hwaddr addr,
else
val = ps2_read_data(s->kbd);
-DPRINTF("kbd: read data=0x%02x\n", val);
+trace_pckbd_kbd_read_data(val);
return val;
}
@@ -335,7 +328,7 @@ static void kbd_write_data(void *opaque, hwaddr addr,
{
KBDState *s = opaque;
-DPRINTF("kbd: write data=0x%02" PRIx64 "\n", val);
+trace_pckbd_kbd_write_data(val);
switch(s->write_cmd) {
case 0:
diff --git a/hw/input/trace-events b/hw/input/trace-events
index 3965a842ae..8e53ae5bbf 100644
--- a/hw/input/trace-events
+++ b/hw/input/trace-events
@@ -14,6 +14,13 @@ adb_mouse_readreg(int reg, uint8_t val0, uint8_t val1) "reg
%d obuf[0] 0x%2.2x o
adb_mouse_request_change_addr(int devaddr) "change addr to 0x%x"
adb_mouse_request_change_addr_and_handler(int devaddr, int handler) "change
addr and handler to 0x%x, 0x%x"
+# hw/input/pckbd.c
+pckbd_kbd_read_data(uint32_t val) "0x%02x"
+pckbd_kbd_read_status(int status) "0x%02x"
+pckbd_outport_write(uint32_t val) "0x%02x"
+pckbd_kbd_write_command(uint64_t val) "0x%02"PRIx64
+pckbd_kbd_write_data(uint64_t val) "0x%02"PRIx64
+
# hw/input/ps2.c
ps2_put_keycode(void *opaque, int keycode) "%p keycode 0x%02x"
ps2_keyboard_event(void *opaque, int qcode, int down, unsigned int modifier,
unsigned int modifiers) "%p qcode %d down %d modifier 0x%x modifiers 0x%x"
--
2.20.1
[Qemu-devel] [PATCH 3/4] kvm: Add kvm_set_ioeventfd* traces
From: "Dr. David Alan Gilbert"
Add a couple of traces around the kvm_set_ioeventfd* calls.
Signed-off-by: Dr. David Alan Gilbert
---
accel/kvm/kvm-all.c| 3 +++
accel/kvm/trace-events | 2 ++
2 files changed, 5 insertions(+)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 4e1de942ce..fd92b6f375 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -657,6 +657,8 @@ static int kvm_set_ioeventfd_mmio(int fd, hwaddr addr,
uint32_t val,
.fd = fd,
};
+trace_kvm_set_ioeventfd_mmio(fd, (uint64_t)addr, val, assign, size,
+ datamatch);
if (!kvm_enabled()) {
return -ENOSYS;
}
@@ -688,6 +690,7 @@ static int kvm_set_ioeventfd_pio(int fd, uint16_t addr,
uint16_t val,
.fd = fd,
};
int r;
+trace_kvm_set_ioeventfd_pio(fd, addr, val, assign, size, datamatch);
if (!kvm_enabled()) {
return -ENOSYS;
}
diff --git a/accel/kvm/trace-events b/accel/kvm/trace-events
index 58e98efe5d..8841025d68 100644
--- a/accel/kvm/trace-events
+++ b/accel/kvm/trace-events
@@ -12,5 +12,7 @@ kvm_irqchip_commit_routes(void) ""
kvm_irqchip_add_msi_route(char *name, int vector, int virq) "dev %s vector %d
virq %d"
kvm_irqchip_update_msi_route(int virq) "Updating MSI route virq=%d"
kvm_irqchip_release_virq(int virq) "virq %d"
+kvm_set_ioeventfd_mmio(int fd, uint64_t addr, uint32_t val, bool assign,
uint32_t size, bool datamatch) "fd: %d @0x%" PRIx64 " val=0x%x assign: %d size:
%d match: %d"
+kvm_set_ioeventfd_pio(int fd, uint16_t addr, uint32_t val, bool assign,
uint32_t size, bool datamatch) "fd: %d @0x%x val=0x%x assign: %d size: %d
match: %d"
kvm_set_user_memory(uint32_t slot, uint32_t flags, uint64_t guest_phys_addr,
uint64_t memory_size, uint64_t userspace_addr, int ret) "Slot#%d flags=0x%x
gpa=0x%"PRIx64 " size=0x%"PRIx64 " ua=0x%"PRIx64 " ret=%d"
--
2.20.1
Re: [Qemu-devel] [PATCH v2] virtio-blk: set correct config size for the host driver
On Tue, Feb 12, 2019 at 02:01:44PM +0800, Changpeng Liu wrote:
> Commit caa1ee43 "vhost-user-blk: add discard/write zeroes features support"
> introduced extra fields to existing struct virtio_blk_config, when
> migration was executed from older QEMU version to current head, it
> will break the migration. While here, set the correct config size
> when initializing the host driver, for now, discard/write zeroes
> are not supported by virtio-blk host driver, so set the config
> size as before, users can change config size when adding the new
> feature bits support.
>
> Signed-off-by: Changpeng Liu
Pls rewrite commit log as suggested in this thread
and repost.
> ---
> hw/block/virtio-blk.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 9a87b3b..846b7b9 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -28,6 +28,9 @@
> #include "hw/virtio/virtio-bus.h"
> #include "hw/virtio/virtio-access.h"
>
> +#define VIRTIO_BLK_CFG_SIZE (offsetof(struct virtio_blk_config, num_queues)
> + \
> + sizeof_field(struct virtio_blk_config,
> num_queues))
> +
I would just do offsetof(max_discard_sectors)
with a comment "we don't support discard yet, hide associated config
fields".
> static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq,
> VirtIOBlockReq *req)
> {
> @@ -761,7 +764,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev,
> uint8_t *config)
> blkcfg.alignment_offset = 0;
> blkcfg.wce = blk_enable_write_cache(s->blk);
> virtio_stw_p(vdev, &blkcfg.num_queues, s->conf.num_queues);
> -memcpy(config, &blkcfg, sizeof(struct virtio_blk_config));
> +memcpy(config, &blkcfg, VIRTIO_BLK_CFG_SIZE);
Let's add QEMU_BUILD_BUG_ON(VIRTIO_BLK_CFG_SIZE <= sizeof(struct
virtio_blk_config)) just for documentation purposes.
> }
>
> static void virtio_blk_set_config(VirtIODevice *vdev, const uint8_t *config)
> @@ -769,7 +772,7 @@ static void virtio_blk_set_config(VirtIODevice *vdev,
> const uint8_t *config)
> VirtIOBlock *s = VIRTIO_BLK(vdev);
> struct virtio_blk_config blkcfg;
>
> -memcpy(&blkcfg, config, sizeof(blkcfg));
> +memcpy(&blkcfg, config, VIRTIO_BLK_CFG_SIZE);
Here too, QEMU_BUILD_BUG_ON(VIRTIO_BLK_CFG_SIZE <= sizeof(blkcfg))
>
> aio_context_acquire(blk_get_aio_context(s->blk));
> blk_set_enable_write_cache(s->blk, blkcfg.wce != 0);
> @@ -952,8 +955,7 @@ static void virtio_blk_device_realize(DeviceState *dev,
> Error **errp)
> return;
> }
>
> -virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
> -sizeof(struct virtio_blk_config));
> +virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, VIRTIO_BLK_CFG_SIZE);
>
> s->blk = conf->conf.blk;
> s->rq = NULL;
> --
> 1.9.3
[Qemu-devel] [PATCH 2/4] HMP: Prepend errors with 'Error:'
From: "Dr. David Alan Gilbert"
Always make error messages start with 'Error:' as a fallback
to make sure that anything parsing them can tell it failed.
Note: Some places don't use hmp_handle_error
Signed-off-by: Dr. David Alan Gilbert
---
hmp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hmp.c b/hmp.c
index b2a2b1f84e..4d14718fea 100644
--- a/hmp.c
+++ b/hmp.c
@@ -62,7 +62,7 @@ static void hmp_handle_error(Monitor *mon, Error **errp)
{
assert(errp);
if (*errp) {
-error_report_err(*errp);
+error_reportf_err(*errp, "Error: ");
}
}
--
2.20.1
