Re: [Qemu-devel] [PATCH v4 09/11] virtio-net: update the head descriptor in a chain lastly
On 2019/2/20 下午12:01, Wei Xu wrote: AFAICT, virtqueue_fill() is a generic API for all relevant userspace virtio devices that do not support batching , without touching virtqueue_fill(), supporting batching changes the meaning of the parameter 'idx' which should be kept overall. To fix it, I got two proposals so far: 1). batching support(two APIs needed to keep compatibility) 2). save a head elem for a vq instead of caching an array of elems like vhost, and introduce a new API(virtqueue_chain_fill()) functioning with an additional parameter 'more' to the current virtqueue_fill() to indicate if there are more descriptor(s) coming in a chain. Either way it changes the API somehow and it does not seem to be clean and clear as wanted. It's as simple as accepting an array of elems in e.g virtqueue_fill_batched()? It is trivial for both, my concern is an array elements need to be allocated dynamically due to vq size which no any other devices are using, a head would be enough for 2. Do you see any issue for this? Thanks
Re: [Qemu-devel] [PATCH 1/5] vfio/migration: define kernel interfaces
On Tue, Feb 19, 2019 at 02:09:18PM +0100, Cornelia Huck wrote:
> On Tue, 19 Feb 2019 16:52:14 +0800
> Yan Zhao wrote:
>
> > - defined 4 device states regions: one control region and 3 data regions
> > - defined layout of control region in struct vfio_device_state_ctl
> > - defined 4 device states: running, stop, running&logging, stop&logging
> > - define 3 device data categories: device config, device memory, system
> > memory
> > - defined 2 device data capabilities: device memory and system memory
> > - defined device state interfaces' version and 12 device state interfaces
> >
> > Signed-off-by: Yan Zhao
> > Signed-off-by: Kevin Tian
> > Signed-off-by: Yulei Zhang
> > ---
> > linux-headers/linux/vfio.h | 260
> > +
> > 1 file changed, 260 insertions(+)
>
> [commenting here for convenience; changes obviously need to be done in
> the Linux patch]
>
yes, you can find the corresponding kernel part code at
https://patchwork.freedesktop.org/series/56876/
> >
> > diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
> > index ceb6453..a124fc1 100644
> > --- a/linux-headers/linux/vfio.h
> > +++ b/linux-headers/linux/vfio.h
> > @@ -303,6 +303,56 @@ struct vfio_region_info_cap_type {
> > #define VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG (2)
> > #define VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG (3)
> >
> > +/* Device State region type and sub-type
> > + *
> > + * A VFIO device driver needs to register up to four device state regions
> > in
> > + * total: two mandatory and another two optional, if it plans to support
> > device
> > + * state management.
>
> Suggest to rephrase:
>
> "A VFIO device driver that plans to support device state management
> needs to register..."
>
ok :)
> > + *
> > + * 1. region CTL :
> > + * Mandatory.
> > + * This is a control region.
> > + * Its layout is defined in struct vfio_device_state_ctl.
> > + * Reading from this region can get version, capabilities and data
> > + * size of device state interfaces.
> > + * Writing to this region can set device state, data size and
> > + * choose which interface to use.
> > + * 2. region DEVICE_CONFIG
> > + * Mandatory.
> > + * This is a data region that holds device config data.
> > + * Device config is such kind of data like MMIOs, page tables...
>
> "Device config is data such as..."
ok :)
>
> > + * Every device is supposed to possess device config data.
> > + * Usually the size of device config data is small (no big
>
> s/no big/no bigger/
right :)
>
> > + * than 10M), and it needs to be loaded in certain strict
> > + * order.
> > + * Therefore no dirty data logging is enabled for device
> > + * config and it must be got/set as a whole.
> > + * Size of device config data is smaller than or equal to that of
> > + * device config region.
>
> Not sure if I understand that sentence correctly... but what if a
> device has more config state than fits into this region? Is that
> supposed to be covered by the device memory region? Or is this assumed
> to be something so exotic that we don't need to plan for it?
>
Device config data and device config region are all provided by vendor
driver, so vendor driver is always able to create a large enough device
config region to hold device config data.
So, if a device has data that are better to be saved after device stop and
saved/loaded in strict order, the data needs to be in device config region.
This kind of data is supposed to be small.
If the device data can be saved/loaded several times, it can also be put
into device memory region.
> > + * It is able to be mmaped into user space.
> > + * 3. region DEVICE_MEMORY
> > + * Optional.
> > + * This is a data region that holds device memory data.
> > + * Device memory is device's internal memory, standalone and
> > outside
>
> s/outside/distinct from/ ?
ok.
>
> > + * system memory. It is usually very big.
> > + * Not all device has device memory. Like IGD only uses system
>
> s/all devices has/all devices have/
>
> s/Like/E.g./
>
ok :)
> > + * memory and has no device memory.
> > + * Size of devie memory is usually larger than that of device
>
> s/devie/device/
>
thanks:)
> > + * memory region. qemu needs to save/load it in chunks of size of
> > + * device memory region.
>
> I'd rather not explicitly mention QEMU in this header. Maybe
> "Userspace"?
>
ok.
> > + * It is able to be mmaped into user space.
> > + * 4. region DIRTY_BITMAP
> > + * Optional.
> > + * This is a data region that holds bitmap of dirty pages in
> > system
> > + * memory that a VFIO devices produces.
> > + * It is able to be mmaped into user space.
> > + */
> > +#define VFIO_REGION_TYPE_D
Re: [Qemu-devel] [PATCH v3 03/25] chardev/wctablet: Use unsigned type to hold unsigned value
On Wed, Feb 20, 2019 at 02:02:10AM +0100, Philippe Mathieu-Daudé wrote:
> TabletChardev::query is an array of uint8_t.
> Use the same type to hold it (this also silent a -Wsign-conversion
> warning in the trace function).
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Gerd Hoffmann
> ---
> chardev/trace-events | 2 +-
> chardev/wctablet.c | 5 +++--
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/chardev/trace-events b/chardev/trace-events
> index d0e5f3bbc1..562bfe70e9 100644
> --- a/chardev/trace-events
> +++ b/chardev/trace-events
> @@ -5,7 +5,7 @@ wct_init(void) ""
> wct_cmd_re(void) ""
> wct_cmd_st(void) ""
> wct_cmd_sp(void) ""
> -wct_cmd_ts(int input) "0x%02x"
> +wct_cmd_ts(uint8_t input) "0x%02x"
> wct_cmd_other(const char *cmd) "%s"
> wct_speed(int speed) "%d"
>
> diff --git a/chardev/wctablet.c b/chardev/wctablet.c
> index 35dbd29a33..cf7a08a363 100644
> --- a/chardev/wctablet.c
> +++ b/chardev/wctablet.c
> @@ -207,7 +207,8 @@ static int wctablet_chr_write(struct Chardev *chr,
>const uint8_t *buf, int len)
> {
> TabletChardev *tablet = WCTABLET_CHARDEV(chr);
> -unsigned int i, clen;
> +size_t i;
> +unsigned int clen;
> char *pos;
>
> if (tablet->line_speed != 9600) {
> @@ -269,7 +270,7 @@ static int wctablet_chr_write(struct Chardev *chr,
>
> } else if (strncmp((char *)tablet->query, "TS", 2) == 0 &&
> clen == 3) {
> -unsigned int input = tablet->query[2];
> +uint8_t input = tablet->query[2];
> uint8_t codes[7] = {
> 0xa3,
> ((input & 0x80) == 0) ? 0x7e : 0x7f,
> --
> 2.20.1
>
Re: [Qemu-devel] [PATCH v3 10/25] usb-redir: Verify usbredirparser_write get called with positive count
On Wed, Feb 20, 2019 at 02:02:17AM +0100, Philippe Mathieu-Daudé wrote: > The usbredirparser_write handler should never be called with a negative > size payload, return an error if this is not the case. > Now that we are sure the 'count' value is positive, make it obvious by > casting it to a size_t. Reviewed-by: Gerd Hoffmann
Re: [Qemu-devel] [PATCH v3 08/25] ui/gtk: Remove pointless cast
On Wed, Feb 20, 2019 at 02:02:15AM +0100, Philippe Mathieu-Daudé wrote: > The 'size' value is of type 'guint' which is already unsigned. > Remove the useless cast. > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Gerd Hoffmann
Re: [Qemu-devel] [PATCH] hw/display: Add basic ATI VGA emulation
Hi, > > So it could be tested with linux guests on x86 too I guess? > > Can the radeon drm driver handle the devices too? > > Yes you can try with x86 guests, I haven't tested that yet. The radeon > driver only supports RV100 and up I think so may only work with the 0x5159 > variant not with Rage 128 Pro which had another driver r128 but not sure > that still exists. Although these two chips are similar, Rage 128 Pro is a > bit simpler that's why I'm targeting it first and also that's what the > PowerMac3,1 (the ppc mac99 machine is converging to) has. The R128Pro is the > last of the previous generation before Radeon, while RV100 is the stripped > down simplest version of the R100 family which has some more 3D capability). > But even if the DRM driver loads, probably only the mode setting part is > useful at the moment as 3D is not implemented yet by this device model. Chances are not too bad that it'll be good enough to bring up a linux console. > > I'd also use model= instead of device_id=... to switch between > > different devices. > > The only problem with that is that there are this many versions with > confusing names (and maybe different device ids for different versions): > > https://www.x.org/wiki/RadeonFeature/#index5h2 Do we want emulate them all? I'd guess picking a few models would be more useful ... > so the only really good way to identify a chip is via device_id. This is not > user friendly but at this stage probably will do and we can add alternative > model property later which aliases some device ids (like it's done for CPU Ok, I'd suggest to rename it to x-device-id (to indicate that it may go away later) if you want stick with device id for now. > OK I thought those are related but if adding it to the default_list[] won't > automatically add a -vga option then that's a good idea. I'll do that and > also move the config lines to pci.mak in next version but wait for a few > more days for more comments. One thing I've noticed is that you use the vbe registers internally. I'd suggest to not do that, I suspect it will only get into the way latter on. Better register your own GraphicsHwOps, then go call the vga ops in vga mode and your ati modesetting code in extended mode. virtio-vga does it that way if you want look at some sample code. Also looking at bochs-display.c is probably more helpful than looking at vga.c when figuring how to handle display updates. cheers, Gerd
Re: [Qemu-devel] [PATCH] hw/ide/ich: Compile ich.c only if CONFIG_PCI is also set
On 20/02/2019 07.37, Wei Yang wrote: > On Tue, Feb 19, 2019 at 04:55:57PM +0100, Thomas Huth wrote: >> With the upcoming Kconfig-like build system, it will be easy to >> build also version of QEMU that only contain a single machine. Some > > Sorry for my poor English. > > What is also version? I mean "it will be easy to build a version of QEMU, too, that ...". Maybe I should have rather written "it will be easy to build a binary of QEMU, too, that ...". Thomas
Re: [Qemu-devel] [PATCH] block/pflash_cfi02: Fix memory leak and potential use-after-free
On Tue, Feb 19, 2019 at 10:37:27AM -0500, Stephen Checkoway wrote:
>Don't dynamically allocate the pflash's timer. But do use timer_del in
>an unrealize function to make sure that the timer can't fire after the
>pflash_t has been freed.
>
>Signed-off-by: Stephen Checkoway
Reviewed-by: Wei Yang
>---
> hw/block/pflash_cfi02.c | 15 +++
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
>diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
>index 0f8b7b8c7b..1588aeff5a 100644
>--- a/hw/block/pflash_cfi02.c
>+++ b/hw/block/pflash_cfi02.c
>@@ -84,7 +84,7 @@ struct pflash_t {
> uint16_t unlock_addr0;
> uint16_t unlock_addr1;
> uint8_t cfi_table[0x52];
>-QEMUTimer *timer;
>+QEMUTimer timer;
> /* The device replicates the flash memory across its memory space.
> Emulate
> * that by having a container (.mem) filled with an array of aliases
> * (.mem_mappings) pointing to the flash memory (.orig_mem).
>@@ -429,7 +429,7 @@ static void pflash_write (pflash_t *pfl, hwaddr offset,
> }
> pfl->status = 0x00;
> /* Let's wait 5 seconds before chip erase is done */
>-timer_mod(pfl->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
>+timer_mod(&pfl->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> (NANOSECONDS_PER_SECOND * 5));
> break;
> case 0x30:
>@@ -444,7 +444,7 @@ static void pflash_write (pflash_t *pfl, hwaddr offset,
> }
> pfl->status = 0x00;
> /* Let's wait 1/2 second before sector erase is done */
>-timer_mod(pfl->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
>+timer_mod(&pfl->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> (NANOSECONDS_PER_SECOND / 2));
> break;
> default:
>@@ -596,7 +596,7 @@ static void pflash_cfi02_realize(DeviceState *dev, Error
>**errp)
> pfl->rom_mode = 1;
> sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
>
>-pfl->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, pflash_timer, pfl);
>+timer_init_ns(&pfl->timer, QEMU_CLOCK_VIRTUAL, pflash_timer, pfl);
> pfl->wcycle = 0;
> pfl->cmd = 0;
> pfl->status = 0;
>@@ -695,11 +695,18 @@ static Property pflash_cfi02_properties[] = {
> DEFINE_PROP_END_OF_LIST(),
> };
>
>+static void pflash_cfi02_unrealize(DeviceState *dev, Error **errp)
>+{
>+pflash_t *pfl = CFI_PFLASH02(dev);
>+timer_del(&pfl->timer);
>+}
>+
> static void pflash_cfi02_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
>
> dc->realize = pflash_cfi02_realize;
>+dc->unrealize = pflash_cfi02_unrealize;
> dc->props = pflash_cfi02_properties;
> set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
> }
>--
>2.17.2 (Apple Git-113)
>
--
Wei Yang
Help you, Help me
Re: [Qemu-devel] [PATCH] hw/ide/ich: Compile ich.c only if CONFIG_PCI is also set
On Tue, Feb 19, 2019 at 04:55:57PM +0100, Thomas Huth wrote: >With the upcoming Kconfig-like build system, it will be easy to >build also version of QEMU that only contain a single machine. Some Sorry for my poor English. What is also version? >of these machines (like the ARM cubieboard) use CONFIG_AHCI for an >AHCI sysbus device, but do not use CONFIG_PCI since they do not feature >a PCI bus. In this case linking fails: > >../hw/ide/ich.o: In function `pci_ich9_ahci_realize': >hw/ide/ich.c:124: undefined reference to `pci_allocate_irq' >hw/ide/ich.c:126: undefined reference to `pci_register_bar' >hw/ide/ich.c:128: undefined reference to `pci_register_bar' >hw/ide/ich.c:131: undefined reference to `pci_add_capability' >hw/ide/ich.c:147: undefined reference to `msi_init' >../hw/ide/ich.o: In function `pci_ich9_uninit': >hw/ide/ich.c:158: undefined reference to `msi_uninit' >../hw/ide/ich.o:(.data.rel+0x50): undefined reference to `vmstate_pci_device' > >Thus we must only compile ich.c if CONFIG_PCI is also set. > >Signed-off-by: Thomas Huth >--- > hw/ide/Makefile.objs | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/hw/ide/Makefile.objs b/hw/ide/Makefile.objs >index a142add..dfe53af 100644 >--- a/hw/ide/Makefile.objs >+++ b/hw/ide/Makefile.objs >@@ -9,6 +9,6 @@ common-obj-$(CONFIG_IDE_MMIO) += mmio.o > common-obj-$(CONFIG_IDE_VIA) += via.o > common-obj-$(CONFIG_MICRODRIVE) += microdrive.o > common-obj-$(CONFIG_AHCI) += ahci.o >-common-obj-$(CONFIG_AHCI) += ich.o >+common-obj-$(call land,$(CONFIG_AHCI),$(CONFIG_PCI)) += ich.o > common-obj-$(CONFIG_ALLWINNER_A10) += ahci-allwinner.o > common-obj-$(CONFIG_IDE_SII3112) += sii3112.o >-- >1.8.3.1 > -- Wei Yang Help you, Help me
Re: [Qemu-devel] [PATCH] hw/pci/pci-stub: Add msi_enabled() and msi_notify() to the pci stubs
On 19/02/2019 21.19, Michael S. Tsirkin wrote: > On Tue, Feb 19, 2019 at 05:07:39PM +0100, Thomas Huth wrote: >> Some machines have an AHCI adapter, but no PCI. To be able to >> compile hw/ide/ahci.c without CONFIG_PCI, we still need the two >> functions msi_enabled() and msi_notify() for linking. >> This is required for the upcoming Kconfig-like build system, if >> a user wants to compile a QEMU binary with just one machine that >> has AHCI, but no PCI, like the ARM "cubieboard" for example. >> >> Signed-off-by: Thomas Huth > > Reviewed-by: Michael S. Tsirkin Thanks! > Do you want me to merge this or do you prefer to > merge it with kconfig patches? If you plan a pci pull request soon, feel free to take it. Otherwise, I can also add it to my Kconfig-for-arm patch series where I need it. Thomas
Re: [Qemu-devel] [PATCH] hw/ide/ich: Compile ich.c only if CONFIG_PCI is also set
On 19/02/2019 19.18, Paolo Bonzini wrote: > On 19/02/19 16:55, Thomas Huth wrote: >> With the upcoming Kconfig-like build system, it will be easy to >> build also version of QEMU that only contain a single machine. Some >> of these machines (like the ARM cubieboard) use CONFIG_AHCI for an >> AHCI sysbus device, but do not use CONFIG_PCI since they do not feature >> a PCI bus. In this case linking fails: >> >> ../hw/ide/ich.o: In function `pci_ich9_ahci_realize': >> hw/ide/ich.c:124: undefined reference to `pci_allocate_irq' >> hw/ide/ich.c:126: undefined reference to `pci_register_bar' >> hw/ide/ich.c:128: undefined reference to `pci_register_bar' >> hw/ide/ich.c:131: undefined reference to `pci_add_capability' >> hw/ide/ich.c:147: undefined reference to `msi_init' >> ../hw/ide/ich.o: In function `pci_ich9_uninit': >> hw/ide/ich.c:158: undefined reference to `msi_uninit' >> ../hw/ide/ich.o:(.data.rel+0x50): undefined reference to `vmstate_pci_device' >> >> Thus we must only compile ich.c if CONFIG_PCI is also set. >> >> Signed-off-by: Thomas Huth >> --- >> hw/ide/Makefile.objs | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/hw/ide/Makefile.objs b/hw/ide/Makefile.objs >> index a142add..dfe53af 100644 >> --- a/hw/ide/Makefile.objs >> +++ b/hw/ide/Makefile.objs >> @@ -9,6 +9,6 @@ common-obj-$(CONFIG_IDE_MMIO) += mmio.o >> common-obj-$(CONFIG_IDE_VIA) += via.o >> common-obj-$(CONFIG_MICRODRIVE) += microdrive.o >> common-obj-$(CONFIG_AHCI) += ahci.o >> -common-obj-$(CONFIG_AHCI) += ich.o >> +common-obj-$(call land,$(CONFIG_AHCI),$(CONFIG_PCI)) += ich.o >> common-obj-$(CONFIG_ALLWINNER_A10) += ahci-allwinner.o >> common-obj-$(CONFIG_IDE_SII3112) += sii3112.o > > What about defining CONFIG_AHCI_ICH instead? It can wait until after > Kconfig is merged. Yeah, that's the ultimate solution once we've got Kconfig merged. I thought this might be a nice intermediate step, but if we want to avoid code churn, this can indeed wait. By the way, what's the current status of the Kconfig series? Will there be another respin soon? ... the next softfreeze is not that far away ... Thomas
Re: [Qemu-devel] [PATCH] doc: update .gitignore and fix typos for docs in tree
On 2019/2/20 11:09, Eric Blake wrote: On 2/20/19 2:55 AM, Like Xu wrote: Signed-off-by: Like Xu This feels like two independent patches - the .gitignore change is different from typo fixes. Actually, for .gitignore, you could just as easily do: echo '*.patch' >> .git/info/exclude and fix it so you never commit patch files locally, while still leaving them visible through 'git status' to other users. I actually like knowing when I have stale patch files lying around, so that I can 'rm' them before creating my next patch batch and using 'git send-email *.patch' - but of course, I could just as easily add '!*.patch' to my .git/info/exclude to override the project default if we decide to go with your patch. Adding '*.patch' to .gitignore is consistent with LKML tree. +++ b/docs/colo-proxy.txt @@ -41,7 +41,7 @@ Below is a COLO proxy ascii figure: | | +--+ | || | |netfilter| | | || | netfilter|| | | +--+ ++ || | +---+ | -| | | | | |out || | | || filter excute order | | +| | | | | |out || | | || filter execute order | | You fixed the typo, but broke the whitespacing on this lineart. (twice) Forgot to check format，thanks! +++ b/docs/qemu-block-drivers.texi @@ -632,7 +632,7 @@ qemu-system-i386 -drive file=iscsi://127.0.0.1/iqn.qemu.test/1 \ @end example -Howto set up a simple iSCSI target on loopback and accessing it via QEMU: +How to set up a simple iSCSI target on loopback and accessing it via QEMU: While here, I would also: s/accessing/access/ +++ b/docs/usb-storage.txt @@ -16,7 +16,7 @@ too): Number two is the newer usb attached scsi transport. This one doesn't -automagically create a scsi disk, so you have to explicitly attach one +automatically create a scsi disk, so you have to explicitly attach one This one may be intentional, as a play on English words. But changing it may sound a bit more professional. Nice play and let's keep it . All your other typo fixes are good. Looking forward to v2.
Re: [Qemu-devel] [PATCH 0/5] QEMU VFIO live migration
On Tue, Feb 19, 2019 at 11:32:13AM +, Dr. David Alan Gilbert wrote:
> * Yan Zhao (yan.y.z...@intel.com) wrote:
> > This patchset enables VFIO devices to have live migration capability.
> > Currently it does not support post-copy phase.
> >
> > It follows Alex's comments on last version of VFIO live migration patches,
> > including device states, VFIO device state region layout, dirty bitmap's
> > query.
>
> Hi,
> I've sent minor comments to later patches; but some minor general
> comments:
>
> a) Never trust the incoming migrations stream - it might be corrupt,
> so check when you can.
hi Dave
Thanks for this suggestion. I'll add more checks for migration streams.
> b) How do we detect if we're migrating from/to the wrong device or
> version of device? Or say to a device with older firmware or perhaps
> a device that has less device memory ?
Actually it's still an open for VFIO migration. Need to think about
whether it's better to check that in libvirt or qemu (like a device magic
along with verion ?).
This patchset is intended to settle down the main device state interfaces
for VFIO migration. So that we can work on that and improve it.
> c) Consider using the trace_ mechanism - it's really useful to
> add to loops writing/reading data so that you can see when it fails.
>
> Dave
>
Got it. many thanks~~
> (P.S. You have a few typo's grep your code for 'devcie', 'devie' and
> 'migrtion'
sorry :)
>
> > Device Data
> > ---
> > Device data is divided into three types: device memory, device config,
> > and system memory dirty pages produced by device.
> >
> > Device config: data like MMIOs, page tables...
> > Every device is supposed to possess device config data.
> > Usually device config's size is small (no big than 10M), and it
> > needs to be loaded in certain strict order.
> > Therefore, device config only needs to be saved/loaded in
> > stop-and-copy phase.
> > The data of device config is held in device config region.
> > Size of device config data is smaller than or equal to that of
> > device config region.
> >
> > Device Memory: device's internal memory, standalone and outside system
> > memory. It is usually very big.
> > This kind of data needs to be saved / loaded in pre-copy and
> > stop-and-copy phase.
> > The data of device memory is held in device memory region.
> > Size of devie memory is usually larger than that of device
> > memory region. qemu needs to save/load it in chunks of size of
> > device memory region.
> > Not all device has device memory. Like IGD only uses system memory.
> >
> > System memory dirty pages: If a device produces dirty pages in system
> > memory, it is able to get dirty bitmap for certain range of system
> > memory. This dirty bitmap is queried in pre-copy and stop-and-copy
> > phase in .log_sync callback. By setting dirty bitmap in .log_sync
> > callback, dirty pages in system memory will be save/loaded by ram's
> > live migration code.
> > The dirty bitmap of system memory is held in dirty bitmap region.
> > If system memory range is larger than that dirty bitmap region can
> > hold, qemu will cut it into several chunks and get dirty bitmap in
> > succession.
> >
> >
> > Device State Regions
> >
> > Vendor driver is required to expose two mandatory regions and another two
> > optional regions if it plans to support device state management.
> >
> > So, there are up to four regions in total.
> > One control region: mandatory.
> > Get access via read/write system call.
> > Its layout is defined in struct vfio_device_state_ctl
> > Three data regions: mmaped into qemu.
> > device config region: mandatory, holding data of device config
> > device memory region: optional, holding data of device memory
> > dirty bitmap region: optional, holding bitmap of system memory
> > dirty pages
> >
> > (The reason why four seperate regions are defined is that the unit of mmap
> > system call is PAGE_SIZE, i.e. 4k bytes. So one read/write region for
> > control and three mmaped regions for data seems better than one big region
> > padded and sparse mmaped).
> >
> >
> > kernel device state interface [1]
> > --
> > #define VFIO_DEVICE_STATE_INTERFACE_VERSION 1
> > #define VFIO_DEVICE_DATA_CAP_DEVICE_MEMORY 1
> > #define VFIO_DEVICE_DATA_CAP_SYSTEM_MEMORY 2
> >
> > #define VFIO_DEVICE_STATE_RUNNING 0
> > #define VFIO_DEVICE_STATE_STOP 1
> > #define VFIO_DEVICE_STATE_LOGGING 2
> >
> > #define VFIO_DEVICE_DATA_ACTION_GET_BUFFER 1
> > #define VFIO_DEVICE_DATA_ACTION_SET_BUFFER 2
> > #define VFIO_DEVICE_DATA_ACTION_GET_BITMAP 3
> >
> > struct vfio_device_state_ctl {
> > __u32 version;/* ro */
> > __u
[Qemu-devel] [PATCH v2] doc: fix typos for documents in tree
Signed-off-by: Like Xu --- docs/COLO-FT.txt | 2 +- docs/amd-memory-encryption.txt | 2 +- docs/can.txt | 2 +- docs/colo-proxy.txt| 6 +++--- docs/cpu-hotplug.rst | 2 +- docs/qcow2-cache.txt | 2 +- docs/qemu-block-drivers.texi | 2 +- docs/qemu-cpu-models.texi | 8 docs/rdma.txt | 4 ++-- docs/replay.txt| 2 +- docs/vfio-ap.txt | 2 +- 11 files changed, 17 insertions(+), 17 deletions(-) diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt index e2686bb..ad24680 100644 --- a/docs/COLO-FT.txt +++ b/docs/COLO-FT.txt @@ -102,7 +102,7 @@ to make sure the state of VM in Secondary side is always consistent with VM in Primary side. COLO Proxy: -Delivers packets to Primary and Seconday, and then compare the responses from +Delivers packets to Primary and Secondary, and then compare the responses from both side. Then decide whether to start a checkpoint according to some rules. Please refer to docs/colo-proxy.txt for more information. diff --git a/docs/amd-memory-encryption.txt b/docs/amd-memory-encryption.txt index f483795..43bf3ee 100644 --- a/docs/amd-memory-encryption.txt +++ b/docs/amd-memory-encryption.txt @@ -97,7 +97,7 @@ References AMD Memory Encryption whitepaper: http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2013/12/AMD_Memory_Encryption_Whitepaper_v7-Public.pdf -Secure Encrypted Virutualization Key Management: +Secure Encrypted Virtualization Key Management: [1] http://support.amd.com/TechDocs/55766_SEV-KM API_Specification.pdf KVM Forum slides: diff --git a/docs/can.txt b/docs/can.txt index 7ba23b2..9fa6ed5 100644 --- a/docs/can.txt +++ b/docs/can.txt @@ -99,7 +99,7 @@ Links to other resources https://gitlab.fel.cvut.cz/canbus/qemu-canbus (3) RTEMS page describing project https://devel.rtems.org/wiki/Developer/Simulators/QEMU/CANEmulation - (4) RTLWS 2015 article about the projevt and its use with CANopen emulation + (4) RTLWS 2015 article about the project and its use with CANopen emulation http://rtime.felk.cvut.cz/publications/public/rtlws2015-qemu-can.pdf Slides http://rtime.felk.cvut.cz/publications/public/rtlws2015-qemu-can-slides.pdf diff --git a/docs/colo-proxy.txt b/docs/colo-proxy.txt index 1f8e4b4..fa1cef0 100644 --- a/docs/colo-proxy.txt +++ b/docs/colo-proxy.txt @@ -41,7 +41,7 @@ Below is a COLO proxy ascii figure: | | +--+ | || | |netfilter| | | || | netfilter|| | | +--+ ++ || | +---+ | -| | | | | |out || | | || filter excute order | | +| | | | | |out || | | || filter execute order | | | | | | +-+|| | | || +---> | | | | | | || | ||| | | || TCP | | | | +-+--+-+ +-v+ +-v+ |pri +++sec|| | | ++ +---++---v+rewriter++ ++ | | @@ -53,7 +53,7 @@ Below is a COLO proxy ascii figure: | | | tx| rx rx | | || | txall | rx | | | | | || | || +---+ | | | | +--+ | | || || -| | | filter excute order | | | || || +| | | filter execute order | | | || || | | | +>| | | ++| | +-+ | | | |||| | | @@ -92,7 +92,7 @@ but do nothing, just pass to next filter. Redirect Server Filter --> COLO-Compare COLO-compare receive primary guest packet then -waiting scondary redirect packet to compare it. +waiting secondary redirect packet to compare
Re: [Qemu-devel] [PATCH 5/5] vfio/migration: support device memory capability
On Tue, Feb 19, 2019 at 11:25:43AM +, Dr. David Alan Gilbert wrote:
> * Yan Zhao (yan.y.z...@intel.com) wrote:
> > If a device has device memory capability, save/load data from device memory
> > in pre-copy and stop-and-copy phases.
> >
> > LOGGING state is set for device memory for dirty page logging:
> > in LOGGING state, get device memory returns whole device memory snapshot;
> > outside LOGGING state, get device memory returns dirty data since last get
> > operation.
> >
> > Usually, device memory is very big, qemu needs to chunk it into several
> > pieces each with size of device memory region.
> >
> > Signed-off-by: Yan Zhao
> > Signed-off-by: Kirti Wankhede
> > ---
> > hw/vfio/migration.c | 235
> > ++--
> > hw/vfio/pci.h | 1 +
> > 2 files changed, 231 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> > index 16d6395..f1e9309 100644
> > --- a/hw/vfio/migration.c
> > +++ b/hw/vfio/migration.c
> > @@ -203,6 +203,201 @@ static int vfio_load_data_device_config(VFIOPCIDevice
> > *vdev,
> > return 0;
> > }
> >
> > +static int vfio_get_device_memory_size(VFIOPCIDevice *vdev)
> > +{
> > +VFIODevice *vbasedev = &vdev->vbasedev;
> > +VFIORegion *region_ctl =
> > +&vdev->migration->region[VFIO_DEVSTATE_REGION_CTL];
> > +uint64_t len;
> > +int sz;
> > +
> > +sz = sizeof(len);
> > +if (pread(vbasedev->fd, &len, sz,
> > +region_ctl->fd_offset +
> > +offsetof(struct vfio_device_state_ctl, device_memory.size))
> > +!= sz) {
> > +error_report("vfio: Failed to get length of device memory");
> > +return -1;
> > +}
> > +vdev->migration->devmem_size = len;
> > +return 0;
> > +}
> > +
> > +static int vfio_set_device_memory_size(VFIOPCIDevice *vdev, uint64_t size)
> > +{
> > +VFIODevice *vbasedev = &vdev->vbasedev;
> > +VFIORegion *region_ctl =
> > +&vdev->migration->region[VFIO_DEVSTATE_REGION_CTL];
> > +int sz;
> > +
> > +sz = sizeof(size);
> > +if (pwrite(vbasedev->fd, &size, sz,
> > +region_ctl->fd_offset +
> > +offsetof(struct vfio_device_state_ctl, device_memory.size))
> > +!= sz) {
> > +error_report("vfio: Failed to set length of device comemory");
> > +return -1;
> > +}
> > +vdev->migration->devmem_size = size;
> > +return 0;
> > +}
> > +
> > +static
> > +int vfio_save_data_device_memory_chunk(VFIOPCIDevice *vdev, QEMUFile *f,
> > +uint64_t pos, uint64_t len)
> > +{
> > +VFIODevice *vbasedev = &vdev->vbasedev;
> > +VFIORegion *region_ctl =
> > +&vdev->migration->region[VFIO_DEVSTATE_REGION_CTL];
> > +VFIORegion *region_devmem =
> > +&vdev->migration->region[VFIO_DEVSTATE_REGION_DATA_DEVICE_MEMORY];
> > +void *dest;
> > +uint32_t sz;
> > +uint8_t *buf = NULL;
> > +uint32_t action = VFIO_DEVICE_DATA_ACTION_GET_BUFFER;
> > +
> > +if (len > region_devmem->size) {
> > +return -1;
> > +}
> > +
> > +sz = sizeof(pos);
> > +if (pwrite(vbasedev->fd, &pos, sz,
> > +region_ctl->fd_offset +
> > +offsetof(struct vfio_device_state_ctl, device_memory.pos))
> > +!= sz) {
> > +error_report("vfio: Failed to set save buffer pos");
> > +return -1;
> > +}
> > +sz = sizeof(action);
> > +if (pwrite(vbasedev->fd, &action, sz,
> > +region_ctl->fd_offset +
> > +offsetof(struct vfio_device_state_ctl,
> > device_memory.action))
> > +!= sz) {
> > +error_report("vfio: Failed to set save buffer action");
> > +return -1;
> > +}
> > +
> > +if (!vfio_device_state_region_mmaped(region_devmem)) {
> > +buf = g_malloc(len);
> > +if (buf == NULL) {
> > +error_report("vfio: Failed to allocate memory for migrate");
> > +return -1;
> > +}
> > +if (pread(vbasedev->fd, buf, len, region_devmem->fd_offset) !=
> > len) {
> > +error_report("vfio: error load device memory buffer");
>
> That's forgotten to g_free(buf)
>
Right, I'll correct that.
> > +return -1;
> > +}
> > +qemu_put_be64(f, len);
> > +qemu_put_be64(f, pos);
> > +qemu_put_buffer(f, buf, len);
> > +g_free(buf);
> > +} else {
> > +dest = region_devmem->mmaps[0].mmap;
> > +qemu_put_be64(f, len);
> > +qemu_put_be64(f, pos);
> > +qemu_put_buffer(f, dest, len);
> > +}
> > +return 0;
> > +}
> > +
> > +static int vfio_save_data_device_memory(VFIOPCIDevice *vdev, QEMUFile *f)
> > +{
> > +VFIORegion *region_devmem =
> > +&vdev->migration->region[VFIO_DEVSTATE_REGION_DATA_DEVICE_MEMORY];
> > +uint64_t total_len = vdev->migration->devmem_size;
> > +uint64_t pos = 0;
Re: [Qemu-devel] [PATCH] migration: Fix cancel state
On Tue, Feb 19, 2019 at 07:59:28PM +, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" > > During a cancelled migration there's a race where the fd can > go into an error state before we get back around the migration loop > and migration_detect_error transitions from cancelling->failed. > > Check for cancelled/cancelling and don't change the state. > > Red Hat bug: https://bugzilla.redhat.com/show_bug.cgi?id=1608649 > > Fixes: b23c2ade250 > Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Peter Xu Thanks, -- Peter Xu
Re: [Qemu-devel] [PATCH 2/5] vfio/migration: support device of device config capability
On Tue, Feb 19, 2019 at 11:01:45AM +, Dr. David Alan Gilbert wrote:
> * Yan Zhao (yan.y.z...@intel.com) wrote:
> > Device config is the default data that every device should have. so
> > device config capability is by default on, no need to set.
> >
> > - Currently two type of resources are saved/loaded for device of device
> > config capability:
> > General PCI config data, and Device config data.
> > They are copies as a whole when precopy is stopped.
> >
> > Migration setup flow:
> > - Setup device state regions, check its device state version and
> > capabilities.
> > Mmap Device Config Region and Dirty Bitmap Region, if available.
> > - If device state regions are failed to get setup, a migration blocker is
> > registered instead.
> > - Added SaveVMHandlers to register device state save/load handlers.
> > - Register VM state change handler to set device's running/stop states.
> > - On migration startup on source machine, set device's state to
> > VFIO_DEVICE_STATE_LOGGING
> >
> > Signed-off-by: Yan Zhao
> > Signed-off-by: Yulei Zhang
> > ---
> > hw/vfio/Makefile.objs | 2 +-
> > hw/vfio/migration.c | 633
> > ++
> > hw/vfio/pci.c | 1 -
> > hw/vfio/pci.h | 25 +-
> > include/hw/vfio/vfio-common.h | 1 +
> > 5 files changed, 659 insertions(+), 3 deletions(-)
> > create mode 100644 hw/vfio/migration.c
> >
> > diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
> > index 8b3f664..f32ff19 100644
> > --- a/hw/vfio/Makefile.objs
> > +++ b/hw/vfio/Makefile.objs
> > @@ -1,6 +1,6 @@
> > ifeq ($(CONFIG_LINUX), y)
> > obj-$(CONFIG_SOFTMMU) += common.o
> > -obj-$(CONFIG_PCI) += pci.o pci-quirks.o display.o
> > +obj-$(CONFIG_PCI) += pci.o pci-quirks.o display.o migration.o
> > obj-$(CONFIG_VFIO_CCW) += ccw.o
> > obj-$(CONFIG_SOFTMMU) += platform.o
> > obj-$(CONFIG_VFIO_XGMAC) += calxeda-xgmac.o
> > diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> > new file mode 100644
> > index 000..16d6395
> > --- /dev/null
> > +++ b/hw/vfio/migration.c
> > @@ -0,0 +1,633 @@
> > +#include "qemu/osdep.h"
> > +
> > +#include "hw/vfio/vfio-common.h"
> > +#include "migration/blocker.h"
> > +#include "migration/register.h"
> > +#include "qapi/error.h"
> > +#include "pci.h"
> > +#include "sysemu/kvm.h"
> > +#include "exec/ram_addr.h"
> > +
> > +#define VFIO_SAVE_FLAG_SETUP 0
> > +#define VFIO_SAVE_FLAG_PCI 1
> > +#define VFIO_SAVE_FLAG_DEVCONFIG 2
> > +#define VFIO_SAVE_FLAG_DEVMEMORY 4
> > +#define VFIO_SAVE_FLAG_CONTINUE 8
> > +
> > +static int vfio_device_state_region_setup(VFIOPCIDevice *vdev,
> > +VFIORegion *region, uint32_t subtype, const char *name)
> > +{
> > +VFIODevice *vbasedev = &vdev->vbasedev;
> > +struct vfio_region_info *info;
> > +int ret;
> > +
> > +ret = vfio_get_dev_region_info(vbasedev, VFIO_REGION_TYPE_DEVICE_STATE,
> > +subtype, &info);
> > +if (ret) {
> > +error_report("Failed to get info of region %s", name);
> > +return ret;
> > +}
> > +
> > +if (vfio_region_setup(OBJECT(vdev), vbasedev,
> > +region, info->index, name)) {
> > +error_report("Failed to setup migrtion region %s", name);
> > +return ret;
> > +}
> > +
> > +if (vfio_region_mmap(region)) {
> > +error_report("Failed to mmap migrtion region %s", name);
> > +}
> > +
> > +return 0;
> > +}
> > +
> > +bool vfio_device_data_cap_system_memory(VFIOPCIDevice *vdev)
> > +{
> > + return !!(vdev->migration->data_caps &
> > VFIO_DEVICE_DATA_CAP_SYSTEM_MEMORY);
> > +}
> > +
> > +bool vfio_device_data_cap_device_memory(VFIOPCIDevice *vdev)
> > +{
> > + return !!(vdev->migration->data_caps &
> > VFIO_DEVICE_DATA_CAP_DEVICE_MEMORY);
> > +}
> > +
> > +static bool vfio_device_state_region_mmaped(VFIORegion *region)
> > +{
> > +bool mmaped = true;
> > +if (region->nr_mmaps != 1 || region->mmaps[0].offset ||
> > +(region->size != region->mmaps[0].size) ||
> > +(region->mmaps[0].mmap == NULL)) {
> > +mmaped = false;
> > +}
> > +
> > +return mmaped;
> > +}
> > +
> > +static int vfio_get_device_config_size(VFIOPCIDevice *vdev)
> > +{
> > +VFIODevice *vbasedev = &vdev->vbasedev;
> > +VFIORegion *region_ctl =
> > +&vdev->migration->region[VFIO_DEVSTATE_REGION_CTL];
> > +VFIORegion *region_config =
> > +&vdev->migration->region[VFIO_DEVSTATE_REGION_DATA_CONFIG];
> > +uint64_t len;
> > +int sz;
> > +
> > +sz = sizeof(len);
> > +if (pread(vbasedev->fd, &len, sz,
> > +region_ctl->fd_offset +
> > +offsetof(struct vfio_device_state_ctl, device_config.size))
> > +!= sz) {
> > +error_report("vfio: Failed to get length of device config");
> > +return -1;
> > +}
> > +if (len > region_config->size) {
> > +error_report("vfio: Error device config length");
>
Re: [Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions command
> From: Aleksandar Markovic [mailto:amarko...@wavecomp.com] > > From: Markus Armbruster > > Subject: Re: [Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions > > command > > > Please rebase. Let me know if you need help. > > Hi, Markus. > > Pavel was probably busy today, so I took the liberty to rebase the patch, > and here is the v2: > > https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg05158.html Thank you! Pavel Dovgalyuk
[Qemu-devel] [PATCH 3/3] target/arm: Implement ARMv8.5-CondM
Signed-off-by: Richard Henderson
---
target/arm/cpu.h | 5
target/arm/translate-a64.c | 58 ++
2 files changed, 63 insertions(+)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 000e778619..0480f9baba 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3380,6 +3380,11 @@ static inline bool isar_feature_aa64_condm_4(const
ARMISARegisters *id)
return FIELD_EX64(id->id_aa64isar0, ID_AA64ISAR0, TS) != 0;
}
+static inline bool isar_feature_aa64_condm_5(const ARMISARegisters *id)
+{
+return FIELD_EX64(id->id_aa64isar0, ID_AA64ISAR0, TS) >= 2;
+}
+
static inline bool isar_feature_aa64_fcma(const ARMISARegisters *id)
{
return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, FCMA) != 0;
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index b88eccef53..1d9bf81c0e 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -1644,6 +1644,48 @@ static void handle_sync(DisasContext *s, uint32_t insn,
}
}
+static void gen_xaflag(void)
+{
+TCGv_i32 z = tcg_temp_new_i32();
+
+tcg_gen_setcondi_i32(TCG_COND_EQ, z, cpu_ZF, 0);
+
+/*
+ * (!C & !Z) << 31
+ * (!(C | Z)) << 31
+ * ~((C | Z) << 31)
+ * ~-(C | Z)
+ * (C | Z) - 1
+ */
+tcg_gen_or_i32(cpu_NF, cpu_CF, z);
+tcg_gen_subi_i32(cpu_NF, cpu_NF, 1);
+
+/* !(Z & C) */
+tcg_gen_and_i32(cpu_ZF, z, cpu_CF);
+tcg_gen_xori_i32(cpu_ZF, cpu_ZF, 1);
+
+/* (!C & Z) << 31 -> -(Z & ~C) */
+tcg_gen_andc_i32(cpu_VF, z, cpu_CF);
+tcg_gen_neg_i32(cpu_VF, cpu_VF);
+
+/* C | Z */
+tcg_gen_or_i32(cpu_CF, cpu_CF, z);
+
+tcg_temp_free_i32(z);
+}
+
+static void gen_axflag(void)
+{
+tcg_gen_sari_i32(cpu_VF, cpu_VF, 31); /* V ? -1 : 0 */
+tcg_gen_andc_i32(cpu_CF, cpu_CF, cpu_VF); /* C & !V */
+
+/* !(Z | V) -> !(!ZF | V) -> ZF & !V -> ZF & ~VF */
+tcg_gen_andc_i32(cpu_ZF, cpu_ZF, cpu_VF);
+
+tcg_gen_movi_i32(cpu_NF, 0);
+tcg_gen_movi_i32(cpu_VF, 0);
+}
+
/* MSR (immediate) - move immediate to processor state field */
static void handle_msr_i(DisasContext *s, uint32_t insn,
unsigned int op1, unsigned int op2, unsigned int crm)
@@ -1663,6 +1705,22 @@ static void handle_msr_i(DisasContext *s, uint32_t insn,
s->base.is_jmp = DISAS_NEXT;
break;
+case 0x01: /* XAFlag */
+if (crm != 0 || !dc_isar_feature(aa64_condm_5, s)) {
+goto do_unallocated;
+}
+gen_xaflag();
+s->base.is_jmp = DISAS_NEXT;
+break;
+
+case 0x02: /* AXFlag */
+if (crm != 0 || !dc_isar_feature(aa64_condm_5, s)) {
+goto do_unallocated;
+}
+gen_axflag();
+s->base.is_jmp = DISAS_NEXT;
+break;
+
case 0x05: /* SPSel */
if (s->current_el == 0) {
goto do_unallocated;
--
2.17.2
[Qemu-devel] [PATCH 0/3] target/arm: Implement ARMv8.5-CondM
Based-on: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg02733.html aka the v3 ARMv8.5-MemTag patch set, or at least some of the early patches that split handle_msr_i. The v8.4 parts have been tested vs FVP, but there's no released version that supports v8.5 yet, so XAFlag and AXFlag are untested. But they seem fairly straightforward, unless I've done something silly. r~ Richard Henderson (3): target/arm: Rearrange disas_data_proc_reg target/arm: Implement ARMv8.4-CondM target/arm: Implement ARMv8.5-CondM target/arm/cpu.h | 10 ++ linux-user/elfload.c | 1 + target/arm/cpu64.c | 1 + target/arm/translate-a64.c | 249 +++-- 4 files changed, 221 insertions(+), 40 deletions(-) -- 2.17.2
[Qemu-devel] [PATCH 2/3] target/arm: Implement ARMv8.4-CondM
Signed-off-by: Richard Henderson
---
target/arm/cpu.h | 5 ++
linux-user/elfload.c | 1 +
target/arm/cpu64.c | 1 +
target/arm/translate-a64.c | 97 +-
4 files changed, 103 insertions(+), 1 deletion(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index f8ff795dcf..000e778619 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3375,6 +3375,11 @@ static inline bool isar_feature_aa64_dp(const
ARMISARegisters *id)
return FIELD_EX64(id->id_aa64isar0, ID_AA64ISAR0, DP) != 0;
}
+static inline bool isar_feature_aa64_condm_4(const ARMISARegisters *id)
+{
+return FIELD_EX64(id->id_aa64isar0, ID_AA64ISAR0, TS) != 0;
+}
+
static inline bool isar_feature_aa64_fcma(const ARMISARegisters *id)
{
return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, FCMA) != 0;
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 3a50d587ff..ef7138839d 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -602,6 +602,7 @@ static uint32_t get_elf_hwcap(void)
GET_FEATURE_ID(aa64_fcma, ARM_HWCAP_A64_FCMA);
GET_FEATURE_ID(aa64_sve, ARM_HWCAP_A64_SVE);
GET_FEATURE_ID(aa64_pauth, ARM_HWCAP_A64_PACA | ARM_HWCAP_A64_PACG);
+GET_FEATURE_ID(aa64_condm_4, ARM_HWCAP_A64_FLAGM);
#undef GET_FEATURE_ID
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 7bd761b8f5..27b95ef787 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -334,6 +334,7 @@ static void aarch64_max_initfn(Object *obj)
t = FIELD_DP64(t, ID_AA64ISAR0, SM3, 1);
t = FIELD_DP64(t, ID_AA64ISAR0, SM4, 1);
t = FIELD_DP64(t, ID_AA64ISAR0, DP, 1);
+t = FIELD_DP64(t, ID_AA64ISAR0, TS, 1);
cpu->isar.id_aa64isar0 = t;
t = cpu->isar.id_aa64isar1;
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 0e16487be0..b88eccef53 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -1655,6 +1655,14 @@ static void handle_msr_i(DisasContext *s, uint32_t insn,
s->base.is_jmp = DISAS_TOO_MANY;
switch (op) {
+case 0x00: /* CFINV */
+if (crm != 0 || !dc_isar_feature(aa64_condm_4, s)) {
+goto do_unallocated;
+}
+tcg_gen_xori_i32(cpu_CF, cpu_CF, 1);
+s->base.is_jmp = DISAS_NEXT;
+break;
+
case 0x05: /* SPSel */
if (s->current_el == 0) {
goto do_unallocated;
@@ -1719,7 +1727,6 @@ static void gen_get_nzcv(TCGv_i64 tcg_rt)
}
static void gen_set_nzcv(TCGv_i64 tcg_rt)
-
{
TCGv_i32 nzcv = tcg_temp_new_i32();
@@ -4729,6 +4736,82 @@ static void disas_adc_sbc(DisasContext *s, uint32_t insn)
}
}
+/* Rotate right into flags
+ * 31 30 2921 15 10 5 4 0
+ * +--+--+--+-++---+--+--+--+
+ * |sf|op| S| 1 1 0 1 0 0 0 0 | imm6 | 0 0 0 0 1 | Rn |o2| mask |
+ * +--+--+--+-++---+--+--+--+
+ */
+static void disas_rotate_right_into_flags(DisasContext *s, uint32_t insn)
+{
+int mask = extract32(insn, 0, 4);
+int o2 = extract32(insn, 4, 1);
+int rn = extract32(insn, 5, 5);
+int imm6 = extract32(insn, 15, 6);
+int sf_op_s = extract32(insn, 29, 3);
+TCGv_i64 tcg_rn;
+TCGv_i32 nzcv;
+
+if (sf_op_s != 5 || o2 != 0 || !dc_isar_feature(aa64_condm_4, s)) {
+unallocated_encoding(s);
+return;
+}
+
+tcg_rn = read_cpu_reg(s, rn, 1);
+tcg_gen_rotri_i64(tcg_rn, tcg_rn, imm6);
+
+nzcv = tcg_temp_new_i32();
+tcg_gen_extrl_i64_i32(nzcv, tcg_rn);
+
+if (mask & 8) { /* N */
+tcg_gen_shli_i32(cpu_NF, nzcv, 31 - 3);
+}
+if (mask & 4) { /* Z */
+tcg_gen_not_i32(cpu_ZF, nzcv);
+tcg_gen_andi_i32(cpu_ZF, cpu_ZF, 4);
+}
+if (mask & 2) { /* C */
+tcg_gen_extract_i32(cpu_CF, nzcv, 1, 1);
+}
+if (mask & 1) { /* V */
+tcg_gen_shli_i32(cpu_VF, nzcv, 31 - 0);
+}
+
+tcg_temp_free_i32(nzcv);
+}
+
+/* Evaluate into flags
+ * 31 30 292115 1410 5 4 0
+ * +--+--+--+-+-++-+--+--+--+
+ * |sf|op| S| 1 1 0 1 0 0 0 0 | opcode2 | sz | 0 0 1 0 | Rn |o3| mask |
+ * +--+--+--+-+-++-+--+--+--+
+ */
+static void disas_evaluate_into_flags(DisasContext *s, uint32_t insn)
+{
+int o3_mask = extract32(insn, 0, 5);
+int rn = extract32(insn, 5, 5);
+int o2 = extract32(insn, 15, 6);
+int sz = extract32(insn, 14, 1);
+int sf_op_s = extract32(insn, 29, 3);
+TCGv_i32 tmp;
+int shift;
+
+if (sf_op_s != 1 || o2 != 0 || o3_mask != 0xd ||
+!dc_isar_feature(aa64_condm_4, s)) {
+unallocated_encoding(s);
+return;
+}
+shift = sz ? 16 : 24; /* SETF16 or SETF8 */
+
+tmp = tcg_temp_new_i32();
+tcg_gen_extrl_i64_i32(tmp, cpu_reg(s, rn));
+tcg_gen_shli_i32(cpu_NF, tmp, shift);
+
[Qemu-devel] [PATCH 1/3] target/arm: Rearrange disas_data_proc_reg
This decoding more closely matches the ARMv8.4 Table C4-6,
Encoding table for Data Processing - Register Group.
In particular, op2 == 0 is now more than just Add/sub (with carry).
Signed-off-by: Richard Henderson
---
target/arm/translate-a64.c | 98 ++
1 file changed, 57 insertions(+), 41 deletions(-)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 951c8f4129..0e16487be0 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -4694,11 +4694,10 @@ static void disas_data_proc_3src(DisasContext *s,
uint32_t insn)
}
/* Add/subtract (with carry)
- * 31 30 29 28 27 26 25 24 23 22 21 20 16 15 10 95 4 0
- * +--+--+--++--+-+--+-+
- * |sf|op| S| 1 1 0 1 0 0 0 0 | rm | opcode2 | Rn | Rd |
- * +--+--+--++--+-+--+-+
- *[00]
+ * 31 30 29 28 27 26 25 24 23 22 21 20 16 15 10 95 4 0
+ * +--+--+--++--+-+--+-+
+ * |sf|op| S| 1 1 0 1 0 0 0 0 | rm | 0 0 0 0 0 0 | Rn | Rd |
+ * +--+--+--++--+-+--+-+
*/
static void disas_adc_sbc(DisasContext *s, uint32_t insn)
@@ -4706,11 +4705,6 @@ static void disas_adc_sbc(DisasContext *s, uint32_t insn)
unsigned int sf, op, setflags, rm, rn, rd;
TCGv_i64 tcg_y, tcg_rn, tcg_rd;
-if (extract32(insn, 10, 6) != 0) {
-unallocated_encoding(s);
-return;
-}
-
sf = extract32(insn, 31, 1);
op = extract32(insn, 30, 1);
setflags = extract32(insn, 29, 1);
@@ -5397,47 +5391,69 @@ static void disas_data_proc_2src(DisasContext *s,
uint32_t insn)
}
}
-/* Data processing - register */
+/*
+ * Data processing - register
+ * 31 30 29 28 2521 20 16 10 0
+ * +--+---+--+---+---+-+---+---+-+
+ * | |op0| |op1| 1 0 1 | op2 | | op3 | |
+ * +--+---+--+---+---+-+---+---+-+
+ */
static void disas_data_proc_reg(DisasContext *s, uint32_t insn)
{
-switch (extract32(insn, 24, 5)) {
-case 0x0a: /* Logical (shifted register) */
-disas_logic_reg(s, insn);
-break;
-case 0x0b: /* Add/subtract */
-if (insn & (1 << 21)) { /* (extended register) */
-disas_add_sub_ext_reg(s, insn);
+int op0 = extract32(insn, 30, 1);
+int op1 = extract32(insn, 28, 1);
+int op2 = extract32(insn, 21, 4);
+int op3 = extract32(insn, 10, 6);
+
+if (!op1) {
+if (op2 & 8) {
+if (op2 & 1) {
+/* Add/sub (extended register) */
+disas_add_sub_ext_reg(s, insn);
+} else {
+/* Add/sub (shifted register) */
+disas_add_sub_reg(s, insn);
+}
} else {
-disas_add_sub_reg(s, insn);
+/* Logical (shifted register) */
+disas_logic_reg(s, insn);
}
-break;
-case 0x1b: /* Data-processing (3 source) */
-disas_data_proc_3src(s, insn);
-break;
-case 0x1a:
-switch (extract32(insn, 21, 3)) {
-case 0x0: /* Add/subtract (with carry) */
+return;
+}
+
+switch (op2) {
+case 0x0:
+switch (op3) {
+case 0x00: /* Add/subtract (with carry) */
disas_adc_sbc(s, insn);
break;
-case 0x2: /* Conditional compare */
-disas_cc(s, insn); /* both imm and reg forms */
-break;
-case 0x4: /* Conditional select */
-disas_cond_select(s, insn);
-break;
-case 0x6: /* Data-processing */
-if (insn & (1 << 30)) { /* (1 source) */
-disas_data_proc_1src(s, insn);
-} else {/* (2 source) */
-disas_data_proc_2src(s, insn);
-}
-break;
+
default:
-unallocated_encoding(s);
-break;
+goto do_unallocated;
}
break;
+
+case 0x2: /* Conditional compare */
+disas_cc(s, insn); /* both imm and reg forms */
+break;
+
+case 0x4: /* Conditional select */
+disas_cond_select(s, insn);
+break;
+
+case 0x6: /* Data-processing */
+if (op0) {/* (1 source) */
+disas_data_proc_1src(s, insn);
+} else { /* (2 source) */
+disas_data_proc_2src(s, insn);
+}
+break;
+case 0x8 ... 0xf: /* (3 source) */
+disas_data_proc_3src(s, insn);
+break;
+
default:
+do_unallocated:
unallocated_encoding(s);
break;
}
--
2.17.2
Re: [Qemu-devel] [PATCH v2] target/mips: implement QMP query-cpu-definitions command
Eric Blake writes:
> On 2/19/19 1:15 PM, Aleksandar Markovic wrote:
>> From: Pavel Dovgalyuk
>>
>> This patch enables QMP-based querying of the available CPU types for
>> MIPS and MIPS64 platforms.
>>
>> Signed-off-by: Pavel Dovgalyuk
>> Signed-off-by: Aleksandar Markovic
>> Reviewed-by: Philippe Mathieu-Daudé
>> Tested-by: Philippe Mathieu-Daudé
>> ---
>
>> +++ b/qapi/target.json
>> @@ -499,7 +499,7 @@
>> 'static': 'bool',
>> '*unavailable-features': [ 'str' ],
>> 'typename': 'str' },
>> - 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386)
>> || defined(TARGET_S390X)' }
>> + 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386)
>> || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
>
> Hmm. Long line; however, the argument to 'if' is pasted literally to an
> #if directive, which would break if we added newlines in the middle.
> And we can't use literal newlines in the middle of a JSON string. About
> the only thing I could think of that might allow for more manageable
> line lengths would be permitting:
>
> 'if': [ 'defined(TARGET_PPC)',
> 'defined(TARGET_ARM)' ...]
>
> where the QAPI generator would in turn form the disjunction of supplying
> the || between each term when the 'if' is an array of strings. But that
> feels like a lot of effort for little gain compared to just living with
> the long lines.
I hate long lines as much as anyone, but we already burned the list
syntax on conjunction:
docs/devel/qapi-code-gen.txt:
=== Configuring the schema ===
The 'struct', 'enum', 'union', 'alternate', 'command' and 'event'
top-level expressions can take an 'if' key. Its value must be a string
or a list of strings. A string is shorthand for a list containing just
that string. The code generated for the top-level expression will then
be guarded by #if COND for each COND in the list.
Example: a conditional struct
{ 'struct': 'IfStruct', 'data': { 'foo': 'int' },
'if': ['defined(CONFIG_FOO)', 'defined(HAVE_BAR)'] }
gets its generated code guarded like this:
#if defined(CONFIG_FOO)
#if defined(HAVE_BAR)
... generated code ...
#endif /* defined(HAVE_BAR) */
#endif /* defined(CONFIG_FOO) */
The problem of long strings requiring long lines is not limited to 'if'.
The common solution in programming languages is to concatenate adjacent
string literals. A more user-friendly QAPI language would support
something like that. By choosing JSON, a language designed "for the
serialization of structured data" (RFC 4627, 7159, 8259), we opted
against user-friendly. We then admitted our regrets halfheartedly by
making our version of JSON incompatible enough to defeat tools like
javascript-mode. Correcting our fundamental mistake now would be a lot
of churn.
Re: [Qemu-devel] [PATCH v4 09/11] virtio-net: update the head descriptor in a chain lastly
On Wed, Feb 20, 2019 at 10:34:32AM +0800, Jason Wang wrote:
>
> On 2019/2/20 上午9:54, Wei Xu wrote:
> >On Tue, Feb 19, 2019 at 09:09:33PM +0800, Jason Wang wrote:
> >>On 2019/2/19 下午6:51, Wei Xu wrote:
> >>>On Tue, Feb 19, 2019 at 03:23:01PM +0800, Jason Wang wrote:
> On 2019/2/14 下午12:26, w...@redhat.com wrote:
> >From: Wei Xu
> >
> >This is a helper for packed ring.
> >
> >To support packed ring, the head descriptor in a chain should be updated
> >lastly since no 'avail_idx' like in packed ring to explicitly tell the
> >driver side that all payload is ready after having done the chain, so
> >the head is always visible immediately.
> >
> >This patch fills the header after done all the other ones.
> >
> >Signed-off-by: Wei Xu
> It's really odd to workaround API issue in the implementation of device.
> Please introduce batched used updating helpers instead.
> >>>Can you elaborate a bit more? I don't get it as well.
> >>>
> >>>The exact batch as vhost-net or dpdk pmd is not supported by userspace
> >>>backend. The change here is to keep the header descriptor updated at
> >>>last in case of a chaining descriptors and the helper might not help
> >>>too much.
> >>>
> >>>Wei
> >>
> >>Of course we can add batching support why not?
> >It is always good to improve performance with anything, while probably
> >this could be done in another separate batch, also we need to bear
> >in mind that usually qemu userspace backend is not the first option for
> >performance oriented user.
>
>
> The point is to hide layout specific things from device emulation. If it
> helps for performance, it could be treated as a good byproduct.
>
>
> >
> >AFAICT, virtqueue_fill() is a generic API for all relevant userspace virtio
> >devices that do not support batching , without touching virtqueue_fill(),
> >supporting batching changes the meaning of the parameter 'idx' which should
> >be kept overall.
> >
> >To fix it, I got two proposals so far:
> >1). batching support(two APIs needed to keep compatibility)
> >2). save a head elem for a vq instead of caching an array of elems like
> >vhost,
> > and introduce a new API(virtqueue_chain_fill()) functioning with an
> > additional parameter 'more' to the current virtqueue_fill() to indicate
> > if
> > there are more descriptor(s) coming in a chain.
> >
> >Either way it changes the API somehow and it does not seem to be clean and
> >clear
> >as wanted.
>
>
> It's as simple as accepting an array of elems in e.g
> virtqueue_fill_batched()?
It is trivial for both, my concern is an array elements need to be allocated
dynamically
due to vq size which no any other devices are using, a head would be enough for
2.
>
>
> >
> >Any better idea?
> >
> >>Your code assumes the device know the virtio layout specific assumption
> >>whih breaks the layer. Device should not care about the actual layout.
> >>
> >Good point, but anyway, change to virtio-net receiving code path is
> >unavoidable to support split and packed rings, and batching is like a new
> >feature somehow.
>
>
> It's ok to change the code as a result of introducing of generic helper but
> it's bad to change to code for working around a bad API.
Agree.
Wei
>
> Thanks
>
>
> >
> >Wei
> >>Thanks
> >>
> >>
> Thanks
>
>
> >---
> > hw/net/virtio-net.c | 11 ++-
> > 1 file changed, 10 insertions(+), 1 deletion(-)
> >
> >diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> >index 3f319ef..330abea 100644
> >--- a/hw/net/virtio-net.c
> >+++ b/hw/net/virtio-net.c
> >@@ -1251,6 +1251,8 @@ static ssize_t
> >virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> > struct virtio_net_hdr_mrg_rxbuf mhdr;
> > unsigned mhdr_cnt = 0;
> > size_t offset, i, guest_offset;
> >+VirtQueueElement head;
> >+int head_len = 0;
> > if (!virtio_net_can_receive(nc)) {
> > return -1;
> >@@ -1328,7 +1330,13 @@ static ssize_t
> >virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> > }
> > /* signal other side */
> >-virtqueue_fill(q->rx_vq, elem, total, i++);
> >+if (i == 0) {
> >+head_len = total;
> >+head = *elem;
> >+} else {
> >+virtqueue_fill(q->rx_vq, elem, len, i);
> >+}
> >+i++;
> > g_free(elem);
> > }
> >@@ -1339,6 +1347,7 @@ static ssize_t
> >virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> > &mhdr.num_buffers, sizeof mhdr.num_buffers);
> > }
> >+virtqueue_fill(q->rx_vq, &head, head_len, 0);
> > virtqueue_flush(q->rx_vq, i);
> > virtio_notify(vdev, q->rx_vq);
>
Re: [Qemu-devel] [PATCH v2] target/mips: implement QMP query-cpu-definitions command
Aleksandar Markovic writes: > From: Pavel Dovgalyuk > > This patch enables QMP-based querying of the available CPU types for > MIPS and MIPS64 platforms. > > Signed-off-by: Pavel Dovgalyuk > Signed-off-by: Aleksandar Markovic > Reviewed-by: Philippe Mathieu-Daudé > Tested-by: Philippe Mathieu-Daudé Almost identical to the ARM code. Factoring out the common core doesn't seem worth the bother for just two copies. Perhaps if we grow more copies. Reviewed-by: Markus Armbruster
Re: [Qemu-devel] [PATCH] ppc/xive: xive does not have a POWER7 interrupt model
On Tue, Feb 19, 2019 at 03:25:30PM +0100, Cédric Le Goater wrote:
> Patch "target/ppc: Add POWER9 external interrupt model" should have
> removed the section covering PPC_FLAGS_INPUT_POWER7.
>
> Signed-off-by: Cédric Le Goater
Applied to ppc-for-4.0, thanks.
> ---
> hw/intc/xive.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 425aa97ef9f6..daa7badc8492 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -481,9 +481,6 @@ static void xive_tctx_realize(DeviceState *dev, Error
> **errp)
>
> env = &cpu->env;
> switch (PPC_INPUT(env)) {
> -case PPC_FLAGS_INPUT_POWER7:
> -tctx->output = env->irq_inputs[POWER7_INPUT_INT];
> -break;
> case PPC_FLAGS_INPUT_POWER9:
> tctx->output = env->irq_inputs[POWER9_INPUT_INT];
> break;
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH] hw/pci/pci-stub: Add msi_enabled() and msi_notify() to the pci stubs
On Tue, Feb 19, 2019 at 06:19:30PM -0500, Paolo Bonzini wrote: > > > > Makes sense, but it is also abstraction time. :) What if instead there > > > was a function > > > > > > void msi_allocate_irqs(PCIDevice *pdev, int num, bool fallback_to_intx); > > > > > > and then ich.c did > > > > > > irqs = msi_allocate_irqs(pdev, 1, true); > > > s->irq = irqs[0]; > > > g_free(irqs); > > > > > > ? "if msi_enabled raise MSI else raise INTX" is really a common idiom. > > > > > > Thanks, > > > > > > Paolo > > > > Maybe it is but the specific issue is not about fallback to INTX of PCI > > (is the fallback broken for ahci? I don't know). > > It works, the above is just a new abstraction. > > > The trick is there's no pdev at all. > > The trick :) is that in ich.c there is a pdev. Right now we are assigning to > s->irq either the INTX irq (if PCI) or a sysbus irq (if sysbus), but then > we need to know about MSI with a wrapper around s->irq. Oh you mean just for PCI. > Instead, my suggestion is to put the wrapper in the PCI core as a qemu_irq > callback---or perhaps in ich.c, but anyway ahci.c should not care that there > could be a PCI AHCI device and it would have two different interrupt modes. I like it very much that devices call pci_set_irq, I'd rather not have callbacks. I think the wrapper thay calls either pci_set_irq isn't a problem, problem is MSI/X has multiple vectors, INTX doesn't. So for many devices there's something extra that happens just in one mode but not the other to deal with multiple vectors. So I don't think it can be an abstraction that everyone uses. But yes it can be a helper function. In fact mptsas_update_interrupt seems not to be PCI spec compliant: it sets both MSI and INTX. CC original contributor with this question. > In fact, doing this would also remove the need for s->container, I think. > > Paolo
Re: [Qemu-devel] [PATCH v5 11/17] spapr: populate PHB DRC entries for root DT node
On Tue, Feb 19, 2019 at 06:18:29PM +0100, Greg Kurz wrote:
> From: Nathan Fontenot
>
> This add entries to the root OF node to advertise our PHBs as being
> DR-capable in accordance with PAPR specification.
>
> Signed-off-by: Nathan Fontenot
> Signed-off-by: Michael Roth
> Reviewed-by: David Gibson
> Signed-off-by: Greg Kurz
Applied, thanks.
> ---
> hw/ppc/spapr.c |8
> 1 file changed, 8 insertions(+)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 96bea7580a3f..fcda17709066 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1365,6 +1365,14 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr)
> exit(1);
> }
>
> +if (smc->dr_phb_enabled) {
> +ret = spapr_drc_populate_dt(fdt, 0, NULL,
> SPAPR_DR_CONNECTOR_TYPE_PHB);
> +if (ret < 0) {
> +error_report("Couldn't set up PHB DR device tree properties");
> +exit(1);
> +}
> +}
> +
> return fdt;
> }
>
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH v5 00/17] spapr: Add support for PHB hotplug
On Tue, Feb 19, 2019 at 06:17:33PM +0100, Greg Kurz wrote: > This allows to hotplug/unplug PHBs. I could successfully test: > - using in-kernel XICS, emulated XICS and XIVE > - hotplug/unplug with e1000 device to validate LSIs > - hotplug/unplug with virtio-net device to validate MSIs > - some simple migration scenarios > > Based on David's ppc-for-4.0 branch SHA1: Applied! > > 6f585625d0d1 target/ppc: Basic POWER9 bare-metal radix MMU support > > Please comment. > > Changes in v5: > - all DRC subtypes generate FDT fragment at configure connector time > - Drop all the LSI bitmap and allocation/typing disintricate stuff > - set IRQ type in KVM at claim time > - fix hotplug call chain > - added PHB unplug test to tests/device-plug-test > > Changes in v4: > - added a LSI bitmap to XICS > - no longer need compat property in XICS > - simplified the patches to access the name and the phandle of the > interrupt controller > - delay the creation of the PHB drc->fdt to RTAS ibm,configure-connector > > Change in v3: > - reworked phandle related code some more > - disintricate allocation/"type setting" of interrupts > - identify LSIs at machine init > > Changes in v2: > - rebased on current ppc-for-4.0 > - added some preliminary cleanup > - call unrealize from realize error path > - advertise PHB hotplug in last patch > - reworked phandle related code > - sync LSIs to KVM > -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v5 13/17] spapr_pci: provide node start offset via spapr_populate_pci_dt()
On Tue, Feb 19, 2019 at 06:18:39PM +0100, Greg Kurz wrote:
> From: Michael Roth
>
> PHB hotplug re-uses PHB device tree generation code and passes
> it to a guest via RTAS. Doing this requires knowledge of where
> exactly in the device tree the node describing the PHB begins.
>
> Provide this via a new optional pointer that can be used to
> store the PHB node's start offset.
>
> Signed-off-by: Michael Roth
> Reviewed-by: David Gibson
> Signed-off-by: Greg Kurz
Applied, thanks.
> ---
> hw/ppc/spapr.c |2 +-
> hw/ppc/spapr_pci.c |5 -
> include/hw/pci-host/spapr.h |2 +-
> 3 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index fcda17709066..76b3c15d5952 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1312,7 +1312,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr)
>
> QLIST_FOREACH(phb, &spapr->phbs, list) {
> ret = spapr_populate_pci_dt(phb, PHANDLE_INTC, fdt,
> -spapr->irq->nr_msis);
> +spapr->irq->nr_msis, NULL);
> if (ret < 0) {
> error_report("couldn't setup PCI devices in fdt");
> exit(1);
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index ede928b0bff3..a0e17694396a 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -2153,7 +2153,7 @@ static void spapr_phb_pci_enumerate(sPAPRPHBState *phb)
> }
>
> int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t intc_phandle, void
> *fdt,
> - uint32_t nr_msis)
> + uint32_t nr_msis, int *node_offset)
> {
> int bus_off, i, j, ret;
> gchar *nodename;
> @@ -2208,6 +2208,9 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t
> intc_phandle, void *fdt,
> nodename = g_strdup_printf("pci@%" PRIx64, phb->buid);
> _FDT(bus_off = fdt_add_subnode(fdt, 0, nodename));
> g_free(nodename);
> +if (node_offset) {
> +*node_offset = bus_off;
> +}
>
> /* Write PHB properties */
> _FDT(fdt_setprop_string(fdt, bus_off, "device_type", "pci"));
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index 4b0443f4cfe4..ab0e3a0a6f72 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -113,7 +113,7 @@ static inline qemu_irq spapr_phb_lsi_qirq(struct
> sPAPRPHBState *phb, int pin)
> }
>
> int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t intc_phandle, void
> *fdt,
> - uint32_t nr_msis);
> + uint32_t nr_msis, int *node_offset);
>
> void spapr_pci_rtas_init(void);
>
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH v5 12/17] spapr_events: add support for phb hotplug events
On Tue, Feb 19, 2019 at 06:18:34PM +0100, Greg Kurz wrote: > From: Michael Roth > > Extend the existing EPOW event format we use for PCI > devices to emit PHB plug/unplug events. > > Signed-off-by: Michael Roth > Reviewed-by: David Gibson > Signed-off-by: Greg Kurz Applied, thanks. > --- > hw/ppc/spapr_events.c |3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c > index b9c7ecb9e987..ab9a1f0063d5 100644 > --- a/hw/ppc/spapr_events.c > +++ b/hw/ppc/spapr_events.c > @@ -526,6 +526,9 @@ static void spapr_hotplug_req_event(uint8_t hp_id, > uint8_t hp_action, > case SPAPR_DR_CONNECTOR_TYPE_CPU: > hp->hotplug_type = RTAS_LOG_V6_HP_TYPE_CPU; > break; > +case SPAPR_DR_CONNECTOR_TYPE_PHB: > +hp->hotplug_type = RTAS_LOG_V6_HP_TYPE_PHB; > +break; > default: > /* we shouldn't be signaling hotplug events for resources > * that don't support them > -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v5 14/17] spapr_pci: add ibm, my-drc-index property for PHB hotplug
On Tue, Feb 19, 2019 at 06:18:44PM +0100, Greg Kurz wrote:
> From: Michael Roth
>
> This is needed to denote a boot-time PHB as being hot-pluggable.
>
> Signed-off-by: Michael Roth
> Reviewed-by: David Gibson
> Signed-off-by: Greg Kurz
Applied, thanks.
> ---
> hw/ppc/spapr_pci.c |9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index a0e17694396a..03fc26985ab1 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -2203,6 +2203,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t
> intc_phandle, void *fdt,
> sPAPRTCETable *tcet;
> PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus;
> sPAPRFDT s_fdt;
> +sPAPRDRConnector *drc;
>
> /* Start populating the FDT */
> nodename = g_strdup_printf("pci@%" PRIx64, phb->buid);
> @@ -2269,6 +2270,14 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t
> intc_phandle, void *fdt,
> tcet->liobn, tcet->bus_offset,
> tcet->nb_table << tcet->page_shift);
>
> +drc = spapr_drc_by_id(TYPE_SPAPR_DRC_PHB, phb->index);
> +if (drc) {
> +uint32_t drc_index = cpu_to_be32(spapr_drc_index(drc));
> +
> +_FDT(fdt_setprop(fdt, bus_off, "ibm,my-drc-index", &drc_index,
> + sizeof(drc_index)));
> +}
> +
> /* Walk the bridges and program the bus numbers*/
> spapr_phb_pci_enumerate(phb);
> _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,phb-enumerated", 0x1));
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH v5 09/17] spapr_pci: add PHB unrealize
On Tue, Feb 19, 2019 at 06:18:18PM +0100, Greg Kurz wrote:
> To support PHB hotplug we need to clean up lingering references,
> memory, child properties, etc. prior to the PHB object being
> finalized. Generally this will be called as a result of calling
> object_unparent() on the PHB object, which in turn would normally
> be called as the result of an unplug() operation.
>
> When the PHB is finalized, child objects will be unparented in
> turn, and finalized if the PHB was the only reference holder. so
> we don't bother to explicitly unparent child objects of the PHB,
> with the notable exception of DRCs. This is needed to avoid a QEMU
> crash when unplugging a PHB and resetting the machine before the
> guest could handle the event. The DRCs are removed from the QOM tree
> by pci_unregister_root_bus() and we must make sure we're not leaving
> stale aliases under the global /dr-connector path.
>
> The formula that gives the number of DMA windows is moved to an
> inline function in the hw/pci-host/spapr.h header because it
> will have other users.
>
> The unrealize function is able to cope with partially realized PHBs.
> It is hence used to implement proper rollback on the realize error
> path.
>
> Signed-off-by: Michael Roth
> Signed-off-by: Greg Kurz
> Reviewed-by: David Gibson
Applied, thanks.
> ---
> v5: - unparent child DRCs at unrealize
> v4: - reverted to v2
> v3: - don't free LSIs at unrealize
> v2: - implement rollback with unrealize function
> ---
> ---
> hw/ppc/spapr_pci.c | 86
> +--
> include/hw/pci-host/spapr.h |5 +++
> 2 files changed, 87 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index e2bc9fec824b..ede928b0bff3 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1570,6 +1570,75 @@ static void spapr_pci_unplug_request(HotplugHandler
> *plug_handler,
> }
> }
>
> +static void spapr_phb_finalizefn(Object *obj)
> +{
> +sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(obj);
> +
> +g_free(sphb->dtbusname);
> +sphb->dtbusname = NULL;
> +}
> +
> +static void spapr_phb_unrealize(DeviceState *dev, Error **errp)
> +{
> +sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> +SysBusDevice *s = SYS_BUS_DEVICE(dev);
> +PCIHostState *phb = PCI_HOST_BRIDGE(s);
> +sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(phb);
> +sPAPRTCETable *tcet;
> +int i;
> +const unsigned windows_supported = spapr_phb_windows_supported(sphb);
> +
> +if (sphb->msi) {
> +g_hash_table_unref(sphb->msi);
> +sphb->msi = NULL;
> +}
> +
> +/*
> + * Remove IO/MMIO subregions and aliases, rest should get cleaned
> + * via PHB's unrealize->object_finalize
> + */
> +for (i = windows_supported - 1; i >= 0; i--) {
> +tcet = spapr_tce_find_by_liobn(sphb->dma_liobn[i]);
> +if (tcet) {
> +memory_region_del_subregion(&sphb->iommu_root,
> +spapr_tce_get_iommu(tcet));
> +}
> +}
> +
> +if (sphb->dr_enabled) {
> +for (i = PCI_SLOT_MAX * 8 - 1; i >= 0; i--) {
> +sPAPRDRConnector *drc = spapr_drc_by_id(TYPE_SPAPR_DRC_PCI,
> +(sphb->index << 16) | i);
> +
> +if (drc) {
> +object_unparent(OBJECT(drc));
> +}
> +}
> +}
> +
> +for (i = PCI_NUM_PINS - 1; i >= 0; i--) {
> +if (sphb->lsi_table[i].irq) {
> +spapr_irq_free(spapr, sphb->lsi_table[i].irq, 1);
> +sphb->lsi_table[i].irq = 0;
> +}
> +}
> +
> +QLIST_REMOVE(sphb, list);
> +
> +memory_region_del_subregion(&sphb->iommu_root, &sphb->msiwindow);
> +
> +address_space_destroy(&sphb->iommu_as);
> +
> +qbus_set_hotplug_handler(BUS(phb->bus), NULL, &error_abort);
> +pci_unregister_root_bus(phb->bus);
> +
> +memory_region_del_subregion(get_system_memory(), &sphb->iowindow);
> +if (sphb->mem64_win_pciaddr != (hwaddr)-1) {
> +memory_region_del_subregion(get_system_memory(), &sphb->mem64window);
> +}
> +memory_region_del_subregion(get_system_memory(), &sphb->mem32window);
> +}
> +
> static void spapr_phb_realize(DeviceState *dev, Error **errp)
> {
> /* We don't use SPAPR_MACHINE() in order to exit gracefully if the user
> @@ -1587,8 +1656,7 @@ static void spapr_phb_realize(DeviceState *dev, Error
> **errp)
> PCIBus *bus;
> uint64_t msi_window_size = 4096;
> sPAPRTCETable *tcet;
> -const unsigned windows_supported =
> -sphb->ddw_enabled ? SPAPR_PCI_DMA_MAX_WINDOWS : 1;
> +const unsigned windows_supported = spapr_phb_windows_supported(sphb);
>
> if (!spapr) {
> error_setg(errp, TYPE_SPAPR_PCI_HOST_BRIDGE " needs a pseries
> machine");
> @@ -1745,6 +1813,10 @@ static void spapr_phb_realize(DeviceState *dev, Error
> **errp)
> if (local_e
Re: [Qemu-devel] [PATCH v5 07/17] spapr: Expose the name of the interrupt controller node
On Tue, Feb 19, 2019 at 06:18:08PM +0100, Greg Kurz wrote:
> This will be needed by PHB hotplug in order to access the "phandle"
> property of the interrupt controller node.
>
> Reviewed-by: Cédric Le Goater
> Signed-off-by: Greg Kurz
> Reviewed-by: David Gibson
Applied, thanks.
> ---
> hw/intc/spapr_xive.c|9 -
> hw/intc/xics_spapr.c|2 +-
> hw/ppc/spapr_irq.c | 21 -
> include/hw/ppc/spapr_irq.h |1 +
> include/hw/ppc/spapr_xive.h |3 +++
> include/hw/ppc/xics_spapr.h |2 ++
> 6 files changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 290a290e43a5..06e3c9fdbfeb 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -317,6 +317,9 @@ static void spapr_xive_realize(DeviceState *dev, Error
> **errp)
> /* Map all regions */
> spapr_xive_map_mmio(xive);
>
> +xive->nodename = g_strdup_printf("interrupt-controller@%" PRIx64,
> + xive->tm_base + XIVE_TM_USER_PAGE * (1 <<
> TM_SHIFT));
> +
> qemu_register_reset(spapr_xive_reset, dev);
> }
>
> @@ -1448,7 +1451,6 @@ void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t
> nr_servers, void *fdt,
> cpu_to_be32(7),/* start */
> cpu_to_be32(0xf8), /* count */
> };
> -gchar *nodename;
>
> /* Thread Interrupt Management Area : User (ring 3) and OS (ring 2) */
> timas[0] = cpu_to_be64(xive->tm_base +
> @@ -1458,10 +1460,7 @@ void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t
> nr_servers, void *fdt,
> XIVE_TM_OS_PAGE * (1ull << TM_SHIFT));
> timas[3] = cpu_to_be64(1ull << TM_SHIFT);
>
> -nodename = g_strdup_printf("interrupt-controller@%" PRIx64,
> - xive->tm_base + XIVE_TM_USER_PAGE * (1 <<
> TM_SHIFT));
> -_FDT(node = fdt_add_subnode(fdt, 0, nodename));
> -g_free(nodename);
> +_FDT(node = fdt_add_subnode(fdt, 0, xive->nodename));
>
> _FDT(fdt_setprop_string(fdt, node, "device_type", "power-ivpe"));
> _FDT(fdt_setprop(fdt, node, "reg", timas, sizeof(timas)));
> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> index e2d8b3818336..53bda6661b2a 100644
> --- a/hw/intc/xics_spapr.c
> +++ b/hw/intc/xics_spapr.c
> @@ -254,7 +254,7 @@ void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t
> nr_servers, void *fdt,
> };
> int node;
>
> -_FDT(node = fdt_add_subnode(fdt, 0, "interrupt-controller"));
> +_FDT(node = fdt_add_subnode(fdt, 0, XICS_NODENAME));
>
> _FDT(fdt_setprop_string(fdt, node, "device_type",
> "PowerPC-External-Interrupt-Presentation"));
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 4297eed600f9..359761494c6e 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -230,6 +230,11 @@ static void spapr_irq_reset_xics(sPAPRMachineState
> *spapr, Error **errp)
> /* TODO: create the KVM XICS device */
> }
>
> +static const char *spapr_irq_get_nodename_xics(sPAPRMachineState *spapr)
> +{
> +return XICS_NODENAME;
> +}
> +
> #define SPAPR_IRQ_XICS_NR_IRQS 0x1000
> #define SPAPR_IRQ_XICS_NR_MSIS \
> (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
> @@ -249,6 +254,7 @@ sPAPRIrq spapr_irq_xics = {
> .post_load = spapr_irq_post_load_xics,
> .reset = spapr_irq_reset_xics,
> .set_irq = spapr_irq_set_irq_xics,
> +.get_nodename = spapr_irq_get_nodename_xics,
> };
>
> /*
> @@ -384,6 +390,11 @@ static void spapr_irq_set_irq_xive(void *opaque, int
> srcno, int val)
> xive_source_set_irq(&spapr->xive->source, srcno, val);
> }
>
> +static const char *spapr_irq_get_nodename_xive(sPAPRMachineState *spapr)
> +{
> +return spapr->xive->nodename;
> +}
> +
> /*
> * XIVE uses the full IRQ number space. Set it to 8K to be compatible
> * with XICS.
> @@ -407,6 +418,7 @@ sPAPRIrq spapr_irq_xive = {
> .post_load = spapr_irq_post_load_xive,
> .reset = spapr_irq_reset_xive,
> .set_irq = spapr_irq_set_irq_xive,
> +.get_nodename = spapr_irq_get_nodename_xive,
> };
>
> /*
> @@ -541,6 +553,11 @@ static void spapr_irq_set_irq_dual(void *opaque, int
> srcno, int val)
> spapr_irq_current(spapr)->set_irq(spapr, srcno, val);
> }
>
> +static const char *spapr_irq_get_nodename_dual(sPAPRMachineState *spapr)
> +{
> +return spapr_irq_current(spapr)->get_nodename(spapr);
> +}
> +
> /*
> * Define values in sync with the XIVE and XICS backend
> */
> @@ -561,7 +578,8 @@ sPAPRIrq spapr_irq_dual = {
> .cpu_intc_create = spapr_irq_cpu_intc_create_dual,
> .post_load = spapr_irq_post_load_dual,
> .reset = spapr_irq_reset_dual,
> -.set_irq = spapr_irq_set_irq_dual
> +.set_irq = spapr_irq_set_irq_dual,
> +.get_nodename = spapr_irq_get_nodename_dual,
> };
>
> /*
> @@ -691,4 +709,5 @@ sPAPRIrq spapr_irq_xics_l
Re: [Qemu-devel] [PATCH v5 06/17] xics: Write source state to KVM at claim time
On Tue, Feb 19, 2019 at 06:18:03PM +0100, Greg Kurz wrote:
> The pseries machine only uses LSIs to support legacy PCI devices. Every
> PHB claims 4 LSIs at realize time. When using in-kernel XICS (or upcoming
> in-kernel XIVE), QEMU synchronizes the state of all irqs, including these
> LSIs, later on at machine reset.
>
> In order to support PHB hotplug, we need a way to tell KVM about the LSIs
> that doesn't require a machine reset. An easy way to do that is to always
> inform KVM when an interrupt is claimed, which really isn't a performance
> path.
>
> Signed-off-by: Greg Kurz
Applied, thanks.
> ---
> hw/intc/xics.c|4 +++
> hw/intc/xics_kvm.c| 74
> -
> include/hw/ppc/xics.h |1 +
> 3 files changed, 48 insertions(+), 31 deletions(-)
>
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index 767fdeb82900..af7dc709abab 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -758,6 +758,10 @@ void ics_set_irq_type(ICSState *ics, int srcno, bool lsi)
>
> ics->irqs[srcno].flags |=
> lsi ? XICS_FLAGS_IRQ_LSI : XICS_FLAGS_IRQ_MSI;
> +
> +if (kvm_irqchip_in_kernel()) {
> +ics_set_kvm_state_one(ics, srcno);
> +}
> }
>
> static void xics_register_types(void)
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index a00d0a7962e1..c6e1b630a404 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -213,45 +213,57 @@ void ics_synchronize_state(ICSState *ics)
> ics_get_kvm_state(ics);
> }
>
> -int ics_set_kvm_state(ICSState *ics)
> +int ics_set_kvm_state_one(ICSState *ics, int srcno)
> {
> uint64_t state;
> -int i;
> Error *local_err = NULL;
> +ICSIRQState *irq = &ics->irqs[srcno];
> +int ret;
>
> -for (i = 0; i < ics->nr_irqs; i++) {
> -ICSIRQState *irq = &ics->irqs[i];
> -int ret;
> -
> -state = irq->server;
> -state |= (uint64_t)(irq->saved_priority & KVM_XICS_PRIORITY_MASK)
> -<< KVM_XICS_PRIORITY_SHIFT;
> -if (irq->priority != irq->saved_priority) {
> -assert(irq->priority == 0xff);
> -state |= KVM_XICS_MASKED;
> -}
> +state = irq->server;
> +state |= (uint64_t)(irq->saved_priority & KVM_XICS_PRIORITY_MASK)
> +<< KVM_XICS_PRIORITY_SHIFT;
> +if (irq->priority != irq->saved_priority) {
> +assert(irq->priority == 0xff);
> +state |= KVM_XICS_MASKED;
> +}
>
> -if (ics->irqs[i].flags & XICS_FLAGS_IRQ_LSI) {
> -state |= KVM_XICS_LEVEL_SENSITIVE;
> -if (irq->status & XICS_STATUS_ASSERTED) {
> -state |= KVM_XICS_PENDING;
> -}
> -} else {
> -if (irq->status & XICS_STATUS_MASKED_PENDING) {
> -state |= KVM_XICS_PENDING;
> -}
> +if (irq->flags & XICS_FLAGS_IRQ_LSI) {
> +state |= KVM_XICS_LEVEL_SENSITIVE;
> +if (irq->status & XICS_STATUS_ASSERTED) {
> +state |= KVM_XICS_PENDING;
> }
> -if (irq->status & XICS_STATUS_PRESENTED) {
> -state |= KVM_XICS_PRESENTED;
> -}
> -if (irq->status & XICS_STATUS_QUEUED) {
> -state |= KVM_XICS_QUEUED;
> +} else {
> +if (irq->status & XICS_STATUS_MASKED_PENDING) {
> +state |= KVM_XICS_PENDING;
> }
> +}
> +if (irq->status & XICS_STATUS_PRESENTED) {
> +state |= KVM_XICS_PRESENTED;
> +}
> +if (irq->status & XICS_STATUS_QUEUED) {
> +state |= KVM_XICS_QUEUED;
> +}
> +
> +ret = kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
> +srcno + ics->offset, &state, true, &local_err);
> +if (local_err) {
> +error_report_err(local_err);
> +return ret;
> +}
> +
> +return 0;
> +}
> +
> +int ics_set_kvm_state(ICSState *ics)
> +{
> +int i;
> +
> +for (i = 0; i < ics->nr_irqs; i++) {
> +int ret;
>
> -ret = kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
> -i + ics->offset, &state, true, &local_err);
> -if (local_err) {
> -error_report_err(local_err);
> +ret = ics_set_kvm_state_one(ics, i);
> +if (ret) {
> return ret;
> }
> }
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index d36bbe11ee2e..eb65ad7e43b7 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -195,6 +195,7 @@ void icp_synchronize_state(ICPState *icp);
> void icp_kvm_realize(DeviceState *dev, Error **errp);
>
> void ics_get_kvm_state(ICSState *ics);
> +int ics_set_kvm_state_one(ICSState *ics, int srcno);
> int ics_set_kvm_state(ICSState *ics);
> void ics_synchronize_state(ICSState *ics);
> void ics_kvm_set_irq(ICSState *ics, int srcno, int val);
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.
Re: [Qemu-devel] [PATCH v5 05/17] spapr/drc: Drop spapr_drc_attach() fdt argument
On Tue, Feb 19, 2019 at 06:17:58PM +0100, Greg Kurz wrote:
> All DRC subtypes have been converted to generate the FDT fragment at
> configure connector time instead of attach time. The fdt and fdt_offset
> arguments of spapr_drc_attach() aren't needed anymore. Drop them and
> make the implementation of the dt_populate() method mandatory.
>
> Signed-off-by: Greg Kurz
I've applied the first 5 patches to ppc-for-4.0, but as a followup...
[...]
> @@ -1113,8 +1104,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU
> *cpu,
>
> drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
>
> -g_assert(drc->fdt || drck->dt_populate);
> -
> if (!drc->fdt) {
..you can now remove this conditional, since it will always be true.
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH v5 08/17] spapr_irq: Expose the phandle of the interrupt controller
On Tue, Feb 19, 2019 at 06:18:13PM +0100, Greg Kurz wrote:
> This will be used by PHB hotplug in order to create the "interrupt-map"
> property of the PHB node.
>
> Signed-off-by: Greg Kurz
Applied, thanks.
> ---
> v5: - return phandle by value
> v4: - return phandle via a pointer
> ---
> hw/ppc/spapr_irq.c | 21 +
> include/hw/ppc/spapr_irq.h |1 +
> 2 files changed, 22 insertions(+)
>
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 359761494c6e..4145079d7fa5 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -638,6 +638,27 @@ void spapr_irq_reset(sPAPRMachineState *spapr, Error
> **errp)
> }
> }
>
> +int spapr_irq_get_phandle(sPAPRMachineState *spapr, void *fdt, Error **errp)
> +{
> +const char *nodename = spapr->irq->get_nodename(spapr);
> +int offset, phandle;
> +
> +offset = fdt_subnode_offset(fdt, 0, nodename);
> +if (offset < 0) {
> +error_setg(errp, "Can't find node \"%s\": %s", nodename,
> + fdt_strerror(offset));
> +return -1;
> +}
> +
> +phandle = fdt_get_phandle(fdt, offset);
> +if (!phandle) {
> +error_setg(errp, "Can't get phandle of node \"%s\"", nodename);
> +return -1;
> +}
> +
> +return phandle;
> +}
> +
> /*
> * XICS legacy routines - to deprecate one day
> */
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index 8bf1a7291966..ec1ee64fa62b 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -61,6 +61,7 @@ void spapr_irq_free(sPAPRMachineState *spapr, int irq, int
> num);
> qemu_irq spapr_qirq(sPAPRMachineState *spapr, int irq);
> int spapr_irq_post_load(sPAPRMachineState *spapr, int version_id);
> void spapr_irq_reset(sPAPRMachineState *spapr, Error **errp);
> +int spapr_irq_get_phandle(sPAPRMachineState *spapr, void *fdt, Error **errp);
>
> /*
> * XICS legacy routines
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH v5 10/17] spapr: create DR connectors for PHBs
On Tue, Feb 19, 2019 at 06:18:23PM +0100, Greg Kurz wrote:
> From: Michael Roth
>
> Signed-off-by: Michael Roth
> Reviewed-by: David Gibson
> Signed-off-by: Greg Kurz
Applied, thanks.
> ---
> hw/ppc/spapr.c | 13 +
> hw/ppc/spapr_drc.c | 17 +
> include/hw/ppc/spapr.h |1 +
> include/hw/ppc/spapr_drc.h |8
> 4 files changed, 39 insertions(+)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 9364d07364ac..96bea7580a3f 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2875,6 +2875,19 @@ static void spapr_machine_init(MachineState *machine)
> /* We always have at least the nvram device on VIO */
> spapr_create_nvram(spapr);
>
> +/*
> + * Setup hotplug / dynamic-reconfiguration connectors. top-level
> + * connectors (described in root DT node's "ibm,drc-types" property)
> + * are pre-initialized here. additional child connectors (such as
> + * connectors for a PHBs PCI slots) are added as needed during their
> + * parent's realization.
> + */
> +if (smc->dr_phb_enabled) {
> +for (i = 0; i < SPAPR_MAX_PHBS; i++) {
> +spapr_dr_connector_new(OBJECT(machine), TYPE_SPAPR_DRC_PHB, i);
> +}
> +}
> +
> /* Set up PCI */
> spapr_pci_rtas_init();
>
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 87ca7d973564..fd6380adb367 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -696,6 +696,15 @@ static void spapr_drc_lmb_class_init(ObjectClass *k,
> void *data)
> drck->dt_populate = spapr_lmb_dt_populate;
> }
>
> +static void spapr_drc_phb_class_init(ObjectClass *k, void *data)
> +{
> +sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
> +
> +drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_PHB;
> +drck->typename = "PHB";
> +drck->drc_name_prefix = "PHB ";
> +}
> +
> static const TypeInfo spapr_dr_connector_info = {
> .name = TYPE_SPAPR_DR_CONNECTOR,
> .parent= TYPE_DEVICE,
> @@ -739,6 +748,13 @@ static const TypeInfo spapr_drc_lmb_info = {
> .class_init= spapr_drc_lmb_class_init,
> };
>
> +static const TypeInfo spapr_drc_phb_info = {
> +.name = TYPE_SPAPR_DRC_PHB,
> +.parent= TYPE_SPAPR_DRC_LOGICAL,
> +.instance_size = sizeof(sPAPRDRConnector),
> +.class_init= spapr_drc_phb_class_init,
> +};
> +
> /* helper functions for external users */
>
> sPAPRDRConnector *spapr_drc_by_index(uint32_t index)
> @@ -1207,6 +1223,7 @@ static void spapr_drc_register_types(void)
> type_register_static(&spapr_drc_cpu_info);
> type_register_static(&spapr_drc_pci_info);
> type_register_static(&spapr_drc_lmb_info);
> +type_register_static(&spapr_drc_phb_info);
>
> spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator",
> rtas_set_indicator);
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 5e3c76072505..b173fd714904 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -104,6 +104,7 @@ struct sPAPRMachineClass {
>
> /*< public >*/
> bool dr_lmb_enabled; /* enable dynamic-reconfig/hotplug of LMBs */
> +bool dr_phb_enabled; /* enable dynamic-reconfig/hotplug of PHBs */
> bool update_dt_enabled;/* enable KVMPPC_H_UPDATE_DT */
> bool use_ohci_by_default; /* use USB-OHCI instead of XHCI */
> bool pre_2_10_has_unused_icps;
> diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
> index f32758ec8487..46b0f6216d83 100644
> --- a/include/hw/ppc/spapr_drc.h
> +++ b/include/hw/ppc/spapr_drc.h
> @@ -71,6 +71,14 @@
> #define SPAPR_DRC_LMB(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
> TYPE_SPAPR_DRC_LMB)
>
> +#define TYPE_SPAPR_DRC_PHB "spapr-drc-phb"
> +#define SPAPR_DRC_PHB_GET_CLASS(obj) \
> +OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_PHB)
> +#define SPAPR_DRC_PHB_CLASS(klass) \
> +OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, TYPE_SPAPR_DRC_PHB)
> +#define SPAPR_DRC_PHB(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
> +TYPE_SPAPR_DRC_PHB)
> +
> /*
> * Various hotplug types managed by sPAPRDRConnector
> *
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
[Qemu-devel] [PATCH v3 2/2] CODING_STYLE: indent example code as all others
All the example code are indented with four spaces except this one.
Fix this by adding four spaces here.
Signed-off-by: Wei Yang
Reviewed-by: Eric Blake
Reviewed-by: Philippe Mathieu-Daudé
---
CODING_STYLE | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/CODING_STYLE b/CODING_STYLE
index 1359f9ab81..b1f5414b73 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -131,10 +131,10 @@ block to a separate function altogether.
When comparing a variable for (in)equality with a constant, list the
constant on the right, as in:
-if (a == 1) {
-/* Reads like: "If a equals 1" */
-do_something();
-}
+if (a == 1) {
+/* Reads like: "If a equals 1" */
+do_something();
+}
Rationale: Yoda conditions (as in 'if (1 == a)') are awkward to read.
Besides, good compilers already warn users when '==' is mis-typed as '=',
--
2.19.1
[Qemu-devel] [PATCH v3 1/2] CODING_STYLE: specify the indent rule for multiline code
We didn't specify the indent rule for multiline code here, which may
mislead users. And in current code, the code use different rules.
Add this rule in CODING_STYLE to make sure this is clear to every one.
Signed-off-by: Wei Yang
Suggested-by: Igor Mammedov
Reviewed-by: Eric Blake
---
v3:
* misleading -> mislead
* add comma after arg2 in example
v2:
* rephrase changelog suggested by Eric Blake
- remove one redundant line
- fix some awkward grammar
- add { ; at the end of example
c1
---
CODING_STYLE | 23 +++
1 file changed, 23 insertions(+)
diff --git a/CODING_STYLE b/CODING_STYLE
index ec075dedc4..1359f9ab81 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -29,6 +29,29 @@ Spaces of course are superior to tabs because:
Do not leave whitespace dangling off the ends of lines.
+1.1 Multiline Indent
+
+There are several places where indent is necessary:
+
+ - struct definition
+ - if/else
+ - while/for
+ - function definition & call
+
+When breaking up a long line to fit within line widths, align the secondary
+lines just after the opening parenthesis of the first.
+
+For example:
+
+if (a == 1 &&
+b == 2) {
+
+while (a == 1 &&
+ b == 2) {
+
+do_something(arg1, arg2,
+ arg3);
+
2. Line width
Lines should be 80 characters; try not to make them longer.
--
2.19.1
[Qemu-devel] [PATCH v3 0/2] CODING_STYLE: trivial update
The first one is suggested by Igor Mammedov to provide rule for multiline code. The second is a trivial fix to make example code all indented with 4 spaces. v3: * fix typo in both changelog and example v2: * adjust Patch 1 as suggested by Eric Wei Yang (2): CODING_STYLE: specify the indent rule for multiline code CODING_STYLE: indent example code as all others CODING_STYLE | 31 +++ 1 file changed, 27 insertions(+), 4 deletions(-) -- 2.19.1
Re: [Qemu-devel] [PATCH] doc: update .gitignore and fix typos for docs in tree
On 2/20/19 2:55 AM, Like Xu wrote: > Signed-off-by: Like Xu This feels like two independent patches - the .gitignore change is different from typo fixes. Actually, for .gitignore, you could just as easily do: echo '*.patch' >> .git/info/exclude and fix it so you never commit patch files locally, while still leaving them visible through 'git status' to other users. I actually like knowing when I have stale patch files lying around, so that I can 'rm' them before creating my next patch batch and using 'git send-email *.patch' - but of course, I could just as easily add '!*.patch' to my .git/info/exclude to override the project default if we decide to go with your patch. > +++ b/docs/colo-proxy.txt > @@ -41,7 +41,7 @@ Below is a COLO proxy ascii figure: > | | +--+ | >|| | > |netfilter| | | || | > netfilter|| | > | +--+ ++ || | > +---+ | > -| | | | | |out || | | >|| filter excute order | | > +| | | | | |out || | | >|| filter execute order | | You fixed the typo, but broke the whitespacing on this lineart. (twice) > +++ b/docs/qemu-block-drivers.texi > @@ -632,7 +632,7 @@ qemu-system-i386 -drive > file=iscsi://127.0.0.1/iqn.qemu.test/1 \ > @end example > > > -Howto set up a simple iSCSI target on loopback and accessing it via QEMU: > +How to set up a simple iSCSI target on loopback and accessing it via QEMU: While here, I would also: s/accessing/access/ > +++ b/docs/usb-storage.txt > @@ -16,7 +16,7 @@ too): > > > Number two is the newer usb attached scsi transport. This one doesn't > -automagically create a scsi disk, so you have to explicitly attach one > +automatically create a scsi disk, so you have to explicitly attach one This one may be intentional, as a play on English words. But changing it may sound a bit more professional. All your other typo fixes are good. Looking forward to v2. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v2 1/2] CODING_STYLE: specify the indent rule for multiline code
On Tue, Feb 19, 2019 at 09:02:34PM -0600, Eric Blake wrote:
>On 2/19/19 6:31 PM, Wei Yang wrote:
>> We didn't specify the indent rule for multiline code here, which may
>> misleading users. And in current code, the code use different rules.
>
>s/misleading/mislead/
>
>>
>> Add this rule in CODING_STYLE to make sure this is clear to every one.
>>
>> Signed-off-by: Wei Yang
>> Suggested-by: Igor Mammedov
>>
>> ---
>> v2:
>>* rephrase changelog suggested by Eric Blake
>> - remove one redundant line
>> - fix some awkward grammar
>> - add { ; at the end of example
>> ---
>> CODING_STYLE | 23 +++
>> 1 file changed, 23 insertions(+)
>
>> +
>> +do_something(arg1, arg2
>> + arg3);
>
>Missing a comma after arg2. With that fixed,
>Reviewed-by: Eric Blake
You are right.
>
>--
>Eric Blake, Principal Software Engineer
>Red Hat, Inc. +1-919-301-3226
>Virtualization: qemu.org | libvirt.org
--
Wei Yang
Help you, Help me
Re: [Qemu-devel] -device ipmi-bmc-sim attached to -netdev vde
On Mon, Feb 11, 2019 at 06:09:24PM +0100, Robin Jarry wrote: > Hi, > > I have several QEMU VMs connected via a vde_switch (with "-netdev vde" > interfaces). I use this to create virtual network topologies without > requiring root access (i.e. no tap + bridge on host). Performance is not > a concern here. To emulate "real" platforms, I would like one of the VMs > to control the others via remote IPMI over this "vde" link without any > implication of the host. > > The current implementation in QEMU does not seem to support this. I only > found ways to create a "local" (i.e. only reachable from the guest > itself) IPMI interface with "isa-ipmi-kcs" or "isa-ipmi-bt" devices. Or > "remote" IPMI interfaces by using an external daemon that runs on the > host [1]. This makes communication from one of the VMs to the others > quite complex. > > I am enclined to try an develop another IPMI device that can be > "attached" to a -netdev with a dedicated mac address + IP configuration > to support my use case. > > However, this would be my first steps into QEMU code and I have a few > questions first: Sorry, I've been overfocused on something else and this has been sitting waiting for a response. The QEMU code is not going to be your problem here, but more on that later. > > - Did I miss something and is that already possible with the current > code base? I assume you want to use ipmitool or something like that to control the remote system. No, it is not possible at the moment, not without something like vbmc like you mention. > - Is there another way to do this or is my idea completely stupid? It's not completely stupid. Building a system with no security and a partial IPMI implementation wouldn't be too hard, though the IPMI spec is very hard to understand. You could probably steal from the openipmi library (what vbmc uses) to get the pieces you want an get it working. Getting all the user management and other standard required IPMI things would be a nightmare. But you don't need all that to make it do what you want. There are obvious security concerns here with an open IPMI interface. > - I am aware that allowing to "attach" a simplistic bmc (without any > authentication) to a netdev may be a security problem. This is not > a concern for me at the moment. Security is always a concern at the moment. If you build a base infrastructure on something that is insecure, it's hard to rearchitect in the future to build a secure system. > - Could someone point me to what would need to be added for this? Do > I need to implement a new ipmi device type that accepts > a "netdev=" argument? If so, would that make sense for this new > device to also accept mac-address and ip configuration for this IPMI > interface? Or does that need to go elsewhere? If you really wanted to do this, you would need to implement the IPMI LAN protocol inside QEMU and sit it on top of a UDP chardev. It could then plug into the standard IPMI infrastructure in QEMU. The power management functions are already there. The openipmi lanserv code is something you can steal from, it's at https://github.com/cminyard/openipmi/tree/master/lanserv My suggestion, though, would be to implement something that ran over TLS with two-way authentication. It doesn't look too hard to do in qemu (though I haven't tried it) but you could have a qemu console running over TLS that would allow you control from another qemu session. Plus it would give you authorization and encryption on your qemu console logins, which is probably a good thing. I have been working on a library that makes it easy (easier? The pain is always in the key management) to make TLS connections. It's at https://github.com/cminyard/gensio and you can use it from C or Python. But there are many tools to accomplish this. -corey > > Thanks in advance for your guidance. > > [1] https://github.com/Zexi/vbmc-qemu > > -- > Robin >
Re: [Qemu-devel] [PATCH v2 1/2] CODING_STYLE: specify the indent rule for multiline code
On 2/19/19 6:31 PM, Wei Yang wrote:
> We didn't specify the indent rule for multiline code here, which may
> misleading users. And in current code, the code use different rules.
s/misleading/mislead/
>
> Add this rule in CODING_STYLE to make sure this is clear to every one.
>
> Signed-off-by: Wei Yang
> Suggested-by: Igor Mammedov
>
> ---
> v2:
>* rephrase changelog suggested by Eric Blake
> - remove one redundant line
> - fix some awkward grammar
> - add { ; at the end of example
> ---
> CODING_STYLE | 23 +++
> 1 file changed, 23 insertions(+)
> +
> +do_something(arg1, arg2
> + arg3);
Missing a comma after arg2. With that fixed,
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH] doc: update .gitignore and fix typos for docs in tree
On Wed, Feb 20, 2019 at 04:55:53PM +0800, Like Xu wrote: >Signed-off-by: Like Xu >--- > .gitignore | 1 + > docs/COLO-FT.txt | 2 +- > docs/amd-memory-encryption.txt | 2 +- > docs/can.txt | 2 +- > docs/colo-proxy.txt| 6 +++--- > docs/cpu-hotplug.rst | 2 +- > docs/qcow2-cache.txt | 2 +- > docs/qemu-block-drivers.texi | 2 +- > docs/qemu-cpu-models.texi | 4 ++-- > docs/rdma.txt | 2 +- > docs/replay.txt| 2 +- > docs/usb-storage.txt | 2 +- > docs/vfio-ap.txt | 2 +- > 13 files changed, 16 insertions(+), 15 deletions(-) > >diff --git a/.gitignore b/.gitignore >index b66b772..6cb0016 100644 >--- a/.gitignore >+++ b/.gitignore >@@ -90,6 +90,7 @@ > *.d > !/scripts/qemu-guest-agent/fsfreeze-hook.d > *.o >+*.patch > .sdk > *.gcda > *.gcno suggest to split this into a standalone one. >diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt >index e2686bb..ad24680 100644 >--- a/docs/COLO-FT.txt >+++ b/docs/COLO-FT.txt >@@ -102,7 +102,7 @@ to make sure the state of VM in Secondary side is always >consistent with VM in > Primary side. > > COLO Proxy: >-Delivers packets to Primary and Seconday, and then compare the responses from >+Delivers packets to Primary and Secondary, and then compare the responses from > both side. Then decide whether to start a checkpoint according to some rules. > Please refer to docs/colo-proxy.txt for more information. > >diff --git a/docs/amd-memory-encryption.txt b/docs/amd-memory-encryption.txt >index f483795..43bf3ee 100644 >--- a/docs/amd-memory-encryption.txt >+++ b/docs/amd-memory-encryption.txt >@@ -97,7 +97,7 @@ References > AMD Memory Encryption whitepaper: > http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2013/12/AMD_Memory_Encryption_Whitepaper_v7-Public.pdf > >-Secure Encrypted Virutualization Key Management: >+Secure Encrypted Virtualization Key Management: > [1] http://support.amd.com/TechDocs/55766_SEV-KM API_Specification.pdf > > KVM Forum slides: >diff --git a/docs/can.txt b/docs/can.txt >index 7ba23b2..9fa6ed5 100644 >--- a/docs/can.txt >+++ b/docs/can.txt >@@ -99,7 +99,7 @@ Links to other resources > https://gitlab.fel.cvut.cz/canbus/qemu-canbus > (3) RTEMS page describing project > https://devel.rtems.org/wiki/Developer/Simulators/QEMU/CANEmulation >- (4) RTLWS 2015 article about the projevt and its use with CANopen emulation >+ (4) RTLWS 2015 article about the project and its use with CANopen emulation > http://rtime.felk.cvut.cz/publications/public/rtlws2015-qemu-can.pdf > Slides > > http://rtime.felk.cvut.cz/publications/public/rtlws2015-qemu-can-slides.pdf >diff --git a/docs/colo-proxy.txt b/docs/colo-proxy.txt >index 1f8e4b4..7074da7 100644 >--- a/docs/colo-proxy.txt >+++ b/docs/colo-proxy.txt >@@ -41,7 +41,7 @@ Below is a COLO proxy ascii figure: > | | +--+ | > || | > |netfilter| | | || | > netfilter|| | > | +--+ ++ || | > +---+ | >-| | | | | |out || | | > || filter excute order | | >+| | | | | |out || | | > || filter execute order | | ^ remove one space here? > | | | | +-+|| | | > || +---> | | > | | | | || | ||| | | > || TCP | | > | | +-+--+-+ +-v+ +-v+ |pri +++sec|| | | > ++ +---++---v+rewriter++ ++ | | >@@ -53,7 +53,7 @@ Below is a COLO proxy ascii figure: > | | | tx| rx rx | | || | > txall | rx | | > | | | || | || > +---+ | > | | | +--+ | | || > || >-| | | filter excute order | | | || > || >+| | | filter
Re: [Qemu-devel] [PATCH v3 15/25] spapr-vty: Let vty_putchars() use size_t
On Wed, Feb 20, 2019 at 02:02:22AM +0100, Philippe Mathieu-Daudé wrote:
> Both callers (h_put_term_char and rtas_display_character) use
> an unsigned value.
>
> Signed-off-by: Philippe Mathieu-Daudé
Acked-by: David Gibson
> ---
> hw/char/spapr_vty.c| 2 +-
> include/hw/ppc/spapr_vio.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/char/spapr_vty.c b/hw/char/spapr_vty.c
> index 6748334ded..92b8c40410 100644
> --- a/hw/char/spapr_vty.c
> +++ b/hw/char/spapr_vty.c
> @@ -83,7 +83,7 @@ static int vty_getchars(VIOsPAPRDevice *sdev, uint8_t *buf,
> int max)
> return n;
> }
>
> -void vty_putchars(VIOsPAPRDevice *sdev, uint8_t *buf, int len)
> +void vty_putchars(VIOsPAPRDevice *sdev, uint8_t *buf, size_t len)
> {
> VIOsPAPRVTYDevice *dev = VIO_SPAPR_VTY_DEVICE(sdev);
>
> diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h
> index e8b006d18f..ed79d2f380 100644
> --- a/include/hw/ppc/spapr_vio.h
> +++ b/include/hw/ppc/spapr_vio.h
> @@ -126,7 +126,7 @@ static inline int spapr_vio_dma_set(VIOsPAPRDevice *dev,
> uint64_t taddr,
> int spapr_vio_send_crq(VIOsPAPRDevice *dev, uint8_t *crq);
>
> VIOsPAPRDevice *vty_lookup(sPAPRMachineState *spapr, target_ulong reg);
> -void vty_putchars(VIOsPAPRDevice *sdev, uint8_t *buf, int len);
> +void vty_putchars(VIOsPAPRDevice *sdev, uint8_t *buf, size_t len);
> void spapr_vty_create(VIOsPAPRBus *bus, Chardev *chardev);
> void spapr_vlan_create(VIOsPAPRBus *bus, NICInfo *nd);
> void spapr_vscsi_create(VIOsPAPRBus *bus);
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH v4 09/11] virtio-net: update the head descriptor in a chain lastly
On 2019/2/20 上午9:54, Wei Xu wrote:
On Tue, Feb 19, 2019 at 09:09:33PM +0800, Jason Wang wrote:
On 2019/2/19 下午6:51, Wei Xu wrote:
On Tue, Feb 19, 2019 at 03:23:01PM +0800, Jason Wang wrote:
On 2019/2/14 下午12:26, w...@redhat.com wrote:
From: Wei Xu
This is a helper for packed ring.
To support packed ring, the head descriptor in a chain should be updated
lastly since no 'avail_idx' like in packed ring to explicitly tell the
driver side that all payload is ready after having done the chain, so
the head is always visible immediately.
This patch fills the header after done all the other ones.
Signed-off-by: Wei Xu
It's really odd to workaround API issue in the implementation of device.
Please introduce batched used updating helpers instead.
Can you elaborate a bit more? I don't get it as well.
The exact batch as vhost-net or dpdk pmd is not supported by userspace
backend. The change here is to keep the header descriptor updated at
last in case of a chaining descriptors and the helper might not help
too much.
Wei
Of course we can add batching support why not?
It is always good to improve performance with anything, while probably
this could be done in another separate batch, also we need to bear
in mind that usually qemu userspace backend is not the first option for
performance oriented user.
The point is to hide layout specific things from device emulation. If it
helps for performance, it could be treated as a good byproduct.
AFAICT, virtqueue_fill() is a generic API for all relevant userspace virtio
devices that do not support batching , without touching virtqueue_fill(),
supporting batching changes the meaning of the parameter 'idx' which should
be kept overall.
To fix it, I got two proposals so far:
1). batching support(two APIs needed to keep compatibility)
2). save a head elem for a vq instead of caching an array of elems like vhost,
and introduce a new API(virtqueue_chain_fill()) functioning with an
additional parameter 'more' to the current virtqueue_fill() to indicate if
there are more descriptor(s) coming in a chain.
Either way it changes the API somehow and it does not seem to be clean and clear
as wanted.
It's as simple as accepting an array of elems in e.g
virtqueue_fill_batched()?
Any better idea?
Your code assumes the device know the virtio layout specific assumption
whih breaks the layer. Device should not care about the actual layout.
Good point, but anyway, change to virtio-net receiving code path is
unavoidable to support split and packed rings, and batching is like a new
feature somehow.
It's ok to change the code as a result of introducing of generic helper
but it's bad to change to code for working around a bad API.
Thanks
Wei
Thanks
Thanks
---
hw/net/virtio-net.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 3f319ef..330abea 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1251,6 +1251,8 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc,
const uint8_t *buf,
struct virtio_net_hdr_mrg_rxbuf mhdr;
unsigned mhdr_cnt = 0;
size_t offset, i, guest_offset;
+VirtQueueElement head;
+int head_len = 0;
if (!virtio_net_can_receive(nc)) {
return -1;
@@ -1328,7 +1330,13 @@ static ssize_t virtio_net_receive_rcu(NetClientState
*nc, const uint8_t *buf,
}
/* signal other side */
-virtqueue_fill(q->rx_vq, elem, total, i++);
+if (i == 0) {
+head_len = total;
+head = *elem;
+} else {
+virtqueue_fill(q->rx_vq, elem, len, i);
+}
+i++;
g_free(elem);
}
@@ -1339,6 +1347,7 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc,
const uint8_t *buf,
&mhdr.num_buffers, sizeof mhdr.num_buffers);
}
+virtqueue_fill(q->rx_vq, &head, head_len, 0);
virtqueue_flush(q->rx_vq, i);
virtio_notify(vdev, q->rx_vq);
Re: [Qemu-devel] [PATCH v4 08/11] virtio: event suppression support for packed ring
On Tue, Feb 19, 2019 at 09:06:42PM +0800, Jason Wang wrote:
>
> On 2019/2/19 下午6:40, Wei Xu wrote:
> >On Tue, Feb 19, 2019 at 03:19:58PM +0800, Jason Wang wrote:
> >>On 2019/2/14 下午12:26, w...@redhat.com wrote:
> >>>From: Wei Xu
> >>>
> >>>Difference between 'avail_wrap_counter' and 'last_avail_wrap_counter':
> >>>For Tx(guest transmitting), they are the same after each pop of a desc.
> >>>
> >>>For Rx(guest receiving), they are also the same when there are enough
> >>>descriptors to carry the payload for a packet(e.g. usually 16 descs are
> >>>needed for a 64k packet in typical iperf tcp connection with tso enabled),
> >>>however, when the ring is running out of descriptors while there are
> >>>still a few free ones, e.g. 6 descriptors are available which is not
> >>>enough to carry an entire packet which needs 16 descriptors, in this
> >>>case the 'avail_wrap_counter' should be set as the first one pending
> >>>being handled by guest driver in order to get a notification, and the
> >>>'last_avail_wrap_counter' should stay unchanged to the head of available
> >>>descriptors, like below:
> >>>
> >>>Mark meaning:
> >>> | | -- available
> >>> |*| -- used
> >>>
> >>>A Snapshot of the queue:
> >>> last_avail_idx = 253
> >>> last_avail_wrap_counter = 1
> >>> |
> >>> +-+
> >>> 0 | | | |*|*|*|*|*|*|*|*|*|*|*|*|*|*|*|*|*| | | | 255
> >>> +-+
> >>>|
> >>> shadow_avail_idx = 3
> >>> avail_wrap_counter = 0
> >>
> >>Well this might not be the good place to describe the difference between
> >>shadow_avail_idx and last_avail_idx. And the comments above their definition
> >>looks good enough?
> >Sorry, I do not get it, can you elaborate more?
>
>
> I meant the comment is good enough to explain what it did. For packed ring,
> the only difference is the wrap counter. You can add e.g "The wrap counter
> of next head to pop" to above last_avail_wrap_counter. And so does
> shadow_avail_wrap_counter.
OK, I will remove the example part.
>
>
> >
> >This is one of the buggy parts of packed ring, it is good to make it clear
> >here.
> >
> >> /* Next head to pop */
> >> uint16_t last_avail_idx;
> >>
> >> /* Last avail_idx read from VQ. */
> >> uint16_t shadow_avail_idx;
> >>
> >What is the meaning of these comment?
>
>
> It's pretty easy to be understood, isn't it?
:)
>
>
> > Do you mean I should replace put them
> >to the comments also? thanks.
> >
> >>Instead, how or why need event suppress is not mentioned ...
> >Yes, but presumably this knowledge has been acquired from reading through the
> >spec, so I skipped this part.
>
>
> You need at least add reference to the spec. Commit log is pretty important
> for starters to understand what has been done in the patch before reading
> the code. I'm pretty sure they will get confused for reading what you wrote
> here.
>
OK.
>
> Thanks
>
>
> >
> >Wei
> >
> >>
> >>
> >>>Signed-off-by: Wei Xu
> >>>---
> >>> hw/virtio/virtio.c | 137
> >>> +
> >>> 1 file changed, 128 insertions(+), 9 deletions(-)
> >>>
> >>>diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> >>>index 7e276b4..8cfc7b6 100644
> >>>--- a/hw/virtio/virtio.c
> >>>+++ b/hw/virtio/virtio.c
> >>>@@ -234,6 +234,34 @@ static void vring_desc_read(VirtIODevice *vdev,
> >>>VRingDesc *desc,
> >>> virtio_tswap16s(vdev, &desc->next);
> >>> }
> >>>+static void vring_packed_event_read(VirtIODevice *vdev,
> >>>+MemoryRegionCache *cache,
> >>>VRingPackedDescEvent *e)
> >>>+{
> >>>+address_space_read_cached(cache, 0, e, sizeof(*e));
> >>>+virtio_tswap16s(vdev, &e->off_wrap);
> >>>+virtio_tswap16s(vdev, &e->flags);
> >>>+}
> >>>+
> >>>+static void vring_packed_off_wrap_write(VirtIODevice *vdev,
> >>>+MemoryRegionCache *cache, uint16_t off_wrap)
> >>>+{
> >>>+virtio_tswap16s(vdev, &off_wrap);
> >>>+address_space_write_cached(cache, offsetof(VRingPackedDescEvent,
> >>>off_wrap),
> >>>+&off_wrap, sizeof(off_wrap));
> >>>+address_space_cache_invalidate(cache,
> >>>+offsetof(VRingPackedDescEvent, off_wrap),
> >>>sizeof(off_wrap));
> >>>+}
> >>>+
> >>>+static void vring_packed_flags_write(VirtIODevice *vdev,
> >>>+MemoryRegionCache *cache, uint16_t flags)
> >>>+{
> >>>+virtio_tswap16s(vdev, &flags);
> >>>+address_space_write_cached(cache, offsetof(VRingPackedDescEvent,
> >>>flags),
> >>>+&flags, sizeof(flags));
> >>>+address_space_cache_invalidate(cache,
> >>>+offsetof(VRingPackedDescEvent, flags),
> >>>sizeof(flags));
> >>>+}
> >>>+
> >>> static VRingMemoryRegionCaches *vring_get_region_cac
Re: [Qemu-devel] [PATCH v4 09/11] virtio-net: update the head descriptor in a chain lastly
On Tue, Feb 19, 2019 at 09:09:33PM +0800, Jason Wang wrote:
>
> On 2019/2/19 下午6:51, Wei Xu wrote:
> >On Tue, Feb 19, 2019 at 03:23:01PM +0800, Jason Wang wrote:
> >>On 2019/2/14 下午12:26, w...@redhat.com wrote:
> >>>From: Wei Xu
> >>>
> >>>This is a helper for packed ring.
> >>>
> >>>To support packed ring, the head descriptor in a chain should be updated
> >>>lastly since no 'avail_idx' like in packed ring to explicitly tell the
> >>>driver side that all payload is ready after having done the chain, so
> >>>the head is always visible immediately.
> >>>
> >>>This patch fills the header after done all the other ones.
> >>>
> >>>Signed-off-by: Wei Xu
> >>
> >>It's really odd to workaround API issue in the implementation of device.
> >>Please introduce batched used updating helpers instead.
> >Can you elaborate a bit more? I don't get it as well.
> >
> >The exact batch as vhost-net or dpdk pmd is not supported by userspace
> >backend. The change here is to keep the header descriptor updated at
> >last in case of a chaining descriptors and the helper might not help
> >too much.
> >
> >Wei
>
>
> Of course we can add batching support why not?
It is always good to improve performance with anything, while probably
this could be done in another separate batch, also we need to bear
in mind that usually qemu userspace backend is not the first option for
performance oriented user.
AFAICT, virtqueue_fill() is a generic API for all relevant userspace virtio
devices that do not support batching , without touching virtqueue_fill(),
supporting batching changes the meaning of the parameter 'idx' which should
be kept overall.
To fix it, I got two proposals so far:
1). batching support(two APIs needed to keep compatibility)
2). save a head elem for a vq instead of caching an array of elems like vhost,
and introduce a new API(virtqueue_chain_fill()) functioning with an
additional parameter 'more' to the current virtqueue_fill() to indicate if
there are more descriptor(s) coming in a chain.
Either way it changes the API somehow and it does not seem to be clean and clear
as wanted.
Any better idea?
>
> Your code assumes the device know the virtio layout specific assumption
> whih breaks the layer. Device should not care about the actual layout.
>
Good point, but anyway, change to virtio-net receiving code path is
unavoidable to support split and packed rings, and batching is like a new
feature somehow.
Wei
> Thanks
>
>
> >>Thanks
> >>
> >>
> >>>---
> >>> hw/net/virtio-net.c | 11 ++-
> >>> 1 file changed, 10 insertions(+), 1 deletion(-)
> >>>
> >>>diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> >>>index 3f319ef..330abea 100644
> >>>--- a/hw/net/virtio-net.c
> >>>+++ b/hw/net/virtio-net.c
> >>>@@ -1251,6 +1251,8 @@ static ssize_t virtio_net_receive_rcu(NetClientState
> >>>*nc, const uint8_t *buf,
> >>> struct virtio_net_hdr_mrg_rxbuf mhdr;
> >>> unsigned mhdr_cnt = 0;
> >>> size_t offset, i, guest_offset;
> >>>+VirtQueueElement head;
> >>>+int head_len = 0;
> >>> if (!virtio_net_can_receive(nc)) {
> >>> return -1;
> >>>@@ -1328,7 +1330,13 @@ static ssize_t
> >>>virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> >>> }
> >>> /* signal other side */
> >>>-virtqueue_fill(q->rx_vq, elem, total, i++);
> >>>+if (i == 0) {
> >>>+head_len = total;
> >>>+head = *elem;
> >>>+} else {
> >>>+virtqueue_fill(q->rx_vq, elem, len, i);
> >>>+}
> >>>+i++;
> >>> g_free(elem);
> >>> }
> >>>@@ -1339,6 +1347,7 @@ static ssize_t virtio_net_receive_rcu(NetClientState
> >>>*nc, const uint8_t *buf,
> >>> &mhdr.num_buffers, sizeof mhdr.num_buffers);
> >>> }
> >>>+virtqueue_fill(q->rx_vq, &head, head_len, 0);
> >>> virtqueue_flush(q->rx_vq, i);
> >>> virtio_notify(vdev, q->rx_vq);
>
Re: [Qemu-devel] [PATCH v2 3/3] pc-dimm: revert "introduce realize callback"
On Wed, Feb 20, 2019 at 02:26:16AM +0100, Philippe Mathieu-Daudé wrote:
>On 2/20/19 1:51 AM, Wei Yang wrote:
>> realize callback in introduced to check if the backend memory is large
>> enough to contain label data and init its memory region, while this task
>> is handled in pre_plug stage.
>>
>> Now it's time to remove it.
>
>Good cleanup!
>
Glad you like it :-)
>Michael, can you add:
>
>"This reverts commit 9f318f8f7e689b9653b42bac73047f9719a1f34e."
>
>Thanks,
>
>Phil.
>
>>
>> Signed-off-by: Wei Yang
>
>Reviewed-by: Philippe Mathieu-Daudé
>
>> ---
>> hw/mem/pc-dimm.c | 5 -
>> include/hw/mem/pc-dimm.h | 3 ---
>> 2 files changed, 8 deletions(-)
>>
>> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
>> index 152400b1fc..5832c0ba92 100644
>> --- a/hw/mem/pc-dimm.c
>> +++ b/hw/mem/pc-dimm.c
>> @@ -159,7 +159,6 @@ static void pc_dimm_init(Object *obj)
>> static void pc_dimm_realize(DeviceState *dev, Error **errp)
>> {
>> PCDIMMDevice *dimm = PC_DIMM(dev);
>> -PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
>>
>> if (!dimm->hostmem) {
>> error_setg(errp, "'" PC_DIMM_MEMDEV_PROP "' property is not set");
>> @@ -178,10 +177,6 @@ static void pc_dimm_realize(DeviceState *dev, Error
>> **errp)
>> return;
>> }
>>
>> -if (ddc->realize) {
>> -ddc->realize(dimm, errp);
>> -}
>> -
>> host_memory_backend_set_mapped(dimm->hostmem, true);
>> }
>>
>> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
>> index 01436b9f50..d18f8246b7 100644
>> --- a/include/hw/mem/pc-dimm.h
>> +++ b/include/hw/mem/pc-dimm.h
>> @@ -59,8 +59,6 @@ typedef struct PCDIMMDevice {
>>
>> /**
>> * PCDIMMDeviceClass:
>> - * @realize: called after common dimm is realized so that the dimm based
>> - * devices get the chance to do specified operations.
>> * @get_vmstate_memory_region: returns #MemoryRegion which indicates the
>> * memory of @dimm should be kept during live migration. Will not fail
>> * after the device was realized.
>> @@ -70,7 +68,6 @@ typedef struct PCDIMMDeviceClass {
>> DeviceClass parent_class;
>>
>> /* public */
>> -void (*realize)(PCDIMMDevice *dimm, Error **errp);
>> MemoryRegion *(*get_vmstate_memory_region)(PCDIMMDevice *dimm,
>> Error **errp);
>> } PCDIMMDeviceClass;
>>
--
Wei Yang
Help you, Help me
Re: [Qemu-devel] [PATCH v2 3/3] pc-dimm: revert "introduce realize callback"
On 2/20/19 1:51 AM, Wei Yang wrote:
> realize callback in introduced to check if the backend memory is large
> enough to contain label data and init its memory region, while this task
> is handled in pre_plug stage.
>
> Now it's time to remove it.
Good cleanup!
Michael, can you add:
"This reverts commit 9f318f8f7e689b9653b42bac73047f9719a1f34e."
Thanks,
Phil.
>
> Signed-off-by: Wei Yang
Reviewed-by: Philippe Mathieu-Daudé
> ---
> hw/mem/pc-dimm.c | 5 -
> include/hw/mem/pc-dimm.h | 3 ---
> 2 files changed, 8 deletions(-)
>
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 152400b1fc..5832c0ba92 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -159,7 +159,6 @@ static void pc_dimm_init(Object *obj)
> static void pc_dimm_realize(DeviceState *dev, Error **errp)
> {
> PCDIMMDevice *dimm = PC_DIMM(dev);
> -PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
>
> if (!dimm->hostmem) {
> error_setg(errp, "'" PC_DIMM_MEMDEV_PROP "' property is not set");
> @@ -178,10 +177,6 @@ static void pc_dimm_realize(DeviceState *dev, Error
> **errp)
> return;
> }
>
> -if (ddc->realize) {
> -ddc->realize(dimm, errp);
> -}
> -
> host_memory_backend_set_mapped(dimm->hostmem, true);
> }
>
> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> index 01436b9f50..d18f8246b7 100644
> --- a/include/hw/mem/pc-dimm.h
> +++ b/include/hw/mem/pc-dimm.h
> @@ -59,8 +59,6 @@ typedef struct PCDIMMDevice {
>
> /**
> * PCDIMMDeviceClass:
> - * @realize: called after common dimm is realized so that the dimm based
> - * devices get the chance to do specified operations.
> * @get_vmstate_memory_region: returns #MemoryRegion which indicates the
> * memory of @dimm should be kept during live migration. Will not fail
> * after the device was realized.
> @@ -70,7 +68,6 @@ typedef struct PCDIMMDeviceClass {
> DeviceClass parent_class;
>
> /* public */
> -void (*realize)(PCDIMMDevice *dimm, Error **errp);
> MemoryRegion *(*get_vmstate_memory_region)(PCDIMMDevice *dimm,
> Error **errp);
> } PCDIMMDeviceClass;
>
Re: [Qemu-devel] [PATCH v2 2/3] mem/nvdimm: remove nvdimm_realize
On 2/20/19 1:51 AM, Wei Yang wrote:
> nvdimm_realize is used to prepare its memory region, while this is done
> in pre_plug stage.
Via the device parent:
pc_dimm_pre_plug()
-> memory_device_pre_plug()
-> MemoryDeviceClass::get_memory_region()
nvdimm_md_get_memory_region()
>
> This is time to remove it.
>
> Signed-off-by: Wei Yang
Reviewed-by: Philippe Mathieu-Daudé
>
> ---
> v2: split nvdimm part here
> ---
> hw/mem/nvdimm.c | 11 ---
> 1 file changed, 11 deletions(-)
>
> diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
> index bf2adf5e16..8f69576926 100644
> --- a/hw/mem/nvdimm.c
> +++ b/hw/mem/nvdimm.c
> @@ -136,15 +136,6 @@ static MemoryRegion
> *nvdimm_md_get_memory_region(MemoryDeviceState *md,
> return nvdimm->nvdimm_mr;
> }
>
> -static void nvdimm_realize(PCDIMMDevice *dimm, Error **errp)
> -{
> -NVDIMMDevice *nvdimm = NVDIMM(dimm);
> -
> -if (!nvdimm->nvdimm_mr) {
> -nvdimm_prepare_memory_region(nvdimm, errp);
> -}
> -}
> -
> /*
> * the caller should check the input parameters before calling
> * label read/write functions.
> @@ -192,12 +183,10 @@ static Property nvdimm_properties[] = {
>
> static void nvdimm_class_init(ObjectClass *oc, void *data)
> {
> -PCDIMMDeviceClass *ddc = PC_DIMM_CLASS(oc);
> MemoryDeviceClass *mdc = MEMORY_DEVICE_CLASS(oc);
> NVDIMMClass *nvc = NVDIMM_CLASS(oc);
> DeviceClass *dc = DEVICE_CLASS(oc);
>
> -ddc->realize = nvdimm_realize;
> mdc->get_memory_region = nvdimm_md_get_memory_region;
> dc->props = nvdimm_properties;
>
>
[Qemu-devel] [PATCH v3 22/25] s390x/sclp: Let write_console_data() take a size_t
Since all callers provide an unsigned value, we can safely
use a size_t argument.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/char/sclpconsole-lm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/char/sclpconsole-lm.c b/hw/char/sclpconsole-lm.c
index 48c76d863e..290d3118a5 100644
--- a/hw/char/sclpconsole-lm.c
+++ b/hw/char/sclpconsole-lm.c
@@ -191,7 +191,7 @@ static int read_event_data(SCLPEvent *event,
EventBufferHeader *evt_buf_hdr,
* - write console data to character layer
* returns < 0 if an error occurred
*/
-static int write_console_data(SCLPEvent *event, const uint8_t *buf, int len)
+static int write_console_data(SCLPEvent *event, const uint8_t *buf, size_t len)
{
SCLPConsoleLM *scon = SCLPLM_CONSOLE(event);
--
2.20.1
[Qemu-devel] [PATCH v3 24/25] chardev: Let qemu_chr_fe_write[_all] use size_t type argument
All caller have been audited and call these functions with
unsigned arguments.
Most of them use a size_t argument, or directly pass sizeof().
One case is unclear: the mux_chr_write() call in chardev/char-mux.c.
There we add an assert (which will be removed in few patches) and
cast the parameter as size_t to make explicit this value is unsigned.
Suggested-by: Paolo Bonzini
Signed-off-by: Philippe Mathieu-Daudé
---
chardev/char-fe.c | 4 ++--
chardev/char-mux.c| 3 ++-
include/chardev/char-fe.h | 4 ++--
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index f3530a90e6..ab2a01709d 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -31,7 +31,7 @@
#include "chardev/char-io.h"
#include "chardev/char-mux.h"
-int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, int len)
+int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, size_t len)
{
Chardev *s = be->chr;
@@ -42,7 +42,7 @@ int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf,
int len)
return qemu_chr_write(s, buf, len, false);
}
-int qemu_chr_fe_write_all(CharBackend *be, const uint8_t *buf, int len)
+int qemu_chr_fe_write_all(CharBackend *be, const uint8_t *buf, size_t len)
{
Chardev *s = be->chr;
diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index 23aa82125d..7a3ff21db4 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -38,7 +38,8 @@ static int mux_chr_write(Chardev *chr, const uint8_t *buf,
int len)
MuxChardev *d = MUX_CHARDEV(chr);
int ret;
if (!d->timestamps) {
-ret = qemu_chr_fe_write(&d->chr, buf, len);
+assert(len >= 0);
+ret = qemu_chr_fe_write(&d->chr, buf, (size_t)len);
} else {
int i;
diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
index aa1b864ccd..5fb2c2e7ec 100644
--- a/include/chardev/char-fe.h
+++ b/include/chardev/char-fe.h
@@ -203,7 +203,7 @@ guint qemu_chr_fe_add_watch(CharBackend *be, GIOCondition
cond,
*
* Returns: the number of bytes consumed (0 if no associated Chardev)
*/
-int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, int len);
+int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, size_t len);
/**
* qemu_chr_fe_write_all:
@@ -217,7 +217,7 @@ int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf,
int len);
*
* Returns: the number of bytes consumed (0 if no associated Chardev)
*/
-int qemu_chr_fe_write_all(CharBackend *be, const uint8_t *buf, int len);
+int qemu_chr_fe_write_all(CharBackend *be, const uint8_t *buf, size_t len);
/**
* qemu_chr_fe_read_all:
--
2.20.1
[Qemu-devel] [PATCH] doc: update .gitignore and fix typos for docs in tree
Signed-off-by: Like Xu --- .gitignore | 1 + docs/COLO-FT.txt | 2 +- docs/amd-memory-encryption.txt | 2 +- docs/can.txt | 2 +- docs/colo-proxy.txt| 6 +++--- docs/cpu-hotplug.rst | 2 +- docs/qcow2-cache.txt | 2 +- docs/qemu-block-drivers.texi | 2 +- docs/qemu-cpu-models.texi | 4 ++-- docs/rdma.txt | 2 +- docs/replay.txt| 2 +- docs/usb-storage.txt | 2 +- docs/vfio-ap.txt | 2 +- 13 files changed, 16 insertions(+), 15 deletions(-) diff --git a/.gitignore b/.gitignore index b66b772..6cb0016 100644 --- a/.gitignore +++ b/.gitignore @@ -90,6 +90,7 @@ *.d !/scripts/qemu-guest-agent/fsfreeze-hook.d *.o +*.patch .sdk *.gcda *.gcno diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt index e2686bb..ad24680 100644 --- a/docs/COLO-FT.txt +++ b/docs/COLO-FT.txt @@ -102,7 +102,7 @@ to make sure the state of VM in Secondary side is always consistent with VM in Primary side. COLO Proxy: -Delivers packets to Primary and Seconday, and then compare the responses from +Delivers packets to Primary and Secondary, and then compare the responses from both side. Then decide whether to start a checkpoint according to some rules. Please refer to docs/colo-proxy.txt for more information. diff --git a/docs/amd-memory-encryption.txt b/docs/amd-memory-encryption.txt index f483795..43bf3ee 100644 --- a/docs/amd-memory-encryption.txt +++ b/docs/amd-memory-encryption.txt @@ -97,7 +97,7 @@ References AMD Memory Encryption whitepaper: http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2013/12/AMD_Memory_Encryption_Whitepaper_v7-Public.pdf -Secure Encrypted Virutualization Key Management: +Secure Encrypted Virtualization Key Management: [1] http://support.amd.com/TechDocs/55766_SEV-KM API_Specification.pdf KVM Forum slides: diff --git a/docs/can.txt b/docs/can.txt index 7ba23b2..9fa6ed5 100644 --- a/docs/can.txt +++ b/docs/can.txt @@ -99,7 +99,7 @@ Links to other resources https://gitlab.fel.cvut.cz/canbus/qemu-canbus (3) RTEMS page describing project https://devel.rtems.org/wiki/Developer/Simulators/QEMU/CANEmulation - (4) RTLWS 2015 article about the projevt and its use with CANopen emulation + (4) RTLWS 2015 article about the project and its use with CANopen emulation http://rtime.felk.cvut.cz/publications/public/rtlws2015-qemu-can.pdf Slides http://rtime.felk.cvut.cz/publications/public/rtlws2015-qemu-can-slides.pdf diff --git a/docs/colo-proxy.txt b/docs/colo-proxy.txt index 1f8e4b4..7074da7 100644 --- a/docs/colo-proxy.txt +++ b/docs/colo-proxy.txt @@ -41,7 +41,7 @@ Below is a COLO proxy ascii figure: | | +--+ | || | |netfilter| | | || | netfilter|| | | +--+ ++ || | +---+ | -| | | | | |out || | | || filter excute order | | +| | | | | |out || | | || filter execute order | | | | | | +-+|| | | || +---> | | | | | | || | ||| | | || TCP | | | | +-+--+-+ +-v+ +-v+ |pri +++sec|| | | ++ +---++---v+rewriter++ ++ | | @@ -53,7 +53,7 @@ Below is a COLO proxy ascii figure: | | | tx| rx rx | | || | txall | rx | | | | | || | || +---+ | | | | +--+ | | || || -| | | filter excute order | | | || || +| | | filter execute order | | | || || | | | +>| | | ++| | +-+ | | | |||| |
Re: [Qemu-devel] [PATCH v2 1/3] pc-dimm: remove check on pc-dimm hotpluggable
On 2/20/19 1:51 AM, Wei Yang wrote:
> Function acpi_memory_plug_cb() is only invoked when dev is a PCDIMM,
> which is hotpluggable. This means it is not necessary to check this
> property again.
>
> This patch removes this check.
>
> Signed-off-by: Wei Yang
> Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Philippe Mathieu-Daudé
> ---
> v2:
> * remove unused dc
> ---
> hw/acpi/memory_hotplug.c | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index 8c7c1013f3..cb5284d36f 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -264,11 +264,6 @@ void acpi_memory_plug_cb(HotplugHandler *hotplug_dev,
> MemHotplugState *mem_st,
> DeviceState *dev, Error **errp)
> {
> MemStatus *mdev;
> -DeviceClass *dc = DEVICE_GET_CLASS(dev);
> -
> -if (!dc->hotpluggable) {
> -return;
> -}
>
> mdev = acpi_memory_slot_status(mem_st, dev, errp);
> if (!mdev) {
>
[Qemu-devel] [PATCH v3 21/25] s390x/sclp: Use size_t in process_mdb()
Since it is unlikely we have sizeof(mdbo->mto.message) < 0,
we can convert this variable to an unsigned type.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/char/sclpconsole-lm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/char/sclpconsole-lm.c b/hw/char/sclpconsole-lm.c
index 49543e2c83..48c76d863e 100644
--- a/hw/char/sclpconsole-lm.c
+++ b/hw/char/sclpconsole-lm.c
@@ -208,7 +208,7 @@ static int write_console_data(SCLPEvent *event, const
uint8_t *buf, int len)
static int process_mdb(SCLPEvent *event, MDBO *mdbo)
{
int rc;
-int len;
+uint16_t len;
uint8_t buffer[SIZE_BUFFER];
const size_t hlen = sizeof(mdbo->length)
+ sizeof(mdbo->type)
@@ -217,6 +217,7 @@ static int process_mdb(SCLPEvent *event, MDBO *mdbo)
+ sizeof(mdbo->mto._reserved);
len = be16_to_cpu(mdbo->length);
+assert(len >= hlen);
len -= hlen;
assert(len <= SIZE_BUFFER);
--
2.20.1
[Qemu-devel] [PATCH v3 18/25] s390x/3270: Let insert_IAC_escape_char() use size_t
This function takes size_t argument and return a size_t.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/char/terminal3270.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/char/terminal3270.c b/hw/char/terminal3270.c
index 35b079d5c4..1cb48a3c6f 100644
--- a/hw/char/terminal3270.c
+++ b/hw/char/terminal3270.c
@@ -199,9 +199,10 @@ static int read_payload_3270(EmulatedCcw3270Device *dev)
}
/* TN3270 uses binary transmission, which needs escape IAC to IAC IAC */
-static int insert_IAC_escape_char(uint8_t *outv, int out_len)
+static size_t insert_IAC_escape_char(uint8_t *outv, size_t out_len)
{
-int IAC_num = 0, new_out_len, i, j;
+size_t new_out_len;
+int IAC_num = 0, i, j;
for (i = 0; i < out_len; i++) {
if (outv[i] == IAC) {
@@ -232,7 +233,7 @@ static int write_payload_3270(EmulatedCcw3270Device *dev,
uint8_t cmd)
int count = ccw_dstream_avail(get_cds(t));
int bound = (OUTPUT_BUFFER_SIZE - 3) / 2;
int len = MIN(count, bound);
-int out_len = 0;
+size_t out_len = 0;
if (!t->handshake_done) {
if (!(t->outv[0] == IAC && t->outv[1] != IAC)) {
--
2.20.1
[Qemu-devel] [PATCH v3 16/25] tpm: Use size_t to hold sizes
Avoid to use a signed type to hold an unsigned value.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/tpm/tpm_emulator.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
index 70f4b10284..931e56f6ed 100644
--- a/hw/tpm/tpm_emulator.c
+++ b/hw/tpm/tpm_emulator.c
@@ -87,17 +87,18 @@ static int tpm_emulator_ctrlcmd(TPMEmulator *tpm, unsigned
long cmd, void *msg,
{
CharBackend *dev = &tpm->ctrl_chr;
uint32_t cmd_no = cpu_to_be32(cmd);
-ssize_t n = sizeof(uint32_t) + msg_len_in;
+size_t sz = sizeof(uint32_t) + msg_len_in;
+ssize_t n;
uint8_t *buf = NULL;
int ret = -1;
qemu_mutex_lock(&tpm->mutex);
-buf = g_alloca(n);
+buf = g_alloca(sz);
memcpy(buf, &cmd_no, sizeof(cmd_no));
memcpy(buf + sizeof(cmd_no), msg, msg_len_in);
-n = qemu_chr_fe_write_all(dev, buf, n);
+n = qemu_chr_fe_write_all(dev, buf, sz);
if (n <= 0) {
goto end;
}
--
2.20.1
[Qemu-devel] [PATCH v3 00/25] chardev: Convert qemu_chr_write() to take a size_t argument
Hi, This series convert the chardev::qemu_chr_write() to take unsigned length argument. To do so I went through all caller and checked if there are no negative value possible. I'm having headaches with the Xen backend, talking with Marc-André he suggested I ask help to the Xen maintainers. Since the series is becoming quite big, I splitted it: - part 1: convert qemu_chr_write() - part 2: convert IOReadHandler part 1 can be reviewed and merged without part 2. The git diffstat is not huge, but there are various chardev subsystems so many maintainers to ask for Ack. v2: https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02396.html v1: https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02200.html (from Prasad J Pandit) Changes requested by Paolo: https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02294.html Please review, Phil. Philippe Mathieu-Daudé (25): chardev: Simplify IOWatchPoll::fd_can_read as a GSourceFunc chardev: Assert IOCanReadHandler can not be negative chardev/wctablet: Use unsigned type to hold unsigned value chardev: Let qemu_chr_be_can_write() return a size_t types gdbstub: Use size_t for strlen() return value gdbstub: Use size_t to hold GDBState::last_packet_len gdbstub: Let put_buffer() use size_t ui/gtk: Remove pointless cast vhost-user: Express sizeof with size_t usb-redir: Verify usbredirparser_write get called with positive count xen: Let xencons_send() take a 'size' argument xen: Let buffer_append() return the size consumed RFC xen: Let buffer_append() return a size_t virtio-serial: Let VirtIOSerialPortClass::have_data() use size_t spapr-vty: Let vty_putchars() use size_t tpm: Use size_t to hold sizes net/filter-mirror: Use size_t s390x/3270: Let insert_IAC_escape_char() use size_t s390/ebcdic: Use size_t to iterate over arrays s390x/sclp: Use a const variable to improve readability s390x/sclp: Use size_t in process_mdb() s390x/sclp: Let write_console_data() take a size_t hw/ipmi: Assert outlen > outpos chardev: Let qemu_chr_fe_write[_all] use size_t type argument chardev: Let qemu_chr_write[_all] use size_t chardev/baum.c| 6 +++--- chardev/char-fd.c | 6 +++--- chardev/char-fe.c | 4 ++-- chardev/char-io.c | 6 +++--- chardev/char-mux.c| 3 ++- chardev/char-pty.c| 8 chardev/char-socket.c | 13 +++-- chardev/char-udp.c| 8 chardev/char-win.c| 2 +- chardev/char.c| 15 +-- chardev/msmouse.c | 4 ++-- chardev/spice.c | 2 +- chardev/trace-events | 2 +- chardev/wctablet.c| 9 + gdbstub.c | 8 hw/bt/hci-csr.c | 2 +- hw/char/sclpconsole-lm.c | 12 +++- hw/char/spapr_vty.c | 2 +- hw/char/terminal3270.c| 7 --- hw/char/virtio-console.c | 2 +- hw/char/xen_console.c | 24 +++- hw/ipmi/ipmi_bmc_extern.c | 3 ++- hw/tpm/tpm_emulator.c | 7 --- hw/usb/redirect.c | 6 +- hw/virtio/vhost-user.c| 14 -- include/chardev/char-fd.h | 2 +- include/chardev/char-fe.h | 4 ++-- include/chardev/char-io.h | 2 +- include/chardev/char.h| 4 ++-- include/hw/ppc/spapr_vio.h| 2 +- include/hw/s390x/ebcdic.h | 8 include/hw/virtio/virtio-serial.h | 2 +- include/sysemu/replay.h | 2 +- net/filter-mirror.c | 2 +- replay/replay-char.c | 2 +- stubs/replay.c| 2 +- ui/console.c | 6 +++--- ui/gtk.c | 2 +- 38 files changed, 119 insertions(+), 96 deletions(-) -- 2.20.1
[Qemu-devel] [PATCH v3 04/25] chardev: Let qemu_chr_be_can_write() return a size_t types
In the previous commit we added an assert to be sure than
qemu_chr_be_can_write() will never return a negative value.
We can now change its prototype to return a size_t.
Adapt the backends accordingly.
Suggested-by: Paolo Bonzini
Signed-off-by: Philippe Mathieu-Daudé
---
chardev/baum.c| 6 +++---
chardev/char-fd.c | 2 +-
chardev/char-pty.c| 4 ++--
chardev/char-socket.c | 7 ---
chardev/char-udp.c| 4 ++--
chardev/char-win.c| 2 +-
chardev/char.c| 2 +-
chardev/msmouse.c | 4 ++--
chardev/spice.c | 2 +-
chardev/wctablet.c| 4 ++--
hw/bt/hci-csr.c | 2 +-
include/chardev/char-fd.h | 2 +-
include/chardev/char.h| 2 +-
ui/console.c | 6 +++---
14 files changed, 25 insertions(+), 24 deletions(-)
diff --git a/chardev/baum.c b/chardev/baum.c
index 78b0c87625..1d69d62158 100644
--- a/chardev/baum.c
+++ b/chardev/baum.c
@@ -265,7 +265,7 @@ static int baum_deferred_init(BaumChardev *baum)
static void baum_chr_accept_input(struct Chardev *chr)
{
BaumChardev *baum = BAUM_CHARDEV(chr);
-int room, first;
+size_t room, first;
if (!baum->out_buf_used)
return;
@@ -292,7 +292,7 @@ static void baum_write_packet(BaumChardev *baum, const
uint8_t *buf, int len)
{
Chardev *chr = CHARDEV(baum);
uint8_t io_buf[1 + 2 * len], *cur = io_buf;
-int room;
+size_t room;
*cur++ = ESC;
while (len--)
if ((*cur++ = *buf++) == ESC)
@@ -303,7 +303,7 @@ static void baum_write_packet(BaumChardev *baum, const
uint8_t *buf, int len)
/* Fits */
qemu_chr_be_write(chr, io_buf, len);
} else {
-int first;
+size_t first;
uint8_t out;
/* Can't fit all, send what can be, and store the rest. */
qemu_chr_be_write(chr, io_buf, room);
diff --git a/chardev/char-fd.c b/chardev/char-fd.c
index 2421d8e216..0fe2822869 100644
--- a/chardev/char-fd.c
+++ b/chardev/char-fd.c
@@ -43,7 +43,7 @@ static gboolean fd_chr_read(QIOChannel *chan, GIOCondition
cond, void *opaque)
{
Chardev *chr = CHARDEV(opaque);
FDChardev *s = FD_CHARDEV(opaque);
-int len;
+size_t len;
uint8_t buf[CHR_READ_BUF_LEN];
ssize_t ret;
diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index f6ddef..eae25f043b 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -34,7 +34,7 @@
typedef struct {
Chardev parent;
QIOChannel *ioc;
-int read_bytes;
+size_t read_bytes;
int connected;
GSource *timer_src;
@@ -132,7 +132,7 @@ static gboolean pty_chr_read(QIOChannel *chan, GIOCondition
cond, void *opaque)
{
Chardev *chr = CHARDEV(opaque);
PtyChardev *s = PTY_CHARDEV(opaque);
-gsize len;
+size_t len;
uint8_t buf[CHR_READ_BUF_LEN];
ssize_t ret;
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 262a59b64f..4010c343e0 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -60,7 +60,7 @@ typedef struct {
GSource *hup_source;
QCryptoTLSCreds *tls_creds;
TCPChardevState state;
-int max_size;
+size_t max_size;
int do_telnetopt;
int do_nodelay;
int *read_msgfds;
@@ -493,10 +493,11 @@ static gboolean tcp_chr_read(QIOChannel *chan,
GIOCondition cond, void *opaque)
Chardev *chr = CHARDEV(opaque);
SocketChardev *s = SOCKET_CHARDEV(opaque);
uint8_t buf[CHR_READ_BUF_LEN];
-int len, size;
+size_t len;
+int size;
if ((s->state != TCP_CHARDEV_STATE_CONNECTED) ||
-s->max_size <= 0) {
+s->max_size == 0) {
return TRUE;
}
len = sizeof(buf);
diff --git a/chardev/char-udp.c b/chardev/char-udp.c
index b6e399e983..d4f40626e4 100644
--- a/chardev/char-udp.c
+++ b/chardev/char-udp.c
@@ -39,7 +39,7 @@ typedef struct {
uint8_t buf[CHR_READ_BUF_LEN];
int bufcnt;
int bufptr;
-int max_size;
+size_t max_size;
} UdpChardev;
#define UDP_CHARDEV(obj) OBJECT_CHECK(UdpChardev, (obj), TYPE_CHARDEV_UDP)
@@ -58,7 +58,7 @@ static void udp_chr_flush_buffer(UdpChardev *s)
Chardev *chr = CHARDEV(s);
while (s->max_size > 0 && s->bufptr < s->bufcnt) {
-int n = MIN(s->max_size, s->bufcnt - s->bufptr);
+size_t n = MIN(s->max_size, s->bufcnt - s->bufptr);
qemu_chr_be_write(chr, &s->buf[s->bufptr], n);
s->bufptr += n;
s->max_size = qemu_chr_be_can_write(chr);
diff --git a/chardev/char-win.c b/chardev/char-win.c
index 05518e0958..30361e8852 100644
--- a/chardev/char-win.c
+++ b/chardev/char-win.c
@@ -29,7 +29,7 @@
static void win_chr_read(Chardev *chr, DWORD len)
{
WinChardev *s = WIN_CHARDEV(chr);
-int max_size = qemu_chr_be_can_write(chr);
+size_t max_size = qemu_chr_be_can_write(chr);
int ret, err;
uint8_t buf[CHR_READ_BUF_LEN];
DWORD size;
diff --git a/chardev/char.c b/chardev/char.c
index 71ecd32b25..3149cd3ba9 100644
--- a/chardev/char.c
+++ b/chardev
[Qemu-devel] [PATCH v3 01/25] chardev: Simplify IOWatchPoll::fd_can_read as a GSourceFunc
IOWatchPoll::fd_can_read() really is a GSourceFunc type, it simply
returns a boolean value.
Update the backends to return a boolean, whether there is data to
read from the source or not.
Suggested-by: Paolo Bonzini
Signed-off-by: Philippe Mathieu-Daudé
---
chardev/char-fd.c | 4 ++--
chardev/char-io.c | 6 +++---
chardev/char-pty.c| 4 ++--
chardev/char-socket.c | 6 +++---
chardev/char-udp.c| 4 ++--
include/chardev/char-io.h | 2 +-
6 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/chardev/char-fd.c b/chardev/char-fd.c
index 2c9b2ce567..2421d8e216 100644
--- a/chardev/char-fd.c
+++ b/chardev/char-fd.c
@@ -69,13 +69,13 @@ static gboolean fd_chr_read(QIOChannel *chan, GIOCondition
cond, void *opaque)
return TRUE;
}
-static int fd_chr_read_poll(void *opaque)
+static gboolean fd_chr_read_poll(void *opaque)
{
Chardev *chr = CHARDEV(opaque);
FDChardev *s = FD_CHARDEV(opaque);
s->max_size = qemu_chr_be_can_write(chr);
-return s->max_size;
+return s->max_size > 0;
}
static GSource *fd_chr_add_watch(Chardev *chr, GIOCondition cond)
diff --git a/chardev/char-io.c b/chardev/char-io.c
index 8ced184160..2c1c69098e 100644
--- a/chardev/char-io.c
+++ b/chardev/char-io.c
@@ -30,7 +30,7 @@ typedef struct IOWatchPoll {
QIOChannel *ioc;
GSource *src;
-IOCanReadHandler *fd_can_read;
+GSourceFunc fd_can_read;
GSourceFunc fd_read;
void *opaque;
} IOWatchPoll;
@@ -44,7 +44,7 @@ static gboolean io_watch_poll_prepare(GSource *source,
gint *timeout)
{
IOWatchPoll *iwp = io_watch_poll_from_source(source);
-bool now_active = iwp->fd_can_read(iwp->opaque) > 0;
+bool now_active = iwp->fd_can_read(iwp->opaque);
bool was_active = iwp->src != NULL;
if (was_active == now_active) {
return FALSE;
@@ -76,7 +76,7 @@ static GSourceFuncs io_watch_poll_funcs = {
GSource *io_add_watch_poll(Chardev *chr,
QIOChannel *ioc,
-IOCanReadHandler *fd_can_read,
+GSourceFunc fd_can_read,
QIOChannelFunc fd_read,
gpointer user_data,
GMainContext *context)
diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index b034332edd..f6ddef 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -119,13 +119,13 @@ static GSource *pty_chr_add_watch(Chardev *chr,
GIOCondition cond)
return qio_channel_create_watch(s->ioc, cond);
}
-static int pty_chr_read_poll(void *opaque)
+static gboolean pty_chr_read_poll(void *opaque)
{
Chardev *chr = CHARDEV(opaque);
PtyChardev *s = PTY_CHARDEV(opaque);
s->read_bytes = qemu_chr_be_can_write(chr);
-return s->read_bytes;
+return s->read_bytes > 0;
}
static gboolean pty_chr_read(QIOChannel *chan, GIOCondition cond, void *opaque)
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 4fcdd8aedd..262a59b64f 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -147,7 +147,7 @@ static void tcp_chr_accept(QIONetListener *listener,
QIOChannelSocket *cioc,
void *opaque);
-static int tcp_chr_read_poll(void *opaque);
+static gboolean tcp_chr_read_poll(void *opaque);
static void tcp_chr_disconnect(Chardev *chr);
/* Called with chr_write_lock held. */
@@ -184,7 +184,7 @@ static int tcp_chr_write(Chardev *chr, const uint8_t *buf,
int len)
}
}
-static int tcp_chr_read_poll(void *opaque)
+static gboolean tcp_chr_read_poll(void *opaque)
{
Chardev *chr = CHARDEV(opaque);
SocketChardev *s = SOCKET_CHARDEV(opaque);
@@ -192,7 +192,7 @@ static int tcp_chr_read_poll(void *opaque)
return 0;
}
s->max_size = qemu_chr_be_can_write(chr);
-return s->max_size;
+return s->max_size > 0;
}
static void tcp_chr_process_IAC_bytes(Chardev *chr,
diff --git a/chardev/char-udp.c b/chardev/char-udp.c
index 097a2f0f42..b6e399e983 100644
--- a/chardev/char-udp.c
+++ b/chardev/char-udp.c
@@ -65,7 +65,7 @@ static void udp_chr_flush_buffer(UdpChardev *s)
}
}
-static int udp_chr_read_poll(void *opaque)
+static gboolean udp_chr_read_poll(void *opaque)
{
Chardev *chr = CHARDEV(opaque);
UdpChardev *s = UDP_CHARDEV(opaque);
@@ -77,7 +77,7 @@ static int udp_chr_read_poll(void *opaque)
*/
udp_chr_flush_buffer(s);
-return s->max_size;
+return s->max_size > 0;
}
static gboolean udp_chr_read(QIOChannel *chan, GIOCondition cond, void *opaque)
diff --git a/include/chardev/char-io.h b/include/chardev/char-io.h
index 9638da5100..a173874538 100644
--- a/include/chardev/char-io.h
+++ b/include/chardev/char-io.h
@@ -31,7 +31,7 @@
/* Can only be used for read */
GSource *io_add_watch_poll(Chardev *chr,
QIOChannel *ioc,
-IOCanReadHandler *fd_can_read,
+GSour
[Qemu-devel] [PATCH v3 25/25] chardev: Let qemu_chr_write[_all] use size_t
We now know all callers use a size_t argument. We can
convert qemu_chr_write() and qemu_chr_write_all() to
use a size_t argument.
Signed-off-by: Philippe Mathieu-Daudé
---
chardev/char.c | 8
include/chardev/char.h | 2 +-
include/sysemu/replay.h | 2 +-
replay/replay-char.c| 2 +-
stubs/replay.c | 2 +-
5 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/chardev/char.c b/chardev/char.c
index 3149cd3ba9..8f1f56a802 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -99,8 +99,8 @@ static void qemu_chr_write_log(Chardev *s, const uint8_t
*buf, size_t len)
}
static int qemu_chr_write_buffer(Chardev *s,
- const uint8_t *buf, int len,
- int *offset, bool write_all)
+ const uint8_t *buf, size_t len,
+ size_t *offset, bool write_all)
{
ChardevClass *cc = CHARDEV_GET_CLASS(s);
int res = 0;
@@ -132,9 +132,9 @@ static int qemu_chr_write_buffer(Chardev *s,
return res;
}
-int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, bool write_all)
+int qemu_chr_write(Chardev *s, const uint8_t *buf, size_t len, bool write_all)
{
-int offset = 0;
+size_t offset = 0;
int res;
if (qemu_chr_replay(s) && replay_mode == REPLAY_MODE_PLAY) {
diff --git a/include/chardev/char.h b/include/chardev/char.h
index 0341dd1ba2..2e3b5a15ca 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -221,7 +221,7 @@ void qemu_chr_set_feature(Chardev *chr,
ChardevFeature feature);
QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename,
bool permit_mux_mon);
-int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, bool write_all);
+int qemu_chr_write(Chardev *s, const uint8_t *buf, size_t len, bool write_all);
#define qemu_chr_write_all(s, buf, len) qemu_chr_write(s, buf, len, true)
int qemu_chr_wait_connected(Chardev *chr, Error **errp);
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 3a7c58e423..334944715d 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -158,7 +158,7 @@ void replay_chr_be_write(struct Chardev *s, uint8_t *buf,
int len);
/*! Writes char write return value to the replay log. */
void replay_char_write_event_save(int res, int offset);
/*! Reads char write return value from the replay log. */
-void replay_char_write_event_load(int *res, int *offset);
+void replay_char_write_event_load(int *res, size_t *offset);
/*! Reads information about read_all character event. */
int replay_char_read_all_load(uint8_t *buf);
/*! Writes character read_all error code into the replay log. */
diff --git a/replay/replay-char.c b/replay/replay-char.c
index 736cc8c2e6..f0308578eb 100644
--- a/replay/replay-char.c
+++ b/replay/replay-char.c
@@ -104,7 +104,7 @@ void replay_char_write_event_save(int res, int offset)
replay_put_dword(offset);
}
-void replay_char_write_event_load(int *res, int *offset)
+void replay_char_write_event_load(int *res, size_t *offset)
{
g_assert(replay_mutex_locked());
diff --git a/stubs/replay.c b/stubs/replay.c
index 4ac607895d..cf584d3191 100644
--- a/stubs/replay.c
+++ b/stubs/replay.c
@@ -44,7 +44,7 @@ void replay_char_write_event_save(int res, int offset)
abort();
}
-void replay_char_write_event_load(int *res, int *offset)
+void replay_char_write_event_load(int *res, size_t *offset)
{
abort();
}
--
2.20.1
[Qemu-devel] [PATCH v3 02/25] chardev: Assert IOCanReadHandler can not be negative
The backend should not return a negative length to read.
We will later change the prototype of IOCanReadHandler to return an
unsigned length. Meanwhile make sure the return length is positive.
Suggested-by: Paolo Bonzini
Signed-off-by: Philippe Mathieu-Daudé
---
chardev/char.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/chardev/char.c b/chardev/char.c
index f6d61fa5f8..71ecd32b25 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -159,12 +159,15 @@ int qemu_chr_write(Chardev *s, const uint8_t *buf, int
len, bool write_all)
int qemu_chr_be_can_write(Chardev *s)
{
CharBackend *be = s->be;
+int receivable_bytes;
if (!be || !be->chr_can_read) {
return 0;
}
-return be->chr_can_read(be->opaque);
+receivable_bytes = be->chr_can_read(be->opaque);
+assert(receivable_bytes >= 0);
+return receivable_bytes;
}
void qemu_chr_be_write_impl(Chardev *s, uint8_t *buf, int len)
--
2.20.1
[Qemu-devel] [PATCH v3 03/25] chardev/wctablet: Use unsigned type to hold unsigned value
TabletChardev::query is an array of uint8_t.
Use the same type to hold it (this also silent a -Wsign-conversion
warning in the trace function).
Signed-off-by: Philippe Mathieu-Daudé
---
chardev/trace-events | 2 +-
chardev/wctablet.c | 5 +++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/chardev/trace-events b/chardev/trace-events
index d0e5f3bbc1..562bfe70e9 100644
--- a/chardev/trace-events
+++ b/chardev/trace-events
@@ -5,7 +5,7 @@ wct_init(void) ""
wct_cmd_re(void) ""
wct_cmd_st(void) ""
wct_cmd_sp(void) ""
-wct_cmd_ts(int input) "0x%02x"
+wct_cmd_ts(uint8_t input) "0x%02x"
wct_cmd_other(const char *cmd) "%s"
wct_speed(int speed) "%d"
diff --git a/chardev/wctablet.c b/chardev/wctablet.c
index 35dbd29a33..cf7a08a363 100644
--- a/chardev/wctablet.c
+++ b/chardev/wctablet.c
@@ -207,7 +207,8 @@ static int wctablet_chr_write(struct Chardev *chr,
const uint8_t *buf, int len)
{
TabletChardev *tablet = WCTABLET_CHARDEV(chr);
-unsigned int i, clen;
+size_t i;
+unsigned int clen;
char *pos;
if (tablet->line_speed != 9600) {
@@ -269,7 +270,7 @@ static int wctablet_chr_write(struct Chardev *chr,
} else if (strncmp((char *)tablet->query, "TS", 2) == 0 &&
clen == 3) {
-unsigned int input = tablet->query[2];
+uint8_t input = tablet->query[2];
uint8_t codes[7] = {
0xa3,
((input & 0x80) == 0) ? 0x7e : 0x7f,
--
2.20.1
[Qemu-devel] [PATCH v3 09/25] vhost-user: Express sizeof with size_t
VHOST_USER_HDR_SIZE uses offsetof(), thus is an expression of type
size_t. Update the format string accordingly.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/virtio/vhost-user.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 564a31d12c..2eb7143d3d 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -215,11 +215,12 @@ static int vhost_user_read(struct vhost_dev *dev,
VhostUserMsg *msg)
struct vhost_user *u = dev->opaque;
CharBackend *chr = u->user->chr;
uint8_t *p = (uint8_t *) msg;
-int r, size = VHOST_USER_HDR_SIZE;
+int r;
+size_t size = VHOST_USER_HDR_SIZE;
r = qemu_chr_fe_read_all(chr, p, size);
if (r != size) {
-error_report("Failed to read msg header. Read %d instead of %d."
+error_report("Failed to read msg header. Read %d instead of %zu."
" Original request %d.", r, size, msg->hdr.request);
goto fail;
}
@@ -235,7 +236,7 @@ static int vhost_user_read(struct vhost_dev *dev,
VhostUserMsg *msg)
/* validate message size is sane */
if (msg->hdr.size > VHOST_USER_PAYLOAD_SIZE) {
error_report("Failed to read msg header."
-" Size %d exceeds the maximum %zu.", msg->hdr.size,
+" Size %u exceeds the maximum %zu.", msg->hdr.size,
VHOST_USER_PAYLOAD_SIZE);
goto fail;
}
@@ -246,7 +247,7 @@ static int vhost_user_read(struct vhost_dev *dev,
VhostUserMsg *msg)
r = qemu_chr_fe_read_all(chr, p, size);
if (r != size) {
error_report("Failed to read msg payload."
- " Read %d instead of %d.", r, msg->hdr.size);
+ " Read %d instead of %u.", r, msg->hdr.size);
goto fail;
}
}
@@ -300,7 +301,8 @@ static int vhost_user_write(struct vhost_dev *dev,
VhostUserMsg *msg,
{
struct vhost_user *u = dev->opaque;
CharBackend *chr = u->user->chr;
-int ret, size = VHOST_USER_HDR_SIZE + msg->hdr.size;
+int ret;
+size_t size = VHOST_USER_HDR_SIZE + msg->hdr.size;
/*
* For non-vring specific requests, like VHOST_USER_SET_MEM_TABLE,
@@ -320,7 +322,7 @@ static int vhost_user_write(struct vhost_dev *dev,
VhostUserMsg *msg,
ret = qemu_chr_fe_write_all(chr, (const uint8_t *) msg, size);
if (ret != size) {
error_report("Failed to write msg."
- " Wrote %d instead of %d.", ret, size);
+ " Wrote %d instead of %zu.", ret, size);
return -1;
}
--
2.20.1
[Qemu-devel] [PATCH v3 05/25] gdbstub: Use size_t for strlen() return value
Since strlen() returns an unsigned value, it is pointless to
convert it to a signed one. Use size_t to hold its return value.
Signed-off-by: Philippe Mathieu-Daudé
---
gdbstub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gdbstub.c b/gdbstub.c
index bc774ae992..76eca3bb7e 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1693,7 +1693,7 @@ static int gdb_handle_packet(GDBState *s, const char
*line_buf)
}
#else /* !CONFIG_USER_ONLY */
else if (strncmp(p, "Rcmd,", 5) == 0) {
-int len = strlen(p + 5);
+size_t len = strlen(p + 5);
if ((len % 2) != 0) {
put_packet(s, "E01");
--
2.20.1
[Qemu-devel] [PATCH v3 08/25] ui/gtk: Remove pointless cast
The 'size' value is of type 'guint' which is already unsigned. Remove the useless cast. Signed-off-by: Philippe Mathieu-Daudé --- ui/gtk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/gtk.c b/ui/gtk.c index 949b143e4e..b5879fdece 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -1764,7 +1764,7 @@ static gboolean gd_vc_in(VteTerminal *terminal, gchar *text, guint size, } } -qemu_chr_be_write(vc->vte.chr, (uint8_t *)text, (unsigned int)size); +qemu_chr_be_write(vc->vte.chr, (uint8_t *)text, size); return TRUE; } -- 2.20.1
[Qemu-devel] [PATCH v3 11/25] xen: Let xencons_send() take a 'size' argument
The single caller of xencons_send(), con_event() already use the
difference 'con->buffer.size - con->buffer.consumed'.
Deduplicate by passing the difference as an argument.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/char/xen_console.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index 91f34ef06c..083b2c8e2a 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -144,11 +144,10 @@ static void xencons_receive(void *opaque, const uint8_t
*buf, int len)
xen_pv_send_notify(&con->xendev);
}
-static void xencons_send(struct XenConsole *con)
+static void xencons_send(struct XenConsole *con, ssize_t size)
{
-ssize_t len, size;
+ssize_t len;
-size = con->buffer.size - con->buffer.consumed;
if (qemu_chr_fe_backend_connected(&con->chr)) {
len = qemu_chr_fe_write(&con->chr,
con->buffer.data + con->buffer.consumed,
@@ -280,10 +279,13 @@ static void con_disconnect(struct XenLegacyDevice *xendev)
static void con_event(struct XenLegacyDevice *xendev)
{
struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
+ssize_t size;
buffer_append(con);
-if (con->buffer.size - con->buffer.consumed)
-xencons_send(con);
+size = con->buffer.size - con->buffer.consumed;
+if (size) {
+xencons_send(con, size);
+}
}
/* */
--
2.20.1
[Qemu-devel] [PATCH v3 07/25] gdbstub: Let put_buffer() use size_t
All callers provide a size_t argument, we can safely use size_t
for this function.
Signed-off-by: Philippe Mathieu-Daudé
---
gdbstub.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 69340d7cd1..860e9bb7c7 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -475,10 +475,10 @@ static int gdb_continue_partial(GDBState *s, char
*newstates)
return res;
}
-static void put_buffer(GDBState *s, const uint8_t *buf, int len)
+static void put_buffer(GDBState *s, const uint8_t *buf, size_t len)
{
#ifdef CONFIG_USER_ONLY
-int ret;
+ssize_t ret;
while (len > 0) {
ret = send(s->fd, buf, len, 0);
--
2.20.1
[Qemu-devel] [PATCH v3 12/25] xen: Let buffer_append() return the size consumed
The buffer.size and buffer.consumed fields are only updated within the
buffer_append() body. We can simply let buffer_append() return the
difference (the buffer consumed).
Signed-off-by: Philippe Mathieu-Daudé
---
hw/char/xen_console.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index 083b2c8e2a..1a30014a11 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -48,7 +48,7 @@ struct XenConsole {
int backlog;
};
-static void buffer_append(struct XenConsole *con)
+static ssize_t buffer_append(struct XenConsole *con)
{
struct buffer *buffer = &con->buffer;
XENCONS_RING_IDX cons, prod, size;
@@ -59,8 +59,9 @@ static void buffer_append(struct XenConsole *con)
xen_mb();
size = prod - cons;
-if ((size == 0) || (size > sizeof(intf->out)))
-return;
+if ((size == 0) || (size > sizeof(intf->out))) {
+goto out;
+}
if ((buffer->capacity - buffer->size) < size) {
buffer->capacity += (size + 1024);
@@ -89,6 +90,9 @@ static void buffer_append(struct XenConsole *con)
if (buffer->consumed > buffer->max_capacity - over)
buffer->consumed = buffer->max_capacity - over;
}
+
+ out:
+return buffer->size - buffer->consumed;
}
static void buffer_advance(struct buffer *buffer, size_t len)
@@ -281,8 +285,7 @@ static void con_event(struct XenLegacyDevice *xendev)
struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
ssize_t size;
-buffer_append(con);
-size = con->buffer.size - con->buffer.consumed;
+size = buffer_append(con);
if (size) {
xencons_send(con, size);
}
--
2.20.1
[Qemu-devel] [PATCH v3 23/25] hw/ipmi: Assert outlen > outpos
A througfull audit show that all time data is added to outbuf[],
'outlen' is incremented. Then at creation and each time
continue_send() returns it pass thru check_reset which resets
'outpos', thus we always have 'outlen >= outpos'.
Also due to the check on entry, we know outlen != 0.
We can then add an assertion on 'outlen > outpos', which will
helps the next patch to safely convert 'outlen - outpos' as an
unsigned type (size_t).
Make this assertion explicit by casting 'outlen - outpos' size_t.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/ipmi/ipmi_bmc_extern.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c
index bf0b7ee0f5..ca61b04942 100644
--- a/hw/ipmi/ipmi_bmc_extern.c
+++ b/hw/ipmi/ipmi_bmc_extern.c
@@ -107,8 +107,9 @@ static void continue_send(IPMIBmcExtern *ibe)
goto check_reset;
}
send:
+assert(ibe->outlen > ibe->outpos);
ret = qemu_chr_fe_write(&ibe->chr, ibe->outbuf + ibe->outpos,
-ibe->outlen - ibe->outpos);
+(size_t)(ibe->outlen - ibe->outpos));
if (ret > 0) {
ibe->outpos += ret;
}
--
2.20.1
[Qemu-devel] [RFC PATCH v3 13/25] xen: Let buffer_append() return a size_t
To the Xen team: this is not trivial to me to demonstrate this assertion can never happen, but then the whole series is justified and I can convert qemu_chr_fe_write() to use size_t argument. Can you help me here? Signed-off-by: Philippe Mathieu-Daudé --- hw/char/xen_console.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c index 1a30014a11..5b672a5a24 100644 --- a/hw/char/xen_console.c +++ b/hw/char/xen_console.c @@ -92,6 +92,7 @@ static ssize_t buffer_append(struct XenConsole *con) } out: +assert(buffer->size >= buffer->consumed); return buffer->size - buffer->consumed; } -- 2.20.1
[Qemu-devel] [PATCH v3 14/25] virtio-serial: Let VirtIOSerialPortClass::have_data() use size_t
Both callers in hw/char/virtio-serial-bus.c provide unsigned values,
even the trace event display an unsigned value.
Convert the have_data() handler to take an unsigned value.
Signed-off-by: Philippe Mathieu-Daudé
---
It is funny/scary that there are big comments about how to treat
errors to set the return value, then the return value is simply
ignored by the caller.
---
hw/char/virtio-console.c | 2 +-
include/hw/virtio/virtio-serial.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
index 2cbe1d4ed5..19639dca3b 100644
--- a/hw/char/virtio-console.c
+++ b/hw/char/virtio-console.c
@@ -45,7 +45,7 @@ static gboolean chr_write_unblocked(GIOChannel *chan,
GIOCondition cond,
/* Callback function that's called when the guest sends us data */
static ssize_t flush_buf(VirtIOSerialPort *port,
- const uint8_t *buf, ssize_t len)
+ const uint8_t *buf, size_t len)
{
VirtConsole *vcon = VIRTIO_CONSOLE(port);
ssize_t ret;
diff --git a/include/hw/virtio/virtio-serial.h
b/include/hw/virtio/virtio-serial.h
index 12657a9f39..f1a5ccf4f7 100644
--- a/include/hw/virtio/virtio-serial.h
+++ b/include/hw/virtio/virtio-serial.h
@@ -81,7 +81,7 @@ typedef struct VirtIOSerialPortClass {
* 'len'. In this case, throttling will be enabled for this port.
*/
ssize_t (*have_data)(VirtIOSerialPort *port, const uint8_t *buf,
- ssize_t len);
+ size_t len);
} VirtIOSerialPortClass;
/*
--
2.20.1
[Qemu-devel] [PATCH v3 15/25] spapr-vty: Let vty_putchars() use size_t
Both callers (h_put_term_char and rtas_display_character) use
an unsigned value.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/char/spapr_vty.c| 2 +-
include/hw/ppc/spapr_vio.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/char/spapr_vty.c b/hw/char/spapr_vty.c
index 6748334ded..92b8c40410 100644
--- a/hw/char/spapr_vty.c
+++ b/hw/char/spapr_vty.c
@@ -83,7 +83,7 @@ static int vty_getchars(VIOsPAPRDevice *sdev, uint8_t *buf,
int max)
return n;
}
-void vty_putchars(VIOsPAPRDevice *sdev, uint8_t *buf, int len)
+void vty_putchars(VIOsPAPRDevice *sdev, uint8_t *buf, size_t len)
{
VIOsPAPRVTYDevice *dev = VIO_SPAPR_VTY_DEVICE(sdev);
diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h
index e8b006d18f..ed79d2f380 100644
--- a/include/hw/ppc/spapr_vio.h
+++ b/include/hw/ppc/spapr_vio.h
@@ -126,7 +126,7 @@ static inline int spapr_vio_dma_set(VIOsPAPRDevice *dev,
uint64_t taddr,
int spapr_vio_send_crq(VIOsPAPRDevice *dev, uint8_t *crq);
VIOsPAPRDevice *vty_lookup(sPAPRMachineState *spapr, target_ulong reg);
-void vty_putchars(VIOsPAPRDevice *sdev, uint8_t *buf, int len);
+void vty_putchars(VIOsPAPRDevice *sdev, uint8_t *buf, size_t len);
void spapr_vty_create(VIOsPAPRBus *bus, Chardev *chardev);
void spapr_vlan_create(VIOsPAPRBus *bus, NICInfo *nd);
void spapr_vscsi_create(VIOsPAPRBus *bus);
--
2.20.1
[Qemu-devel] [PATCH v3 10/25] usb-redir: Verify usbredirparser_write get called with positive count
The usbredirparser_write handler should never be called with a negative
size payload, return an error if this is not the case.
Now that we are sure the 'count' value is positive, make it obvious by
casting it to a size_t.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/usb/redirect.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 18a42d1938..131eae2e7e 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -285,7 +285,11 @@ static int usbredir_write(void *priv, uint8_t *data, int
count)
return 0;
}
-r = qemu_chr_fe_write(&dev->cs, data, count);
+if (count < 0) {
+ERROR("Illegal write count: %i\n", count);
+return 0;
+}
+r = qemu_chr_fe_write(&dev->cs, data, (size_t)count);
if (r < count) {
if (!dev->watch) {
dev->watch = qemu_chr_fe_add_watch(&dev->cs, G_IO_OUT | G_IO_HUP,
--
2.20.1
[Qemu-devel] [PATCH v3 06/25] gdbstub: Use size_t to hold GDBState::last_packet_len
In put_packet_binary() we have:
uint8_t *p;
for(;;) {
p = s->last_packet;
*(p++) = ...
s->last_packet_len = p - s->last_packet;
put_buffer(s, (uint8_t *)s->last_packet, s->last_packet_len);
The 'p' pointer start at s->last_packet, then is only incremented.
Since we have "p >= s->last_packet", we are sure than
"p - s->last_packet >= 0", thus "p - s->last_packet" is positive.
The few other places where s->last_packet_len is set is with constant
positive values.
It makes sense to use size_t to hold last_packet_len values.
Signed-off-by: Philippe Mathieu-Daudé
---
gdbstub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gdbstub.c b/gdbstub.c
index 76eca3bb7e..69340d7cd1 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -323,7 +323,7 @@ typedef struct GDBState {
int line_sum; /* running checksum */
int line_csum; /* checksum at the end of the packet */
uint8_t last_packet[MAX_PACKET_LENGTH + 4];
-int last_packet_len;
+size_t last_packet_len;
int signal;
#ifdef CONFIG_USER_ONLY
int fd;
--
2.20.1
[Qemu-devel] [PATCH v3 20/25] s390x/sclp: Use a const variable to improve readability
We will reuse this variable in the next patch. Signed-off-by: Philippe Mathieu-Daudé --- hw/char/sclpconsole-lm.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/char/sclpconsole-lm.c b/hw/char/sclpconsole-lm.c index dbc91a1e5b..49543e2c83 100644 --- a/hw/char/sclpconsole-lm.c +++ b/hw/char/sclpconsole-lm.c @@ -210,13 +210,14 @@ static int process_mdb(SCLPEvent *event, MDBO *mdbo) int rc; int len; uint8_t buffer[SIZE_BUFFER]; - -len = be16_to_cpu(mdbo->length); -len -= sizeof(mdbo->length) + sizeof(mdbo->type) +const size_t hlen = sizeof(mdbo->length) ++ sizeof(mdbo->type) + sizeof(mdbo->mto.line_type_flags) + sizeof(mdbo->mto.alarm_control) + sizeof(mdbo->mto._reserved); +len = be16_to_cpu(mdbo->length); +len -= hlen; assert(len <= SIZE_BUFFER); /* convert EBCDIC SCLP contents to ASCII console message */ -- 2.20.1
[Qemu-devel] [PATCH v3 19/25] s390/ebcdic: Use size_t to iterate over arrays
Signed-off-by: Philippe Mathieu-Daudé
---
include/hw/s390x/ebcdic.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/hw/s390x/ebcdic.h b/include/hw/s390x/ebcdic.h
index 69a04cab62..d89174e113 100644
--- a/include/hw/s390x/ebcdic.h
+++ b/include/hw/s390x/ebcdic.h
@@ -83,18 +83,18 @@ static const uint8_t ascii2ebcdic[] = {
0x90, 0x3F, 0x3F, 0x3F, 0x3F, 0xEA, 0x3F, 0xFF
};
-static inline void ebcdic_put(uint8_t *p, const char *ascii, int len)
+static inline void ebcdic_put(uint8_t *p, const char *ascii, size_t len)
{
-int i;
+size_t i;
for (i = 0; i < len; i++) {
p[i] = ascii2ebcdic[(uint8_t)ascii[i]];
}
}
-static inline void ascii_put(uint8_t *p, const char *ebcdic, int len)
+static inline void ascii_put(uint8_t *p, const char *ebcdic, size_t len)
{
-int i;
+size_t i;
for (i = 0; i < len; i++) {
p[i] = ebcdic2ascii[(uint8_t)ebcdic[i]];
--
2.20.1
[Qemu-devel] [PATCH v3 17/25] net/filter-mirror: Use size_t
Since iov_size() returns a size_t, no need to use a signed type.
Signed-off-by: Philippe Mathieu-Daudé
---
net/filter-mirror.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/filter-mirror.c b/net/filter-mirror.c
index 3a61cf21e8..97b52d0544 100644
--- a/net/filter-mirror.c
+++ b/net/filter-mirror.c
@@ -48,7 +48,7 @@ static int filter_send(MirrorState *s,
{
NetFilterState *nf = NETFILTER(s);
int ret = 0;
-ssize_t size = 0;
+size_t size = 0;
uint32_t len = 0;
char *buf;
--
2.20.1
[Qemu-devel] [PATCH v2 2/3] mem/nvdimm: remove nvdimm_realize
nvdimm_realize is used to prepare its memory region, while this is done
in pre_plug stage.
This is time to remove it.
Signed-off-by: Wei Yang
---
v2: split nvdimm part here
---
hw/mem/nvdimm.c | 11 ---
1 file changed, 11 deletions(-)
diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
index bf2adf5e16..8f69576926 100644
--- a/hw/mem/nvdimm.c
+++ b/hw/mem/nvdimm.c
@@ -136,15 +136,6 @@ static MemoryRegion
*nvdimm_md_get_memory_region(MemoryDeviceState *md,
return nvdimm->nvdimm_mr;
}
-static void nvdimm_realize(PCDIMMDevice *dimm, Error **errp)
-{
-NVDIMMDevice *nvdimm = NVDIMM(dimm);
-
-if (!nvdimm->nvdimm_mr) {
-nvdimm_prepare_memory_region(nvdimm, errp);
-}
-}
-
/*
* the caller should check the input parameters before calling
* label read/write functions.
@@ -192,12 +183,10 @@ static Property nvdimm_properties[] = {
static void nvdimm_class_init(ObjectClass *oc, void *data)
{
-PCDIMMDeviceClass *ddc = PC_DIMM_CLASS(oc);
MemoryDeviceClass *mdc = MEMORY_DEVICE_CLASS(oc);
NVDIMMClass *nvc = NVDIMM_CLASS(oc);
DeviceClass *dc = DEVICE_CLASS(oc);
-ddc->realize = nvdimm_realize;
mdc->get_memory_region = nvdimm_md_get_memory_region;
dc->props = nvdimm_properties;
--
2.19.1
[Qemu-devel] [PATCH v2 0/3] PCDIMM cleanup
Three trivial cleanup for pc-dimm. Patch [1] remove the check on class->hotpluggable since pc-dimm is always hotpluggable. Patch [2] remove nvdimm_realize Patch [2] remove pcdimm realize-callback v2: * fix warning in Patch 1 * split Patch 2 into two Wei Yang (3): pc-dimm: remove check on pc-dimm hotpluggable mem/nvdimm: remove nvdimm_realize pc-dimm: revert "introduce realize callback" hw/acpi/memory_hotplug.c | 5 - hw/mem/nvdimm.c | 11 --- hw/mem/pc-dimm.c | 5 - include/hw/mem/pc-dimm.h | 3 --- 4 files changed, 24 deletions(-) -- 2.19.1
[Qemu-devel] [PATCH v2 1/3] pc-dimm: remove check on pc-dimm hotpluggable
Function acpi_memory_plug_cb() is only invoked when dev is a PCDIMM,
which is hotpluggable. This means it is not necessary to check this
property again.
This patch removes this check.
Signed-off-by: Wei Yang
Reviewed-by: Philippe Mathieu-Daudé
---
v2:
* remove unused dc
---
hw/acpi/memory_hotplug.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index 8c7c1013f3..cb5284d36f 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -264,11 +264,6 @@ void acpi_memory_plug_cb(HotplugHandler *hotplug_dev,
MemHotplugState *mem_st,
DeviceState *dev, Error **errp)
{
MemStatus *mdev;
-DeviceClass *dc = DEVICE_GET_CLASS(dev);
-
-if (!dc->hotpluggable) {
-return;
-}
mdev = acpi_memory_slot_status(mem_st, dev, errp);
if (!mdev) {
--
2.19.1
[Qemu-devel] [PATCH v2 3/3] pc-dimm: revert "introduce realize callback"
realize callback in introduced to check if the backend memory is large
enough to contain label data and init its memory region, while this task
is handled in pre_plug stage.
Now it's time to remove it.
Signed-off-by: Wei Yang
---
hw/mem/pc-dimm.c | 5 -
include/hw/mem/pc-dimm.h | 3 ---
2 files changed, 8 deletions(-)
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 152400b1fc..5832c0ba92 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -159,7 +159,6 @@ static void pc_dimm_init(Object *obj)
static void pc_dimm_realize(DeviceState *dev, Error **errp)
{
PCDIMMDevice *dimm = PC_DIMM(dev);
-PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
if (!dimm->hostmem) {
error_setg(errp, "'" PC_DIMM_MEMDEV_PROP "' property is not set");
@@ -178,10 +177,6 @@ static void pc_dimm_realize(DeviceState *dev, Error **errp)
return;
}
-if (ddc->realize) {
-ddc->realize(dimm, errp);
-}
-
host_memory_backend_set_mapped(dimm->hostmem, true);
}
diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
index 01436b9f50..d18f8246b7 100644
--- a/include/hw/mem/pc-dimm.h
+++ b/include/hw/mem/pc-dimm.h
@@ -59,8 +59,6 @@ typedef struct PCDIMMDevice {
/**
* PCDIMMDeviceClass:
- * @realize: called after common dimm is realized so that the dimm based
- * devices get the chance to do specified operations.
* @get_vmstate_memory_region: returns #MemoryRegion which indicates the
* memory of @dimm should be kept during live migration. Will not fail
* after the device was realized.
@@ -70,7 +68,6 @@ typedef struct PCDIMMDeviceClass {
DeviceClass parent_class;
/* public */
-void (*realize)(PCDIMMDevice *dimm, Error **errp);
MemoryRegion *(*get_vmstate_memory_region)(PCDIMMDevice *dimm,
Error **errp);
} PCDIMMDeviceClass;
--
2.19.1
Re: [Qemu-devel] [PATCH v4 11/11] virtio: CLI and provide packed ring feature bit by default
On Tue, Feb 19, 2019 at 09:33:40PM +0800, Jason Wang wrote: > > On 2019/2/19 下午7:23, Wei Xu wrote: > >On Tue, Feb 19, 2019 at 03:32:19PM +0800, Jason Wang wrote: > >>On 2019/2/14 下午12:26,w...@redhat.com wrote: > >>>From: Wei Xu > >>> > >>>Add userspace and vhost kernel/user support. > >>> > >>>Add CLI "ring_packed=true/false" to enable/disable packed ring provision. > >>>Usage: > >>> -device > >>> virtio-net-pci,netdev=xx,mac=xx:xx:xx:xx:xx:xx,ring_packed=false > >>> > >>>By default it is provided. > >>Please compat this for old machine types. > >It is provided by default, how to make it compatible for old machine types? > >Hide or provide it? > > > >Wei > > > > Take a look at e.g how pc_compat_3_1 and hw_compat_3_1 was used. OK, thanks. Wei > > Thanks > >
[Qemu-devel] [PATCH v2 1/2] CODING_STYLE: specify the indent rule for multiline code
We didn't specify the indent rule for multiline code here, which may
misleading users. And in current code, the code use different rules.
Add this rule in CODING_STYLE to make sure this is clear to every one.
Signed-off-by: Wei Yang
Suggested-by: Igor Mammedov
---
v2:
* rephrase changelog suggested by Eric Blake
- remove one redundant line
- fix some awkward grammar
- add { ; at the end of example
---
CODING_STYLE | 23 +++
1 file changed, 23 insertions(+)
diff --git a/CODING_STYLE b/CODING_STYLE
index ec075dedc4..403904667a 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -29,6 +29,29 @@ Spaces of course are superior to tabs because:
Do not leave whitespace dangling off the ends of lines.
+1.1 Multiline Indent
+
+There are several places where indent is necessary:
+
+ - struct definition
+ - if/else
+ - while/for
+ - function definition & call
+
+When breaking up a long line to fit within line widths, align the secondary
+lines just after the opening parenthesis of the first.
+
+For example:
+
+if (a == 1 &&
+b == 2) {
+
+while (a == 1 &&
+ b == 2) {
+
+do_something(arg1, arg2
+ arg3);
+
2. Line width
Lines should be 80 characters; try not to make them longer.
--
2.19.1
[Qemu-devel] [PATCH v2 0/2] CODING_STYLE: trivial update
The first one is suggested by Igor Mammedov to provide rule for multiline code. The second is a trivial fix to make example code all indented with 4 spaces. v2: * adjust Patch 1 as suggested by Eric Wei Yang (2): CODING_STYLE: specify the indent rule for multiline code CODING_STYLE: indent example code as all others CODING_STYLE | 31 +++ 1 file changed, 27 insertions(+), 4 deletions(-) -- 2.19.1
[Qemu-devel] [PATCH v2 2/2] CODING_STYLE: indent example code as all others
All the example code are indented with four spaces except this one.
Fix this by adding four spaces here.
Signed-off-by: Wei Yang
Reviewed-by: Eric Blake
Reviewed-by: Philippe Mathieu-Daudé
---
CODING_STYLE | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/CODING_STYLE b/CODING_STYLE
index 403904667a..5679b2fd69 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -131,10 +131,10 @@ block to a separate function altogether.
When comparing a variable for (in)equality with a constant, list the
constant on the right, as in:
-if (a == 1) {
-/* Reads like: "If a equals 1" */
-do_something();
-}
+if (a == 1) {
+/* Reads like: "If a equals 1" */
+do_something();
+}
Rationale: Yoda conditions (as in 'if (1 == a)') are awkward to read.
Besides, good compilers already warn users when '==' is mis-typed as '=',
--
2.19.1
Re: [Qemu-devel] [PATCH v2 0/3] target/arm: Reduce overhead of cpu_get_tb_cpu_state
Patchew URL: https://patchew.org/QEMU/20190219233421.388-1-richard.hender...@linaro.org/ Hi, This series seems to have some coding style problems. See output below for more information: Message-id: 20190219233421.388-1-richard.hender...@linaro.org Subject: [Qemu-devel] [PATCH v2 0/3] target/arm: Reduce overhead of cpu_get_tb_cpu_state Type: series === TEST SCRIPT BEGIN === #!/bin/bash git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/20190219233421.388-1-richard.hender...@linaro.org -> patchew/20190219233421.388-1-richard.hender...@linaro.org Switched to a new branch 'test' fdca6b67d3 target/arm: Rely on hflags correct in cpu_get_tb_cpu_state 8615480629 target/arm: Rebuild hflags at el changes and MSR writes 0d2a83ccfd target/arm: Split out recompute_hflags et al === OUTPUT BEGIN === 1/3 Checking commit 0d2a83ccfd5e (target/arm: Split out recompute_hflags et al) WARNING: Block comments use a leading /* on a separate line #277: FILE: target/arm/helper.c:13972: +/* If SVE is disabled, but FP is enabled, WARNING: Block comments use a leading /* on a separate line #358: FILE: target/arm/helper.c:14053: +/* The SS_ACTIVE and PSTATE_SS bits correspond to the state machine total: 0 errors, 2 warnings, 363 lines checked Patch 1/3 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 2/3 Checking commit 86154806299c (target/arm: Rebuild hflags at el changes and MSR writes) 3/3 Checking commit fdca6b67d324 (target/arm: Rely on hflags correct in cpu_get_tb_cpu_state) ERROR: Use g_assert or g_assert_not_reached #73: FILE: target/arm/helper.c:14049: +g_assert_cmphex(flags, ==, check_flags); total: 1 errors, 0 warnings, 34 lines checked Patch 3/3 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20190219233421.388-1-richard.hender...@linaro.org/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH v2 0/3] target/arm: Reduce overhead of cpu_get_tb_cpu_state
Patchew URL: https://patchew.org/QEMU/20190219233421.388-1-richard.hender...@linaro.org/ Hi, This series seems to have some coding style problems. See output below for more information: Message-id: 20190219233421.388-1-richard.hender...@linaro.org Subject: [Qemu-devel] [PATCH v2 0/3] target/arm: Reduce overhead of cpu_get_tb_cpu_state Type: series === TEST SCRIPT BEGIN === #!/bin/bash git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/20190219233421.388-1-richard.hender...@linaro.org -> patchew/20190219233421.388-1-richard.hender...@linaro.org Submodule 'capstone' (https://git.qemu.org/git/capstone.git) registered for path 'capstone' Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc' Submodule 'roms/QemuMacDrivers' (https://git.qemu.org/git/QemuMacDrivers.git) registered for path 'roms/QemuMacDrivers' Submodule 'roms/SLOF' (https://git.qemu.org/git/SLOF.git) registered for path 'roms/SLOF' Submodule 'roms/ipxe' (https://git.qemu.org/git/ipxe.git) registered for path 'roms/ipxe' Submodule 'roms/openbios' (https://git.qemu.org/git/openbios.git) registered for path 'roms/openbios' Submodule 'roms/openhackware' (https://git.qemu.org/git/openhackware.git) registered for path 'roms/openhackware' Submodule 'roms/qemu-palcode' (https://git.qemu.org/git/qemu-palcode.git) registered for path 'roms/qemu-palcode' Submodule 'roms/seabios' (https://git.qemu.org/git/seabios.git/) registered for path 'roms/seabios' Submodule 'roms/seabios-hppa' (https://github.com/hdeller/seabios-hppa.git) registered for path 'roms/seabios-hppa' Submodule 'roms/sgabios' (https://git.qemu.org/git/sgabios.git) registered for path 'roms/sgabios' Submodule 'roms/skiboot' (https://git.qemu.org/git/skiboot.git) registered for path 'roms/skiboot' Submodule 'roms/u-boot' (https://git.qemu.org/git/u-boot.git) registered for path 'roms/u-boot' Submodule 'roms/u-boot-sam460ex' (https://git.qemu.org/git/u-boot-sam460ex.git) registered for path 'roms/u-boot-sam460ex' Submodule 'tests/fp/berkeley-softfloat-3' (https://github.com/cota/berkeley-softfloat-3) registered for path 'tests/fp/berkeley-softfloat-3' Submodule 'tests/fp/berkeley-testfloat-3' (https://github.com/cota/berkeley-testfloat-3) registered for path 'tests/fp/berkeley-testfloat-3' Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) registered for path 'ui/keycodemapdb' Cloning into 'capstone'... Submodule path 'capstone': checked out '22ead3e0bfdb87516656453336160e0a37b066bf' Cloning into 'dtc'... Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536' Cloning into 'roms/QemuMacDrivers'... Submodule path 'roms/QemuMacDrivers': checked out '90c488d5f4a407342247b9ea869df1c2d9c8e266' Cloning into 'roms/SLOF'... Submodule path 'roms/SLOF': checked out 'a5b428e1c1eae703bdd62a3f527223c291ee3fdc' Cloning into 'roms/ipxe'... Submodule path 'roms/ipxe': checked out 'de4565cbe76ea9f7913a01f331be3ee901bb6e17' Cloning into 'roms/openbios'... Submodule path 'roms/openbios': checked out '3464681b2b5983df80086a40179d324102347da3' Cloning into 'roms/openhackware'... Submodule path 'roms/openhackware': checked out 'c559da7c8eec5e45ef1f67978827af6f0b9546f5' Cloning into 'roms/qemu-palcode'... Submodule path 'roms/qemu-palcode': checked out '51c237d7e20d05100eacadee2f61abc17e6bc097' Cloning into 'roms/seabios'... Submodule path 'roms/seabios': checked out 'a698c8995ffb2838296ec284fe3c4ad33dfca307' Cloning into 'roms/seabios-hppa'... Submodule path 'roms/seabios-hppa': checked out '1ef99a01572c2581c30e16e6fe69e9ea2ef92ce0' Cloning into 'roms/sgabios'... Submodule path 'roms/sgabios': checked out 'cbaee52287e5f32373181cff50a00b6c4ac9015a' Cloning into 'roms/skiboot'... Submodule path 'roms/skiboot': checked out 'e0ee24c27a172bcf482f6f2bc905e6211c134bcc' Cloning into 'roms/u-boot'... Submodule path 'roms/u-boot': checked out 'd85ca029f257b53a96da6c2fb421e78a003a9943' Cloning into 'roms/u-boot-sam460ex'... Submodule path 'roms/u-boot-sam460ex': checked out '60b3916f33e617a815973c5a6df77055b2e3a588' Cloning into 'tests/fp/berkeley-softfloat-3'... Submodule path 'tests/fp/berkeley-softfloat-3': checked out 'b64af41c3276f97f0e181920400ee056b9c88037' Cloning into 'tests/fp/berkeley-testfloat-3'... Submodule path 'tests/fp/berkeley-testfloat-3': checked out '5a59dcec19327396a011a17fd924aed4fec416b3' Cloning into 'ui/keycodemapdb'... Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce' Switched to a new branch 'test' fdca6b6 target/arm: Rely on hflags correct in cpu_get_tb_cpu_state 8615480 target/arm: Rebuild hflags at el changes and MSR writes 0d2a83c target/arm: Split out recompute_hflags et al === OUTPUT BEGIN ===
[Qemu-devel] [PATCH] block/iscsi: Restrict Linux-specific code
Some Linux specific code is missing guards, leading to
build failure on OSX:
$ sudo brew install libiscsi
$ ./configure && make
[...]
CC block/iscsi.o
qemu/block/iscsi.c:338:24: error: 'iscsi_aiocb_info' defined but not used
[-Werror=unused-const-variable=]
static const AIOCBInfo iscsi_aiocb_info = {
^~~~
qemu/block/iscsi.c:168:1: error: 'iscsi_schedule_bh' defined but not used
[-Werror=unused-function]
iscsi_schedule_bh(IscsiAIOCB *acb)
^
cc1: all warnings being treated as errors
Add guards to restrict this code for Linux.
Signed-off-by: Philippe Mathieu-Daudé
---
There are various Linux specific code, we could split it out to
another file such block/iscsi-lnx.o and guard with Makefile,
but that's another patch.
block/iscsi.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/block/iscsi.c b/block/iscsi.c
index ff473206e6..85daa9a481 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -145,6 +145,8 @@ static const unsigned iscsi_retry_times[] = {8, 32, 128,
512, 2048, 8192, 32768}
* unallocated. */
#define ISCSI_CHECKALLOC_THRES 64
+#ifdef __linux__
+
static void
iscsi_bh_cb(void *p)
{
@@ -172,6 +174,8 @@ iscsi_schedule_bh(IscsiAIOCB *acb)
qemu_bh_schedule(acb->bh);
}
+#endif
+
static void iscsi_co_generic_bh_cb(void *opaque)
{
struct IscsiTask *iTask = opaque;
@@ -290,6 +294,8 @@ static void iscsi_co_init_iscsitask(IscsiLun *iscsilun,
struct IscsiTask *iTask)
};
}
+#ifdef __linux__
+
/* Called (via iscsi_service) with QemuMutex held. */
static void
iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void
*command_data,
@@ -338,6 +344,7 @@ static const AIOCBInfo iscsi_aiocb_info = {
.cancel_async = iscsi_aio_cancel,
};
+#endif
static void iscsi_process_read(void *arg);
static void iscsi_process_write(void *arg);
--
2.20.1
Re: [Qemu-devel] [PATCH 2/2] CODING_STYLE: indent example code as all others
On 2/19/19 11:20 PM, Wei Yang wrote:
> On Tue, Feb 19, 2019 at 07:55:31PM +0100, Philippe Mathieu-Daudé wrote:
>> On 2/19/19 6:56 PM, Eric Blake wrote:
>>> On 2/19/19 11:38 AM, Philippe Mathieu-Daudé wrote:
>>>
> +if (a == 1) {
> +/* Reads like: "If a equals 1" */
I guess you found a bug in the documentation :)
Since 8c06fbdf36bf4d the style asked is:
We now require Linux-kernel-style multiline comments:
/*
* line one
* line two
*/
> +do_something();
>>>
>>> We only require winged multiline comments when the comment is actually
>>> multiline. In this case, the comment is a one-liner, and is just fine
>>> as written.
>>
>> Hmm I have a series where I moved code and changed from /* one line */
>> to the multi-line style, I wonder why and remember checkpatch errors.
>> Maybe a side-effect from what b94e809d3e fixed.
>>
>> Anyway, Wei do you mind adding a multi-line example here too?
>>
>
> A multi-line example for multiline comments?
>
> This looks not relavant to this sectioin. I am afraid I will not add
> this example here. Sorry for that.
No worries, R-b stands.
>> With/without multi-line example:
>> Reviewed-by: Philippe Mathieu-Daudé
>>
>> Thanks!
>>
>> Phil.
>
[Qemu-devel] [PATCH v2 3/3] target/arm: Rely on hflags correct in cpu_get_tb_cpu_state
This is the payoff.
>From perf record -g data of ubuntu 18 boot and shutdown:
BEFORE:
- 23.02% 2.82% qemu-system-aar [.] helper_lookup_tb_ptr
- 20.22% helper_lookup_tb_ptr
+ 10.05% tb_htable_lookup
- 9.13% cpu_get_tb_cpu_state
3.20% aa64_va_parameters_both
0.55% fp_exception_el
- 11.66% 4.74% qemu-system-aar [.] cpu_get_tb_cpu_state
- 6.96% cpu_get_tb_cpu_state
3.63% aa64_va_parameters_both
0.60% fp_exception_el
0.53% sve_exception_el
AFTER:
- 16.40% 3.40% qemu-system-aar [.] helper_lookup_tb_ptr
- 13.03% helper_lookup_tb_ptr
+ 11.19% tb_htable_lookup
0.55% cpu_get_tb_cpu_state
0.98% 0.71% qemu-system-aar [.] cpu_get_tb_cpu_state
0.87% 0.24% qemu-system-aar [.] rebuild_hflags_a64
Before, helper_lookup_tb_ptr is the second hottest function in the
application, consuming almost a quarter of the runtime. Within the
entire execution, cpu_get_tb_cpu_state consumes about 12%.
After, helper_lookup_tb_ptr has dropped to the fourth hottest function,
with consumption dropping to a sixth of the runtime. Within the
entire execution, cpu_get_tb_cpu_state has dropped below 1%, and the
supporting function to rebuild hflags also consumes about 1%.
Assertions are retained for --enable-debug-tcg.
Signed-off-by: Richard Henderson
---
v2: Retain asserts for future debugging.
---
target/arm/helper.c | 20 +++-
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 909535a3e3..990a87876f 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -14034,19 +14034,29 @@ void HELPER(rebuild_hflags_a64)(CPUARMState *env,
uint32_t el)
void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
target_ulong *cs_base, uint32_t *pflags)
{
-int current_el = arm_current_el(env);
-uint32_t flags;
+uint32_t flags = env->hflags;
uint32_t pstate_for_ss;
+#ifdef CONFIG_DEBUG_TCG
+{
+int el = arm_current_el(env);
+uint32_t check_flags;
+if (is_a64(env)) {
+check_flags = rebuild_hflags_a64(env, el);
+} else {
+check_flags = rebuild_hflags_a32(env, el);
+}
+g_assert_cmphex(flags, ==, check_flags);
+}
+#endif
+
*cs_base = 0;
-if (is_a64(env)) {
+if (FIELD_EX32(flags, TBFLAG_ANY, AARCH64_STATE)) {
*pc = env->pc;
-flags = rebuild_hflags_a64(env, current_el);
flags = FIELD_DP32(flags, TBFLAG_A64, BTYPE, env->btype);
pstate_for_ss = env->pstate;
} else {
*pc = env->regs[15];
-flags = rebuild_hflags_a32(env, current_el);
flags = FIELD_DP32(flags, TBFLAG_A32, THUMB, env->thumb);
flags = FIELD_DP32(flags, TBFLAG_A32, CONDEXEC, env->condexec_bits);
pstate_for_ss = env->uncached_cpsr;
--
2.17.1
[Qemu-devel] [PATCH v2 1/3] target/arm: Split out recompute_hflags et al
We will use these to minimize the computation for every call to
cpu_get_tb_cpu_state. For now, the env->hflags variable is not used.
Reviewed-by: Alex Bennée
Signed-off-by: Richard Henderson
---
target/arm/cpu.h | 22 +++-
target/arm/helper.h| 3 +
target/arm/internals.h | 3 +
target/arm/helper.c| 267 -
4 files changed, 179 insertions(+), 116 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 84ae6849c2..848f0926eb 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -240,6 +240,9 @@ typedef struct CPUARMState {
uint32_t pstate;
uint32_t aarch64; /* 1 if CPU is in aarch64 state; inverse of PSTATE.nRW */
+/* Cached TBFLAGS state. See below for which bits are included. */
+uint32_t hflags;
+
/* Frequently accessed CPSR bits are stored separately for efficiency.
This contains all the other bits. Use cpsr_{read,write} to access
the whole CPSR. */
@@ -3065,25 +3068,28 @@ static inline bool
arm_cpu_data_is_big_endian(CPUARMState *env)
#include "exec/cpu-all.h"
-/* Bit usage in the TB flags field: bit 31 indicates whether we are
+/*
+ * Bit usage in the TB flags field: bit 31 indicates whether we are
* in 32 or 64 bit mode. The meaning of the other bits depends on that.
* We put flags which are shared between 32 and 64 bit mode at the top
* of the word, and flags which apply to only one mode at the bottom.
+ *
+ * Unless otherwise noted, these bits are cached in env->hflags.
*/
FIELD(TBFLAG_ANY, AARCH64_STATE, 31, 1)
FIELD(TBFLAG_ANY, MMUIDX, 28, 3)
FIELD(TBFLAG_ANY, SS_ACTIVE, 27, 1)
-FIELD(TBFLAG_ANY, PSTATE_SS, 26, 1)
+FIELD(TBFLAG_ANY, PSTATE_SS, 26, 1) /* Not cached. */
/* Target EL if we take a floating-point-disabled exception */
FIELD(TBFLAG_ANY, FPEXC_EL, 24, 2)
FIELD(TBFLAG_ANY, BE_DATA, 23, 1)
/* Bit usage when in AArch32 state: */
-FIELD(TBFLAG_A32, THUMB, 0, 1)
+FIELD(TBFLAG_A32, THUMB, 0, 1) /* Not cached. */
FIELD(TBFLAG_A32, VECLEN, 1, 3)
FIELD(TBFLAG_A32, VECSTRIDE, 4, 2)
FIELD(TBFLAG_A32, VFPEN, 7, 1)
-FIELD(TBFLAG_A32, CONDEXEC, 8, 8)
+FIELD(TBFLAG_A32, CONDEXEC, 8, 8) /* Not cached. */
FIELD(TBFLAG_A32, SCTLR_B, 16, 1)
/* We store the bottom two bits of the CPAR as TB flags and handle
* checks on the other bits at runtime
@@ -3105,7 +3111,7 @@ FIELD(TBFLAG_A64, SVEEXC_EL, 2, 2)
FIELD(TBFLAG_A64, ZCR_LEN, 4, 4)
FIELD(TBFLAG_A64, PAUTH_ACTIVE, 8, 1)
FIELD(TBFLAG_A64, BT, 9, 1)
-FIELD(TBFLAG_A64, BTYPE, 10, 2)
+FIELD(TBFLAG_A64, BTYPE, 10, 2) /* Not cached. */
FIELD(TBFLAG_A64, TBID, 12, 2)
static inline bool bswap_code(bool sctlr_b)
@@ -3190,6 +3196,12 @@ void arm_register_pre_el_change_hook(ARMCPU *cpu,
ARMELChangeHookFn *hook,
void arm_register_el_change_hook(ARMCPU *cpu, ARMELChangeHookFn *hook, void
*opaque);
+/**
+ * arm_rebuild_hflags:
+ * Rebuild the cached TBFLAGS for arbitrary changed processor state.
+ */
+void arm_rebuild_hflags(CPUARMState *env);
+
/**
* aa32_vfp_dreg:
* Return a pointer to the Dn register within env in 32-bit mode.
diff --git a/target/arm/helper.h b/target/arm/helper.h
index 923e8e1525..bbc1a48089 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -89,6 +89,9 @@ DEF_HELPER_4(msr_banked, void, env, i32, i32, i32)
DEF_HELPER_2(get_user_reg, i32, env, i32)
DEF_HELPER_3(set_user_reg, void, env, i32, i32)
+DEF_HELPER_FLAGS_2(rebuild_hflags_a32, TCG_CALL_NO_RWG, void, env, i32)
+DEF_HELPER_FLAGS_2(rebuild_hflags_a64, TCG_CALL_NO_RWG, void, env, i32)
+
DEF_HELPER_1(vfp_get_fpscr, i32, env)
DEF_HELPER_2(vfp_set_fpscr, void, env, i32)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index a4bd1becb7..8c1b813364 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -968,4 +968,7 @@ ARMVAParameters aa64_va_parameters_both(CPUARMState *env,
uint64_t va,
ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va,
ARMMMUIdx mmu_idx, bool data);
+uint32_t rebuild_hflags_a32(CPUARMState *env, int el);
+uint32_t rebuild_hflags_a64(CPUARMState *env, int el);
+
#endif
diff --git a/target/arm/helper.c b/target/arm/helper.c
index a018eb23fe..189e97a083 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -13886,122 +13886,15 @@ ARMMMUIdx arm_stage1_mmu_idx(CPUARMState *env)
}
#endif
-void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
- target_ulong *cs_base, uint32_t *pflags)
+static uint32_t common_hflags(CPUARMState *env, int el, ARMMMUIdx mmu_idx,
+ int fp_el, uint32_t flags)
{
-ARMMMUIdx mmu_idx = arm_mmu_idx(env);
-int current_el = arm_current_el(env);
-int fp_el = fp_exception_el(env, current_el);
-uint32_t flags = 0;
-
-if (is_a64(env)) {
-ARMCPU *cpu = arm_env_get_cpu(env);
-uint64_t sctlr;
-
-*pc = env->pc;
-flags = FIELD_DP32(flags, TBFLAG_ANY, AAR
[Qemu-devel] [PATCH v2 2/3] target/arm: Rebuild hflags at el changes and MSR writes
Now setting, but not relying upon, env->hflags.
Signed-off-by: Richard Henderson
---
v2: Fixed partial conversion to assignment to env->hflags.
---
target/arm/internals.h | 1 +
linux-user/syscall.c | 1 +
target/arm/cpu.c | 1 +
target/arm/helper-a64.c| 3 +++
target/arm/helper.c| 2 ++
target/arm/machine.c | 1 +
target/arm/op_helper.c | 1 +
target/arm/translate-a64.c | 6 +-
target/arm/translate.c | 14 --
9 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 8c1b813364..235f4fafec 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -970,5 +970,6 @@ ARMVAParameters aa64_va_parameters(CPUARMState *env,
uint64_t va,
uint32_t rebuild_hflags_a32(CPUARMState *env, int el);
uint32_t rebuild_hflags_a64(CPUARMState *env, int el);
+void rebuild_hflags_any(CPUARMState *env);
#endif
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 5bbb72f3d5..123f342bdc 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9691,6 +9691,7 @@ static abi_long do_syscall1(void *cpu_env, int num,
abi_long arg1,
aarch64_sve_narrow_vq(env, vq);
}
env->vfp.zcr_el[1] = vq - 1;
+arm_rebuild_hflags(env);
ret = vq * 16;
}
return ret;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index edf6e0e1f1..e4da513eb3 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -390,6 +390,7 @@ static void arm_cpu_reset(CPUState *s)
hw_breakpoint_update_all(cpu);
hw_watchpoint_update_all(cpu);
+arm_rebuild_hflags(env);
}
bool arm_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
diff --git a/target/arm/helper-a64.c b/target/arm/helper-a64.c
index 70850e564d..17200f1288 100644
--- a/target/arm/helper-a64.c
+++ b/target/arm/helper-a64.c
@@ -995,6 +995,7 @@ void HELPER(exception_return)(CPUARMState *env, uint64_t
new_pc)
} else {
env->regs[15] = new_pc & ~0x3;
}
+env->hflags = rebuild_hflags_a32(env, new_el);
qemu_log_mask(CPU_LOG_INT, "Exception return from AArch64 EL%d to "
"AArch32 EL%d PC 0x%" PRIx32 "\n",
cur_el, new_el, env->regs[15]);
@@ -1006,10 +1007,12 @@ void HELPER(exception_return)(CPUARMState *env,
uint64_t new_pc)
}
aarch64_restore_sp(env, new_el);
env->pc = new_pc;
+env->hflags = rebuild_hflags_a64(env, new_el);
qemu_log_mask(CPU_LOG_INT, "Exception return from AArch64 EL%d to "
"AArch64 EL%d PC 0x%" PRIx64 "\n",
cur_el, new_el, env->pc);
}
+
/*
* Note that cur_el can never be 0. If new_el is 0, then
* el0_a64 is return_to_aa64, else el0_a64 is ignored.
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 189e97a083..909535a3e3 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -9201,6 +9201,7 @@ static void take_aarch32_exception(CPUARMState *env, int
new_mode,
env->regs[14] = env->regs[15] + offset;
}
env->regs[15] = newpc;
+env->hflags = rebuild_hflags_a32(env, arm_current_el(env));
}
static void arm_cpu_do_interrupt_aarch32_hyp(CPUState *cs)
@@ -9546,6 +9547,7 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
pstate_write(env, PSTATE_DAIF | new_mode);
env->aarch64 = 1;
+env->hflags = rebuild_hflags_a64(env, new_el);
aarch64_restore_sp(env, new_el);
env->pc = addr;
diff --git a/target/arm/machine.c b/target/arm/machine.c
index 124192bfc2..e944d6b736 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -743,6 +743,7 @@ static int cpu_post_load(void *opaque, int version_id)
if (!kvm_enabled()) {
pmu_op_finish(&cpu->env);
}
+arm_rebuild_hflags(&cpu->env);
return 0;
}
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index c998eadfaa..f82eeae7e4 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -571,6 +571,7 @@ void HELPER(cpsr_write_eret)(CPUARMState *env, uint32_t val)
*/
env->regs[15] &= (env->thumb ? ~1 : ~3);
+env->hflags = rebuild_hflags_a32(env, arm_current_el(env));
qemu_mutex_lock_iothread();
arm_call_el_change_hook(arm_env_get_cpu(env));
qemu_mutex_unlock_iothread();
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index af8e4fd4be..a786c7ef5f 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -1841,11 +1841,15 @@ static void handle_sys(DisasContext *s, uint32_t insn,
bool isread,
/* I/O operations must end the TB here (whether read or write) */
gen_io_end();
s->base.is_jmp = DISAS_UPDATE;
-} else if (!isread && !(ri->type & ARM_CP_SUPPRESS_TB_END)) {
+}
+if (!isread && !(ri->type & ARM_CP_SUPPRESS_TB_END)) {
/* We default to ending
[Qemu-devel] [PATCH v2 0/3] target/arm: Reduce overhead of cpu_get_tb_cpu_state
Changes since v1: * Apparently I had started a last-minute API change, and failed to covert all of the users, and also failed to re-test afterward. * Retain assertions for --enable-debug-tcg. r~ Richard Henderson (3): target/arm: Split out recompute_hflags et al target/arm: Rebuild hflags at el changes and MSR writes target/arm: Rely on hflags correct in cpu_get_tb_cpu_state target/arm/cpu.h | 22 ++- target/arm/helper.h| 3 + target/arm/internals.h | 4 + linux-user/syscall.c | 1 + target/arm/cpu.c | 1 + target/arm/helper-a64.c| 3 + target/arm/helper.c| 279 ++--- target/arm/machine.c | 1 + target/arm/op_helper.c | 1 + target/arm/translate-a64.c | 6 +- target/arm/translate.c | 14 +- 11 files changed, 216 insertions(+), 119 deletions(-) -- 2.17.1
Re: [Qemu-devel] [PATCH] hw/pci/pci-stub: Add msi_enabled() and msi_notify() to the pci stubs
> > Makes sense, but it is also abstraction time. :) What if instead there > > was a function > > > > void msi_allocate_irqs(PCIDevice *pdev, int num, bool fallback_to_intx); > > > > and then ich.c did > > > > irqs = msi_allocate_irqs(pdev, 1, true); > > s->irq = irqs[0]; > > g_free(irqs); > > > > ? "if msi_enabled raise MSI else raise INTX" is really a common idiom. > > > > Thanks, > > > > Paolo > > Maybe it is but the specific issue is not about fallback to INTX of PCI > (is the fallback broken for ahci? I don't know). It works, the above is just a new abstraction. > The trick is there's no pdev at all. The trick :) is that in ich.c there is a pdev. Right now we are assigning to s->irq either the INTX irq (if PCI) or a sysbus irq (if sysbus), but then we need to know about MSI with a wrapper around s->irq. Instead, my suggestion is to put the wrapper in the PCI core as a qemu_irq callback---or perhaps in ich.c, but anyway ahci.c should not care that there could be a PCI AHCI device and it would have two different interrupt modes. In fact, doing this would also remove the need for s->container, I think. Paolo
Re: [Qemu-devel] [PATCH 4/4] target/arm: Rely on hflags correct in cpu_get_tb_cpu_state
On 2/19/19 12:17 PM, Alex Bennée wrote:
> While debugging I came up with this monstrosity:
>
> if (FIELD_EX32(flags, TBFLAG_ANY, AARCH64_STATE)) {
> #ifdef CONFIG_DEBUG_TCG
> static uint32_t tb_state = 0;
> uint32_t recalc_flags = rebuild_hflags_a64(env, arm_current_el(env));
> tb_state++;
> if (flags != recalc_flags) {
> fprintf(stderr, "%s: flags %#x, should be %#x (%#x/%d)\n",
> __func__,
> flags, recalc_flags, flags ^ recalc_flags, tb_state);
> abort();
> }
> #endif
> *pc = env->pc;
> flags = FIELD_DP32(flags, TBFLAG_A64, BTYPE, env->btype);
> pstate_for_ss = env->pstate;
> } else {
I have now included
+#ifdef CONFIG_DEBUG_TCG
+{
+int el = arm_current_el(env);
+uint32_t check_flags;
+if (is_a64(env)) {
+check_flags = rebuild_hflags_a64(env, el);
+} else {
+check_flags = rebuild_hflags_a32(env, el);
+}
+g_assert_cmphex(flags, ==, check_flags);
+}
+#endif
r~
[Qemu-devel] [PATCH v5 1/5] target/arm: Add helpers for FMLAL
Note that float16_to_float32 rightly squashes SNaN to QNaN.
But of course pickNaNMulAdd, for ARM, selects SNaNs first.
So we have to preserve SNaN long enough for the correct NaN
to be selected. Thus float16_to_float32_by_bits.
Signed-off-by: Richard Henderson
---
target/arm/helper.h | 9 +++
target/arm/vec_helper.c | 148
2 files changed, 157 insertions(+)
diff --git a/target/arm/helper.h b/target/arm/helper.h
index 747cb64d29..d363904278 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -677,6 +677,15 @@ DEF_HELPER_FLAGS_5(gvec_sqsub_s, TCG_CALL_NO_RWG,
DEF_HELPER_FLAGS_5(gvec_sqsub_d, TCG_CALL_NO_RWG,
void, ptr, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_5(gvec_fmlal_a32, TCG_CALL_NO_RWG,
+ void, ptr, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_5(gvec_fmlal_a64, TCG_CALL_NO_RWG,
+ void, ptr, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_5(gvec_fmlal_idx_a32, TCG_CALL_NO_RWG,
+ void, ptr, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_5(gvec_fmlal_idx_a64, TCG_CALL_NO_RWG,
+ void, ptr, ptr, ptr, ptr, i32)
+
#ifdef TARGET_AARCH64
#include "helper-a64.h"
#include "helper-sve.h"
diff --git a/target/arm/vec_helper.c b/target/arm/vec_helper.c
index dfc635cf9a..dedef62403 100644
--- a/target/arm/vec_helper.c
+++ b/target/arm/vec_helper.c
@@ -898,3 +898,151 @@ void HELPER(gvec_sqsub_d)(void *vd, void *vq, void *vn,
}
clear_tail(d, oprsz, simd_maxsz(desc));
}
+
+/*
+ * Convert float16 to float32, raising no exceptions and
+ * preserving exceptional values, including SNaN.
+ * This is effectively an unpack+repack operation.
+ */
+static float32 float16_to_float32_by_bits(uint32_t f16, bool fz16)
+{
+const int f16_bias = 15;
+const int f32_bias = 127;
+uint32_t sign = extract32(f16, 15, 1);
+uint32_t exp = extract32(f16, 10, 5);
+uint32_t frac = extract32(f16, 0, 10);
+
+if (exp == 0x1f) {
+/* Inf or NaN */
+exp = 0xff;
+} else if (exp == 0) {
+/* Zero or denormal. */
+if (frac != 0) {
+if (fz16) {
+frac = 0;
+} else {
+/*
+ * Denormal; these are all normal float32.
+ * Shift the fraction so that the msb is at bit 11,
+ * then remove bit 11 as the implicit bit of the
+ * normalized float32. Note that we still go through
+ * the shift for normal numbers below, to put the
+ * float32 fraction at the right place.
+ */
+int shift = clz32(frac) - 21;
+frac = (frac << shift) & 0x3ff;
+exp = f32_bias - f16_bias - shift + 1;
+}
+}
+} else {
+/* Normal number; adjust the bias. */
+exp += f32_bias - f16_bias;
+}
+sign <<= 31;
+exp <<= 23;
+frac <<= 23 - 10;
+
+return sign | exp | frac;
+}
+
+static uint64_t load4_f16(uint64_t *ptr, int is_q, int is_2)
+{
+/*
+ * Branchless load of u32[0], u64[0], u32[1], or u64[1].
+ * Load the 2nd qword iff is_q & is_2.
+ * Shift to the 2nd dword iff !is_q & is_2.
+ * For !is_q & !is_2, the upper bits of the result are garbage.
+ */
+return ptr[is_q & is_2] >> ((is_2 & ~is_q) << 5);
+}
+
+/*
+ * Note that FMLAL requires oprsz == 8 or oprsz == 16,
+ * as there is not yet SVE versions that might use blocking.
+ */
+
+static void do_fmlal(float32 *d, void *vn, void *vm, float_status *fpst,
+ uint32_t desc, bool fz16)
+{
+intptr_t i, oprsz = simd_oprsz(desc);
+int is_s = extract32(desc, SIMD_DATA_SHIFT, 1);
+int is_2 = extract32(desc, SIMD_DATA_SHIFT + 1, 1);
+int is_q = oprsz == 16;
+uint64_t n_4, m_4;
+
+/* Pre-load all of the f16 data, avoiding overlap issues. */
+n_4 = load4_f16(vn, is_q, is_2);
+m_4 = load4_f16(vm, is_q, is_2);
+
+/* Negate all inputs for FMLSL at once. */
+if (is_s) {
+n_4 ^= 0x8000800080008000ull;
+}
+
+for (i = 0; i < oprsz / 4; i++) {
+float32 n_1 = float16_to_float32_by_bits(n_4 >> (i * 16), fz16);
+float32 m_1 = float16_to_float32_by_bits(m_4 >> (i * 16), fz16);
+d[H4(i)] = float32_muladd(n_1, m_1, d[H4(i)], 0, fpst);
+}
+clear_tail(d, oprsz, simd_maxsz(desc));
+}
+
+void HELPER(gvec_fmlal_a32)(void *vd, void *vn, void *vm,
+void *venv, uint32_t desc)
+{
+CPUARMState *env = venv;
+do_fmlal(vd, vn, vm, &env->vfp.standard_fp_status, desc,
+ get_flush_inputs_to_zero(&env->vfp.fp_status_f16));
+}
+
+void HELPER(gvec_fmlal_a64)(void *vd, void *vn, void *vm,
+void *venv, uint32_t desc)
+{
+CPUARMState *env = venv;
+do_fmlal(vd, vn, vm, &env->vfp.fp_status, desc,
+ get_flush_inputs_to_zero(&env->vfp.fp_status_f16));
+}
+
+static void do_fmlal_idx(float32 *d, void *vn,
