Re: [PATCH 00/10] vhost/qemu: thread per IO SCSI vq
On 2020/11/18 下午2:57, Mike Christie wrote: On 11/17/20 11:17 PM, Jason Wang wrote: On 2020/11/18 上午12:40, Stefan Hajnoczi wrote: On Thu, Nov 12, 2020 at 05:18:59PM -0600, Mike Christie wrote: The following kernel patches were made over Michael's vhost branch: https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/log/?h=vhost__;!!GqivPVa7Brio!MzCv3wdRfz5dltunazRWGCeUkMg91pPEOLpIivsebLX9vhYDSi_E1V36e9H0NoRys_hU$ and the vhost-scsi bug fix patchset: https://urldefense.com/v3/__https://lore.kernel.org/linux-scsi/20201112170008.GB1555653@stefanha-x1.localdomain/T/*t__;Iw!!GqivPVa7Brio!MzCv3wdRfz5dltunazRWGCeUkMg91pPEOLpIivsebLX9vhYDSi_E1V36e9H0NmuPE_m8$ And the qemu patch was made over the qemu master branch. vhost-scsi currently supports multiple queues with the num_queues setting, but we end up with a setup where the guest's scsi/block layer can do a queue per vCPU and the layers below vhost can do a queue per CPU. vhost-scsi will then do a num_queue virtqueues, but all IO gets set on and completed on a single vhost-scsi thread. After 2 - 4 vqs this becomes a bottleneck. This patchset allows us to create a worker thread per IO vq, so we can better utilize multiple CPUs with the multiple queues. It implments Jason's suggestion to create the initial worker like normal, then create the extra workers for IO vqs with the VHOST_SET_VRING_ENABLE ioctl command added in this patchset. How does userspace find out the tids and set their CPU affinity? What is the meaning of the new VHOST_SET_VRING_ENABLE ioctl? It doesn't really "enable" or "disable" the vq, requests are processed regardless. Actually I think it should do the real "enable/disable" that tries to follow the virtio spec. What does real mean here? I think it means when a vq is disabled, vhost won't process any request from that virtqueue. For the vdpa enable call for example, would it be like ifcvf_vdpa_set_vq_ready where it sets the ready bit or more like mlx5_vdpa_set_vq_ready where it can do some more work in the disable case? For vDPA, it would be more complicated. E.g for IFCVF, it just delay the setting of queue_enable when it get DRIVER_OK. Technically it can passthrough the queue_enable to the hardware as what mlx5e did. For net and something like ifcvf_vdpa_set_vq_ready's design would we have vhost_ring_ioctl() set some vhost_virtqueue enable bit. We then have some helper vhost_vq_is_enabled() and some code to detect if userspace supports the new ioctl. Yes, vhost support backend capability. When userspace negotiate the new capability, we should depend on SET_VRING_ENABLE, if not we can do vhost_vq_is_enable(). And then in vhost_net_set_backend do we call vhost_vq_is_enabled()? What is done for disable then? It needs more thought, but the question is not specific to SET_VRING_ENABLE. Consider guest may zero ring address as well. For disabling, we can simply flush the work and disable all the polls. It doesn't seem to buy a lot of new functionality. Is it just so we follow the spec? My understanding is that, since spec defines queue_enable, we should support it in vhost. And we can piggyback the delayed vq creation with this feature. Otherwise we will duplicate the function if we want to support queue_enable. Or do you want it work more like mlx5_vdpa_set_vq_ready? For this in vhost_ring_ioctl when we get the new ioctl we would call into the drivers and have it start queues and stop queues? For enable, what we you do for net for this case? Net is something different, we can simply use SET_BACKEND to disable a specific virtqueue without introducing new ioctls. Notice that, net mq is kind of different with scsi which have a per queue pair vhost device, and the API allows us to set backend for a specific virtqueue. For disable, would you do something like vhost_net_stop_vq (we don't free up anything allocated in vhost_vring_ioctl calls, but we can stop what we setup in the net driver)? It's up to you, if you think you should free the resources you can do that. Is this useful for the current net mq design or is this for something like where you would do one vhost net device with multiple vqs? I think SET_VRING_ENABLE is more useful for SCSI since it have a model of multiple vqs per vhost device. My issue/convern is that in general these calls seems useful, but we don't really need them for scsi because vhost scsi is already stuck creating vqs like how it does due to existing users. If we do the ifcvf_vdpa_set_vq_ready type of design where we just set some bit, then the new ioctl does not give us a lot. It's just an extra check and extra code. And for the mlx5_vdpa_set_vq_ready type of design, it doesn't seem like it's going to happen a lot where the admin is going to want to remove vqs from a running device. In this case, qemu may just disable the queues of vhost-scsi via SET_VRING_ENABLE and then we can free resou
Re: [PATCH] Fix build with 64 bits time_t
On Tue, Nov 17, 2020 at 09:28:46PM +0100, Fabrice Fontaine wrote:
> time element is deprecated on new input_event structure in kernel's
> input.h [1]
>
> This will avoid the following build failure:
>
> hw/input/virtio-input-host.c: In function 'virtio_input_host_handle_status':
> hw/input/virtio-input-host.c:198:28: error: 'struct input_event' has no
> member named 'time'
> 198 | if (gettimeofday(&evdev.time, NULL)) {
> |^
>
> Fixes:
> -
> http://autobuild.buildroot.org/results/a538167e288c14208d557cd45446df86d3d599d5
> -
> http://autobuild.buildroot.org/results/efd4474fb4b6c0ce0ab3838ce130429c51e43bbb
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=152194fe9c3f
>
> Signed-off-by: Fabrice Fontaine
> ---
> contrib/vhost-user-input/main.c | 10 +-
> hw/input/virtio-input-host.c| 10 +-
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/vhost-user-input/main.c b/contrib/vhost-user-input/main.c
> index 6020c6f33a..e688c3e0a9 100644
> --- a/contrib/vhost-user-input/main.c
> +++ b/contrib/vhost-user-input/main.c
> @@ -17,6 +17,11 @@
> #include "standard-headers/linux/virtio_input.h"
> #include "qapi/error.h"
>
> +#ifndef input_event_sec
> +#define input_event_sec time.tv_sec
> +#define input_event_usec time.tv_usec
> +#endif
> +
> enum {
> VHOST_USER_INPUT_MAX_QUEUES = 2,
> };
Just update ./include/standard-headers/linux/input.h, we'll get these
defines for free.
> @@ -115,13 +120,16 @@ vi_evdev_watch(VuDev *dev, int condition, void *data)
> static void vi_handle_status(VuInput *vi, virtio_input_event *event)
> {
> struct input_event evdev;
> +struct timeval tval;
> int rc;
>
> -if (gettimeofday(&evdev.time, NULL)) {
> +if (gettimeofday(&tval, NULL)) {
> perror("vi_handle_status: gettimeofday");
> return;
> }
>
> +evdev.input_event_sec = tval.tv_sec;
> +evdev.input_event_usec = tval.tv_usec;
> evdev.type = le16toh(event->type);
> evdev.code = le16toh(event->code);
> evdev.value = le32toh(event->value);
> diff --git a/hw/input/virtio-input-host.c b/hw/input/virtio-input-host.c
> index 85daf73f1a..2e261737e1 100644
> --- a/hw/input/virtio-input-host.c
> +++ b/hw/input/virtio-input-host.c
> @@ -16,6 +16,11 @@
> #include
> #include "standard-headers/linux/input.h"
>
> +#ifndef input_event_sec
> +#define input_event_sec time.tv_sec
> +#define input_event_usec time.tv_usec
> +#endif
> +
> /* - */
>
> static struct virtio_input_config virtio_input_host_config[] = {
> @@ -193,13 +198,16 @@ static void virtio_input_host_handle_status(VirtIOInput
> *vinput,
> {
> VirtIOInputHost *vih = VIRTIO_INPUT_HOST(vinput);
> struct input_event evdev;
> +struct timeval tval;
> int rc;
>
> -if (gettimeofday(&evdev.time, NULL)) {
> +if (gettimeofday(&tval, NULL)) {
> perror("virtio_input_host_handle_status: gettimeofday");
> return;
> }
>
> +evdev.input_event_sec = tval.tv_sec;
> +evdev.input_event_usec = tval.tv_usec;
> evdev.type = le16_to_cpu(event->type);
> evdev.code = le16_to_cpu(event->code);
> evdev.value = le32_to_cpu(event->value);
> --
> 2.29.2
Re: [PATCH for-5.2] s390x/pci: fix endianness issues
On Tue, 17 Nov 2020 14:49:53 -0500
Matthew Rosato wrote:
> On 11/17/20 2:21 PM, Thomas Huth wrote:
> > On 17/11/2020 18.13, Cornelia Huck wrote:
> >> zPCI control blocks are big endian, we need to take care that we
> >> do proper accesses in order not to break tcg guests on little endian
> >> hosts.
> >>
> >> Fixes: 28dc86a07299 ("s390x/pci: use a PCI Group structure")
> >> Fixes: 9670ee752727 ("s390x/pci: use a PCI Function structure")
> >> Fixes: 1e7552ff5c34 ("s390x/pci: get zPCI function info from host")
> >
> > This fixes the problem with my old Fedora 28 under TCG, too, but... do we
> > really want to store this information in big endian on the QEMU side (e.g.
> > in the QTAILQ lists)? ... that smells like trouble again in the future...
> >
> > I think it would be better if we rather replace all those memcpy() functions
> > in the patches that you cited in the "Fixes:" lines above with code that
> > changes the endianess when we copy the date from QEMU space to guest space
> > and vice versa. What do you think?
>
> Hmm, that's actually a fair point... Such an approach would have the
> advantage of avoiding weird lines like the following:
>
> memory_region_add_subregion(&pbdev->iommu->mr,
> -pbdev->pci_group->zpci_group.msia,
> +ldq_p(&pbdev->pci_group->zpci_group.msia),
> &pbdev->msix_notify_mr);
>
>
> And would keep messing with endianness largely contained to the code
> that handles CLPs. It does take away the niceness of being able to
> gather the CLP response in one fell memcpy but... It's not like these
> are done very often (device init).
>
Not opposed to it, can try to put a patch together and see what it
looks like. As long as we get this into 5.2 :)
Re: [PATCH for-5.2] docs: Fix some typos (found by codespell)
On Tue, Nov 17, 2020 at 08:34:48PM +0100, Stefan Weil wrote: > Fix also a similar typo in a code comment. > > Signed-off-by: Stefan Weil Reviewed-by: Michael S. Tsirkin > --- > docs/can.txt | 8 > docs/interop/vhost-user.rst | 2 +- > docs/replay.txt | 2 +- > docs/specs/ppc-spapr-numa.rst | 2 +- > docs/system/deprecated.rst| 4 ++-- > docs/tools/virtiofsd.rst | 2 +- > hw/vfio/igd.c | 2 +- > 7 files changed, 11 insertions(+), 11 deletions(-) > > diff --git a/docs/can.txt b/docs/can.txt > index 5838f6620c..0d310237df 100644 > --- a/docs/can.txt > +++ b/docs/can.txt > @@ -19,7 +19,7 @@ interface to implement because such device can be easily > connected > to systems with different CPU architectures (x86, PowerPC, Arm, etc.). > > In 2020, CTU CAN FD controller model has been added as part > -of the bachelor theses of Jan Charvat. This controller is complete > +of the bachelor thesis of Jan Charvat. This controller is complete > open-source/design/hardware solution. The core designer > of the project is Ondrej Ille, the financial support has been > provided by CTU, and more companies including Volkswagen subsidiaries. > @@ -31,7 +31,7 @@ testing lead to goal change to provide environment which > provides complete > emulated environment for testing and RTEMS GSoC slot has been donated > to work on CAN hardware emulation on QEMU. > > -Examples how to use CAN emulation for SJA1000 based borads > +Examples how to use CAN emulation for SJA1000 based boards > == > > When QEMU with CAN PCI support is compiled then one of the next > @@ -106,8 +106,8 @@ This open-source core provides CAN FD support. CAN FD > drames are > delivered even to the host systems when SocketCAN interface is found > CAN FD capable. > > -The PCIe borad emulation is provided for now (the device identifier is > -ctucan_pci). The defauld build defines two CTU CAN FD cores > +The PCIe board emulation is provided for now (the device identifier is > +ctucan_pci). The default build defines two CTU CAN FD cores > on the board. > > Example how to connect the canbus0-bus (virtual wire) to the host > diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst > index 988f154144..72b2e8c7ba 100644 > --- a/docs/interop/vhost-user.rst > +++ b/docs/interop/vhost-user.rst > @@ -513,7 +513,7 @@ descriptor table (split virtqueue) or descriptor ring > (packed > virtqueue). However, it can't work when we process descriptors > out-of-order because some entries which store the information of > inflight descriptors in available ring (split virtqueue) or descriptor > -ring (packed virtqueue) might be overrided by new entries. To solve > +ring (packed virtqueue) might be overridden by new entries. To solve > this problem, slave need to allocate an extra buffer to store this > information of inflight descriptors and share it with master for > persistent. ``VHOST_USER_GET_INFLIGHT_FD`` and > diff --git a/docs/replay.txt b/docs/replay.txt > index 87a64ae068..5b008ca491 100644 > --- a/docs/replay.txt > +++ b/docs/replay.txt > @@ -328,7 +328,7 @@ between the snapshots. Each of the passes include the > following steps: > 1. loading the snapshot > 2. replaying to examine the breakpoints > 3. if breakpoint or watchpoint was met > -- loading the snaphot again > +- loading the snapshot again > - replaying to the required breakpoint > 4. else > - proceeding to the p.1 with the earlier snapshot > diff --git a/docs/specs/ppc-spapr-numa.rst b/docs/specs/ppc-spapr-numa.rst > index 5fca2bdd8e..ffa687dc89 100644 > --- a/docs/specs/ppc-spapr-numa.rst > +++ b/docs/specs/ppc-spapr-numa.rst > @@ -198,7 +198,7 @@ This is how it is being done: > * user distance 121 and beyond will be interpreted as 160 > * user distance 10 stays 10 > > -The reasoning behind this aproximation is to avoid any round up to the local > +The reasoning behind this approximation is to avoid any round up to the local > distance (10), keeping it exclusive to the 4th NUMA level (which is still > exclusive to the node_id). All other ranges were chosen under the developer > discretion of what would be (somewhat) sensible considering the user input. > diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst > index 32a0e620db..63e9db1463 100644 > --- a/docs/system/deprecated.rst > +++ b/docs/system/deprecated.rst > @@ -465,7 +465,7 @@ default configuration. > > The CPU model runnability guarantee won't apply anymore to > existing CPU models. Management software that needs runnability > -guarantees must resolve the CPU model aliases using te > +guarantees must resolve the CPU model aliases using the > ``alias-of`` field returned by the ``query-cpu-definitions`` QMP > command. > > @@ -637,7 +637,7 @@ Splitting RAM by default between NUMA nodes had the same > issues as ``mem`` > parameter with th
Re: [PATCH 00/10] vhost/qemu: thread per IO SCSI vq
On 11/18/20 12:57 AM, Mike Christie wrote: > On 11/17/20 11:17 PM, Jason Wang wrote: >> >> On 2020/11/18 上午12:40, Stefan Hajnoczi wrote: >>> On Thu, Nov 12, 2020 at 05:18:59PM -0600, Mike Christie wrote: The following kernel patches were made over Michael's vhost branch: https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/log/?h=vhost__;!!GqivPVa7Brio!MzCv3wdRfz5dltunazRWGCeUkMg91pPEOLpIivsebLX9vhYDSi_E1V36e9H0NoRys_hU$ and the vhost-scsi bug fix patchset: https://urldefense.com/v3/__https://lore.kernel.org/linux-scsi/20201112170008.GB1555653@stefanha-x1.localdomain/T/*t__;Iw!!GqivPVa7Brio!MzCv3wdRfz5dltunazRWGCeUkMg91pPEOLpIivsebLX9vhYDSi_E1V36e9H0NmuPE_m8$ And the qemu patch was made over the qemu master branch. vhost-scsi currently supports multiple queues with the num_queues setting, but we end up with a setup where the guest's scsi/block layer can do a queue per vCPU and the layers below vhost can do a queue per CPU. vhost-scsi will then do a num_queue virtqueues, but all IO gets set on and completed on a single vhost-scsi thread. After 2 - 4 vqs this becomes a bottleneck. This patchset allows us to create a worker thread per IO vq, so we can better utilize multiple CPUs with the multiple queues. It implments Jason's suggestion to create the initial worker like normal, then create the extra workers for IO vqs with the VHOST_SET_VRING_ENABLE ioctl command added in this patchset. >>> How does userspace find out the tids and set their CPU affinity? >>> >>> What is the meaning of the new VHOST_SET_VRING_ENABLE ioctl? It doesn't >>> really "enable" or "disable" the vq, requests are processed regardless. >> >> >> Actually I think it should do the real "enable/disable" that tries to follow >> the virtio spec. >> > > What does real mean here? For the vdpa enable call for example, would it be > like > ifcvf_vdpa_set_vq_ready where it sets the ready bit or more like > mlx5_vdpa_set_vq_ready > where it can do some more work in the disable case? > > For net and something like ifcvf_vdpa_set_vq_ready's design would we have > vhost_ring_ioctl() set some vhost_virtqueue enable bit. We then have some > helper > vhost_vq_is_enabled() and some code to detect if userspace supports the new > ioctl. > And then in vhost_net_set_backend do we call vhost_vq_is_enabled()? What is > done > for disable then? It doesn't seem to buy a lot of new functionality. Is it > just > so we follow the spec? > > Or do you want it work more like mlx5_vdpa_set_vq_ready? For this in > vhost_ring_ioctl > when we get the new ioctl we would call into the drivers and have it start > queues > and stop queues? For enable, what we you do for net for this case? For > disable, > would you do something like vhost_net_stop_vq (we don't free up anything > allocated > in vhost_vring_ioctl calls, but we can stop what we setup in the net driver)? > Is this useful for the current net mq design or is this for something like > where > you would do one vhost net device with multiple vqs? > > My issue/convern is that in general these calls seems useful, but we don't > really > need them for scsi because vhost scsi is already stuck creating vqs like how > it does > due to existing users. If we do the ifcvf_vdpa_set_vq_ready type of design > where > we just set some bit, then the new ioctl does not give us a lot. It's just an > extra > check and extra code. > > And for the mlx5_vdpa_set_vq_ready type of design, it doesn't seem like it's > going > to happen a lot where the admin is going to want to remove vqs from a running > device. > And for both addition/removal for scsi we would need code in virtio scsi to > handle > hot plug removal/addition of a queue and then redoing the multiqueue mappings > which > would be difficult to add with no one requesting it. Actually I want to half take this last chunk back. When I said in general these calls seem useful, I meant for the mlx5_vdpa_set_vq_ready type design. For example, if a user was going to remove/add vCPUs then this functionality where we are completely adding/removing virtqueues would be useful. We would need a lot more than just the new ioctl though, because we would want to completely create/setup a new virtqueue I do not have any of our users asking for this. You guys work on this more so you know better. Another option is to kick it down the road again since I'm not sure my patches here have a lot to do with this. We could also just do the kernel only approach (no new ioctl) and then add some new design when we have users asking for it.
Re: [PATCH for-5.2] Revert series "spapr/xive: Allocate vCPU IPIs from the vCPU contexts"
On 11/18/20 6:24 AM, David Gibson wrote:
> On Mon, Nov 16, 2020 at 04:34:22PM +0100, Greg Kurz wrote:
>> This series was largely built on the assumption that IPI numbers are
>> numerically equal to vCPU ids. Even if this happens to be the case
>> in practice with the default machine settings, this ceases to be true
>> if VSMT is set to a different value than the number of vCPUs per core.
>> This causes bogus IPI numbers to be created in KVM from a guest stand
>> point. This leads to unknow results in the guest, including crashes
>> or missing vCPUs (see BugLink) and even non-fatal oopses in current
>> KVM that lacks a check before accessing misconfigured HW (see [1]).
>>
>> A tentative patch was sent (see [2]) but it seems too complex to be
>> merged in an RC. Since the original changes are essentially an
>> optimization, it seems safer to revert them for now. The damage is
>> done by commit acbdb9956fe9 ("spapr/xive: Allocate IPIs independently
>> from the other sources") but the previous patches in the series are
>> really preparatory patches. So this reverts the whole series:
>>
>> eab0a2d06e97 ("spapr/xive: Allocate vCPU IPIs from the vCPU contexts")
>> acbdb9956fe9 ("spapr/xive: Allocate IPIs independently from the other
>> sources")
>> fa94447a2cd6 ("spapr/xive: Use kvmppc_xive_source_reset() in post_load")
>> 235d3b116213 ("spapr/xive: Modify kvm_cpu_is_enabled() interface")
>>
>> [1] https://marc.info/?l=kvm-ppc&m=160458409722959&w=4
>> [2] https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg03626.html
>>
>> Reported-by: Satheesh Rajendran
>> Fixes: acbdb9956fe9 ("spapr/xive: Allocate IPIs independently from the other
>> sources")
>> BugLink: https://bugs.launchpad.net/qemu/+bug/1900241
>> Signed-off-by: Greg Kurz
>> ---
>>
>> Peter,
>>
>> I'm Cc'ing you because we really want to fix this regression in 5.2.
>> Reverting the culprit optimization seems a lot safer than the changes
>> proposed in my other patch. David is on PTO right now and I'm not sure
>> if he can post a PR anytime soon. Just in case: would it be acceptable
>> to you if I send a PR after it got some positive feedback from the
>> people on the Cc: list ? The better the sooner or do we wait for David
>> to get a chance to step in ?
>
> I am indeed on holiday, and I'm not going to review this until next
> week. I trust Greg's judgement, though, so I'm happy for this to be
> applied directly.
Acked-by: Cédric Le Goater
C.
>
>
>> ---
>> hw/intc/spapr_xive_kvm.c | 102
>> --
>> 1 file changed, 18 insertions(+), 84 deletions(-)
>>
>> diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
>> index 66bf4c06fe55..e8667ce5f621 100644
>> --- a/hw/intc/spapr_xive_kvm.c
>> +++ b/hw/intc/spapr_xive_kvm.c
>> @@ -36,9 +36,10 @@ typedef struct KVMEnabledCPU {
>> static QLIST_HEAD(, KVMEnabledCPU)
>> kvm_enabled_cpus = QLIST_HEAD_INITIALIZER(&kvm_enabled_cpus);
>>
>> -static bool kvm_cpu_is_enabled(unsigned long vcpu_id)
>> +static bool kvm_cpu_is_enabled(CPUState *cs)
>> {
>> KVMEnabledCPU *enabled_cpu;
>> +unsigned long vcpu_id = kvm_arch_vcpu_id(cs);
>>
>> QLIST_FOREACH(enabled_cpu, &kvm_enabled_cpus, node) {
>> if (enabled_cpu->vcpu_id == vcpu_id) {
>> @@ -146,45 +147,6 @@ int kvmppc_xive_cpu_synchronize_state(XiveTCTX *tctx,
>> Error **errp)
>> return s.ret;
>> }
>>
>> -/*
>> - * Allocate the vCPU IPIs from the vCPU context. This will allocate
>> - * the XIVE IPI interrupt on the chip on which the vCPU is running.
>> - * This gives a better distribution of IPIs when the guest has a lot
>> - * of vCPUs. When the vCPUs are pinned, this will make the IPI local
>> - * to the chip of the vCPU. It will reduce rerouting between interrupt
>> - * controllers and gives better performance.
>> - */
>> -typedef struct {
>> -SpaprXive *xive;
>> -Error *err;
>> -int rc;
>> -} XiveInitIPI;
>> -
>> -static void kvmppc_xive_reset_ipi_on_cpu(CPUState *cs, run_on_cpu_data arg)
>> -{
>> -unsigned long ipi = kvm_arch_vcpu_id(cs);
>> -XiveInitIPI *s = arg.host_ptr;
>> -uint64_t state = 0;
>> -
>> -s->rc = kvm_device_access(s->xive->fd, KVM_DEV_XIVE_GRP_SOURCE, ipi,
>> - &state, true, &s->err);
>> -}
>> -
>> -static int kvmppc_xive_reset_ipi(SpaprXive *xive, CPUState *cs, Error
>> **errp)
>> -{
>> -XiveInitIPI s = {
>> -.xive = xive,
>> -.err = NULL,
>> -.rc = 0,
>> -};
>> -
>> -run_on_cpu(cs, kvmppc_xive_reset_ipi_on_cpu, RUN_ON_CPU_HOST_PTR(&s));
>> -if (s.err) {
>> -error_propagate(errp, s.err);
>> -}
>> -return s.rc;
>> -}
>> -
>> int kvmppc_xive_cpu_connect(XiveTCTX *tctx, Error **errp)
>> {
>> ERRP_GUARD();
>> @@ -195,7 +157,7 @@ int kvmppc_xive_cpu_connect(XiveTCTX *tctx, Error **errp)
>> assert(xive->fd != -1);
>>
>> /* Check if CPU was hot unplugged and replugged. */
>> -if (kvm_cpu_is_enabled(kvm_arch_vcpu_
Re: [PATCH for-5.2] meson: Fix argument for makensis (build regression)
On Tue, Nov 17, 2020 at 11:15 PM Stefan Weil wrote:
>
> `make installer` with a DLL directory was broken.
>
> Signed-off-by: Stefan Weil
Reviewed-by: Marc-André Lureau
> ---
> scripts/nsis.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/nsis.py b/scripts/nsis.py
> index e1c409344e..5135a05831 100644
> --- a/scripts/nsis.py
> +++ b/scripts/nsis.py
> @@ -65,7 +65,7 @@ def main():
> dlldir = "w64"
> makensis += ["-DW64"]
> if os.path.exists(os.path.join(args.srcdir, "dll")):
> -makensis += "-DDLLDIR={0}/dll/{1}".format(args.srcdir, dlldir)
> +makensis += ["-DDLLDIR={0}/dll/{1}".format(args.srcdir, dlldir)]
>
> makensis += ["-DOUTFILE=" + args.outfile] + args.nsisargs
> subprocess.run(makensis)
> --
> 2.29.2
>
Re: [PATCH 00/10] vhost/qemu: thread per IO SCSI vq
On 11/17/20 11:17 PM, Jason Wang wrote: > > On 2020/11/18 上午12:40, Stefan Hajnoczi wrote: >> On Thu, Nov 12, 2020 at 05:18:59PM -0600, Mike Christie wrote: >>> The following kernel patches were made over Michael's vhost branch: >>> >>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/log/?h=vhost__;!!GqivPVa7Brio!MzCv3wdRfz5dltunazRWGCeUkMg91pPEOLpIivsebLX9vhYDSi_E1V36e9H0NoRys_hU$ >>> and the vhost-scsi bug fix patchset: >>> >>> https://urldefense.com/v3/__https://lore.kernel.org/linux-scsi/20201112170008.GB1555653@stefanha-x1.localdomain/T/*t__;Iw!!GqivPVa7Brio!MzCv3wdRfz5dltunazRWGCeUkMg91pPEOLpIivsebLX9vhYDSi_E1V36e9H0NmuPE_m8$ >>> And the qemu patch was made over the qemu master branch. >>> >>> vhost-scsi currently supports multiple queues with the num_queues >>> setting, but we end up with a setup where the guest's scsi/block >>> layer can do a queue per vCPU and the layers below vhost can do >>> a queue per CPU. vhost-scsi will then do a num_queue virtqueues, >>> but all IO gets set on and completed on a single vhost-scsi thread. >>> After 2 - 4 vqs this becomes a bottleneck. >>> >>> This patchset allows us to create a worker thread per IO vq, so we >>> can better utilize multiple CPUs with the multiple queues. It >>> implments Jason's suggestion to create the initial worker like >>> normal, then create the extra workers for IO vqs with the >>> VHOST_SET_VRING_ENABLE ioctl command added in this patchset. >> How does userspace find out the tids and set their CPU affinity? >> >> What is the meaning of the new VHOST_SET_VRING_ENABLE ioctl? It doesn't >> really "enable" or "disable" the vq, requests are processed regardless. > > > Actually I think it should do the real "enable/disable" that tries to follow > the virtio spec. > What does real mean here? For the vdpa enable call for example, would it be like ifcvf_vdpa_set_vq_ready where it sets the ready bit or more like mlx5_vdpa_set_vq_ready where it can do some more work in the disable case? For net and something like ifcvf_vdpa_set_vq_ready's design would we have vhost_ring_ioctl() set some vhost_virtqueue enable bit. We then have some helper vhost_vq_is_enabled() and some code to detect if userspace supports the new ioctl. And then in vhost_net_set_backend do we call vhost_vq_is_enabled()? What is done for disable then? It doesn't seem to buy a lot of new functionality. Is it just so we follow the spec? Or do you want it work more like mlx5_vdpa_set_vq_ready? For this in vhost_ring_ioctl when we get the new ioctl we would call into the drivers and have it start queues and stop queues? For enable, what we you do for net for this case? For disable, would you do something like vhost_net_stop_vq (we don't free up anything allocated in vhost_vring_ioctl calls, but we can stop what we setup in the net driver)? Is this useful for the current net mq design or is this for something like where you would do one vhost net device with multiple vqs? My issue/convern is that in general these calls seems useful, but we don't really need them for scsi because vhost scsi is already stuck creating vqs like how it does due to existing users. If we do the ifcvf_vdpa_set_vq_ready type of design where we just set some bit, then the new ioctl does not give us a lot. It's just an extra check and extra code. And for the mlx5_vdpa_set_vq_ready type of design, it doesn't seem like it's going to happen a lot where the admin is going to want to remove vqs from a running device. And for both addition/removal for scsi we would need code in virtio scsi to handle hot plug removal/addition of a queue and then redoing the multiqueue mappings which would be difficult to add with no one requesting it.
[PATCH] qapi: Normalize version references x.y.0 to just x.y
We use x.y most of the time, and x.y.0 sometimes. Normalize for
consistency.
Reported-by: Eduardo Habkost
Signed-off-by: Markus Armbruster
---
qapi/block-core.json | 28
qapi/block-export.json | 6 +++---
qapi/block.json | 2 +-
qapi/char.json | 4 ++--
qapi/control.json| 14 ++--
qapi/machine-target.json | 22 +--
qapi/machine.json| 46
qapi/migration.json | 16 +++---
qapi/misc-target.json| 2 +-
qapi/misc.json | 30 +-
qapi/net.json| 6 +++---
qapi/pci.json| 12 +--
qapi/qdev.json | 2 +-
qapi/run-state.json | 16 +++---
qapi/ui.json | 40 +-
15 files changed, 123 insertions(+), 123 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 04ad80bc1e..04c5196e59 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -302,7 +302,7 @@
# @ro: true if the backing device was open read-only
#
# @drv: the name of the block format used to open the backing device. As of
-# 0.14.0 this can be: 'blkdebug', 'bochs', 'cloop', 'cow', 'dmg',
+# 0.14 this can be: 'blkdebug', 'bochs', 'cloop', 'cow', 'dmg',
# 'file', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device',
# 'http', 'https', 'luks', 'nbd', 'parallels', 'qcow',
# 'qcow2', 'raw', 'vdi', 'vmdk', 'vpc', 'vvfat'
@@ -389,7 +389,7 @@
# @deprecated: Member @encryption_key_missing is deprecated. It is
# always false.
#
-# Since: 0.14.0
+# Since: 0.14
#
##
{ 'struct': 'BlockDeviceInfo',
@@ -607,7 +607,7 @@
# @deprecated: Member @dirty-bitmaps is deprecated. Use @inserted
# member @dirty-bitmaps instead.
#
-# Since: 0.14.0
+# Since: 0.14
##
{ 'struct': 'BlockInfo',
'data': {'device': 'str', '*qdev': 'str', 'type': 'str', 'removable': 'bool',
@@ -655,7 +655,7 @@
# Returns: a list of @BlockInfo describing each virtual block device. Filter
# nodes that were created implicitly are skipped over.
#
-# Since: 0.14.0
+# Since: 0.14
#
# Example:
#
@@ -812,17 +812,17 @@
# @wr_operations: The number of write operations performed by the device.
#
# @flush_operations: The number of cache flush operations performed by the
-#device (since 0.15.0)
+#device (since 0.15)
#
# @unmap_operations: The number of unmap operations performed by the device
#(Since 4.2)
#
-# @rd_total_time_ns: Total time spent on reads in nanoseconds (since 0.15.0).
+# @rd_total_time_ns: Total time spent on reads in nanoseconds (since 0.15).
#
-# @wr_total_time_ns: Total time spent on writes in nanoseconds (since 0.15.0).
+# @wr_total_time_ns: Total time spent on writes in nanoseconds (since 0.15).
#
# @flush_total_time_ns: Total time spent on cache flushes in nanoseconds
-# (since 0.15.0).
+# (since 0.15).
#
# @unmap_total_time_ns: Total time spent on unmap operations in nanoseconds
# (Since 4.2)
@@ -884,7 +884,7 @@
#
# @flush_latency_histogram: @BlockLatencyHistogramInfo. (Since 4.0)
#
-# Since: 0.14.0
+# Since: 0.14
##
{ 'struct': 'BlockDeviceStats',
'data': {'rd_bytes': 'int', 'wr_bytes': 'int', 'unmap_bytes' : 'int',
@@ -987,7 +987,7 @@
# @backing: This describes the backing block device if it has one.
# (Since 2.0)
#
-# Since: 0.14.0
+# Since: 0.14
##
{ 'struct': 'BlockStats',
'data': {'*device': 'str', '*qdev': 'str', '*node-name': 'str',
@@ -1011,7 +1011,7 @@
#
# Returns: A list of @BlockStats for each virtual block devices.
#
-# Since: 0.14.0
+# Since: 0.14
#
# Example:
#
@@ -1299,7 +1299,7 @@
# Returns: - nothing on success
# - If @device is not a valid block device, DeviceNotFound
#
-# Since: 0.14.0
+# Since: 0.14
#
# Example:
#
@@ -1484,7 +1484,7 @@
# Returns: - nothing on success
# - If @device is not a valid block device, DeviceNotFound
#
-# Since: 0.14.0
+# Since: 0.14
#
# Example:
#
@@ -4852,7 +4852,7 @@
# Note: If action is "stop", a STOP event will eventually follow the
# BLOCK_IO_ERROR event
#
-# Since: 0.13.0
+# Since: 0.13
#
# Example:
#
diff --git a/qapi/block-export.json b/qapi/block-export.json
index a9f488f99c..4eeac7842d 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -54,7 +54,7 @@
#
# Returns: error if the server is already running.
#
-# Since: 1.3.0
+# Since: 1.3
##
{ 'command': 'nbd-server-start',
'data': { 'addr': 'SocketAddressLegacy',
@@ -155,7 +155,7 @@
# Returns: error if the server is not running, or export with the same name
# already exists.
#
-# Since: 1.3.0
+# Since: 1.3
##
{ 'command': 'nbd-server-add',
'data': 'NbdServerAddOptions', 'boxed': true, 'features': ['deprecated'] }
@@ -211,7 +211,7 @@
# Stop QEMU's e
Re: [PATCH] virtio-net: purge queued rx packets on queue deletion
Hi Jason,
Sorry, there is a mistake in the title: should be 'net' instead of
'virtio-net'.
Can you please fix it?
Thanks,
Yuri Benditovich
On Wed, Nov 18, 2020 at 5:59 AM Jason Wang wrote:
>
> On 2020/11/12 下午5:46, Yuri Benditovich wrote:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1829272
> > When deleting queue pair, purge pending RX packets if any.
> > Example of problematic flow:
> > 1. Bring up q35 VM with tap (vhost off) and virtio-net or e1000e
> > 2. Run ping flood to the VM NIC ( 1 ms interval)
> > 3. Hot unplug the NIC device (device_del)
> > During unplug process one or more packets come, the NIC
> > can't receive, tap disables read_poll
> > 4. Hot plug the device (device_add) with the same netdev
> > The tap stays with read_poll disabled and does not receive
> > any packets anymore (tap_send never triggered)
> >
> > Signed-off-by: Yuri Benditovich
>
>
> Applied.
>
> Thanks
>
>
> > ---
> > net/net.c | 12
> > 1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/net.c b/net/net.c
> > index 7a2a0fb5ac..a95b417300 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -411,10 +411,14 @@ void qemu_del_nic(NICState *nic)
> >
> > qemu_macaddr_set_free(&nic->conf->macaddr);
> >
> > -/* If this is a peer NIC and peer has already been deleted, free it
> now. */
> > -if (nic->peer_deleted) {
> > -for (i = 0; i < queues; i++) {
> > -qemu_free_net_client(qemu_get_subqueue(nic, i)->peer);
> > +for (i = 0; i < queues; i++) {
> > +NetClientState *nc = qemu_get_subqueue(nic, i);
> > +/* If this is a peer NIC and peer has already been deleted,
> free it now. */
> > +if (nic->peer_deleted) {
> > +qemu_free_net_client(nc->peer);
> > +} else if (nc->peer) {
> > +/* if there are RX packets pending, complete them */
> > +qemu_purge_queued_packets(nc->peer);
> > }
> > }
> >
>
>
Re: [PATCH v2 5/7] qapi: Introduce QAPI_LIST_APPEND
Eric Blake writes:
> On 11/17/20 6:51 AM, Markus Armbruster wrote:
>> Eric Blake writes:
>>
>>> Similar to the existing QAPI_LIST_PREPEND, but designed for use where
>>> we want to preserve insertion order. Callers will be added in
>>> upcoming patches. Note the difference in signature: PREPEND takes
>>> List*, APPEND takes List**.
>>>
>>> Signed-off-by: Eric Blake
>
>>> +#define QAPI_LIST_APPEND(tail, element) do { \
>>> +*(tail) = g_malloc0(sizeof(**(tail))); \
>>> +(*(tail))->value = (element); \
>>> +(tail) = &(*tail)->next; \
>
> Hmm; I'm inconsistent on whether to spell things '*tail' or '*(tail)'.
> I don't think any of the callers converted in patches 6 or 7 care about
> the difference, but for maximal copy-paste portability, the use of the
> macro parameter should be surrounded by () anywhere that could otherwise
> cause a mis-parse on some arbitrary expression with an operator at
> higher precedence than unary * (hmm, the only such operators are all
> suffix operators; so maybe the *(tail) is overkill...)
Good habit: enclose macro parameter in parenthesis unless there is a
reason not to. Let's do it here, too.
>> Reviewed-by: Markus Armbruster
Re: [PATCH v3 2/5] memory: Add IOMMUTLBEvent
On Mon, Nov 16, 2020 at 05:55:03PM +0100, Eugenio Pérez wrote:
> This way we can tell between regular IOMMUTLBEntry (entry of IOMMU
> hardware) and notifications.
>
> In the notifications, we set explicitly if it is a MAPs or an UNMAP,
> instead of trusting in entry permissions to differentiate them.
>
> Signed-off-by: Eugenio Pérez
> Reviewed-by: Peter Xu
> Reviewed-by: Juan Quintela
> Acked-by: Jason Wang
ppc parts
Acked-by: David Gibson
> ---
> include/exec/memory.h| 27 ++--
> hw/arm/smmu-common.c | 13 +++---
> hw/arm/smmuv3.c | 13 +++---
> hw/i386/intel_iommu.c| 88 ++--
> hw/misc/tz-mpc.c | 32 ---
> hw/ppc/spapr_iommu.c | 15 +++
> hw/s390x/s390-pci-inst.c | 27 +++-
> hw/virtio/virtio-iommu.c | 30 +++---
> softmmu/memory.c | 20 -
> 9 files changed, 143 insertions(+), 122 deletions(-)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index d8456ccf52..e86b5e92da 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -116,6 +116,11 @@ struct IOMMUNotifier {
> };
> typedef struct IOMMUNotifier IOMMUNotifier;
>
> +typedef struct IOMMUTLBEvent {
> +IOMMUNotifierFlag type;
> +IOMMUTLBEntry entry;
> +} IOMMUTLBEvent;
> +
> /* RAM is pre-allocated and passed into qemu_ram_alloc_from_ptr */
> #define RAM_PREALLOC (1 << 0)
>
> @@ -1326,24 +1331,18 @@ uint64_t
> memory_region_iommu_get_min_page_size(IOMMUMemoryRegion *iommu_mr);
> /**
> * memory_region_notify_iommu: notify a change in an IOMMU translation entry.
> *
> - * The notification type will be decided by entry.perm bits:
> - *
> - * - For UNMAP (cache invalidation) notifies: set entry.perm to IOMMU_NONE.
> - * - For MAP (newly added entry) notifies: set entry.perm to the
> - * permission of the page (which is definitely !IOMMU_NONE).
> - *
> * Note: for any IOMMU implementation, an in-place mapping change
> * should be notified with an UNMAP followed by a MAP.
> *
> * @iommu_mr: the memory region that was changed
> * @iommu_idx: the IOMMU index for the translation table which has changed
> - * @entry: the new entry in the IOMMU translation table. The entry
> - * replaces all old entries for the same virtual I/O address range.
> - * Deleted entries have .@perm == 0.
> + * @event: TLB event with the new entry in the IOMMU translation table.
> + * The entry replaces all old entries for the same virtual I/O
> address
> + * range.
> */
> void memory_region_notify_iommu(IOMMUMemoryRegion *iommu_mr,
> int iommu_idx,
> -IOMMUTLBEntry entry);
> +IOMMUTLBEvent event);
>
> /**
> * memory_region_notify_iommu_one: notify a change in an IOMMU translation
> @@ -1353,12 +1352,12 @@ void memory_region_notify_iommu(IOMMUMemoryRegion
> *iommu_mr,
> * notifies a specific notifier, not all of them.
> *
> * @notifier: the notifier to be notified
> - * @entry: the new entry in the IOMMU translation table. The entry
> - * replaces all old entries for the same virtual I/O address range.
> - * Deleted entries have .@perm == 0.
> + * @event: TLB event with the new entry in the IOMMU translation table.
> + * The entry replaces all old entries for the same virtual I/O
> address
> + * range.
> */
> void memory_region_notify_iommu_one(IOMMUNotifier *notifier,
> - IOMMUTLBEntry *entry);
> +IOMMUTLBEvent *event);
>
> /**
> * memory_region_register_iommu_notifier: register a notifier for changes to
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index 88d2c454f0..405d5c5325 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -465,14 +465,15 @@ IOMMUMemoryRegion *smmu_iommu_mr(SMMUState *s, uint32_t
> sid)
> /* Unmap the whole notifier's range */
> static void smmu_unmap_notifier_range(IOMMUNotifier *n)
> {
> -IOMMUTLBEntry entry;
> +IOMMUTLBEvent event;
>
> -entry.target_as = &address_space_memory;
> -entry.iova = n->start;
> -entry.perm = IOMMU_NONE;
> -entry.addr_mask = n->end - n->start;
> +event.type = IOMMU_NOTIFIER_UNMAP;
> +event.entry.target_as = &address_space_memory;
> +event.entry.iova = n->start;
> +event.entry.perm = IOMMU_NONE;
> +event.entry.addr_mask = n->end - n->start;
>
> -memory_region_notify_iommu_one(n, &entry);
> +memory_region_notify_iommu_one(n, &event);
> }
>
> /* Unmap all notifiers attached to @mr */
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 273f5f7dce..bbca0e9f20 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -800,7 +800,7 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
> uint8_t tg, uint64_t num_pages)
> {
> SMMUDevice *sdev = container
Re: [PATCH for-5.2] Revert series "spapr/xive: Allocate vCPU IPIs from the vCPU contexts"
On Mon, Nov 16, 2020 at 04:34:22PM +0100, Greg Kurz wrote:
> This series was largely built on the assumption that IPI numbers are
> numerically equal to vCPU ids. Even if this happens to be the case
> in practice with the default machine settings, this ceases to be true
> if VSMT is set to a different value than the number of vCPUs per core.
> This causes bogus IPI numbers to be created in KVM from a guest stand
> point. This leads to unknow results in the guest, including crashes
> or missing vCPUs (see BugLink) and even non-fatal oopses in current
> KVM that lacks a check before accessing misconfigured HW (see [1]).
>
> A tentative patch was sent (see [2]) but it seems too complex to be
> merged in an RC. Since the original changes are essentially an
> optimization, it seems safer to revert them for now. The damage is
> done by commit acbdb9956fe9 ("spapr/xive: Allocate IPIs independently
> from the other sources") but the previous patches in the series are
> really preparatory patches. So this reverts the whole series:
>
> eab0a2d06e97 ("spapr/xive: Allocate vCPU IPIs from the vCPU contexts")
> acbdb9956fe9 ("spapr/xive: Allocate IPIs independently from the other
> sources")
> fa94447a2cd6 ("spapr/xive: Use kvmppc_xive_source_reset() in post_load")
> 235d3b116213 ("spapr/xive: Modify kvm_cpu_is_enabled() interface")
>
> [1] https://marc.info/?l=kvm-ppc&m=160458409722959&w=4
> [2] https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg03626.html
>
> Reported-by: Satheesh Rajendran
> Fixes: acbdb9956fe9 ("spapr/xive: Allocate IPIs independently from the other
> sources")
> BugLink: https://bugs.launchpad.net/qemu/+bug/1900241
> Signed-off-by: Greg Kurz
> ---
>
> Peter,
>
> I'm Cc'ing you because we really want to fix this regression in 5.2.
> Reverting the culprit optimization seems a lot safer than the changes
> proposed in my other patch. David is on PTO right now and I'm not sure
> if he can post a PR anytime soon. Just in case: would it be acceptable
> to you if I send a PR after it got some positive feedback from the
> people on the Cc: list ? The better the sooner or do we wait for David
> to get a chance to step in ?
I am indeed on holiday, and I'm not going to review this until next
week. I trust Greg's judgement, though, so I'm happy for this to be
applied directly.
> ---
> hw/intc/spapr_xive_kvm.c | 102
> --
> 1 file changed, 18 insertions(+), 84 deletions(-)
>
> diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
> index 66bf4c06fe55..e8667ce5f621 100644
> --- a/hw/intc/spapr_xive_kvm.c
> +++ b/hw/intc/spapr_xive_kvm.c
> @@ -36,9 +36,10 @@ typedef struct KVMEnabledCPU {
> static QLIST_HEAD(, KVMEnabledCPU)
> kvm_enabled_cpus = QLIST_HEAD_INITIALIZER(&kvm_enabled_cpus);
>
> -static bool kvm_cpu_is_enabled(unsigned long vcpu_id)
> +static bool kvm_cpu_is_enabled(CPUState *cs)
> {
> KVMEnabledCPU *enabled_cpu;
> +unsigned long vcpu_id = kvm_arch_vcpu_id(cs);
>
> QLIST_FOREACH(enabled_cpu, &kvm_enabled_cpus, node) {
> if (enabled_cpu->vcpu_id == vcpu_id) {
> @@ -146,45 +147,6 @@ int kvmppc_xive_cpu_synchronize_state(XiveTCTX *tctx,
> Error **errp)
> return s.ret;
> }
>
> -/*
> - * Allocate the vCPU IPIs from the vCPU context. This will allocate
> - * the XIVE IPI interrupt on the chip on which the vCPU is running.
> - * This gives a better distribution of IPIs when the guest has a lot
> - * of vCPUs. When the vCPUs are pinned, this will make the IPI local
> - * to the chip of the vCPU. It will reduce rerouting between interrupt
> - * controllers and gives better performance.
> - */
> -typedef struct {
> -SpaprXive *xive;
> -Error *err;
> -int rc;
> -} XiveInitIPI;
> -
> -static void kvmppc_xive_reset_ipi_on_cpu(CPUState *cs, run_on_cpu_data arg)
> -{
> -unsigned long ipi = kvm_arch_vcpu_id(cs);
> -XiveInitIPI *s = arg.host_ptr;
> -uint64_t state = 0;
> -
> -s->rc = kvm_device_access(s->xive->fd, KVM_DEV_XIVE_GRP_SOURCE, ipi,
> - &state, true, &s->err);
> -}
> -
> -static int kvmppc_xive_reset_ipi(SpaprXive *xive, CPUState *cs, Error **errp)
> -{
> -XiveInitIPI s = {
> -.xive = xive,
> -.err = NULL,
> -.rc = 0,
> -};
> -
> -run_on_cpu(cs, kvmppc_xive_reset_ipi_on_cpu, RUN_ON_CPU_HOST_PTR(&s));
> -if (s.err) {
> -error_propagate(errp, s.err);
> -}
> -return s.rc;
> -}
> -
> int kvmppc_xive_cpu_connect(XiveTCTX *tctx, Error **errp)
> {
> ERRP_GUARD();
> @@ -195,7 +157,7 @@ int kvmppc_xive_cpu_connect(XiveTCTX *tctx, Error **errp)
> assert(xive->fd != -1);
>
> /* Check if CPU was hot unplugged and replugged. */
> -if (kvm_cpu_is_enabled(kvm_arch_vcpu_id(tctx->cs))) {
> +if (kvm_cpu_is_enabled(tctx->cs)) {
> return 0;
> }
>
> @@ -214,12 +176,6 @@ int kvmppc_xive_cpu_connect(XiveTCTX *tctx, Error **errp)
> return ret;
>
Re: [PATCH 00/10] vhost/qemu: thread per IO SCSI vq
On 2020/11/18 上午12:40, Stefan Hajnoczi wrote:
On Thu, Nov 12, 2020 at 05:18:59PM -0600, Mike Christie wrote:
The following kernel patches were made over Michael's vhost branch:
https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/log/?h=vhost
and the vhost-scsi bug fix patchset:
https://lore.kernel.org/linux-scsi/20201112170008.GB1555653@stefanha-x1.localdomain/T/#t
And the qemu patch was made over the qemu master branch.
vhost-scsi currently supports multiple queues with the num_queues
setting, but we end up with a setup where the guest's scsi/block
layer can do a queue per vCPU and the layers below vhost can do
a queue per CPU. vhost-scsi will then do a num_queue virtqueues,
but all IO gets set on and completed on a single vhost-scsi thread.
After 2 - 4 vqs this becomes a bottleneck.
This patchset allows us to create a worker thread per IO vq, so we
can better utilize multiple CPUs with the multiple queues. It
implments Jason's suggestion to create the initial worker like
normal, then create the extra workers for IO vqs with the
VHOST_SET_VRING_ENABLE ioctl command added in this patchset.
How does userspace find out the tids and set their CPU affinity?
What is the meaning of the new VHOST_SET_VRING_ENABLE ioctl? It doesn't
really "enable" or "disable" the vq, requests are processed regardless.
Actually I think it should do the real "enable/disable" that tries to
follow the virtio spec.
(E.g both PCI and MMIO have something similar).
The purpose of the ioctl isn't clear to me because the kernel could
automatically create 1 thread per vq without a new ioctl.
It's not necessarily to create or destroy kthread according to
VRING_ENABLE but could be a hint.
On the other
hand, if userspace is supposed to control worker threads then a
different interface would be more powerful:
struct vhost_vq_worker_info {
/*
* The pid of an existing vhost worker that this vq will be
* assigned to. When pid is 0 the virtqueue is assigned to the
* default vhost worker. When pid is -1 a new worker thread is
* created for this virtqueue. When pid is -2 the virtqueue's
* worker thread is unchanged.
*
* If a vhost worker no longer has any virtqueues assigned to it
* then it will terminate.
*
* The pid of the vhost worker is stored to this field when the
* ioctl completes successfully. Use pid -2 to query the current
* vhost worker pid.
*/
__kernel_pid_t pid; /* in/out */
/* The virtqueue index*/
unsigned int vq_idx; /* in */
};
ioctl(vhost_fd, VHOST_SET_VQ_WORKER, &info);
This seems to leave the question to userspace which I'm not sure it's
good since it tries to introduce another scheduling layer.
Per vq worker seems be good enough to start with.
Thanks
Stefan
[Bug 1904652] [NEW] Assertion failure in usb-ohci
Public bug reported: Hello, Using hypervisor fuzzer, hyfuzz, I found an assertion failure through usb-ohci. A malicious guest user/process could use this flaw to abort the QEMU process on the host, resulting in a denial of service. This was found in version 5.2.0 (master) ``` Program terminated with signal SIGABRT, Aborted. #0 __GI_raise (sig=sig@entry=0x6) at ../sysdeps/unix/sysv/linux/raise.c:51 51 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory. [Current thread is 1 (Thread 0x7f34d0411440 (LWP 9418))] gdb-peda$ bt #0 0x7f34c8d4ef47 in __GI_raise (sig=sig@entry=0x6) at ../sysdeps/unix/sysv/linux/raise.c:51 #1 0x7f34c8d508b1 in __GI_abort () at abort.c:79 #2 0x55d3a2081844 in ohci_frame_boundary (opaque=0x55d3a4ecdaf0) at ../hw/usb/hcd-ohci.c:1297 #3 0x55d3a25be155 in timerlist_run_timers (timer_list=0x55d3a3fd9840) at ../util/qemu-timer.c:574 #4 0x55d3a25beaba in qemu_clock_run_timers (type=QEMU_CLOCK_VIRTUAL) at ../util/qemu-timer.c:588 #5 0x55d3a25beaba in qemu_clock_run_all_timers () at ../util/qemu-timer.c:670 #6 0x55d3a25e69a1 in main_loop_wait (nonblocking=) at ../util/main-loop.c:531 #7 0x55d3a2433972 in qemu_main_loop () at ../softmmu/vl.c:1678 #8 0x55d3a1d0969b in main (argc=, argc@entry=0x15, argv=, argv@entry=0x7ffc6de722a8, envp=) at ../softmmu/main.c:50 #9 0x7f34c8d31b97 in __libc_start_main (main= 0x55d3a1d09690 , argc=0x15, argv=0x7ffc6de722a8, init=, fini=, rtld_fini=, stack_end=0x7ffc6de72298) at ../csu/libc-start.c:310 #10 0x55d3a1d095aa in _start () ``` To reproduce the assertion failure, please run the QEMU with the following command line. ``` [Terminal 1] $ qemu-system-i386 -m 512 -drive file=./fs.img,index=1,media=disk,format=raw -drive file=./hyfuzz.img,index=0,media=disk,format=raw -drive if=none,id=stick,file=./usbdisk.img,format=raw -device pci-ohci,id=usb -device usb-storage,bus=usb.0,drive=stick [Terminal 2] $ ./repro_log ./fs.img ./pci-ohci ``` Please let me know if I can provide any further info. -Cheolwoo, Myung (Seoul National University) ** Affects: qemu Importance: Undecided Status: New ** Attachment added: "attachment.zip" https://bugs.launchpad.net/bugs/1904652/+attachment/5435350/+files/attachment.zip -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1904652 Title: Assertion failure in usb-ohci Status in QEMU: New Bug description: Hello, Using hypervisor fuzzer, hyfuzz, I found an assertion failure through usb-ohci. A malicious guest user/process could use this flaw to abort the QEMU process on the host, resulting in a denial of service. This was found in version 5.2.0 (master) ``` Program terminated with signal SIGABRT, Aborted. #0 __GI_raise (sig=sig@entry=0x6) at ../sysdeps/unix/sysv/linux/raise.c:51 51 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory. [Current thread is 1 (Thread 0x7f34d0411440 (LWP 9418))] gdb-peda$ bt #0 0x7f34c8d4ef47 in __GI_raise (sig=sig@entry=0x6) at ../sysdeps/unix/sysv/linux/raise.c:51 #1 0x7f34c8d508b1 in __GI_abort () at abort.c:79 #2 0x55d3a2081844 in ohci_frame_boundary (opaque=0x55d3a4ecdaf0) at ../hw/usb/hcd-ohci.c:1297 #3 0x55d3a25be155 in timerlist_run_timers (timer_list=0x55d3a3fd9840) at ../util/qemu-timer.c:574 #4 0x55d3a25beaba in qemu_clock_run_timers (type=QEMU_CLOCK_VIRTUAL) at ../util/qemu-timer.c:588 #5 0x55d3a25beaba in qemu_clock_run_all_timers () at ../util/qemu-timer.c:670 #6 0x55d3a25e69a1 in main_loop_wait (nonblocking=) at ../util/main-loop.c:531 #7 0x55d3a2433972 in qemu_main_loop () at ../softmmu/vl.c:1678 #8 0x55d3a1d0969b in main (argc=, argc@entry=0x15, argv=, argv@entry=0x7ffc6de722a8, envp=) at ../softmmu/main.c:50 #9 0x7f34c8d31b97 in __libc_start_main (main= 0x55d3a1d09690 , argc=0x15, argv=0x7ffc6de722a8, init=, fini=, rtld_fini=, stack_end=0x7ffc6de72298) at ../csu/libc-start.c:310 #10 0x55d3a1d095aa in _start () ``` To reproduce the assertion failure, please run the QEMU with the following command line. ``` [Terminal 1] $ qemu-system-i386 -m 512 -drive file=./fs.img,index=1,media=disk,format=raw -drive file=./hyfuzz.img,index=0,media=disk,format=raw -drive if=none,id=stick,file=./usbdisk.img,format=raw -device pci-ohci,id=usb -device usb-storage,bus=usb.0,drive=stick [Terminal 2] $ ./repro_log ./fs.img ./pci-ohci ``` Please let me know if I can provide any further info. -Cheolwoo, Myung (Seoul National University) To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1904652/+subscriptions
Re: [PATCH] virtio-net: purge queued rx packets on queue deletion
On 2020/11/12 下午5:46, Yuri Benditovich wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1829272
When deleting queue pair, purge pending RX packets if any.
Example of problematic flow:
1. Bring up q35 VM with tap (vhost off) and virtio-net or e1000e
2. Run ping flood to the VM NIC ( 1 ms interval)
3. Hot unplug the NIC device (device_del)
During unplug process one or more packets come, the NIC
can't receive, tap disables read_poll
4. Hot plug the device (device_add) with the same netdev
The tap stays with read_poll disabled and does not receive
any packets anymore (tap_send never triggered)
Signed-off-by: Yuri Benditovich
Applied.
Thanks
---
net/net.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/net/net.c b/net/net.c
index 7a2a0fb5ac..a95b417300 100644
--- a/net/net.c
+++ b/net/net.c
@@ -411,10 +411,14 @@ void qemu_del_nic(NICState *nic)
qemu_macaddr_set_free(&nic->conf->macaddr);
-/* If this is a peer NIC and peer has already been deleted, free it now. */
-if (nic->peer_deleted) {
-for (i = 0; i < queues; i++) {
-qemu_free_net_client(qemu_get_subqueue(nic, i)->peer);
+for (i = 0; i < queues; i++) {
+NetClientState *nc = qemu_get_subqueue(nic, i);
+/* If this is a peer NIC and peer has already been deleted, free it
now. */
+if (nic->peer_deleted) {
+qemu_free_net_client(nc->peer);
+} else if (nc->peer) {
+/* if there are RX packets pending, complete them */
+qemu_purge_queued_packets(nc->peer);
}
}
Re: [PATCH v2] net/e1000e_core: adjust count if RDH exceeds RDT in e1000e_ring_advance()
On 2020/11/13 下午6:31, Mauro Matteo Cascella wrote: The e1000e_write_packet_to_guest() function iterates over a set of receive descriptors by advancing rx descriptor head register (RDH) from its initial value to rx descriptor tail register (RDT). The check in e1000e_ring_empty() is responsible for detecting whether RDH has reached RDT, terminating the loop if that's the case. Additional checks have been added in the past to deal with bogus values submitted by the guest to prevent possible infinite loop. This is done by "wrapping around" RDH at some point and detecting whether it assumes the original value during the loop. However, when e1000e is configured to use the packet split feature, RDH is incremented by two instead of one, as the packet split descriptors are 32 bytes while regular descriptors are 16 bytes. A malicious or buggy guest may set RDT to an odd value and transmit only null RX descriptors. This corner case would prevent RDH from ever matching RDT, leading to an infinite loop. This patch adds a check in e1000e_ring_advance() to make sure RDH does not exceed RDT in a single incremental step, adjusting the count value accordingly. Can this patch solve this issue in another way? https://patchew.org/QEMU/2020130636.2208620-1-ppan...@redhat.com/ Thanks
[ANNOUNCE] QEMU 5.2.0-rc2 is now available
Hello, On behalf of the QEMU Team, I'd like to announce the availability of the third release candidate for the QEMU 5.2 release. This release is meant for testing purposes and should not be used in a production environment. http://download.qemu-project.org/qemu-5.2.0-rc2.tar.xz http://download.qemu-project.org/qemu-5.2.0-rc2.tar.xz.sig A note from the maintainer: Note that QEMU has switched build systems so you will need to install ninja to compile it. See the "Build Information" section of the Changelog for more information about this change. You can help improve the quality of the QEMU 5.2 release by testing this release and reporting bugs on Launchpad: https://bugs.launchpad.net/qemu/ The release plan, as well a documented known issues for release candidates, are available at: http://wiki.qemu.org/Planning/5.2 Please add entries to the ChangeLog for the 5.2 release below: http://wiki.qemu.org/ChangeLog/5.2 Thank you to everyone involved! Changes since rc1: 66a300a107: Update version for v5.2.0-rc2 release (Peter Maydell) 922d42bb0d: json: Fix a memleak in parse_pair() (Alex Chen) 5351f4075d: linux-user,netlink: add IFLA_BRPORT_MRP_RING_OPEN, IFLA_BRPORT_MRP_IN_OPEN (Laurent Vivier) f536612dc1: linux-user,netlink: fix message translation with ip command (Laurent Vivier) ab135622cf: tmp105: Correct handling of temperature limit checks (Peter Maydell) e1919889ef: hw/misc/tmp105: reset the T_low and T_High registers (Peter Maydell) 13ceae6663: configure: Make "does libgio work" test pull in some actual functions (Peter Maydell) 6d7ccc576d: util/cutils: Fix Coverity array overrun in freq_to_str() (Philippe Mathieu-Daudé) ea2d7fcf35: register: Remove unnecessary NULL check (Alistair Francis) 7b0263cb14: target/openrisc: Remove dead code attempting to check "is timer disabled" (Peter Maydell) 019294db68: hw/input/ps2.c: Remove remnants of printf debug (Peter Maydell) 63192565f9: exynos: Fix bad printf format specifiers (Alex Chen) 3362c56835: hw/arm/virt: ARM_VIRT must select ARM_GIC (Andrew Jones) c61c644f59: iotests/081: Test rewrite-corrupted without WRITE (Max Reitz) 55f2c014d7: iotests/081: Filter image format after testdir (Max Reitz) 9ca5b0e842: quorum: Require WRITE perm with rewrite-corrupted (Max Reitz) bd89f93603: io_uring: do not use pointer after free (Paolo Bonzini) ece4fa9152: file-posix: allow -EBUSY errors during write zeros on raw block devices (Maxim Levitsky) 5aaabf9161: iotests: Replace deprecated ConfigParser.readfp() (Kevin Wolf) 6deb20f668: char-stdio: Fix QMP default for 'signal' (Kevin Wolf) 575094b786: hw/sd: Fix 2 GiB card CSD register values (Bin Meng) 46b42f715d: max111x: put it into the 'misc' category (Gan Qixin) 84aab60c12: nand: put it into the 'storage' category (Gan Qixin) be3701eae3: ads7846: put it into the 'input' category (Gan Qixin) 1352711561: ssd0323: put it into the 'display' category (Gan Qixin) 91010f0407: vhost-user-blk/scsi: Fix broken error handling for socket call (AlexChen) 5fd6921ccc: contrib/libvhost-user: Fix bad printf format specifiers (AlexChen) ca905bec44: gitlab-ci: Use $CI_REGISTRY instead of hard-coding registry.gitlab.com (Rebecca Cran) f25c7ca0ce: target/microblaze: Fix possible array out of bounds in mmu_write() (AlexChen) 844d35b9c2: tests/vm: update NetBSD to 9.1 (Brad Smith) 9fc33bf4e1: tests/vm: Add Haiku test based on their vagrant images (Alexander von Gluck IV) ded5d78c1e: configure: Add a proper check for sys/ioccom.h and use it in tpm_ioctl.h (Thomas Huth) 7000a12e08: configure: Do not build pc-bios/optionrom on Haiku (Thomas Huth) cde9925362: configure: Fix the _BSD_SOURCE define for the Haiku build (Thomas Huth) 949eaaad53: qemu/bswap: Remove unused qemu_bswap_len() (Philippe Mathieu-Daudé) 2f3c1fd396: iotests: Replace deprecated ConfigParser.readfp() (Kevin Wolf) c0b21f2e22: nbd: Silence Coverity false positive (Eric Blake) 1370d61ae3: memory: Skip dirty tracking for un-migratable memory regions (Zenghui Yu) 42ccce1981: target/i386: avoid theoretical leak on MCE injection (Paolo Bonzini) 3b12a7fd39: scsi-disk: convert more errno values back to SCSI statuses (Paolo Bonzini) b430b51395: util/vfio-helpers.c: Use ram_block_discard_disable() in qemu_vfio_open_pci() (David Hildenbrand) 2654ace151: kvm/i386: Set proper nested state format for SVM (Tom Lendacky) a8aa94b5f8: qga: update schema for guest-get-disks 'dependents' field (Michael Roth) 7025111a19: .gitlab-ci.d/check-patch: tweak output for CI logs (Alex Bennée) b48580ad92: tests/acceptance: Disable Spartan-3A DSP 1800A test (Philippe Mathieu-Daudé) 811c74fb65: hw/i386/acpi-build: Fix maybe-uninitialized error when ACPI hotplug off (Philippe Mathieu-Daudé) 4bdccdec70: accel/stubs: drop unused cpu.h include (Alex Bennée) d67ef04cb8: stubs/xen-hw-stub: drop xenstore_store_pv_console_info stub (Alex Bennée) 97d351b476: include/hw/xen.h: drop superfluous struct (Alex Bennée) 0c3e41d408: meson.build: fix building of Xen support for aarch64
Re: [PATCH for-6.0 1/6] qapi: Add query-accel command
On Tue, Nov 17, 2020 at 09:51:58AM +0100, Markus Armbruster wrote:
> Eduardo Habkost writes:
>
> > On Mon, Nov 16, 2020 at 10:20:04AM -0600, Eric Blake wrote:
> >> On 11/16/20 7:10 AM, Roman Bolshakov wrote:
> >> > There's a problem for management applications to determine if certain
> >> > accelerators available. Generic QMP command should help with that.
> >> >
> >> > Signed-off-by: Roman Bolshakov
> >> > ---
> >> > monitor/qmp-cmds.c | 15 +++
> >> > qapi/machine.json | 19 +++
> >> > 2 files changed, 34 insertions(+)
> >> >
> >>
> >> > +++ b/qapi/machine.json
> >> > @@ -591,6 +591,25 @@
> >> > ##
> >> > { 'command': 'query-kvm', 'returns': 'KvmInfo' }
> >> >
> >> > +##
> >> > +# @query-accel:
> >> > +#
> >> > +# Returns information about an accelerator
> >> > +#
> >> > +# Returns: @KvmInfo
>
> Is reusing KvmInfo here is good idea? Recall:
>
> ##
> # @KvmInfo:
> #
> # Information about support for KVM acceleration
> #
> # @enabled: true if KVM acceleration is active
> #
> # @present: true if KVM acceleration is built into this executable
> #
> # Since: 0.14.0
> ##
> { 'struct': 'KvmInfo', 'data': {'enabled': 'bool', 'present': 'bool'} }
>
> I figure @present will always be true with query-accel. In other words,
> it's useless noise.
>
Hi Markus,
Actually, no. Provided implementation returns present = true only if we
can find the accel in QOM, i.e. on macOS we get present = false for kvm.
And on Linux we get present = false for hvf, e.g.:
C: {"execute": "query-accel", "arguments": {"name": "hvf"}}
S: {"return": {"enabled": true, "present": true}}
C: {"execute": "query-accel", "arguments": {"name": "kvm"}}
S: {"return": {"enabled": false, "present": false}}
C: {"execute": "query-accel", "arguments": {"name": "tcg"}}
S: {"return": {"enabled": false, "present": true}}
> If we return information on all accelerators, KvmInfo won't do, because
> it lacks the accelerator name.
>
> If we choose to reuse KvmInfo regardless, it needs to be renamed to
> something like AccelInfo, and the doc comment generalized beyond KVM.
>
I have renamed it to AccelInfo in another patch in the series :)
> >> > +#
> >> > +# Since: 6.0.0
> >>
> >> We're inconsistent on whether we have 'Since: x.y' or 'Since: x.y.z',
> >> although I prefer the shorter form. Maybe Markus has an opnion on that.
>
> Yes: use the shorter form, unless .z != .0.
>
> The shorter form is much more common:
>
> $ sed -En 's/.*since:? *([0-9][0-9.]*).*/\1/pi' qapi/*json | sed
> 's/[^.]//g' | sort | uniq -c
>1065 .
> 129 ..
>
> .z != 0 should be rare: the stable branch is for bug fixes, and bug
> fixes rarely require schema changes. It is: there is just one instance,
> from commit ab9ba614556 (v3.0.0) and 0779afdc897 (v2.12.1).
>
Thanks, I'll use 6.0 then.
> >> > +#
> >> > +# Example:
> >> > +#
> >> > +# -> { "execute": "query-accel", "arguments": { "name": "kvm" } }
> >> > +# <- { "return": { "enabled": true, "present": true } }
> >> > +#
> >> > +##
> >> > +{ 'command': 'query-accel',
> >> > + 'data': { 'name': 'str' },
> >> > + 'returns': 'KvmInfo' }
> >>
> >> '@name' is undocumented and an open-coded string. Better would be
> >> requiring 'name' to be one of an enum type. [...]
> >
> > This seem similar to CPU models, machine types, device types, and
> > backend object types: the set of valid values is derived from the
> > list of subtypes of a QOM type. We don't duplicate lists of QOM
> > types in the QAPI schema, today.
>
> Yes.
>
> > Do we want to duplicate the list of accelerators in the QAPI
> > schema, or should we wait for a generic solution that works for
> > any QOM type?
>
> There are only a few accelerators, so duplicating them wouldn't be too
> bad. Still, we need a better reason than "because we can".
>
> QAPI enum vs. 'str' doesn't affect the wire format: both use JSON
> string.
>
'str' is quite flexible. If we remove an accel, provided QOM command
won't complain. It'll just reply that the accel is not present :)
> With a QAPI enum, the values available in this QEMU build are visible in
> query-qmp-schema.
>
> Without a QAPI enum, we need a separate, ad hoc query. For QOM types,
> we have qom-list-types.
>
> If you're familiar with qom-list-types, you may want to skip to
> "Conclusion:" now.
>
> Ad hoc queries can take additional arguments. qom-list-types does:
> "implements" and "abstract" limit output. Default is "all
> non-abstract".
>
> This provides a way to find accelerators: use "implements": "accel" to
> return only concrete subtypes of "accel":
>
>---> {"execute": "qom-list-types", "arguments": {"implements": "accel"}}
><--- {"return": [{"name": "qtest-accel", "parent": "accel"}, {"name":
> "tcg-accel", "parent": "accel"}, {"name": "xen-accel", "parent": "accel"},
> {"name": "kvm-accel", "parent": "accel"}, {"name": "accel", "parent":
> "object"}]}
>
> Aside: the reply inc
Re: [PATCH v6] hw/i386/pc: add max combined fw size as machine configuration option
Thanks Michael.
-Erich
From: Michael S. Tsirkin
Sent: Tuesday, November 17, 2020 10:53 AM
To: McMillan, Erich
Cc: qemu-devel@nongnu.org ; ler...@redhat.com
; dgilb...@redhat.com ;
marcel.apfelb...@gmail.com ; imamm...@redhat.com
; kra...@redhat.com
Subject: Re: [PATCH v6] hw/i386/pc: add max combined fw size as machine
configuration option
On Fri, Sep 25, 2020 at 05:57:51PM +, Erich Mcmillan wrote:
> From: Erich McMillan
>
> At Hewlett Packard Inc. we have a need for increased fw size to enable
> testing of our custom fw.
> Move return statement for early return
>
> Signed-off-by: Erich McMillan
My bad that I dropped it by mistake before code freeze.
I will queue it for the next release.
> ---
>
> Changes since v5:
>
> Move return statement for pc_machine_set_max_fw_size() to follow
> error_setg() as early return.
>
> hw/i386/pc.c | 51
> hw/i386/pc_sysfw.c | 13 ++-
> include/hw/i386/pc.h | 2 ++
> 3 files changed, 55 insertions(+), 11 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index d11daacc23..70c8c9adcf 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1869,6 +1869,50 @@ static void pc_machine_set_max_ram_below_4g(Object
> *obj, Visitor *v,
> pcms->max_ram_below_4g = value;
> }
>
> +static void pc_machine_get_max_fw_size(Object *obj, Visitor *v,
> + const char *name, void *opaque,
> + Error **errp)
> +{
> +PCMachineState *pcms = PC_MACHINE(obj);
> +uint64_t value = pcms->max_fw_size;
> +
> +visit_type_size(v, name, &value, errp);
> +}
> +
> +static void pc_machine_set_max_fw_size(Object *obj, Visitor *v,
> + const char *name, void *opaque,
> + Error **errp)
> +{
> +PCMachineState *pcms = PC_MACHINE(obj);
> +Error *error = NULL;
> +uint64_t value;
> +
> +visit_type_size(v, name, &value, &error);
> +if (error) {
> +error_propagate(errp, error);
> +return;
> +}
> +
> +/*
> +* We don't have a theoretically justifiable exact lower bound on the base
> +* address of any flash mapping. In practice, the IO-APIC MMIO range is
> +* [0xFEE0..0xFEE01000] -- see IO_APIC_DEFAULT_ADDRESS --, leaving
> free
> +* only 18MB-4KB below 4G. For now, restrict the cumulative mapping to
> 8MB in
> +* size.
> +*/
> +if (value > 16 * MiB) {
> +error_setg(errp,
> + "User specified max allowed firmware size %" PRIu64 " is "
> + "greater than 16MiB. If combined firwmare size exceeds "
> + "16MiB the system may not boot, or experience
> intermittent"
> + "stability issues.",
> + value);
> +return;
> +}
> +
> +pcms->max_fw_size = value;
> +}
> +
> static void pc_machine_initfn(Object *obj)
> {
> PCMachineState *pcms = PC_MACHINE(obj);
> @@ -1884,6 +1928,7 @@ static void pc_machine_initfn(Object *obj)
> pcms->smbus_enabled = true;
> pcms->sata_enabled = true;
> pcms->pit_enabled = true;
> +pcms->max_fw_size = 8 * MiB;
>
> pc_system_flash_create(pcms);
> pcms->pcspk = isa_new(TYPE_PC_SPEAKER);
> @@ -2004,6 +2049,12 @@ static void pc_machine_class_init(ObjectClass *oc,
> void *data)
>
> object_class_property_add_bool(oc, PC_MACHINE_PIT,
> pc_machine_get_pit, pc_machine_set_pit);
> +
> +object_class_property_add(oc, PC_MACHINE_MAX_FW_SIZE, "size",
> +pc_machine_get_max_fw_size, pc_machine_set_max_fw_size,
> +NULL, NULL);
> +object_class_property_set_description(oc, PC_MACHINE_MAX_FW_SIZE,
> +"Maximum combined firmware size");
> }
>
> static const TypeInfo pc_machine_info = {
> diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
> index b6c0822fe3..22450ba0ef 100644
> --- a/hw/i386/pc_sysfw.c
> +++ b/hw/i386/pc_sysfw.c
> @@ -39,15 +39,6 @@
> #include "hw/block/flash.h"
> #include "sysemu/kvm.h"
>
> -/*
> - * We don't have a theoretically justifiable exact lower bound on the base
> - * address of any flash mapping. In practice, the IO-APIC MMIO range is
> - * [0xFEE0..0xFEE01000] -- see IO_APIC_DEFAULT_ADDRESS --, leaving free
> - * only 18MB-4KB below 4G. For now, restrict the cumulative mapping to 8MB in
> - * size.
> - */
> -#define FLASH_SIZE_LIMIT (8 * MiB)
> -
> #define FLASH_SECTOR_SIZE 4096
>
> static void pc_isa_bios_init(MemoryRegion *rom_memory,
> @@ -182,10 +173,10 @@ static void pc_system_flash_map(PCMachineState *pcms,
> }
> if ((hwaddr)size != size
> || total_size > HWADDR_MAX - size
> -|| total_size + size > FLASH_SIZE_LIMIT) {
> +|| total_size + size > pcms->max_fw_size) {
> error_report("combined size of system firmware exceeds "
> "%" PRI
Re: [PATCH v2 5/7] qapi: Introduce QAPI_LIST_APPEND
On 11/17/20 6:51 AM, Markus Armbruster wrote:
> Eric Blake writes:
>
>> Similar to the existing QAPI_LIST_PREPEND, but designed for use where
>> we want to preserve insertion order. Callers will be added in
>> upcoming patches. Note the difference in signature: PREPEND takes
>> List*, APPEND takes List**.
>>
>> Signed-off-by: Eric Blake
>> +#define QAPI_LIST_APPEND(tail, element) do { \
>> +*(tail) = g_malloc0(sizeof(**(tail))); \
>> +(*(tail))->value = (element); \
>> +(tail) = &(*tail)->next; \
Hmm; I'm inconsistent on whether to spell things '*tail' or '*(tail)'.
I don't think any of the callers converted in patches 6 or 7 care about
the difference, but for maximal copy-paste portability, the use of the
macro parameter should be surrounded by () anywhere that could otherwise
cause a mis-parse on some arbitrary expression with an operator at
higher precedence than unary * (hmm, the only such operators are all
suffix operators; so maybe the *(tail) is overkill...)
>
> Reviewed-by: Markus Armbruster
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v9 08/12] hw/block/nvme: Support Zoned Namespace Command Set
On Thu, Nov 12, 2020 at 08:36:39PM +0100, Klaus Jensen wrote:
> On Nov 5 11:53, Dmitry Fomichev wrote:
> > @@ -133,6 +300,12 @@ static Property nvme_ns_props[] = {
> > DEFINE_BLOCK_PROPERTIES(NvmeNamespace, blkconf),
> > DEFINE_PROP_UINT32("nsid", NvmeNamespace, params.nsid, 0),
> > DEFINE_PROP_UUID("uuid", NvmeNamespace, params.uuid),
> > +DEFINE_PROP_BOOL("zoned", NvmeNamespace, params.zoned, false),
>
> I disagree on this. Using the "magic" value ensures that only one
> command set can be selected. We can do a custom property so we can set
> `iocs=zoned` as well as `iocs=0x2` if that makes it more user friendly?
IMO, 'iocs' is a rather difficult parameter name for a user to remember
compared to 'zoned=true'. While 'iocs' is a spec field, the spec isn't
very user friendly either, and these user parameters can hide the spec
terms behind human comprehensible names.
[PATCH v4] fcntl: Add 32bit filesystem mode
It was brought to my attention that this bug from 2018 was
still unresolved: 32 bit emulators like QEMU were given
64 bit hashes when running 32 bit emulation on 64 bit systems.
This adds a flag to the fcntl() F_GETFD and F_SETFD operations
to set the underlying filesystem into 32bit mode even if the
file handle was opened using 64bit mode without the compat
syscalls.
Programs that need the 32 bit file system behavior need to
issue a fcntl() system call such as in this example:
#define FD_32BIT_MODE 2
int main(int argc, char** argv) {
DIR* dir;
int err;
int mode;
int fd;
dir = opendir("/boot");
fd = dirfd(dir);
mode = fcntl(fd, F_GETFD);
mode |= FD_32BIT_MODE;
err = fcntl(fd, F_SETFD, mode);
if (err) {
printf("fcntl() failed! err=%d\n", err);
return 1;
}
printf("dir=%p\n", dir);
printf("readdir(dir)=%p\n", readdir(dir));
printf("errno=%d: %s\n", errno, strerror(errno));
return 0;
}
This can be pretty hard to test since C libraries and linux
userspace security extensions aggressively filter the parameters
that are passed down and allowed to commit into actual system
calls.
Cc: Florian Weimer
Cc: Peter Maydell
Cc: Andy Lutomirski
Cc: Eric Blake
Reported-by: 罗勇刚(Yonggang Luo)
Suggested-by: Theodore Ts'o
Link: https://bugs.launchpad.net/qemu/+bug/1805913
Link: https://lore.kernel.org/lkml/87bm56vqg4@mid.deneb.enyo.de/
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=205957
Signed-off-by: Linus Walleij
---
ChangeLog v3 RESEND 1-> v4:
- Update the example in the commit message to a read/modify/write
version.
- Notice that Yonggang Luo sees the sema problem on i386 binaries
as we see on ARM 32bit binaries.
ChangeLog v3->v3 RESEND 1:
- Resending during the v5.10 merge window to get attention.
ChangeLog v2->v3:
- Realized that I also have to clear the flag correspondingly
if someone ask for !FD_32BIT_MODE after setting it the
first time.
ChangeLog v1->v2:
- Use a new flag FD_32BIT_MODE to F_GETFD and F_SETFD
instead of a new fcntl operation, there is already a fcntl
operation to set random flags.
- Sorry for taking forever to respin this patch :(
---
fs/fcntl.c | 7 +++
include/uapi/asm-generic/fcntl.h | 8
2 files changed, 15 insertions(+)
diff --git a/fs/fcntl.c b/fs/fcntl.c
index 19ac5baad50f..6c32edc4099a 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -335,10 +335,17 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned
long arg,
break;
case F_GETFD:
err = get_close_on_exec(fd) ? FD_CLOEXEC : 0;
+ /* Report 32bit file system mode */
+ if (filp->f_mode & FMODE_32BITHASH)
+ err |= FD_32BIT_MODE;
break;
case F_SETFD:
err = 0;
set_close_on_exec(fd, arg & FD_CLOEXEC);
+ if (arg & FD_32BIT_MODE)
+ filp->f_mode |= FMODE_32BITHASH;
+ else
+ filp->f_mode &= ~FMODE_32BITHASH;
break;
case F_GETFL:
err = filp->f_flags;
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index 9dc0bf0c5a6e..edd3573cb7ef 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -160,6 +160,14 @@ struct f_owner_ex {
/* for F_[GET|SET]FL */
#define FD_CLOEXEC 1 /* actually anything with low bit set goes */
+/*
+ * This instructs the kernel to provide 32bit semantics (such as hashes) from
+ * the file system layer, when running a userland that depend on 32bit
+ * semantics on a kernel that supports 64bit userland, but does not use the
+ * compat ioctl() for e.g. open(), so that the kernel would otherwise assume
+ * that the userland process is capable of dealing with 64bit semantics.
+ */
+#define FD_32BIT_MODE 2
/* for posix fcntl() and lockf() */
#ifndef F_RDLCK
--
2.26.2
Re: [PATCH v3 RESEND] fcntl: Add 32bit filesystem mode
On Tue, Oct 13, 2020 at 11:22 AM Dave Martin wrote: > > case F_SETFD: > > err = 0; > > set_close_on_exec(fd, arg & FD_CLOEXEC); > > + if (arg & FD_32BIT_MODE) > > + filp->f_mode |= FMODE_32BITHASH; > > + else > > + filp->f_mode &= ~FMODE_32BITHASH; > > This seems inconsistent? F_SETFD is for setting flags on a file > descriptor. Won't setting a flag on filp here instead cause the > behaviour to change for all file descriptors across the system that are > open on this struct file? Compare set_close_on_exec(). > > I don't see any discussion on whether this should be an F_SETFL or an > F_SETFD, though I see F_SETFD was Ted's suggestion originally. I cannot honestly say I know the semantic difference. I would ask the QEMU people how a user program would expect the flag to behave. Yours, Linus Walleij
Re: [PULL 0/2] Linux user for 5.2 patches
On Tue, 17 Nov 2020 at 15:18, Laurent Vivier wrote: > > The following changes since commit cb5ed407a1ddadf788fd373fed41c87c9e81e5b0: > > Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2020-11= > -15' into staging (2020-11-16 17:00:36 +) > > are available in the Git repository at: > > git://github.com/vivier/qemu.git tags/linux-user-for-5.2-pull-request > > for you to fetch changes up to 5351f4075dc17825df8e0628a93f9baa9b9bda4b: > > linux-user,netlink: add IFLA_BRPORT_MRP_RING_OPEN, IFLA_BRPORT_MRP_IN_OPEN = > (2020-11-17 15:22:52 +0100) > > > Fix netlink with latest iproute > > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2 for any user-visible changes. -- PMM
question about bdrv_replace_node
Hi!
bdrv_replace_node_common() keeps old node parents in a list and call
bdrv_replace_child_noperm() in a loop..
But bdrv_replace_child_noperm() may do aio_poll, which may trigger any graph
change, up to freeing child which we keep in a loop.
Actually I've reach something similar with a lot modified code, not sure that
it may be reproduced on master. Still, here is a backtrace, to illustrate what
I mean:
#0 bdrv_detach_child (child=0x557e50e0) at ../block.c:3073
#1 0x55609d53 in bdrv_root_unref_child (child=0x557e50e0) at
../block.c:3084
#2 0x55609e57 in bdrv_unref_child (parent=0x5582de10,
child=0x557e50e0)
at ../block.c:3124
#3 0x5560db2a in bdrv_close (bs=0x5582de10) at ../block.c:4728
#4 0x5560e7eb in bdrv_delete (bs=0x5582de10) at ../block.c:5056
#5 0x55610ea6 in bdrv_unref (bs=0x5582de10) at ../block.c:6409
#6 0x55609d5f in bdrv_root_unref_child (child=0x557e00d0) at
../block.c:3085
#7 0x55588122 in blk_remove_bs (blk=0x55838df0) at
../block/block-backend.c:831
#8 0x555875c0 in blk_delete (blk=0x55838df0) at
../block/block-backend.c:447
#9 0x55587864 in blk_unref (blk=0x55838df0) at
../block/block-backend.c:502
#10 0x555aeb84 in block_job_free (job=0x55839150) at
../blockjob.c:89
#11 0x555caad3 in job_unref (job=0x55839150) at ../job.c:380
#12 0x555cbc7f in job_exit (opaque=0x55839150) at ../job.c:894
#13 0x5569a375 in aio_bh_call (bh=0x558215f0) at ../util/async.c:136
#14 0x5569a47f in aio_bh_poll (ctx=0x55810e90) at
../util/async.c:164
#15 0x556ac65d in aio_poll (ctx=0x55810e90, blocking=true) at
../util/aio-posix.c:659
#16 0x55639c2b in bdrv_unapply_subtree_drain (child=0x557ef080,
old_parent=0x55815050)
at ../block/io.c:530
#17 0x556062e1 in bdrv_child_cb_detach (child=0x557ef080) at
../block.c:1326
#18 0x5560918e in bdrv_replace_child_noperm (child=0x557ef080,
new_bs=0x0) at ../block.c:2779
#19 0x55607f11 in bdrv_replace_child_safe (child=0x557ef080,
new_bs=0x0, tran=0x7fffda08)
at ../block.c:2189
#20 0x5560dfce in bdrv_remove_backing (bs=0x55815050,
tran=0x7fffda08) at ../block.c:4884
#21 0x5560e3fc in bdrv_replace_node_common (from=0x55815050,
to=0x5581c1e0,
auto_skip=false, detach_subchain=true, errp=0x7fffda80) at
../block.c:4972
#22 0x5560ee57 in bdrv_drop_intermediate (top=0x55815050,
base=0x5581c1e0,
backing_file_str=0x5581c211 "json:{\"driver\": \"test\"}") at
../block.c:5318
#23 0x55583939 in test_drop_intermediate_poll () at
../tests/test-bdrv-drain.c:1822
Here a child is detached which is kept in a updated_children list in
bdrv_drop_intermediate(). And we'll crash soon with use-after-free.
I'll try to find a similar reproduce on master, but anyway, it seems to be a
wrong design to loop through children with possible intermediate aio_poll..
This problem breaks now my work on trying to move child-replacement to prepare
phase of transactional graph updates (to correctly update permissions on new
graph).
In short, do several updates with bdrv_replace_child_noperm(), than do
permission update. If permission update fails, rollback
bdrv_replace_child_noperm() calls in reverse order.
But what to do with unexpected aio_poll? Seems we need a way to do several
child replacement operations not triggering any drain-poll and do all needed
drain-poll things at the end.. But how to achieve it I have no idea.
Any thoughts?
--
Best regards,
Vladimir
Re: [Virtio-fs] [PATCH] virtiofsd: Use --thread-pool-size=0 to mean no thread pool
On Tue, Nov 17, 2020 at 04:00:09PM +, Venegas Munoz, Jose Carlos wrote: > > Not sure what the default is for 9p, but comparing > > default to default will definitely not be apples to apples since this > > mode is nonexistent in 9p. > > In Kata we are looking for the best config for fs compatibility and > performance. So even if are no apples to apples, > we are for the best config for both and comparing the best that each of them > can do. Can we run tests in more than one configuration. Right now you are using cache=mmap for virtio-9p and cache=auto for virtiofs and as Miklos said this is not apples to apples comparison. So you can continue to run above but also run additional tests if time permits. virtio-9p virtio-fs cache=mmap cache=none + DAX cache=none cache=none cache=loose cache=always Given you are using cache=mmap for virtio-9p, "cache=none + DAX" is somewhat equivalent of that. Provides strong coherency as well as allow for mmap() to work. Now kernel virtiofs DAX support got merged in 5.10-rc1. For qemu, you can use virtio-fs-dev branch. https://gitlab.com/virtio-fs/qemu/-/commits/virtio-fs-dev Thanks Vivek
[Bug 1894836] Re: kernel panic using hvf with CPU passthrough
Thanks for the response Jessica! The option you provided fixes the problem and everything works flawlessly now. Thank you!! -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1894836 Title: kernel panic using hvf with CPU passthrough Status in QEMU: New Bug description: Host Details QEMU 5.1 (Homebrew) macOS 10.15.6 Catalina Late 2014 iMac i5-4690 @ 3.5 GHz 8 GB RAM Guest Details Ubuntu Desktop 20.04.1 Installer ISO Problem Whenever I boot with "-accel hvf -cpu host", the Ubuntu desktop installer will immediately crash with a kernel panic after the initial splash screen. See the attached picture of the kernel panic for more details. Steps to recreate From https://www.jwillikers.com/posts/virtualize_ubuntu_desktop_on_macos_with_qemu/ 1. Install QEMU with Homebrew. $ brew install qemu 2. Create a qcow2 disk image to which to install. $ qemu-img create -f qcow2 ubuntu2004.qcow2 60G 3. Download the ISO. $ curl -L -o ubuntu-20.04.1-desktop-amd64.iso https://releases.ubuntu.com/20.04/ubuntu-20.04.1-desktop-amd64.iso 4. Run the installer in QEMU. $ qemu-system-x86_64 \ -accel hvf \ -cpu host \ -smp 2 \ -m 4G \ -usb \ -device usb-tablet \ -vga virtio \ -display default,show-cursor=on \ -device virtio-net,netdev=vmnic -netdev user,id=vmnic \ -audiodev coreaudio,id=snd0 \ -device ich9-intel-hda -device hda-output,audiodev=snd0 \ -cdrom ubuntu-20.04.1-desktop-amd64.iso \ -drive file=ubuntu2004.qcow2,if=virtio Workaround Emulating the CPU with "-cpu qemu64" does not result in a kernel panic. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1894836/+subscriptions
Re: [PULL 0/1] QObject patches patches for 2020-11-17
On Tue, 17 Nov 2020 at 14:57, Markus Armbruster wrote: > > The following changes since commit 1c7ab0930a3e02e6e54722c20b6b586364f387a7: > > Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging > (2020-11-17 11:50:11 +) > > are available in the Git repository at: > > git://repo.or.cz/qemu/armbru.git tags/pull-qobject-2020-11-17 > > for you to fetch changes up to 922d42bb0d08c154602dd9112da00d22d2b46579: > > json: Fix a memleak in parse_pair() (2020-11-17 15:39:53 +0100) > > > QObject patches patches for 2020-11-17 > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2 for any user-visible changes. -- PMM
[PATCH] Fix build with 64 bits time_t
time element is deprecated on new input_event structure in kernel's
input.h [1]
This will avoid the following build failure:
hw/input/virtio-input-host.c: In function 'virtio_input_host_handle_status':
hw/input/virtio-input-host.c:198:28: error: 'struct input_event' has no member
named 'time'
198 | if (gettimeofday(&evdev.time, NULL)) {
|^
Fixes:
-
http://autobuild.buildroot.org/results/a538167e288c14208d557cd45446df86d3d599d5
-
http://autobuild.buildroot.org/results/efd4474fb4b6c0ce0ab3838ce130429c51e43bbb
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=152194fe9c3f
Signed-off-by: Fabrice Fontaine
---
contrib/vhost-user-input/main.c | 10 +-
hw/input/virtio-input-host.c| 10 +-
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/contrib/vhost-user-input/main.c b/contrib/vhost-user-input/main.c
index 6020c6f33a..e688c3e0a9 100644
--- a/contrib/vhost-user-input/main.c
+++ b/contrib/vhost-user-input/main.c
@@ -17,6 +17,11 @@
#include "standard-headers/linux/virtio_input.h"
#include "qapi/error.h"
+#ifndef input_event_sec
+#define input_event_sec time.tv_sec
+#define input_event_usec time.tv_usec
+#endif
+
enum {
VHOST_USER_INPUT_MAX_QUEUES = 2,
};
@@ -115,13 +120,16 @@ vi_evdev_watch(VuDev *dev, int condition, void *data)
static void vi_handle_status(VuInput *vi, virtio_input_event *event)
{
struct input_event evdev;
+struct timeval tval;
int rc;
-if (gettimeofday(&evdev.time, NULL)) {
+if (gettimeofday(&tval, NULL)) {
perror("vi_handle_status: gettimeofday");
return;
}
+evdev.input_event_sec = tval.tv_sec;
+evdev.input_event_usec = tval.tv_usec;
evdev.type = le16toh(event->type);
evdev.code = le16toh(event->code);
evdev.value = le32toh(event->value);
diff --git a/hw/input/virtio-input-host.c b/hw/input/virtio-input-host.c
index 85daf73f1a..2e261737e1 100644
--- a/hw/input/virtio-input-host.c
+++ b/hw/input/virtio-input-host.c
@@ -16,6 +16,11 @@
#include
#include "standard-headers/linux/input.h"
+#ifndef input_event_sec
+#define input_event_sec time.tv_sec
+#define input_event_usec time.tv_usec
+#endif
+
/* - */
static struct virtio_input_config virtio_input_host_config[] = {
@@ -193,13 +198,16 @@ static void virtio_input_host_handle_status(VirtIOInput
*vinput,
{
VirtIOInputHost *vih = VIRTIO_INPUT_HOST(vinput);
struct input_event evdev;
+struct timeval tval;
int rc;
-if (gettimeofday(&evdev.time, NULL)) {
+if (gettimeofday(&tval, NULL)) {
perror("virtio_input_host_handle_status: gettimeofday");
return;
}
+evdev.input_event_sec = tval.tv_sec;
+evdev.input_event_usec = tval.tv_usec;
evdev.type = le16_to_cpu(event->type);
evdev.code = le16_to_cpu(event->code);
evdev.value = le32_to_cpu(event->value);
--
2.29.2
Re: [PATCH 06/15] target/arm: Enforce M-profile VMRS/VMSR register restrictions
On Tue, 17 Nov 2020 at 19:42, Richard Henderson
wrote:
>
> On 11/16/20 8:08 AM, Peter Maydell wrote:
> > -if (a->rt == 15 && (!a->l || a->reg != ARM_VFP_FPSCR)) {
> > +if (a->reg != ARM_VFP_FPSCR) {
> > +return false;
> > +}
> > +if (a->rt == 15 && !a->l) {
>
> Alternately, the parenthesis are just off:
>
> if ((a->rt == 15 && !a->l) || a->reg != ARM_VFP_FPSCR)
Mmm. As you've probably discovered by now, the refactoring
in the subsequent patches means that this code gets moved
and changed anyway; I just wanted it in this separate
patch so the bugfix wasn't hidden in the refactoring.
thanks
-- PMM
Re: [PULL 0/9] target-arm queue
On Tue, 17 Nov 2020 at 13:48, Peter Maydell wrote: > > Arm queue; bugfixes only. > > thanks > -- PMM > > The following changes since commit 48aa8f0ac536db3550a35c295ff7de94e4c33739: > > Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2020-11-16' into > staging (2020-11-17 11:07:00 +) > > are available in the Git repository at: > > https://git.linaro.org/people/pmaydell/qemu-arm.git > tags/pull-target-arm-20201117 > > for you to fetch changes up to ab135622cf478585bdfcb68b85e4a817d74a0c42: > > tmp105: Correct handling of temperature limit checks (2020-11-17 12:56:33 > +) > > > target-arm queue: > * hw/arm/virt: ARM_VIRT must select ARM_GIC > * exynos: Fix bad printf format specifiers > * hw/input/ps2.c: Remove remnants of printf debug > * target/openrisc: Remove dead code attempting to check "is timer disabled" > * register: Remove unnecessary NULL check > * util/cutils: Fix Coverity array overrun in freq_to_str() > * configure: Make "does libgio work" test pull in some actual functions > * tmp105: reset the T_low and T_High registers > * tmp105: Correct handling of temperature limit checks > > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2 for any user-visible changes. -- PMM
Re: [PATCH v2] hw/char/cmsdk-apb-uart: Fix rx interrupt handling
On 17. 11. 20 17:38, Peter Maydell wrote:
> On Mon, 16 Nov 2020 at 19:58, Tadej Pečar wrote:
>>
>> Previously, the RX interrupt got missed if:
>> - the character backend provided next character before
>>the RX IRQ Handler managed to clear the currently served interrupt.
>> - the character backend provided next character while the RX interrupt
>>was disabled. Enabling the interrupt did not trigger the interrupt
>>even if the RXFULL status bit was set.
>>
>> These bugs become apparent when the terminal emulator buffers the line
>> before sending it to qemu stdin (Eclipse IDE console does this).
>>
>>
>> diff --git a/hw/char/cmsdk-apb-uart.c b/hw/char/cmsdk-apb-uart.c
>> index 626b68f2ec..d76ca76e01 100644
>> --- a/hw/char/cmsdk-apb-uart.c
>> +++ b/hw/char/cmsdk-apb-uart.c
>> @@ -96,19 +96,34 @@ static void uart_update_parameters(CMSDKAPBUART *s)
>>
>> static void cmsdk_apb_uart_update(CMSDKAPBUART *s)
>> {
>> -/* update outbound irqs, including handling the way the rxo and txo
>> - * interrupt status bits are just logical AND of the overrun bit in
>> - * STATE and the overrun interrupt enable bit in CTRL.
>> +/*
>> + * update outbound irqs
>> + * (
>> + * state [rxo, txo, rxbf, txbf ] at bit [3, 2, 1, 0]
>> + * | intstatus [rxo, txo, rx, tx ] at bit [3, 2, 1, 0]
>> + * )
>> + * & ctrl[rxoe, txoe, rxe, txe ] at bit [5, 4, 3, 2]
>> + * = masked_intstatus
>> + *
>> + * state: status register
>> + * intstatus: pending interrupts and is sticky (has to be cleared by sw)
>> + * masked_intstatus: masked (by ctrl) pending interrupts
>> + *
>> + * intstatus [rxo, txo, rx] bits are set here
>> + * intstatus [tx] is managed in uart_transmit
>>*/
>> -uint32_t omask = (R_INTSTATUS_RXO_MASK | R_INTSTATUS_TXO_MASK);
>> -s->intstatus &= ~omask;
>> -s->intstatus |= (s->state & (s->ctrl >> 2) & omask);
>> -
>> -qemu_set_irq(s->txint, !!(s->intstatus & R_INTSTATUS_TX_MASK));
>> -qemu_set_irq(s->rxint, !!(s->intstatus & R_INTSTATUS_RX_MASK));
>> -qemu_set_irq(s->txovrint, !!(s->intstatus & R_INTSTATUS_TXO_MASK));
>> -qemu_set_irq(s->rxovrint, !!(s->intstatus & R_INTSTATUS_RXO_MASK));
>> -qemu_set_irq(s->uartint, !!(s->intstatus));
>> +s->intstatus |= s->state &
>> +(R_INTSTATUS_RXO_MASK | R_INTSTATUS_TXO_MASK | R_INTSTATUS_RX_MASK);
>> +
>> +uint32_t masked_intstatus = s->intstatus & (s->ctrl >> 2);
>
> I don't think this logic is correct. It makes the values we
> send out on the output interrupt lines different from the
> values visible in the INTSTATUS register bits, and I don't
> think that's how the hardware behaves.
>
> thanks
> -- PMM
>
Yep, it seems that I stand corrected. More so, it seems that I was completely
wrong.
I've had a closer look at the cmsdk_apb_uart.v HDL file, available in the
Cortex-M0/M3 DesignStart, and the interrupt lines are directly assigned to
INTSTATUS.
Additionally, it seems that the actual hardware suffers from the same issues
described as "fixed" by this patch:
- If you happen to be in the interrupt handler for long enough to receive
the next character, STATE will report RX buffer full / overrun, depending
on whether you've already read the current character from DATA.
Which is fine and dandy - but if you happen to clear INTSTATUS _after_ you
have received the next character, you've essentially cleared the next
interrupt,
so the next character won't get handled.
This is because INTSTATUS register logic is independent from the STATE
register.
- RX / TX interrupt lines are masked by interrupt enable _before_ they are
registered, and the rx'd / tx'd status from the state machine
is present only for one clock cycle.
So the interrupts are ignored when they are disabled by CTRL, and they don't
get fired at interrupt enable, regardless of the current STATE.
Interestingly enough, RX / TX overrun interrupts are masked _after_ they
are registered, so enabling the interrupt for those should trigger the
interrupt (as correctly modeled by the current cmsdk-apb-uart).
Guess I wanted my hardware emulation to be better than the actual hardware ;)
Jokes aside, I guess these issues weren't apparent in hardware because serial
communication is usually so much slower that
- the mcu could always clear the interrupt before the next character was
received.
- the time when the rx interrupt was disabled
was always shorter than the time required to receive the next character.
The uart emulation could be made a little more forgiving by postponing the
next character until the current interrupt is finished, but that's probably for
some other discussion.
In the end, the patch is unnecessary, as the current cmsdk-apb-uart
provides a more faithful emulation of the actual hardware, along with its warts.
It was educational, at least.
Regards, TP
[PATCH for-5.2] meson: Fix build with --disable-guest-agent-msi
The QGA MSI target requires several macros which are only available
without --disable-guest-agent-msi.
Don't define that target if configure was called with --disable-guest-agent-msi.
Signed-off-by: Stefan Weil
---
qga/meson.build | 36 +++-
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/qga/meson.build b/qga/meson.build
index 53ba6de5f8..520af6ce9b 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -61,23 +61,25 @@ if targetos == 'windows'
if 'CONFIG_QGA_VSS' in config_host and 'QEMU_GA_MSI_WITH_VSS' in
config_host
deps += qga_vss
endif
-qga_msi = custom_target('QGA MSI',
-input: files('installer/qemu-ga.wxs'),
-output:
'qemu-ga-@0@.msi'.format(config_host['ARCH']),
-depends: deps,
-command: [
- find_program('env'),
- 'QEMU_GA_VERSION=' +
config_host['QEMU_GA_VERSION'],
- 'QEMU_GA_MANUFACTURER=' +
config_host['QEMU_GA_MANUFACTURER'],
- 'QEMU_GA_DISTRO=' +
config_host['QEMU_GA_DISTRO'],
- 'BUILD_DIR=' + meson.build_root(),
- wixl, '-o', '@OUTPUT0@', '@INPUT0@',
- config_host['QEMU_GA_MSI_ARCH'].split(),
- config_host['QEMU_GA_MSI_WITH_VSS'].split(),
-
config_host['QEMU_GA_MSI_MINGW_DLL_PATH'].split(),
-])
-all_qga += [qga_msi]
-alias_target('msi', qga_msi)
+if 'CONFIG_QGA_MSI' in config_host
+ qga_msi = custom_target('QGA MSI',
+ input: files('installer/qemu-ga.wxs'),
+ output:
'qemu-ga-@0@.msi'.format(config_host['ARCH']),
+ depends: deps,
+ command: [
+find_program('env'),
+'QEMU_GA_VERSION=' +
config_host['QEMU_GA_VERSION'],
+'QEMU_GA_MANUFACTURER=' +
config_host['QEMU_GA_MANUFACTURER'],
+'QEMU_GA_DISTRO=' +
config_host['QEMU_GA_DISTRO'],
+'BUILD_DIR=' + meson.build_root(),
+wixl, '-o', '@OUTPUT0@', '@INPUT0@',
+config_host['QEMU_GA_MSI_ARCH'].split(),
+config_host['QEMU_GA_MSI_WITH_VSS'].split(),
+
config_host['QEMU_GA_MSI_MINGW_DLL_PATH'].split(),
+ ])
+ all_qga += [qga_msi]
+ alias_target('msi', qga_msi)
+endif
endif
else
install_subdir('run', install_dir: get_option('localstatedir'))
--
2.29.2
Re: [PATCH 13/13] bcm2835_cprman: put some peripherals of bcm2835 cprman into the 'misc' category
On 16/11/2020 18.00, Markus Armbruster wrote: > Thomas Huth writes: > >> On 16/11/2020 14.25, Philippe Mathieu-Daudé wrote: >>> Hi Gan, >>> >>> On 11/15/20 7:49 PM, Gan Qixin wrote: Some peripherals of bcm2835 cprman have no category, put them into the 'misc' category. Signed-off-by: Gan Qixin --- Cc: Philippe Mathieu-Daudé --- hw/misc/bcm2835_cprman.c | 4 1 file changed, 4 insertions(+) diff --git a/hw/misc/bcm2835_cprman.c b/hw/misc/bcm2835_cprman.c index 7e415a017c..c62958a99e 100644 --- a/hw/misc/bcm2835_cprman.c +++ b/hw/misc/bcm2835_cprman.c @@ -136,6 +136,7 @@ static void pll_class_init(ObjectClass *klass, void *data) dc->reset = pll_reset; dc->vmsd = &pll_vmstate; +set_bit(DEVICE_CATEGORY_MISC, dc->categories); >>> >>> Well, this is not an usable device but a part of a bigger device, >>> so here we want the opposite: not list this device in any category. >>> >>> Maybe we could add a DEVICE_CATEGORY_COMPOSITE for all such QOM >>> types so management apps can filter them out? (And so we are sure >>> all QOM is classified). >>> >>> Thomas, you already dealt with categorizing devices in the past, >>> what do you think about this? Who else could help? Maybe add >>> someone from libvirt in the thread? >> >> My 0.02 € : Mark the device as user_creatable = false if it can not really >> be used by the user with the -device CLI parameter. Then it also does not >> need a category. I know Markus will likely have a different opinion, but in > > You're hurting my feelings! ;-P > >> my eyes it's just ugly if we present devices to the users that they can not >> use. > > If we believe a device should only ever be used from C, then we should > keep it away from the UI. > > However, I'm wary of overloading user_creatable. Even though it has > shifted shape a number of times (cannot_instantiate_with_device_add_yet, > no_user, and now user_creatable), its purpose has always been focused: > distinguishing devices that can be instantiated by generic code from the > ones that need device-specific code. See user_creatable's comment in > qdev-core.h. > > I don't want to lose that distinction. That's all. Well, currently we have the user_creatable flag and the hotpluggable flag. I guess that's simply not enough. I think in the long run, we should maybe replace the two flags with a "creatable" type instead that could take the following values: CREATABLE_AS_SUBDEVICE /* Device is part of another device and can only by added by code */ CREATABLE_BY_QOM/* Some fancy new QOM function can be used to e.g. create this as part of a machine */ CREATABLE_BY_COLDPLUG /* For cold-plugging via -device */ CREATABLE_BY_HOTPLUG/* For hot-plugging via device_add */ ... but that's likely something for the distant future... >> (By the way, this device here seems to be a decendant of TYPE_SYS_BUS_DEVICE >> ... shouldn't these show up as user_creatable = false automatically?) > > Yes, unless it is a dynamic sysbus device (which I consider a flawed > concept). > > But TYPE_CPRMAN_PLL is *not* a descendant of TYPE_SYS_BUS_DEVICE, it's a > bus-less device: Oops, I obviously looked at the wrong device in that file (TYPE_BCM2835_CPRMAN instead of TYPE_CPRMAN_PLL) - thanks for the clarification! Thomas
Re: Regressions in build process introduced since August
Am 17.11.20 um 19:01 schrieb Paolo Bonzini: On 17/11/20 18:50, Stefano Garzarella wrote: Running `configure [...] --extra-cflags="-I /xyz"` results in compiler flags `-I [...] /xyz`, so the `-I` and `/xyz` are separated by other compiler flags which obviously cannot work as expected. I could work around that by removing the space and using a pattern like `-I/xyz`. This regression is not restricted to builds targeting Windows. I'll take a look at fixing this (in meson). Thanks. Here is another regression for builds targeting Windows: Running `../configure --disable-guest-agent-msi [...]` fails with "../qga/meson.build:64:4: ERROR: Key QEMU_GA_VERSION is not in dict". QEMU_GA_VERSION is only set with enabled guest-agent-msi, but currently used with enabled guest-agent even when guest-agent-msi is disabled. Regards, Stefan
Re: [PATCH 12/15] target/arm: Factor out preserve-fp-state from full_vfp_access_check()
On 11/16/20 8:08 AM, Peter Maydell wrote: > Factor out the code which handles M-profile lazy FP state preservation > from full_vfp_access_check(); accesses to the FPCXT_NS register are > a special case which need to do just this part (corresponding in the > pseudocode to the PreserveFPState() function), and not the full > set of actions matching the pseudocode ExecuteFPCheck() which > normal FP instructions need to do. > > Signed-off-by: Peter Maydell > --- > target/arm/translate-vfp.c.inc | 45 -- > 1 file changed, 27 insertions(+), 18 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 11/15] target/arm: Use new FPCR_NZCV_MASK constant
On 11/16/20 8:08 AM, Peter Maydell wrote: > We defined a constant name for the mask of NZCV bits in the FPCR/FPSCR > in the previous commit; use it in a couple of places in existing code, > where we're masking out everything except NZCV for the "load to Rt=15 > sets CPSR.NZCV" special case. > > Signed-off-by: Peter Maydell > --- > target/arm/translate-vfp.c.inc | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH for-5.2] s390x/pci: fix endianness issues
On 11/17/20 2:21 PM, Thomas Huth wrote:
On 17/11/2020 18.13, Cornelia Huck wrote:
zPCI control blocks are big endian, we need to take care that we
do proper accesses in order not to break tcg guests on little endian
hosts.
Fixes: 28dc86a07299 ("s390x/pci: use a PCI Group structure")
Fixes: 9670ee752727 ("s390x/pci: use a PCI Function structure")
Fixes: 1e7552ff5c34 ("s390x/pci: get zPCI function info from host")
This fixes the problem with my old Fedora 28 under TCG, too, but... do we
really want to store this information in big endian on the QEMU side (e.g.
in the QTAILQ lists)? ... that smells like trouble again in the future...
I think it would be better if we rather replace all those memcpy() functions
in the patches that you cited in the "Fixes:" lines above with code that
changes the endianess when we copy the date from QEMU space to guest space
and vice versa. What do you think?
Hmm, that's actually a fair point... Such an approach would have the
advantage of avoiding weird lines like the following:
memory_region_add_subregion(&pbdev->iommu->mr,
-pbdev->pci_group->zpci_group.msia,
+ldq_p(&pbdev->pci_group->zpci_group.msia),
&pbdev->msix_notify_mr);
And would keep messing with endianness largely contained to the code
that handles CLPs. It does take away the niceness of being able to
gather the CLP response in one fell memcpy but... It's not like these
are done very often (device init).
Re: [PATCH v2 02/16] qapi/expr.py: Check for dict instead of OrderedDict
On 11/17/20 1:08 PM, Eduardo Habkost wrote: On Tue, Nov 17, 2020 at 05:30:04PM +0100, Markus Armbruster wrote: John Snow writes: OrderedDict is a subtype of dict, so we can check for a more general form. Signed-off-by: John Snow Reviewed-by: Eduardo Habkost Reviewed-by: Cleber Rosa --- scripts/qapi/expr.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index 35695c4c653b..5694c501fa38 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -14,7 +14,6 @@ # This work is licensed under the terms of the GNU GPL, version 2. # See the COPYING file in the top-level directory. -from collections import OrderedDict import re from .common import c_name @@ -131,7 +130,7 @@ def check_if_str(ifcond): def normalize_members(members): -if isinstance(members, OrderedDict): +if isinstance(members, dict): for key, arg in members.items(): if isinstance(arg, dict): continue @@ -162,7 +161,7 @@ def check_type(value, info, source, if not allow_dict: raise QAPISemError(info, "%s should be a type name" % source) -if not isinstance(value, OrderedDict): +if not isinstance(value, dict): raise QAPISemError(info, "%s should be an object or type name" % source) Plain dict remembers insertion order since Python 3.6, but it wasn't formally promised until 3.7. Can we simply ditch OrderedDict entirely? In theory, our build requirement is "Python >= 3.6", not "CPython >= 3.6". In practice, I don't expect anybody to ever use any other Python implementation except CPython to build QEMU. I think we can get rid of OrderedDict if you really want to. No harm in keeping it right now either, it doesn't make typing harder. The OrderedDict is created in the parser, so we can cover ditching OrderedDict when we get to that module if desired. --js
Re: [PATCH 08/15] target/arm: Move general-use constant expanders up in translate.c
On 11/16/20 8:08 AM, Peter Maydell wrote: > The constant-expander functions like negate, plus_2, etc, are > generally useful; move them up in translate.c so we can use them in > the VFP/Neon decoders as well as in the A32/T32/T16 decoders. > > Signed-off-by: Peter Maydell > --- > target/arm/translate.c | 46 +++--- > 1 file changed, 25 insertions(+), 21 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 06/15] target/arm: Enforce M-profile VMRS/VMSR register restrictions
On 11/16/20 8:08 AM, Peter Maydell wrote:
> -if (a->rt == 15 && (!a->l || a->reg != ARM_VFP_FPSCR)) {
> +if (a->reg != ARM_VFP_FPSCR) {
> +return false;
> +}
> +if (a->rt == 15 && !a->l) {
Alternately, the parenthesis are just off:
if ((a->rt == 15 && !a->l) || a->reg != ARM_VFP_FPSCR)
Either way,
Reviewed-by: Richard Henderson
r~
[PATCH v3] virtiofsd: Use --thread-pool-size=0 to mean no thread pool
This is V3 of the patch. A minor change since V2 is to reverse the list
before executing requests. We are prepending elements to the list and that
means when we start processing requests, we are processing these in
the reverse order. Though we don't guarantee any ordering but it does
not hurt to process requests in same order as received as much as
possible.
Right now we create a thread pool and main thread hands over the request
to thread in thread pool to process. Number of threads in thread pool
can be managed by option --thread-pool-size.
In tests we have noted that many of the workloads are getting better
performance if we don't use a thread pool at all and process all
the requests in the context of a thread receiving the request.
Hence give user an option to be able to run virtiofsd without using
a thread pool.
To implement this, I have used existing option --thread-pool-size. This
option defines how many maximum threads can be in the thread pool.
Thread pool size zero freezes thead pool. I can't see why will one
start virtiofsd with a frozen thread pool (hence frozen file system).
So I am redefining --thread-pool-size=0 to mean, don't use a thread pool.
Instead process the request in the context of thread receiving request
from the queue.
Signed-off-by: Vivek Goyal
---
tools/virtiofsd/fuse_virtio.c | 37 ++-
1 file changed, 28 insertions(+), 9 deletions(-)
diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
index 83ba07c6cd..6c56e606ef 100644
--- a/tools/virtiofsd/fuse_virtio.c
+++ b/tools/virtiofsd/fuse_virtio.c
@@ -588,13 +588,18 @@ static void *fv_queue_thread(void *opaque)
struct VuDev *dev = &qi->virtio_dev->dev;
struct VuVirtq *q = vu_get_queue(dev, qi->qidx);
struct fuse_session *se = qi->virtio_dev->se;
-GThreadPool *pool;
-
-pool = g_thread_pool_new(fv_queue_worker, qi, se->thread_pool_size, FALSE,
- NULL);
-if (!pool) {
-fuse_log(FUSE_LOG_ERR, "%s: g_thread_pool_new failed\n", __func__);
-return NULL;
+GThreadPool *pool = NULL;
+GList *req_list = NULL;
+
+if (se->thread_pool_size) {
+fuse_log(FUSE_LOG_DEBUG, "%s: Creating thread pool for Queue %d\n",
+ __func__, qi->qidx);
+pool = g_thread_pool_new(fv_queue_worker, qi, se->thread_pool_size,
+ FALSE, NULL);
+if (!pool) {
+fuse_log(FUSE_LOG_ERR, "%s: g_thread_pool_new failed\n", __func__);
+return NULL;
+}
}
fuse_log(FUSE_LOG_INFO, "%s: Start for queue %d kick_fd %d\n", __func__,
@@ -669,14 +674,28 @@ static void *fv_queue_thread(void *opaque)
req->reply_sent = false;
-g_thread_pool_push(pool, req, NULL);
+if (!se->thread_pool_size) {
+req_list = g_list_prepend(req_list, req);
+} else {
+g_thread_pool_push(pool, req, NULL);
+}
}
pthread_mutex_unlock(&qi->vq_lock);
pthread_rwlock_unlock(&qi->virtio_dev->vu_dispatch_rwlock);
+
+/* Process all the requests. */
+if (!se->thread_pool_size && req_list != NULL) {
+req_list = g_list_reverse(req_list);
+g_list_foreach(req_list, fv_queue_worker, qi);
+g_list_free(req_list);
+req_list = NULL;
+}
}
-g_thread_pool_free(pool, FALSE, TRUE);
+if (pool) {
+g_thread_pool_free(pool, FALSE, TRUE);
+}
return NULL;
}
--
2.25.4
[PATCH for-5.2] docs: Fix some typos (found by codespell)
Fix also a similar typo in a code comment. Signed-off-by: Stefan Weil --- docs/can.txt | 8 docs/interop/vhost-user.rst | 2 +- docs/replay.txt | 2 +- docs/specs/ppc-spapr-numa.rst | 2 +- docs/system/deprecated.rst| 4 ++-- docs/tools/virtiofsd.rst | 2 +- hw/vfio/igd.c | 2 +- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/can.txt b/docs/can.txt index 5838f6620c..0d310237df 100644 --- a/docs/can.txt +++ b/docs/can.txt @@ -19,7 +19,7 @@ interface to implement because such device can be easily connected to systems with different CPU architectures (x86, PowerPC, Arm, etc.). In 2020, CTU CAN FD controller model has been added as part -of the bachelor theses of Jan Charvat. This controller is complete +of the bachelor thesis of Jan Charvat. This controller is complete open-source/design/hardware solution. The core designer of the project is Ondrej Ille, the financial support has been provided by CTU, and more companies including Volkswagen subsidiaries. @@ -31,7 +31,7 @@ testing lead to goal change to provide environment which provides complete emulated environment for testing and RTEMS GSoC slot has been donated to work on CAN hardware emulation on QEMU. -Examples how to use CAN emulation for SJA1000 based borads +Examples how to use CAN emulation for SJA1000 based boards == When QEMU with CAN PCI support is compiled then one of the next @@ -106,8 +106,8 @@ This open-source core provides CAN FD support. CAN FD drames are delivered even to the host systems when SocketCAN interface is found CAN FD capable. -The PCIe borad emulation is provided for now (the device identifier is -ctucan_pci). The defauld build defines two CTU CAN FD cores +The PCIe board emulation is provided for now (the device identifier is +ctucan_pci). The default build defines two CTU CAN FD cores on the board. Example how to connect the canbus0-bus (virtual wire) to the host diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst index 988f154144..72b2e8c7ba 100644 --- a/docs/interop/vhost-user.rst +++ b/docs/interop/vhost-user.rst @@ -513,7 +513,7 @@ descriptor table (split virtqueue) or descriptor ring (packed virtqueue). However, it can't work when we process descriptors out-of-order because some entries which store the information of inflight descriptors in available ring (split virtqueue) or descriptor -ring (packed virtqueue) might be overrided by new entries. To solve +ring (packed virtqueue) might be overridden by new entries. To solve this problem, slave need to allocate an extra buffer to store this information of inflight descriptors and share it with master for persistent. ``VHOST_USER_GET_INFLIGHT_FD`` and diff --git a/docs/replay.txt b/docs/replay.txt index 87a64ae068..5b008ca491 100644 --- a/docs/replay.txt +++ b/docs/replay.txt @@ -328,7 +328,7 @@ between the snapshots. Each of the passes include the following steps: 1. loading the snapshot 2. replaying to examine the breakpoints 3. if breakpoint or watchpoint was met -- loading the snaphot again +- loading the snapshot again - replaying to the required breakpoint 4. else - proceeding to the p.1 with the earlier snapshot diff --git a/docs/specs/ppc-spapr-numa.rst b/docs/specs/ppc-spapr-numa.rst index 5fca2bdd8e..ffa687dc89 100644 --- a/docs/specs/ppc-spapr-numa.rst +++ b/docs/specs/ppc-spapr-numa.rst @@ -198,7 +198,7 @@ This is how it is being done: * user distance 121 and beyond will be interpreted as 160 * user distance 10 stays 10 -The reasoning behind this aproximation is to avoid any round up to the local +The reasoning behind this approximation is to avoid any round up to the local distance (10), keeping it exclusive to the 4th NUMA level (which is still exclusive to the node_id). All other ranges were chosen under the developer discretion of what would be (somewhat) sensible considering the user input. diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 32a0e620db..63e9db1463 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -465,7 +465,7 @@ default configuration. The CPU model runnability guarantee won't apply anymore to existing CPU models. Management software that needs runnability -guarantees must resolve the CPU model aliases using te +guarantees must resolve the CPU model aliases using the ``alias-of`` field returned by the ``query-cpu-definitions`` QMP command. @@ -637,7 +637,7 @@ Splitting RAM by default between NUMA nodes had the same issues as ``mem`` parameter with the difference that the role of the user plays QEMU using implicit generic or board specific splitting rule. Use ``memdev`` with *memory-backend-ram* backend or ``mem`` (if -it's supported by used machine type) to define mapping explictly instead. +it's supported by used machine type) to define mapping expl
Re: [PATCH 05/15] target/arm: Implement CLRM instruction
On 11/16/20 8:08 AM, Peter Maydell wrote: > In v8.1M the new CLRM instruction allows zeroing an arbitrary set of > the general-purpose registers and APSR. Implement this. > > The encoding is a subset of the LDMIA T2 encoding, using what would > be Rn=0b (which UNDEFs for LDMIA). > > Signed-off-by: Peter Maydell > --- > target/arm/t32.decode | 6 +- > target/arm/translate.c | 38 ++ > 2 files changed, 43 insertions(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v4 2/2] arm64: kvm: Introduce MTE VCPU feature
Hi Steven,
On 2020-10-26 15:57, Steven Price wrote:
Add a new VM feature 'KVM_ARM_CAP_MTE' which enables memory tagging
for a VM. This exposes the feature to the guest and automatically tags
memory pages touched by the VM as PG_mte_tagged (and clears the tags
storage) to ensure that the guest cannot see stale tags, and so that
the
tags are correctly saved/restored across swap.
Signed-off-by: Steven Price
Reviewed-by: Andrew Jones
---
arch/arm64/include/asm/kvm_emulate.h | 3 +++
arch/arm64/include/asm/kvm_host.h| 3 +++
arch/arm64/kvm/arm.c | 9 +
arch/arm64/kvm/mmu.c | 20
arch/arm64/kvm/sys_regs.c| 6 +-
include/uapi/linux/kvm.h | 1 +
6 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/kvm_emulate.h
b/arch/arm64/include/asm/kvm_emulate.h
index 5ef2669ccd6c..66c0d9e7c2b4 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -79,6 +79,9 @@ static inline void vcpu_reset_hcr(struct kvm_vcpu
*vcpu)
if (cpus_have_const_cap(ARM64_MISMATCHED_CACHE_TYPE) ||
vcpu_el1_is_32bit(vcpu))
vcpu->arch.hcr_el2 |= HCR_TID2;
+
+ if (vcpu->kvm->arch.mte_enabled)
Please add a predicate (vcpu_has_mte() or kvm_has_mte()?) for this.
+ vcpu->arch.hcr_el2 |= HCR_ATA;
}
static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/include/asm/kvm_host.h
b/arch/arm64/include/asm/kvm_host.h
index 95ab7345dcc8..cd993aec0440 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -118,6 +118,9 @@ struct kvm_arch {
*/
unsigned long *pmu_filter;
unsigned int pmuver;
+
+ /* Memory Tagging Extension enabled for the guest */
+ bool mte_enabled;
};
struct kvm_vcpu_fault_info {
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index f56122eedffc..7ee93bcac017 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -89,6 +89,12 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
r = 0;
kvm->arch.return_nisv_io_abort_to_user = true;
break;
+ case KVM_CAP_ARM_MTE:
+ if (!system_supports_mte() || kvm->created_vcpus)
+ return -EINVAL;
You also want to avoid 32bit guests. Also, what is the rational for
this being a VM capability as opposed to a CPU feature, similar
to SVE and PMU?
+ r = 0;
+ kvm->arch.mte_enabled = true;
+ break;
default:
r = -EINVAL;
break;
@@ -210,6 +216,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm,
long ext)
*/
r = 1;
break;
+ case KVM_CAP_ARM_MTE:
+ r = system_supports_mte();
Same comment about 32bit.
+ break;
case KVM_CAP_STEAL_TIME:
r = kvm_arm_pvtime_supported();
break;
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 19aacc7d64de..38fe25310ca1 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -862,6 +862,26 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
phys_addr_t fault_ipa,
if (vma_pagesize == PAGE_SIZE && !force_pte)
vma_pagesize = transparent_hugepage_adjust(memslot, hva,
&pfn, &fault_ipa);
+
+ /*
+* The otherwise redundant test for system_supports_mte() allows the
+* code to be compiled out when CONFIG_ARM64_MTE is not present.
+*/
+ if (system_supports_mte() && kvm->arch.mte_enabled && pfn_valid(pfn))
{
+ /*
+* VM will be able to see the page's tags, so we must ensure
+* they have been initialised.
+*/
+ struct page *page = pfn_to_page(pfn);
+ long i, nr_pages = compound_nr(page);
+
+ /* if PG_mte_tagged is set, tags have already been initialised
*/
+ for (i = 0; i < nr_pages; i++, page++) {
+ if (!test_and_set_bit(PG_mte_tagged, &page->flags))
+ mte_clear_page_tags(page_address(page));
+ }
+ }
What are the visibility requirements for the tags, specially if the
guest has its MMU off? Is there any cache management that needs to
occur?
Another thing is device-like memory that is managed by userspace,
such as the QEMU emulated flash, for which there also might be tags.
How is that dealt with? In general, what are the expectations for
tags on memory shared between host and guest? Who owns them?
+
if (writable) {
prot |= KVM_PGTABLE_PROT_W;
kvm_set_pfn_dirty(pfn);
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 430e36e1a13d..35a3dc448231 100644
--- a/arch/arm64/kvm
Re: [PATCH 04/15] target/arm: Implement VSCCLRM insn
On 11/16/20 8:08 AM, Peter Maydell wrote:
> +aspen = load_cpu_field(v7m.fpccr[M_REG_S]);
> +sfpa = load_cpu_field(v7m.control[M_REG_S]);
> +tcg_gen_andi_i32(aspen, aspen, R_V7M_FPCCR_ASPEN_MASK);
> +tcg_gen_subi_i32(aspen, aspen, R_V7M_FPCCR_ASPEN_MASK);
xori would be clearer, i think.
> +/* Zero the Sregs from btmreg to topreg inclusive. */
> +zero64 = tcg_const_i64(0);
> +zero32 = tcg_const_i32(0);
> +if (btmreg & 1) {
> +write_neon_element32(zero32, btmreg >> 1, 1, MO_32);
> +btmreg++;
> +}
> +for (; btmreg + 1 <= topreg; btmreg += 2) {
> +write_neon_element64(zero64, btmreg >> 1, 0, MO_64);
> +}
> +if (btmreg == topreg) {
> +write_neon_element32(zero32, btmreg >> 1, 0, MO_32);
> +btmreg++;
> +}
I hadn't implemented MO_32 for write_neon_element64 because there were no
users. Better to just add the case there using tcg_gen_st32_i64, then you
don't need a 32-bit zero.
Otherwise,
Reviewed-by: Richard Henderson
r~
Re: [PATCH for-5.2] s390x/pci: fix endianness issues
On 17/11/2020 19.49, Philippe Mathieu-Daudé wrote:
> On 11/17/20 7:39 PM, Thomas Huth wrote:
>> On 17/11/2020 19.30, Philippe Mathieu-Daudé wrote:
>>> On 11/17/20 7:19 PM, Matthew Rosato wrote:
On 11/17/20 12:59 PM, Philippe Mathieu-Daudé wrote:
> On 11/17/20 6:13 PM, Cornelia Huck wrote:
>> zPCI control blocks are big endian, we need to take care that we
>> do proper accesses in order not to break tcg guests on little endian
>> hosts.
>>
>> Fixes: 28dc86a07299 ("s390x/pci: use a PCI Group structure")
>> Fixes: 9670ee752727 ("s390x/pci: use a PCI Function structure")
>> Fixes: 1e7552ff5c34 ("s390x/pci: get zPCI function info from host")
>> Signed-off-by: Cornelia Huck
>> ---
>>
>> Works for me with virtio-pci devices for tcg on x86 and s390x, and
>> for kvm.
>> The vfio changes are not strictly needed; did not test them due to
>> lack of
>> hardware -- testing appreciated. >>
>> As this fixes a regression, I want this in 5.2.
>>
>> ---
>> hw/s390x/s390-pci-bus.c | 12 ++--
>> hw/s390x/s390-pci-inst.c | 4 ++--
>> hw/s390x/s390-pci-vfio.c | 8
>> 3 files changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
>> index e0dc20ce4a56..17e64e0b1200 100644
>> --- a/hw/s390x/s390-pci-bus.c
>> +++ b/hw/s390x/s390-pci-bus.c
>> @@ -787,12 +787,12 @@ static void s390_pci_init_default_group(void)
>> static void set_pbdev_info(S390PCIBusDevice *pbdev)
>> {
>> - pbdev->zpci_fn.sdma = ZPCI_SDMA_ADDR;
>> - pbdev->zpci_fn.edma = ZPCI_EDMA_ADDR;
>> - pbdev->zpci_fn.pchid = 0;
>> + stq_p(&pbdev->zpci_fn.sdma, ZPCI_SDMA_ADDR);
>
> "zPCI control blocks are big endian" so don't we
> need the _be_ accessors? stq_be_p() etc...
>
I don't think this is necessary. This is only available for target
s390x, which is always big endian... cpu-all.h should define stq_p as
stq_be_p for example inside the #if defined(TARGET_WORDS_BIGENDIAN).
>>>
>>> But if you run on little-endian host, you need to byte-swap that,
>>> isn't it?
>>
>> It's done by the macros. They depend on the target endianess. See cpu-all.h.
>
> I'm confused because the description is about target endianness,
> but stq_p() is about host alignment.
stq_p() is apparently also about endianess. Why would it depend on
TARGET_WORDS_BIGENDIAN otherwise?
Thomas
Re: [PATCH for-5.2] s390x/pci: fix endianness issues
On 17/11/2020 18.13, Cornelia Huck wrote:
> zPCI control blocks are big endian, we need to take care that we
> do proper accesses in order not to break tcg guests on little endian
> hosts.
>
> Fixes: 28dc86a07299 ("s390x/pci: use a PCI Group structure")
> Fixes: 9670ee752727 ("s390x/pci: use a PCI Function structure")
> Fixes: 1e7552ff5c34 ("s390x/pci: get zPCI function info from host")
This fixes the problem with my old Fedora 28 under TCG, too, but... do we
really want to store this information in big endian on the QEMU side (e.g.
in the QTAILQ lists)? ... that smells like trouble again in the future...
I think it would be better if we rather replace all those memcpy() functions
in the patches that you cited in the "Fixes:" lines above with code that
changes the endianess when we copy the date from QEMU space to guest space
and vice versa. What do you think?
Thomas
Re: [PATCH for-5.2] s390x/pci: fix endianness issues
On 11/17/20 12:13 PM, Cornelia Huck wrote:
zPCI control blocks are big endian, we need to take care that we
do proper accesses in order not to break tcg guests on little endian
hosts.
Fixes: 28dc86a07299 ("s390x/pci: use a PCI Group structure")
Fixes: 9670ee752727 ("s390x/pci: use a PCI Function structure")
Fixes: 1e7552ff5c34 ("s390x/pci: get zPCI function info from host")
Signed-off-by: Cornelia Huck
---
Works for me with virtio-pci devices for tcg on x86 and s390x, and for kvm.
The vfio changes are not strictly needed; did not test them due to lack of
hardware -- testing appreciated.
As this fixes a regression, I want this in 5.2.
---
hw/s390x/s390-pci-bus.c | 12 ++--
hw/s390x/s390-pci-inst.c | 4 ++--
hw/s390x/s390-pci-vfio.c | 8
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index e0dc20ce4a56..17e64e0b1200 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -787,12 +787,12 @@ static void s390_pci_init_default_group(void)
static void set_pbdev_info(S390PCIBusDevice *pbdev)
{
-pbdev->zpci_fn.sdma = ZPCI_SDMA_ADDR;
-pbdev->zpci_fn.edma = ZPCI_EDMA_ADDR;
-pbdev->zpci_fn.pchid = 0;
+stq_p(&pbdev->zpci_fn.sdma, ZPCI_SDMA_ADDR);
+stq_p(&pbdev->zpci_fn.edma, ZPCI_EDMA_ADDR);
+stw_p(&pbdev->zpci_fn.pchid, 0);
pbdev->zpci_fn.pfgid = ZPCI_DEFAULT_FN_GRP;
-pbdev->zpci_fn.fid = pbdev->fid;
-pbdev->zpci_fn.uid = pbdev->uid;
+stl_p(&pbdev->zpci_fn.fid, pbdev->fid);
+stl_p(&pbdev->zpci_fn.uid, pbdev->uid);
pbdev->pci_group = s390_group_find(ZPCI_DEFAULT_FN_GRP);
}
@@ -871,7 +871,7 @@ static int s390_pci_msix_init(S390PCIBusDevice *pbdev)
memory_region_init_io(&pbdev->msix_notify_mr, OBJECT(pbdev),
&s390_msi_ctrl_ops, pbdev, name, PAGE_SIZE);
memory_region_add_subregion(&pbdev->iommu->mr,
-pbdev->pci_group->zpci_group.msia,
+ldq_p(&pbdev->pci_group->zpci_group.msia),
&pbdev->msix_notify_mr);
g_free(name);
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 58cd041d17fb..7bc6b79f10ce 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -305,7 +305,7 @@ int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra)
ClpReqQueryPciGrp *reqgrp = (ClpReqQueryPciGrp *)reqh;
S390PCIGroup *group;
-group = s390_group_find(reqgrp->g);
+group = s390_group_find(ldl_p(&reqgrp->g));
So as I alluded to in my other email, this should really be:
+group = s390_group_find(ldl_p(&reqgrp->g) &
CLP_REQ_QPCIG_MASK_PFGID);
To ensure that only the 8b pfgid is used from the 32b 'g' - doing it
this way ensure we've already accounted for endianness. The other 24b
are reserved 0s so that's why things are working anyway without this
mask. If you'd prefer to split this change off, we can do that too.
I took this for a spin (with AND without that 1-liner change) with
vfio-pci, driving network and disk workloads using a fairly recent
(5.10-rc3) kernel in host/guest. I also rolled back the host to an
older kernel (so, no hardware capabilities from vfio) to drive the
default clp paths against vfio -- Everything works fine there. I also
tested (with AND without that 1-liner change) against a tcg guest on x86
using a virtio-blk-pci device.
So either way, thank you and:
Reviewed-by: Matthew Rosato
if (!group) {
/* We do not allow access to unknown groups */
/* The group must have been obtained with a vfio device */
@@ -788,7 +788,7 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t
r3, uint64_t gaddr,
/* Length must be greater than 8, a multiple of 8 */
/* and not greater than maxstbl */
if ((len <= 8) || (len % 8) ||
-(len > pbdev->pci_group->zpci_group.maxstbl)) {
+(len > lduw_p(&pbdev->pci_group->zpci_group.maxstbl))) {
goto specification_error;
}
/* Do not cross a 4K-byte boundary */
diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c
index d5c78063b5bc..f455c6f20a1a 100644
--- a/hw/s390x/s390-pci-vfio.c
+++ b/hw/s390x/s390-pci-vfio.c
@@ -116,10 +116,10 @@ static void s390_pci_read_base(S390PCIBusDevice *pbdev,
}
cap = (void *) hdr;
-pbdev->zpci_fn.sdma = cap->start_dma;
-pbdev->zpci_fn.edma = cap->end_dma;
-pbdev->zpci_fn.pchid = cap->pchid;
-pbdev->zpci_fn.vfn = cap->vfn;
+stq_p(&pbdev->zpci_fn.sdma, cap->start_dma);
+stq_p(&pbdev->zpci_fn.edma, cap->end_dma);
+stw_p(&pbdev->zpci_fn.pchid, cap->pchid);
+stw_p(&pbdev->zpci_fn.vfn, cap->vfn);
pbdev->zpci_fn.pfgid = cap->gid;
/* The following values remain 0 until we support other FMB formats */
pbdev->zpci_fn.fmbl = 0;
Re: [PATCH v4 1/2] arm64: kvm: Save/restore MTE registers
Hi Steven,
These patches unfortunately don't apply to -rc4 anymore, as we repainted
quite a bit while working on fixes. I'd be grateful if you could rebase
them.
A few other things though:
On 2020-10-26 15:57, Steven Price wrote:
Define the new system registers that MTE introduces and context switch
them. The MTE feature is still hidden from the ID register as it isn't
supported in a VM yet.
Signed-off-by: Steven Price
Reviewed-by: Andrew Jones
---
arch/arm64/include/asm/kvm_host.h | 4
arch/arm64/include/asm/sysreg.h| 3 ++-
arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 14 ++
arch/arm64/kvm/sys_regs.c | 14 ++
4 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h
b/arch/arm64/include/asm/kvm_host.h
index 0aecbab6a7fb..95ab7345dcc8 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -134,6 +134,8 @@ enum vcpu_sysreg {
SCTLR_EL1, /* System Control Register */
ACTLR_EL1, /* Auxiliary Control Register */
CPACR_EL1, /* Coprocessor Access Control */
+ RGSR_EL1, /* Random Allocation Tag Seed Register */
+ GCR_EL1,/* Tag Control Register */
ZCR_EL1,/* SVE Control */
TTBR0_EL1, /* Translation Table Base Register 0 */
TTBR1_EL1, /* Translation Table Base Register 1 */
@@ -150,6 +152,8 @@ enum vcpu_sysreg {
TPIDR_EL1, /* Thread ID, Privileged */
AMAIR_EL1, /* Aux Memory Attribute Indirection Register */
CNTKCTL_EL1,/* Timer Control Register (EL1) */
+ TFSRE0_EL1, /* Tag Fault Status Register (EL0) */
+ TFSR_EL1, /* Tag Fault Stauts Register (EL1) */
PAR_EL1,/* Physical Address Register */
MDSCR_EL1, /* Monitor Debug System Control Register */
MDCCINT_EL1,/* Monitor Debug Comms Channel Interrupt Enable Reg */
diff --git a/arch/arm64/include/asm/sysreg.h
b/arch/arm64/include/asm/sysreg.h
index d52c1b3ce589..7727df0bc09d 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -565,7 +565,8 @@
#define SCTLR_ELx_M(BIT(0))
#define SCTLR_ELx_FLAGS(SCTLR_ELx_M | SCTLR_ELx_A | SCTLR_ELx_C | \
-SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB)
+SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB | \
+SCTLR_ELx_ITFSB)
/* SCTLR_EL2 specific flags. */
#define SCTLR_EL2_RES1 ((BIT(4)) | (BIT(5)) | (BIT(11)) | (BIT(16))
| \
diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
index 7a986030145f..a124ffa49ba3 100644
--- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
+++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
@@ -18,6 +18,11 @@
static inline void __sysreg_save_common_state(struct kvm_cpu_context
*ctxt)
{
ctxt_sys_reg(ctxt, MDSCR_EL1) = read_sysreg(mdscr_el1);
+ if (system_supports_mte()) {
+ ctxt_sys_reg(ctxt, RGSR_EL1)= read_sysreg_s(SYS_RGSR_EL1);
+ ctxt_sys_reg(ctxt, GCR_EL1) = read_sysreg_s(SYS_GCR_EL1);
+ ctxt_sys_reg(ctxt, TFSRE0_EL1) = read_sysreg_s(SYS_TFSRE0_EL1);
As far as I can tell, HCR_EL2.ATA is still clear when running a guest.
So why, do we save/restore this state yet?
Also, I wonder whether we should keep these in the C code. If one day
we enable MTE in the kernel, we will have to move them to the assembly
part, much like we do for PAuth. And I fear that "one day" is pretty
soon:
https://lore.kernel.org/linux-arm-kernel/cover.1605046192.git.andreyk...@google.com/
+ }
}
static inline void __sysreg_save_user_state(struct kvm_cpu_context
*ctxt)
@@ -45,6 +50,8 @@ static inline void __sysreg_save_el1_state(struct
kvm_cpu_context *ctxt)
ctxt_sys_reg(ctxt, CNTKCTL_EL1) = read_sysreg_el1(SYS_CNTKCTL);
ctxt_sys_reg(ctxt, PAR_EL1) = read_sysreg(par_el1);
ctxt_sys_reg(ctxt, TPIDR_EL1) = read_sysreg(tpidr_el1);
+ if (system_supports_mte())
+ ctxt_sys_reg(ctxt, TFSR_EL1) = read_sysreg_el1(SYS_TFSR);
ctxt_sys_reg(ctxt, SP_EL1) = read_sysreg(sp_el1);
ctxt_sys_reg(ctxt, ELR_EL1) = read_sysreg_el1(SYS_ELR);
@@ -63,6 +70,11 @@ static inline void
__sysreg_save_el2_return_state(struct kvm_cpu_context *ctxt)
static inline void __sysreg_restore_common_state(struct
kvm_cpu_context *ctxt)
{
write_sysreg(ctxt_sys_reg(ctxt, MDSCR_EL1), mdscr_el1);
+ if (system_supports_mte()) {
+ write_sysreg_s(ctxt_sys_reg(ctxt, RGSR_EL1), SYS_RGSR_EL1);
+ write_sysreg_s(ctxt_sys_reg(ctxt, GCR_EL1), SYS_GCR_EL1);
+ write_sysreg_s(ctxt_sys_reg(ctxt, TFSRE0_EL1), SYS_TFSRE0_EL1);
+ }
}
static inline void __sysreg_restore_user_state(struct kvm_cpu_context
*ctxt)
@@ -106,6 +118,8 @@ static inline void
_
Re: [PATCH 00/10] vhost/qemu: thread per IO SCSI vq
On 11/17/20 10:40 AM, Stefan Hajnoczi wrote: > On Thu, Nov 12, 2020 at 05:18:59PM -0600, Mike Christie wrote: >> The following kernel patches were made over Michael's vhost branch: >> >> https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/log/?h=vhost >> >> and the vhost-scsi bug fix patchset: >> >> https://lore.kernel.org/linux-scsi/20201112170008.GB1555653@stefanha-x1.localdomain/T/#t >> >> And the qemu patch was made over the qemu master branch. >> >> vhost-scsi currently supports multiple queues with the num_queues >> setting, but we end up with a setup where the guest's scsi/block >> layer can do a queue per vCPU and the layers below vhost can do >> a queue per CPU. vhost-scsi will then do a num_queue virtqueues, >> but all IO gets set on and completed on a single vhost-scsi thread. >> After 2 - 4 vqs this becomes a bottleneck. >> >> This patchset allows us to create a worker thread per IO vq, so we >> can better utilize multiple CPUs with the multiple queues. It >> implments Jason's suggestion to create the initial worker like >> normal, then create the extra workers for IO vqs with the >> VHOST_SET_VRING_ENABLE ioctl command added in this patchset. > > How does userspace find out the tids and set their CPU affinity? > When we create the worker thread we add it to the device owner's cgroup, so we end up inheriting those settings like affinity. However, are you more asking about finer control like if the guest is doing mq, and the mq hw queue is bound to cpu0, it would perform better if we could bind vhost vq's worker thread to cpu0? I think the problem might is if you are in the cgroup then we can't set a specific threads CPU affinity to just one specific CPU. So you can either do cgroups or not. > What is the meaning of the new VHOST_SET_VRING_ENABLE ioctl? It doesn't > really "enable" or "disable" the vq, requests are processed regardless. > Yeah, I agree. The problem I've mentioned before is: 1. For net and vsock, it's not useful because the vqs are hard coded in the kernel and userspace, so you can't disable a vq and you never need to enable one. 2. vdpa has it's own enable ioctl. 3. For scsi, because we already are doing multiple vqs based on the num_queues value, we have to have some sort of compat support and code to detect if userspace is even going to send the new ioctl. In this patchset, compat just meant enable/disable the extra functionality of extra worker threads for a vq. We will still use the vq if userspace set it up. > The purpose of the ioctl isn't clear to me because the kernel could > automatically create 1 thread per vq without a new ioctl. On the other > hand, if userspace is supposed to control worker threads then a > different interface would be more powerful: > My preference has been: 1. If we were to ditch cgroups, then add a new interface that would allow us to bind threads to a specific CPU, so that it lines up with the guest's mq to CPU mapping. 2. If we continue with cgroups then I think just creating the worker threads from vhost_scsi_set_endpoint is best, because that is the point we do the other final vq setup ops vhost_vq_set_backend and vhost_vq_init_access. For option number 2 it would be simple. Instead of the vring enable patches: [PATCH 08/10] vhost: move msg_handler to new ops struct [PATCH 09/10] vhost: add VHOST_SET_VRING_ENABLE support [PATCH 10/10] vhost-scsi: create a woker per IO vq and [PATCH 1/1] qemu vhost scsi: add VHOST_SET_VRING_ENABLE support we could do this patch like I had done in previous versions: From bcc4c29c28daf04679ce6566d06845b9e1b31eb4 Mon Sep 17 00:00:00 2001 From: Mike Christie Date: Wed, 11 Nov 2020 22:50:56 -0600 Subject:
Re: [PATCH 03/15] target/arm: Don't clobber ID_PFR1.Security on M-profile cores
On 11/16/20 8:08 AM, Peter Maydell wrote: > In arm_cpu_realizefn() we check whether the board code disabled EL3 > via the has_el3 CPU object property, which we create if the CPU > starts with the ARM_FEATURE_EL3 feature bit. If it is disabled, then > we turn off ARM_FEATURE_EL3 and also zero out the relevant fields in > the ID_PFR1 and ID_AA64PFR0 registers. > > This codepath was incorrectly being taken for M-profile CPUs, which > do not have an EL3 and don't set ARM_FEATURE_EL3, but which may have > the M-profile Security extension and so should have non-zero values > in the ID_PFR1.Security field. > > Restrict the handling of the feature flag to A/R-profile cores. > > Signed-off-by: Peter Maydell > --- > target/arm/cpu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 02/15] target/arm: Implement v8.1M PXN extension
On 11/16/20 8:08 AM, Peter Maydell wrote: > In v8.1M the PXN architecture extension adds a new PXN bit to the > MPU_RLAR registers, which forbids execution of code in the region > from a privileged mode. > > This is another feature which is just in the generic "in v8.1M" set > and has no ID register field indicating its presence. > > Signed-off-by: Peter Maydell > --- > target/arm/helper.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
[PATCH for-5.2] meson: Fix argument for makensis (build regression)
`make installer` with a DLL directory was broken.
Signed-off-by: Stefan Weil
---
scripts/nsis.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/nsis.py b/scripts/nsis.py
index e1c409344e..5135a05831 100644
--- a/scripts/nsis.py
+++ b/scripts/nsis.py
@@ -65,7 +65,7 @@ def main():
dlldir = "w64"
makensis += ["-DW64"]
if os.path.exists(os.path.join(args.srcdir, "dll")):
-makensis += "-DDLLDIR={0}/dll/{1}".format(args.srcdir, dlldir)
+makensis += ["-DDLLDIR={0}/dll/{1}".format(args.srcdir, dlldir)]
makensis += ["-DOUTFILE=" + args.outfile] + args.nsisargs
subprocess.run(makensis)
--
2.29.2
Re: [PATCH 01/15] hw/intc/armv7m_nvic: Make all of system PPB range be RAZWI/BusFault
On 11/16/20 8:08 AM, Peter Maydell wrote: > For M-profile CPUs, the range from 0xe000 to 0xe00f is the > Private Peripheral Bus range, which includes all of the memory mapped > devices and registers that are part of the CPU itself, including the > NVIC, systick timer, and debug and trace components like the Data > Watchpoint and Trace unit (DWT). Within this large region, the range > 0xe000e000 to 0xe000efff is the System Control Space (NVIC, system > registers, systick) and 0xe002e000 to 0exe002efff is its Non-secure > alias. > > The architecture is clear that within the SCS unimplemented registers > should be RES0 for privileged accesses and generate BusFault for > unprivileged accesses, and we currently implement this. > > It is less clear about how to handle accesses to unimplemented > regions of the wider PPB. Unprivileged accesses should definitely > cause BusFaults (R_DQQS), but the behaviour of privileged accesses is > not given as a general rule. However, the register definitions of > individual registers for components like the DWT all state that they > are RES0 if the relevant component is not implemented, so the > simplest way to provide that is to provide RAZ/WI for the whole range > for privileged accesses. (The v7M Arm ARM does say that reserved > registers should be UNK/SBZP.) > > Expand the container MemoryRegion that the NVIC exposes so that > it covers the whole PPB space. This means: > * moving the address that the ARMV7M device maps it to down by >0xe000 bytes > * moving the off and the offsets within the container of all the >subregions forward by 0xe000 bytes > * adding a new default MemoryRegion that covers the whole container >at a lower priority than anything else and which provides the >RAZWI/BusFault behaviour > > Signed-off-by: Peter Maydell > --- Reviewed-by: Richard Henderson r~
Re: [Virtio-fs] [PATCH] virtiofsd: Use --thread-pool-size=0 to mean no thread pool
On Tue, Nov 17, 2020 at 04:00:09PM +, Venegas Munoz, Jose Carlos wrote:
> > Not sure what the default is for 9p, but comparing
> > default to default will definitely not be apples to apples since this
> > mode is nonexistent in 9p.
>
> In Kata we are looking for the best config for fs compatibility and
> performance. So even if are no apples to apples,
> we are for the best config for both and comparing the best that each of them
> can do.
>
> In the case of Kata for 9pfs(this is the config we have found has better
> performance and fs compatibility in general) we have:
> ```
> -device virtio-9p-pci # device type
> ,disable-modern=false
> ,fsdev=extra-9p-kataShared # attr: device id for fsdev
> ,mount_tag=kataShared # attr: tag on how will be found de sharedfs
> ,romfile=
> -fsdev local #local: Simply lets QEMU call the individual VFS functions
> (more or less) directly on host.
> ,id=extra-9p-kataShared
> ,path=${SHARED_PATH} # attrs: path to share
> ,security_model=none #
> #passthrough: Files are stored using the same credentials as they are
> created on the guest. This requires QEMU to run as # root.
> #none: Same as "passthrough" except the sever won't report failures if it
> fails to set file attributes like ownership #
> # (chown). This makes a passthrough like security model usable for people
> who run kvm as non root.
> ,multidevs=remap
> ```
>
> The mount options are:
> ```
> trans=virtio
> ,version=9p2000.L
> ,cache=mmap
> ,"nodev" # Security: The nodev mount option specifies that the filesystem
> cannot contain special devices.
> ,"msize=8192" # msize: Maximum packet size including any headers.
> ```
How much RAM you are giving to these containers when using virtio-9p?
Vivek
Re: [PATCH for-5.2] s390x/pci: fix endianness issues
On 11/17/20 7:39 PM, Thomas Huth wrote:
> On 17/11/2020 19.30, Philippe Mathieu-Daudé wrote:
>> On 11/17/20 7:19 PM, Matthew Rosato wrote:
>>> On 11/17/20 12:59 PM, Philippe Mathieu-Daudé wrote:
On 11/17/20 6:13 PM, Cornelia Huck wrote:
> zPCI control blocks are big endian, we need to take care that we
> do proper accesses in order not to break tcg guests on little endian
> hosts.
>
> Fixes: 28dc86a07299 ("s390x/pci: use a PCI Group structure")
> Fixes: 9670ee752727 ("s390x/pci: use a PCI Function structure")
> Fixes: 1e7552ff5c34 ("s390x/pci: get zPCI function info from host")
> Signed-off-by: Cornelia Huck
> ---
>
> Works for me with virtio-pci devices for tcg on x86 and s390x, and
> for kvm.
> The vfio changes are not strictly needed; did not test them due to
> lack of
> hardware -- testing appreciated. >>
> As this fixes a regression, I want this in 5.2.
>
> ---
> hw/s390x/s390-pci-bus.c | 12 ++--
> hw/s390x/s390-pci-inst.c | 4 ++--
> hw/s390x/s390-pci-vfio.c | 8
> 3 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index e0dc20ce4a56..17e64e0b1200 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -787,12 +787,12 @@ static void s390_pci_init_default_group(void)
> static void set_pbdev_info(S390PCIBusDevice *pbdev)
> {
> - pbdev->zpci_fn.sdma = ZPCI_SDMA_ADDR;
> - pbdev->zpci_fn.edma = ZPCI_EDMA_ADDR;
> - pbdev->zpci_fn.pchid = 0;
> + stq_p(&pbdev->zpci_fn.sdma, ZPCI_SDMA_ADDR);
"zPCI control blocks are big endian" so don't we
need the _be_ accessors? stq_be_p() etc...
>>>
>>> I don't think this is necessary. This is only available for target
>>> s390x, which is always big endian... cpu-all.h should define stq_p as
>>> stq_be_p for example inside the #if defined(TARGET_WORDS_BIGENDIAN).
>>
>> But if you run on little-endian host, you need to byte-swap that,
>> isn't it?
>
> It's done by the macros. They depend on the target endianess. See cpu-all.h.
I'm confused because the description is about target endianness,
but stq_p() is about host alignment.
If there is no alignment problem, doesn't using stq_p() make code
harder to review?
[Bug 1715203] Re: Maintain Haiku support
https://git.qemu.org/?p=qemu.git;a=commitdiff;h=9fc33bf4e1d694225 ... the test suite is still failing, but at least we can compile test Haiku now. Thanks! ** Changed in: qemu Status: Confirmed => Fix Committed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1715203 Title: Maintain Haiku support Status in QEMU: Fix Committed Bug description: It was pointed out that the 2.10 release notes are pushing to drop Haiku support. The qemu port is currently working as-is under Haiku. Was there a reason this was recommended? Is there anything Haiku can do to keep it from being dropped? We're working on a docker container to cross-compile rust-lang for Haiku, could this be of some use to qemu when complete? To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1715203/+subscriptions
Re: [PATCH for-5.2] s390x/pci: fix endianness issues
On 17/11/2020 19.30, Philippe Mathieu-Daudé wrote:
> On 11/17/20 7:19 PM, Matthew Rosato wrote:
>> On 11/17/20 12:59 PM, Philippe Mathieu-Daudé wrote:
>>> On 11/17/20 6:13 PM, Cornelia Huck wrote:
zPCI control blocks are big endian, we need to take care that we
do proper accesses in order not to break tcg guests on little endian
hosts.
Fixes: 28dc86a07299 ("s390x/pci: use a PCI Group structure")
Fixes: 9670ee752727 ("s390x/pci: use a PCI Function structure")
Fixes: 1e7552ff5c34 ("s390x/pci: get zPCI function info from host")
Signed-off-by: Cornelia Huck
---
Works for me with virtio-pci devices for tcg on x86 and s390x, and
for kvm.
The vfio changes are not strictly needed; did not test them due to
lack of
hardware -- testing appreciated. >>
As this fixes a regression, I want this in 5.2.
---
hw/s390x/s390-pci-bus.c | 12 ++--
hw/s390x/s390-pci-inst.c | 4 ++--
hw/s390x/s390-pci-vfio.c | 8
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index e0dc20ce4a56..17e64e0b1200 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -787,12 +787,12 @@ static void s390_pci_init_default_group(void)
static void set_pbdev_info(S390PCIBusDevice *pbdev)
{
- pbdev->zpci_fn.sdma = ZPCI_SDMA_ADDR;
- pbdev->zpci_fn.edma = ZPCI_EDMA_ADDR;
- pbdev->zpci_fn.pchid = 0;
+ stq_p(&pbdev->zpci_fn.sdma, ZPCI_SDMA_ADDR);
>>>
>>> "zPCI control blocks are big endian" so don't we
>>> need the _be_ accessors? stq_be_p() etc...
>>>
>>
>> I don't think this is necessary. This is only available for target
>> s390x, which is always big endian... cpu-all.h should define stq_p as
>> stq_be_p for example inside the #if defined(TARGET_WORDS_BIGENDIAN).
>
> But if you run on little-endian host, you need to byte-swap that,
> isn't it?
It's done by the macros. They depend on the target endianess. See cpu-all.h.
Thomas
Re: [PATCH for-5.2] s390x/pci: fix endianness issues
On 11/17/20 7:19 PM, Matthew Rosato wrote:
> On 11/17/20 12:59 PM, Philippe Mathieu-Daudé wrote:
>> On 11/17/20 6:13 PM, Cornelia Huck wrote:
>>> zPCI control blocks are big endian, we need to take care that we
>>> do proper accesses in order not to break tcg guests on little endian
>>> hosts.
>>>
>>> Fixes: 28dc86a07299 ("s390x/pci: use a PCI Group structure")
>>> Fixes: 9670ee752727 ("s390x/pci: use a PCI Function structure")
>>> Fixes: 1e7552ff5c34 ("s390x/pci: get zPCI function info from host")
>>> Signed-off-by: Cornelia Huck
>>> ---
>>>
>>> Works for me with virtio-pci devices for tcg on x86 and s390x, and
>>> for kvm.
>>> The vfio changes are not strictly needed; did not test them due to
>>> lack of
>>> hardware -- testing appreciated. >>
>>> As this fixes a regression, I want this in 5.2.
>>>
>>> ---
>>> hw/s390x/s390-pci-bus.c | 12 ++--
>>> hw/s390x/s390-pci-inst.c | 4 ++--
>>> hw/s390x/s390-pci-vfio.c | 8
>>> 3 files changed, 12 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
>>> index e0dc20ce4a56..17e64e0b1200 100644
>>> --- a/hw/s390x/s390-pci-bus.c
>>> +++ b/hw/s390x/s390-pci-bus.c
>>> @@ -787,12 +787,12 @@ static void s390_pci_init_default_group(void)
>>> static void set_pbdev_info(S390PCIBusDevice *pbdev)
>>> {
>>> - pbdev->zpci_fn.sdma = ZPCI_SDMA_ADDR;
>>> - pbdev->zpci_fn.edma = ZPCI_EDMA_ADDR;
>>> - pbdev->zpci_fn.pchid = 0;
>>> + stq_p(&pbdev->zpci_fn.sdma, ZPCI_SDMA_ADDR);
>>
>> "zPCI control blocks are big endian" so don't we
>> need the _be_ accessors? stq_be_p() etc...
>>
>
> I don't think this is necessary. This is only available for target
> s390x, which is always big endian... cpu-all.h should define stq_p as
> stq_be_p for example inside the #if defined(TARGET_WORDS_BIGENDIAN).
But if you run on little-endian host, you need to byte-swap that,
isn't it?
Re: [RFC PATCH-for-5.2] hw/s390x/pci: Fix endianness issue
On 17/11/2020 15.34, Matthew Rosato wrote:
> On 11/17/20 9:13 AM, Cornelia Huck wrote:
>> On Tue, 17 Nov 2020 09:02:37 -0500
>> Matthew Rosato wrote:
>>
>>> On 11/17/20 8:31 AM, Cornelia Huck wrote:
On Tue, 17 Nov 2020 14:23:57 +0100
Pierre Morel wrote:
> On 11/17/20 2:00 PM, Peter Maydell wrote:
>> On Tue, 17 Nov 2020 at 12:03, Philippe Mathieu-Daudé
>> wrote:
>>>
>>> Fix an endianness issue reported by Cornelia:
>>>
s390x tcg guest on x86, virtio-pci devices are not detected. The
relevant feature bits are visible to the guest. Same breakage with
different guest kernels.
KVM guests and s390x tcg guests on s390x are fine.
>>>
>>> Fixes: 28dc86a0729 ("s390x/pci: use a PCI Group structure")
>>> Reported-by: Cornelia Huck
>>> Signed-off-by: Philippe Mathieu-Daudé
>>> ---
>>> RFC because review-only patch, untested
>>> ---
>>> hw/s390x/s390-pci-inst.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
>>> index 58cd041d17f..cfb54b4d8ec 100644
>>> --- a/hw/s390x/s390-pci-inst.c
>>> +++ b/hw/s390x/s390-pci-inst.c
>>> @@ -305,7 +305,7 @@ int clp_service_call(S390CPU *cpu, uint8_t r2,
>>> uintptr_t ra)
>>> ClpReqQueryPciGrp *reqgrp = (ClpReqQueryPciGrp *)reqh;
>>> S390PCIGroup *group;
>>>
>>> - group = s390_group_find(reqgrp->g);
>>> + group = s390_group_find(ldl_p(&reqgrp->g));
>>
>> 'g' in the ClpReqQueryPciGrp struct is a uint32_t, so
>> adding the ldl_p() will have no effect unless (a) the
>> structure is not 4-aligned and (b) the host will fault on
>> unaligned accesses, which isn't the case for x86 hosts.
>>
>> Q: is this struct really in host order, or should we
>> be using ldl_le_p() or ldl_be_p() and friends here and
>> elsewhere?
>>
>> thanks
>> -- PMM
>>
>
> Hi, I think we better modify the structure here, g should be a byte.
>
> Connie, can you please try this if it resolves the issue?
>
> diff --git a/hw/s390x/s390-pci-inst.h b/hw/s390x/s390-pci-inst.h
> index fa3bf8b5aa..641d19c815 100644
> --- a/hw/s390x/s390-pci-inst.h
> +++ b/hw/s390x/s390-pci-inst.h
> @@ -146,7 +146,8 @@ typedef struct ClpReqQueryPciGrp {
> uint32_t fmt;
> uint64_t reserved1;
> #define CLP_REQ_QPCIG_MASK_PFGID 0xff
> - uint32_t g;
> + uint32_t g0 :24;
> + uint32_t g :8;
> uint32_t reserved2;
> uint64_t reserved3;
> } QEMU_PACKED ClpReqQueryPciGrp;
>
No, same crash... I fear there are more things broken wrt endianness.
>>>
>>> Sorry, just getting online now, looking at the code Are the 2
>>> memcpy calls added in 9670ee75 and 28dc86a0 the issue? Won't they just
>>> present the Q PCI FN / Q PCI FN GRP results in host endianness?
>>>
>>
>> I just re-added some st?_p operations in set_pbdev_info and that fixes
>> at least the crash I was seeing with Phil's patch applied. Still, no
>> pci functions get detected, so that's not enough. Those memcpy calls
>> look like a possible culprit.
>>
>
> OK, so if everything in set_pbdev_info and s390_pci_init_default_group() is
> handled with st?_p operations, then the memcpy should be OK...
>
> Pierre was on to something with his recommendation, as the group id is only
> 1B of the 'g' field (see CLP_REQ_QPCIG_MASK_PFGID) - the other bits just
> happen to be unused.
>
> Did you include his change with your st?_p changes to set_pbdev_info
As Peter also already wrote: Bitfields are not endianess safe either. You'd
need to replace the g0:24 with "uint8_t g0[3]" to get it working that way.
Thomas
Re: [PATCH for-5.2] s390x/pci: fix endianness issues
On 11/17/20 12:59 PM, Philippe Mathieu-Daudé wrote:
On 11/17/20 6:13 PM, Cornelia Huck wrote:
zPCI control blocks are big endian, we need to take care that we
do proper accesses in order not to break tcg guests on little endian
hosts.
Fixes: 28dc86a07299 ("s390x/pci: use a PCI Group structure")
Fixes: 9670ee752727 ("s390x/pci: use a PCI Function structure")
Fixes: 1e7552ff5c34 ("s390x/pci: get zPCI function info from host")
Signed-off-by: Cornelia Huck
---
Works for me with virtio-pci devices for tcg on x86 and s390x, and for kvm.
The vfio changes are not strictly needed; did not test them due to lack of
hardware -- testing appreciated. >>
As this fixes a regression, I want this in 5.2.
---
hw/s390x/s390-pci-bus.c | 12 ++--
hw/s390x/s390-pci-inst.c | 4 ++--
hw/s390x/s390-pci-vfio.c | 8
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index e0dc20ce4a56..17e64e0b1200 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -787,12 +787,12 @@ static void s390_pci_init_default_group(void)
static void set_pbdev_info(S390PCIBusDevice *pbdev)
{
-pbdev->zpci_fn.sdma = ZPCI_SDMA_ADDR;
-pbdev->zpci_fn.edma = ZPCI_EDMA_ADDR;
-pbdev->zpci_fn.pchid = 0;
+stq_p(&pbdev->zpci_fn.sdma, ZPCI_SDMA_ADDR);
"zPCI control blocks are big endian" so don't we
need the _be_ accessors? stq_be_p() etc...
I don't think this is necessary. This is only available for target
s390x, which is always big endian... cpu-all.h should define stq_p as
stq_be_p for example inside the #if defined(TARGET_WORDS_BIGENDIAN).
Re: [PATCH v2 02/16] qapi/expr.py: Check for dict instead of OrderedDict
On Tue, Nov 17, 2020 at 05:30:04PM +0100, Markus Armbruster wrote: > John Snow writes: > > > OrderedDict is a subtype of dict, so we can check for a more general form. > > > > Signed-off-by: John Snow > > Reviewed-by: Eduardo Habkost > > Reviewed-by: Cleber Rosa > > --- > > scripts/qapi/expr.py | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py > > index 35695c4c653b..5694c501fa38 100644 > > --- a/scripts/qapi/expr.py > > +++ b/scripts/qapi/expr.py > > @@ -14,7 +14,6 @@ > > # This work is licensed under the terms of the GNU GPL, version 2. > > # See the COPYING file in the top-level directory. > > > > -from collections import OrderedDict > > import re > > > > from .common import c_name > > @@ -131,7 +130,7 @@ def check_if_str(ifcond): > > > > > > def normalize_members(members): > > -if isinstance(members, OrderedDict): > > +if isinstance(members, dict): > > for key, arg in members.items(): > > if isinstance(arg, dict): > > continue > > @@ -162,7 +161,7 @@ def check_type(value, info, source, > > if not allow_dict: > > raise QAPISemError(info, "%s should be a type name" % source) > > > > -if not isinstance(value, OrderedDict): > > +if not isinstance(value, dict): > > raise QAPISemError(info, > > "%s should be an object or type name" % source) > > Plain dict remembers insertion order since Python 3.6, but it wasn't > formally promised until 3.7. > > Can we simply ditch OrderedDict entirely? In theory, our build requirement is "Python >= 3.6", not "CPython >= 3.6". In practice, I don't expect anybody to ever use any other Python implementation except CPython to build QEMU. I think we can get rid of OrderedDict if you really want to. -- Eduardo
Re: [PATCH for-5.2? 2/2] authz-simple: Check that 'identity' property is set
On 11/17/20 5:30 PM, Kevin Wolf wrote: > If the 'identify' property is not set, we'll pass a NULL pointer to > g_str_equal() and crash. Catch the error condition during the creation > of the object. > > Signed-off-by: Kevin Wolf > --- > authz/simple.c | 14 ++ > 1 file changed, 14 insertions(+) Reviewed-by: Philippe Mathieu-Daudé
Re: Regressions in build process introduced since August
On 17/11/20 18:50, Stefano Garzarella wrote: Running `configure [...] --extra-cflags="-I /xyz"` results in compiler flags `-I [...] /xyz`, so the `-I` and `/xyz` are separated by other compiler flags which obviously cannot work as expected. I could work around that by removing the space and using a pattern like `-I/xyz`. This regression is not restricted to builds targeting Windows. I'll take a look at fixing this (in meson). meson.build sets a hard name for the Windows installer executable: installer = 'qemu-setup-' + meson.project_version() + '.exe'. Previously the installer name could be changed by running `make installer INSTALLER=qemu-setup-something.exe`. This no longer works. Is there an alternative solution how the name of the installer executable can be set? Or how could I reimplement the lost functionality? No, there's no way to do this apart from patching meson.build. Thanks, Paolo
Re: [PATCH for-5.2? 1/2] authz-pam: Check that 'service' property is set
On 11/17/20 5:30 PM, Kevin Wolf wrote: > If the 'service' property is not set, we'll call pam_start() with a NULL > pointer for the service name. This fails and leaves a message like this > in the syslog: > > qemu-storage-daemon[294015]: PAM pam_start: invalid argument: service == NULL > > Make specifying the property mandatory and catch the error already > during the creation of the object. > > Signed-off-by: Kevin Wolf > --- > authz/pamacct.c | 6 ++ > 1 file changed, 6 insertions(+) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-5.2] s390x/pci: fix endianness issues
On 11/17/20 6:13 PM, Cornelia Huck wrote:
> zPCI control blocks are big endian, we need to take care that we
> do proper accesses in order not to break tcg guests on little endian
> hosts.
>
> Fixes: 28dc86a07299 ("s390x/pci: use a PCI Group structure")
> Fixes: 9670ee752727 ("s390x/pci: use a PCI Function structure")
> Fixes: 1e7552ff5c34 ("s390x/pci: get zPCI function info from host")
> Signed-off-by: Cornelia Huck
> ---
>
> Works for me with virtio-pci devices for tcg on x86 and s390x, and for kvm.
> The vfio changes are not strictly needed; did not test them due to lack of
> hardware -- testing appreciated.
>
> As this fixes a regression, I want this in 5.2.
>
> ---
> hw/s390x/s390-pci-bus.c | 12 ++--
> hw/s390x/s390-pci-inst.c | 4 ++--
> hw/s390x/s390-pci-vfio.c | 8
> 3 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index e0dc20ce4a56..17e64e0b1200 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -787,12 +787,12 @@ static void s390_pci_init_default_group(void)
>
> static void set_pbdev_info(S390PCIBusDevice *pbdev)
> {
> -pbdev->zpci_fn.sdma = ZPCI_SDMA_ADDR;
> -pbdev->zpci_fn.edma = ZPCI_EDMA_ADDR;
> -pbdev->zpci_fn.pchid = 0;
> +stq_p(&pbdev->zpci_fn.sdma, ZPCI_SDMA_ADDR);
"zPCI control blocks are big endian" so don't we
need the _be_ accessors? stq_be_p() etc...
> +stq_p(&pbdev->zpci_fn.edma, ZPCI_EDMA_ADDR);
> +stw_p(&pbdev->zpci_fn.pchid, 0);
> pbdev->zpci_fn.pfgid = ZPCI_DEFAULT_FN_GRP;
> -pbdev->zpci_fn.fid = pbdev->fid;
> -pbdev->zpci_fn.uid = pbdev->uid;
> +stl_p(&pbdev->zpci_fn.fid, pbdev->fid);
> +stl_p(&pbdev->zpci_fn.uid, pbdev->uid);
> pbdev->pci_group = s390_group_find(ZPCI_DEFAULT_FN_GRP);
> }
>
> @@ -871,7 +871,7 @@ static int s390_pci_msix_init(S390PCIBusDevice *pbdev)
> memory_region_init_io(&pbdev->msix_notify_mr, OBJECT(pbdev),
>&s390_msi_ctrl_ops, pbdev, name, PAGE_SIZE);
> memory_region_add_subregion(&pbdev->iommu->mr,
> -pbdev->pci_group->zpci_group.msia,
> +ldq_p(&pbdev->pci_group->zpci_group.msia),
> &pbdev->msix_notify_mr);
> g_free(name);
>
> diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
> index 58cd041d17fb..7bc6b79f10ce 100644
> --- a/hw/s390x/s390-pci-inst.c
> +++ b/hw/s390x/s390-pci-inst.c
> @@ -305,7 +305,7 @@ int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t
> ra)
> ClpReqQueryPciGrp *reqgrp = (ClpReqQueryPciGrp *)reqh;
> S390PCIGroup *group;
>
> -group = s390_group_find(reqgrp->g);
> +group = s390_group_find(ldl_p(&reqgrp->g));
> if (!group) {
> /* We do not allow access to unknown groups */
> /* The group must have been obtained with a vfio device */
> @@ -788,7 +788,7 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t
> r3, uint64_t gaddr,
> /* Length must be greater than 8, a multiple of 8 */
> /* and not greater than maxstbl */
> if ((len <= 8) || (len % 8) ||
> -(len > pbdev->pci_group->zpci_group.maxstbl)) {
> +(len > lduw_p(&pbdev->pci_group->zpci_group.maxstbl))) {
> goto specification_error;
> }
> /* Do not cross a 4K-byte boundary */
> diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c
> index d5c78063b5bc..f455c6f20a1a 100644
> --- a/hw/s390x/s390-pci-vfio.c
> +++ b/hw/s390x/s390-pci-vfio.c
> @@ -116,10 +116,10 @@ static void s390_pci_read_base(S390PCIBusDevice *pbdev,
> }
> cap = (void *) hdr;
>
> -pbdev->zpci_fn.sdma = cap->start_dma;
> -pbdev->zpci_fn.edma = cap->end_dma;
> -pbdev->zpci_fn.pchid = cap->pchid;
> -pbdev->zpci_fn.vfn = cap->vfn;
> +stq_p(&pbdev->zpci_fn.sdma, cap->start_dma);
> +stq_p(&pbdev->zpci_fn.edma, cap->end_dma);
> +stw_p(&pbdev->zpci_fn.pchid, cap->pchid);
> +stw_p(&pbdev->zpci_fn.vfn, cap->vfn);
> pbdev->zpci_fn.pfgid = cap->gid;
> /* The following values remain 0 until we support other FMB formats */
> pbdev->zpci_fn.fmbl = 0;
>
Re: [PATCH v1 2/6] tests: add prefixes to the bare mkdtemp calls
On 11/17/20 6:36 PM, Alex Bennée wrote: > The first step to debug a thing is to know what created the thing in > the first place. Add some prefixes so random tmpdir's have something > grep in the code. > > Signed-off-by: Alex Bennée > > --- > v2 > - fix long lines > --- > python/qemu/machine.py| 3 ++- > tests/acceptance/avocado_qemu/__init__.py | 3 ++- > 2 files changed, 4 insertions(+), 2 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: Regressions in build process introduced since August
CCing Paolo and Marc-André who worked on meson integrations. On Sun, Nov 15, 2020 at 11:57:25AM +0100, Stefan Weil wrote: Dear all, yesterday I tried to build new QEMU installers for Windows and noticed two regressions which break my build process: *** Change in handling of --extra-cflags Running `configure [...] --extra-cflags="-I /xyz"` results in compiler flags `-I [...] /xyz`, so the `-I` and `/xyz` are separated by other compiler flags which obviously cannot work as expected. I could work around that by removing the space and using a pattern like `-I/xyz`. This regression is not restricted to builds targeting Windows. *** Setting INSTALLER no longer handled meson.build sets a hard name for the Windows installer executable: installer = 'qemu-setup-' + meson.project_version() + '.exe'. Previously the installer name could be changed by running `make installer INSTALLER=qemu-setup-something.exe`. This no longer works. Is there an alternative solution how the name of the installer executable can be set? Or how could I reimplement the lost functionality? Kind regards Stefan Weil
[PATCH v1 4/6] gitlab: move remaining x86 check-tcg targets to gitlab
The GCC check-tcg (user) test in particular was very prone to timing
out on Travis. We only actually need to move the some-softmmu builds
across as we already have coverage for linux-user.
As --enable-debug-tcg does increase the run time somewhat as more
debug is put in let's restrict that to just the plugins build. It's
unlikely that a plugins enabled build is going to hide a sanity
failure in core TCG code so let the plugin builds do the heavy lifting
on checking TCG sanity so the non-plugin builds can run swiftly.
Now the only remaining check-tcg builds on Travis are for the various
non-x86 arches.
Signed-off-by: Alex Bennée
Message-Id: <20201110192316.26397-10-alex.ben...@linaro.org>
---
.gitlab-ci.yml | 17 +
.travis.yml| 26 --
2 files changed, 17 insertions(+), 26 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 9a8b375188..b406027a55 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -247,6 +247,15 @@ build-user:
CONFIGURE_ARGS: --disable-tools --disable-system
MAKE_CHECK_ARGS: check-tcg
+# Only build the softmmu targets we have check-tcg tests for
+build-some-softmmu:
+ <<: *native_build_job_definition
+ variables:
+IMAGE: debian-all-test-cross
+CONFIGURE_ARGS: --disable-tools --enable-debug-tcg
+TARGETS: xtensa-softmmu arm-softmmu aarch64-softmmu alpha-softmmu
+MAKE_CHECK_ARGS: check-tcg
+
# Run check-tcg against linux-user (with plugins)
# we skip sparc64-linux-user until it has been fixed somewhat
# we skip cris-linux-user as it doesn't use the common run loop
@@ -258,6 +267,14 @@ build-user-plugins:
MAKE_CHECK_ARGS: check-tcg
timeout: 1h 30m
+build-some-softmmu-plugins:
+ <<: *native_build_job_definition
+ variables:
+IMAGE: debian-all-test-cross
+CONFIGURE_ARGS: --disable-tools --disable-user --enable-plugins
--enable-debug-tcg
+TARGETS: xtensa-softmmu arm-softmmu aarch64-softmmu alpha-softmmu
+MAKE_CHECK_ARGS: check-tcg
+
build-clang:
<<: *native_build_job_definition
variables:
diff --git a/.travis.yml b/.travis.yml
index a3d78171ca..bac085f800 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -301,32 +301,6 @@ jobs:
- ${SRC_DIR}/configure ${CONFIG} --extra-cflags="-g3 -O0
-fsanitize=thread" || { cat config.log meson-logs/meson-log.txt && exit 1; }
-# Run check-tcg against linux-user
-- name: "GCC check-tcg (user)"
- env:
-- CONFIG="--disable-system --enable-debug-tcg"
-- TEST_BUILD_CMD="make build-tcg"
-- TEST_CMD="make check-tcg"
-- CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-debug-tcg"
-
-
-# Run check-tcg against softmmu targets
-- name: "GCC check-tcg (some-softmmu)"
- env:
-- CONFIG="--enable-debug-tcg
--target-list=xtensa-softmmu,arm-softmmu,aarch64-softmmu,alpha-softmmu"
-- TEST_BUILD_CMD="make build-tcg"
-- TEST_CMD="make check-tcg"
-- CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-debug-tcg"
-
-
-# Run check-tcg against softmmu targets (with plugins)
-- name: "GCC plugins check-tcg (some-softmmu)"
- env:
-- CONFIG="--enable-plugins --enable-debug-tcg
--target-list=xtensa-softmmu,arm-softmmu,aarch64-softmmu,alpha-softmmu"
-- TEST_BUILD_CMD="make build-tcg"
-- TEST_CMD="make check-tcg"
-- CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-debug-tcg"
-
- name: "[aarch64] GCC check-tcg"
arch: arm64
dist: focal
--
2.20.1
[PATCH v1 3/6] tests/avocado: clean-up socket directory after run
Previously we were leaving temporary directories behind. While the
QEMUMachine does make efforts to clean up after itself the directory
belongs to the calling function. We use TemporaryDirectory to wrap
this although we explicitly clear the reference in tearDown() as it
doesn't get cleaned up otherwise.
Signed-off-by: Alex Bennée
---
tests/acceptance/avocado_qemu/__init__.py | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tests/acceptance/avocado_qemu/__init__.py
b/tests/acceptance/avocado_qemu/__init__.py
index 3033b2cabe..bf54e419da 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -171,8 +171,8 @@ class Test(avocado.Test):
self.cancel("No QEMU binary defined or found in the build tree")
def _new_vm(self, *args):
-sd = tempfile.mkdtemp(prefix="avocado_qemu_sock_")
-vm = QEMUMachine(self.qemu_bin, sock_dir=sd)
+self._sd = tempfile.TemporaryDirectory(prefix="avo_qemu_sock_")
+vm = QEMUMachine(self.qemu_bin, sock_dir=self._sd.name)
if args:
vm.add_args(*args)
return vm
@@ -193,6 +193,7 @@ class Test(avocado.Test):
def tearDown(self):
for vm in self._vms.values():
vm.shutdown()
+self._sd = None
def fetch_asset(self, name,
asset_hash=None, algorithm=None,
--
2.20.1
[PATCH v1 5/6] tests/docker: Install liblttng-ust-dev package in Ubuntu 20.04 image
From: Philippe Mathieu-Daudé Install the liblttng-ust-dev package to be able to build QEMU using the User-Space Tracer trace backend (configure --enable-trace-backends=ust). Suggested-by: Wainer dos Santos Moschetta Signed-off-by: Philippe Mathieu-Daudé Message-Id: <2020121234.3246812-2-phi...@redhat.com> Signed-off-by: Alex Bennée --- tests/docker/dockerfiles/ubuntu2004.docker | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/docker/dockerfiles/ubuntu2004.docker b/tests/docker/dockerfiles/ubuntu2004.docker index 355bbb3c63..ae889d8482 100644 --- a/tests/docker/dockerfiles/ubuntu2004.docker +++ b/tests/docker/dockerfiles/ubuntu2004.docker @@ -23,6 +23,7 @@ ENV PACKAGES flex bison \ libiscsi-dev \ libjemalloc-dev \ libjpeg-turbo8-dev \ +liblttng-ust-dev \ liblzo2-dev \ libncurses5-dev \ libncursesw5-dev \ -- 2.20.1
[PATCH v1 1/6] scripts/ci: clean up default args logic a little
This allows us to do:
./scripts/ci/gitlab-pipeline-status -w -b HEAD -p 2961854
to check out own pipeline status of a recently pushed branch.
Signed-off-by: Alex Bennée
---
scripts/ci/gitlab-pipeline-status | 24 +---
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/scripts/ci/gitlab-pipeline-status
b/scripts/ci/gitlab-pipeline-status
index bac8233079..78e72f6008 100755
--- a/scripts/ci/gitlab-pipeline-status
+++ b/scripts/ci/gitlab-pipeline-status
@@ -31,7 +31,7 @@ class NoPipelineFound(Exception):
"""Communication is successfull but pipeline is not found."""
-def get_local_branch_commit(branch='staging'):
+def get_local_branch_commit(branch):
"""
Returns the commit sha1 for the *local* branch named "staging"
"""
@@ -126,18 +126,16 @@ def create_parser():
help=('The GitLab project ID. Defaults to the project '
'for https://gitlab.com/qemu-project/qemu, that '
'is, "%(default)s"'))
-try:
-default_commit = get_local_branch_commit()
-commit_required = False
-except ValueError:
-default_commit = ''
-commit_required = True
-parser.add_argument('-c', '--commit', required=commit_required,
-default=default_commit,
+parser.add_argument('-b', '--branch', type=str, default="staging",
+help=('Specify the branch to check. '
+ 'Use HEAD for your current branch. '
+ 'Otherwise looks at "%(default)s"'))
+parser.add_argument('-c', '--commit',
+default=None,
help=('Look for a pipeline associated with the given '
'commit. If one is not explicitly given, the '
- 'commit associated with the local branch named '
- '"staging" is used. Default: %(default)s'))
+ 'commit associated with the default branch '
+ 'is used.'))
parser.add_argument('--verbose', action='store_true', default=False,
help=('A minimal verbosity level that prints the '
'overall result of the check/wait'))
@@ -149,6 +147,10 @@ def main():
"""
parser = create_parser()
args = parser.parse_args()
+
+if not args.commit:
+args.commit = get_local_branch_commit(args.branch)
+
success = False
try:
if args.wait:
--
2.20.1
[PATCH v1 2/6] tests: add prefixes to the bare mkdtemp calls
The first step to debug a thing is to know what created the thing in
the first place. Add some prefixes so random tmpdir's have something
grep in the code.
Signed-off-by: Alex Bennée
---
v2
- fix long lines
---
python/qemu/machine.py| 3 ++-
tests/acceptance/avocado_qemu/__init__.py | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 6420f01bed..64d966aeeb 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -303,7 +303,8 @@ class QEMUMachine:
return args
def _pre_launch(self) -> None:
-self._temp_dir = tempfile.mkdtemp(dir=self._test_dir)
+self._temp_dir = tempfile.mkdtemp(prefix="qemu-machine-",
+ dir=self._test_dir)
self._qemu_log_path = os.path.join(self._temp_dir, self._name + ".log")
self._qemu_log_file = open(self._qemu_log_path, 'wb')
diff --git a/tests/acceptance/avocado_qemu/__init__.py
b/tests/acceptance/avocado_qemu/__init__.py
index 4cda037187..3033b2cabe 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -171,7 +171,8 @@ class Test(avocado.Test):
self.cancel("No QEMU binary defined or found in the build tree")
def _new_vm(self, *args):
-vm = QEMUMachine(self.qemu_bin, sock_dir=tempfile.mkdtemp())
+sd = tempfile.mkdtemp(prefix="avocado_qemu_sock_")
+vm = QEMUMachine(self.qemu_bin, sock_dir=sd)
if args:
vm.add_args(*args)
return vm
--
2.20.1
[PATCH for 5.3-rc3 v1 0/6] testing fixes (avocado, gitlab)
Hi, Here are a few more minor fixes for 5.2-rc3: - a tweak to the gitlab status script (from last series) - moving check-tcg to gitlab (also in last series) - fix some tempdir names and left overs - moving some more tests to gitlab The following need review: - gitlab: move remaining x86 check-tcg targets to gitlab - tests/avocado: clean-up socket directory after run - tests: add prefixes to the bare mkdtemp calls - scripts/ci: clean up default args logic a little Alex Bennée (4): scripts/ci: clean up default args logic a little tests: add prefixes to the bare mkdtemp calls tests/avocado: clean-up socket directory after run gitlab: move remaining x86 check-tcg targets to gitlab Philippe Mathieu-Daudé (2): tests/docker: Install liblttng-ust-dev package in Ubuntu 20.04 image gitlab-ci: Move trace backend tests across to gitlab .gitlab-ci.yml | 35 + .travis.yml| 45 -- python/qemu/machine.py | 3 +- scripts/ci/gitlab-pipeline-status | 24 ++-- tests/acceptance/avocado_qemu/__init__.py | 4 +- tests/docker/dockerfiles/ubuntu2004.docker | 1 + 6 files changed, 54 insertions(+), 58 deletions(-) -- 2.20.1
[PATCH v1 6/6] gitlab-ci: Move trace backend tests across to gitlab
From: Philippe Mathieu-Daudé Similarly to commit 8cdb2cef3f1, move the trace backend tests to GitLab. Note the User-Space Tracer backend is still tested on Ubuntu by the s390x jobs on Travis-CI. Signed-off-by: Philippe Mathieu-Daudé Message-Id: <2020121234.3246812-3-phi...@redhat.com> Signed-off-by: Alex Bennée --- .gitlab-ci.yml | 18 ++ .travis.yml| 19 --- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index b406027a55..d0173e82b1 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -415,6 +415,24 @@ check-crypto-only-gnutls: IMAGE: centos7 MAKE_CHECK_ARGS: check +# We don't need to exercise every backend with every front-end +build-trace-multi-user: + <<: *native_build_job_definition + variables: +IMAGE: ubuntu2004 +CONFIGURE_ARGS: --enable-trace-backends=log,simple,syslog --disable-system + +build-trace-ftrace-system: + <<: *native_build_job_definition + variables: +IMAGE: ubuntu2004 +CONFIGURE_ARGS: --enable-trace-backends=ftrace --target-list=x86_64-softmmu + +build-trace-ust-system: + <<: *native_build_job_definition + variables: +IMAGE: ubuntu2004 +CONFIGURE_ARGS: --enable-trace-backends=ust --target-list=x86_64-softmmu check-patch: stage: build diff --git a/.travis.yml b/.travis.yml index bac085f800..1f80bdb624 100644 --- a/.travis.yml +++ b/.travis.yml @@ -232,25 +232,6 @@ jobs: - TEST_CMD="" -# We don't need to exercise every backend with every front-end -- name: "GCC trace log,simple,syslog (user)" - env: -- CONFIG="--enable-trace-backends=log,simple,syslog --disable-system" -- TEST_CMD="" - - -- name: "GCC trace ftrace (x86_64-softmmu)" - env: -- CONFIG="--enable-trace-backends=ftrace --target-list=x86_64-softmmu" -- TEST_CMD="" - - -- name: "GCC trace ust (x86_64-softmmu)" - env: -- CONFIG="--enable-trace-backends=ust --target-list=x86_64-softmmu" -- TEST_CMD="" - - # Using newer GCC with sanitizers - name: "GCC9 with sanitizers (softmmu)" dist: bionic -- 2.20.1
Re: [PATCH v3 00/41] Mirror map JIT memory for TCG
Joelle van Dyne writes: > Sorry, are you asking for a review from me? I don’t know if I’m > qualified to review the other patches but I did review the iOS patch. Anyone can review code and given you wrote the original patches you certainly know enough about the flow to give some opinion. If things aren't clear then please do ask questions. The pool of TCG backend reviewers is small enough and helping out does help. Failing that you can always send Tested-by: tags once you've tested a series on the HW. > > -j > > On Tue, Nov 17, 2020 at 9:20 AM Richard Henderson > wrote: >> >> On 11/16/20 7:47 PM, Joelle van Dyne wrote: >> > Hi, I'm wondering what the progress is for this patch set and the iOS >> > support one? I know 5.2 is frozen, so will this be considered for 6.0? >> > Apple Silicon Macs are out now and a few people are asking about QEMU >> > support :) >> >> Yes, this will be considered for 6.0. >> >> It does need to be reviewed more completely than a "LGTM", but there's time >> for >> that. >> >> >> r~ -- Alex Bennée
Re: [PATCH V13 2/9] meson.build: Re-enable KVM support for MIPS
Hi Huacai,
On 10/7/20 10:39 AM, Huacai Chen wrote:
> After converting from configure to meson, KVM support is lost for MIPS,
> so re-enable it in meson.build.
>
> Fixes: fdb75aeff7c212e1afaaa3a43 ("configure: remove target configuration")
> Fixes: 8a19980e3fc42239aae054bc9 ("configure: move accelerator logic to
> meson")
> Cc: aolo Bonzini
> Signed-off-by: Huacai Chen
> ---
> meson.build | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/meson.build b/meson.build
> index 17c89c8..b407ff4 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -59,6 +59,8 @@ elif cpu == 's390x'
>kvm_targets = ['s390x-softmmu']
> elif cpu in ['ppc', 'ppc64']
>kvm_targets = ['ppc-softmmu', 'ppc64-softmmu']
> +elif cpu in ['mips', 'mips64']
> + kvm_targets = ['mips-softmmu', 'mipsel-softmmu', 'mips64-softmmu',
> 'mips64el-softmmu']
Are you sure both 32-bit hosts and targets are supported?
I don't have hardware to test. If you are not working with
32-bit hardware I'd remove them.
> else
>kvm_targets = []
> endif
>
Re: [PATCH v2 4/8] qnum: qnum_value_is_equal() function
On Tue, Nov 17, 2020 at 08:53:19PM +0400, Marc-André Lureau wrote: > On Tue, Nov 17, 2020 at 7:49 PM Eduardo Habkost wrote: > > > On Tue, Nov 17, 2020 at 12:42:47PM +0400, Marc-André Lureau wrote: > > > On Tue, Nov 17, 2020 at 2:42 AM Eduardo Habkost > > wrote: > > > > > > > Extract the QNum value comparison logic to a function that takes > > > > QNumValue* as argument. > > > > > > > > Signed-off-by: Eduardo Habkost > > > > --- > > > > include/qapi/qmp/qnum.h | 1 + > > > > qobject/qnum.c | 29 +++-- > > > > 2 files changed, 20 insertions(+), 10 deletions(-) > > > > > > > > diff --git a/include/qapi/qmp/qnum.h b/include/qapi/qmp/qnum.h > > > > index 62fbdfda68..0327ecd0f0 100644 > > > > --- a/include/qapi/qmp/qnum.h > > > > +++ b/include/qapi/qmp/qnum.h > > > > @@ -106,6 +106,7 @@ double qnum_get_double(const QNum *qn); > > > > > > > > char *qnum_to_string(QNum *qn); > > > > > > > > +bool qnum_value_is_equal(const QNumValue *num_x, const QNumValue > > *num_y); > > > > bool qnum_is_equal(const QObject *x, const QObject *y); > > > > void qnum_destroy_obj(QObject *obj); > > > > > > > > diff --git a/qobject/qnum.c b/qobject/qnum.c > > > > index f80d4efd76..6a0f948b16 100644 > > > > --- a/qobject/qnum.c > > > > +++ b/qobject/qnum.c > > > > @@ -207,9 +207,9 @@ char *qnum_to_string(QNum *qn) > > > > } > > > > > > > > /** > > > > - * qnum_is_equal(): Test whether the two QNums are equal > > > > - * @x: QNum object > > > > - * @y: QNum object > > > > + * qnum_value_is_equal(): Test whether two QNumValues are equal > > > > + * @num_x: QNum value > > > > + * @num_y: QNum value > > > > > > > > > > val_x: a QNumValue ? > > > > Do you mean: > > @num_x: a QNumValue > > > ? > > > > I was not planning to rename the existing num_x/num_y variables. > > > > > Not renaming because that would make some churn? But this is already quite > confusing, so it's better to use "val" for QNumVal and "num" for QNum I > guess. > > If you don't want to rename it in the code, may I suggest doing it at the > declaration side? Not sure it's a better idea. Yeah, I was not renaming them just to avoid churn. However, I'm already replacing `qn` variables with `qv` at patch 3/8. Replacing num_x/num_y with val_x/val_y at qnum_is_equal() (at patch 3/8 as well) wouldn't hurt too much. -- Eduardo
[PATCH for-5.2] s390x/pci: fix endianness issues
zPCI control blocks are big endian, we need to take care that we
do proper accesses in order not to break tcg guests on little endian
hosts.
Fixes: 28dc86a07299 ("s390x/pci: use a PCI Group structure")
Fixes: 9670ee752727 ("s390x/pci: use a PCI Function structure")
Fixes: 1e7552ff5c34 ("s390x/pci: get zPCI function info from host")
Signed-off-by: Cornelia Huck
---
Works for me with virtio-pci devices for tcg on x86 and s390x, and for kvm.
The vfio changes are not strictly needed; did not test them due to lack of
hardware -- testing appreciated.
As this fixes a regression, I want this in 5.2.
---
hw/s390x/s390-pci-bus.c | 12 ++--
hw/s390x/s390-pci-inst.c | 4 ++--
hw/s390x/s390-pci-vfio.c | 8
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index e0dc20ce4a56..17e64e0b1200 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -787,12 +787,12 @@ static void s390_pci_init_default_group(void)
static void set_pbdev_info(S390PCIBusDevice *pbdev)
{
-pbdev->zpci_fn.sdma = ZPCI_SDMA_ADDR;
-pbdev->zpci_fn.edma = ZPCI_EDMA_ADDR;
-pbdev->zpci_fn.pchid = 0;
+stq_p(&pbdev->zpci_fn.sdma, ZPCI_SDMA_ADDR);
+stq_p(&pbdev->zpci_fn.edma, ZPCI_EDMA_ADDR);
+stw_p(&pbdev->zpci_fn.pchid, 0);
pbdev->zpci_fn.pfgid = ZPCI_DEFAULT_FN_GRP;
-pbdev->zpci_fn.fid = pbdev->fid;
-pbdev->zpci_fn.uid = pbdev->uid;
+stl_p(&pbdev->zpci_fn.fid, pbdev->fid);
+stl_p(&pbdev->zpci_fn.uid, pbdev->uid);
pbdev->pci_group = s390_group_find(ZPCI_DEFAULT_FN_GRP);
}
@@ -871,7 +871,7 @@ static int s390_pci_msix_init(S390PCIBusDevice *pbdev)
memory_region_init_io(&pbdev->msix_notify_mr, OBJECT(pbdev),
&s390_msi_ctrl_ops, pbdev, name, PAGE_SIZE);
memory_region_add_subregion(&pbdev->iommu->mr,
-pbdev->pci_group->zpci_group.msia,
+ldq_p(&pbdev->pci_group->zpci_group.msia),
&pbdev->msix_notify_mr);
g_free(name);
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 58cd041d17fb..7bc6b79f10ce 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -305,7 +305,7 @@ int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra)
ClpReqQueryPciGrp *reqgrp = (ClpReqQueryPciGrp *)reqh;
S390PCIGroup *group;
-group = s390_group_find(reqgrp->g);
+group = s390_group_find(ldl_p(&reqgrp->g));
if (!group) {
/* We do not allow access to unknown groups */
/* The group must have been obtained with a vfio device */
@@ -788,7 +788,7 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t
r3, uint64_t gaddr,
/* Length must be greater than 8, a multiple of 8 */
/* and not greater than maxstbl */
if ((len <= 8) || (len % 8) ||
-(len > pbdev->pci_group->zpci_group.maxstbl)) {
+(len > lduw_p(&pbdev->pci_group->zpci_group.maxstbl))) {
goto specification_error;
}
/* Do not cross a 4K-byte boundary */
diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c
index d5c78063b5bc..f455c6f20a1a 100644
--- a/hw/s390x/s390-pci-vfio.c
+++ b/hw/s390x/s390-pci-vfio.c
@@ -116,10 +116,10 @@ static void s390_pci_read_base(S390PCIBusDevice *pbdev,
}
cap = (void *) hdr;
-pbdev->zpci_fn.sdma = cap->start_dma;
-pbdev->zpci_fn.edma = cap->end_dma;
-pbdev->zpci_fn.pchid = cap->pchid;
-pbdev->zpci_fn.vfn = cap->vfn;
+stq_p(&pbdev->zpci_fn.sdma, cap->start_dma);
+stq_p(&pbdev->zpci_fn.edma, cap->end_dma);
+stw_p(&pbdev->zpci_fn.pchid, cap->pchid);
+stw_p(&pbdev->zpci_fn.vfn, cap->vfn);
pbdev->zpci_fn.pfgid = cap->gid;
/* The following values remain 0 until we support other FMB formats */
pbdev->zpci_fn.fmbl = 0;
--
2.26.2
Re: Use of g_return_if_fail(), g_return_val_if_fail()
On Tue, Nov 17, 2020 at 04:14:38PM +0100, Markus Armbruster wrote: > * block/export/vhost-user-blk-server.c:270:g_return_val_if_fail(len <= > sizeof(struct virtio_blk_config), -1); > > Stefan, why is len > sizeof(struct virtio_blk_config) a programming > error? > > Why is returning safe? Thanks for pointing this out. The vhost-user frontend passed an invalid len and we're validating input. This and the other instances in vhost-user config function in contrib/ should be replaced with explicit input validation. I'll send a patch. Stefan signature.asc Description: PGP signature
Re: [PATCH-for-5.2? v5 0/2] ci: Move trace backend tests across to gitlab
Philippe Mathieu-Daudé writes: > Extracted from "ci: Move various jobs from Travis to GitLab CI": > https://www.mail-archive.com/qemu-devel@nongnu.org/msg758314.html Queued to for-5.2/fixes-for-rc3, thanks. > > v5: > - addressed Wainer's review comment > > Philippe Mathieu-Daudé (2): > tests/docker: Install liblttng-ust-dev package in Ubuntu 20.04 image > gitlab-ci: Move trace backend tests across to gitlab > > .gitlab-ci.yml | 18 ++ > .travis.yml| 19 --- > tests/docker/dockerfiles/ubuntu2004.docker | 1 + > 3 files changed, 19 insertions(+), 19 deletions(-) -- Alex Bennée
Re: [PATCH v6] hw/i386/pc: add max combined fw size as machine configuration option
On Fri, Sep 25, 2020 at 05:57:51PM +, Erich Mcmillan wrote:
> From: Erich McMillan
>
> At Hewlett Packard Inc. we have a need for increased fw size to enable
> testing of our custom fw.
> Move return statement for early return
>
> Signed-off-by: Erich McMillan
My bad that I dropped it by mistake before code freeze.
I will queue it for the next release.
> ---
>
> Changes since v5:
>
> Move return statement for pc_machine_set_max_fw_size() to follow
> error_setg() as early return.
>
> hw/i386/pc.c | 51
> hw/i386/pc_sysfw.c | 13 ++-
> include/hw/i386/pc.h | 2 ++
> 3 files changed, 55 insertions(+), 11 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index d11daacc23..70c8c9adcf 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1869,6 +1869,50 @@ static void pc_machine_set_max_ram_below_4g(Object
> *obj, Visitor *v,
> pcms->max_ram_below_4g = value;
> }
>
> +static void pc_machine_get_max_fw_size(Object *obj, Visitor *v,
> + const char *name, void *opaque,
> + Error **errp)
> +{
> +PCMachineState *pcms = PC_MACHINE(obj);
> +uint64_t value = pcms->max_fw_size;
> +
> +visit_type_size(v, name, &value, errp);
> +}
> +
> +static void pc_machine_set_max_fw_size(Object *obj, Visitor *v,
> + const char *name, void *opaque,
> + Error **errp)
> +{
> +PCMachineState *pcms = PC_MACHINE(obj);
> +Error *error = NULL;
> +uint64_t value;
> +
> +visit_type_size(v, name, &value, &error);
> +if (error) {
> +error_propagate(errp, error);
> +return;
> +}
> +
> +/*
> +* We don't have a theoretically justifiable exact lower bound on the base
> +* address of any flash mapping. In practice, the IO-APIC MMIO range is
> +* [0xFEE0..0xFEE01000] -- see IO_APIC_DEFAULT_ADDRESS --, leaving
> free
> +* only 18MB-4KB below 4G. For now, restrict the cumulative mapping to
> 8MB in
> +* size.
> +*/
> +if (value > 16 * MiB) {
> +error_setg(errp,
> + "User specified max allowed firmware size %" PRIu64 " is "
> + "greater than 16MiB. If combined firwmare size exceeds "
> + "16MiB the system may not boot, or experience
> intermittent"
> + "stability issues.",
> + value);
> +return;
> +}
> +
> +pcms->max_fw_size = value;
> +}
> +
> static void pc_machine_initfn(Object *obj)
> {
> PCMachineState *pcms = PC_MACHINE(obj);
> @@ -1884,6 +1928,7 @@ static void pc_machine_initfn(Object *obj)
> pcms->smbus_enabled = true;
> pcms->sata_enabled = true;
> pcms->pit_enabled = true;
> +pcms->max_fw_size = 8 * MiB;
>
> pc_system_flash_create(pcms);
> pcms->pcspk = isa_new(TYPE_PC_SPEAKER);
> @@ -2004,6 +2049,12 @@ static void pc_machine_class_init(ObjectClass *oc,
> void *data)
>
> object_class_property_add_bool(oc, PC_MACHINE_PIT,
> pc_machine_get_pit, pc_machine_set_pit);
> +
> +object_class_property_add(oc, PC_MACHINE_MAX_FW_SIZE, "size",
> +pc_machine_get_max_fw_size, pc_machine_set_max_fw_size,
> +NULL, NULL);
> +object_class_property_set_description(oc, PC_MACHINE_MAX_FW_SIZE,
> +"Maximum combined firmware size");
> }
>
> static const TypeInfo pc_machine_info = {
> diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
> index b6c0822fe3..22450ba0ef 100644
> --- a/hw/i386/pc_sysfw.c
> +++ b/hw/i386/pc_sysfw.c
> @@ -39,15 +39,6 @@
> #include "hw/block/flash.h"
> #include "sysemu/kvm.h"
>
> -/*
> - * We don't have a theoretically justifiable exact lower bound on the base
> - * address of any flash mapping. In practice, the IO-APIC MMIO range is
> - * [0xFEE0..0xFEE01000] -- see IO_APIC_DEFAULT_ADDRESS --, leaving free
> - * only 18MB-4KB below 4G. For now, restrict the cumulative mapping to 8MB in
> - * size.
> - */
> -#define FLASH_SIZE_LIMIT (8 * MiB)
> -
> #define FLASH_SECTOR_SIZE 4096
>
> static void pc_isa_bios_init(MemoryRegion *rom_memory,
> @@ -182,10 +173,10 @@ static void pc_system_flash_map(PCMachineState *pcms,
> }
> if ((hwaddr)size != size
> || total_size > HWADDR_MAX - size
> -|| total_size + size > FLASH_SIZE_LIMIT) {
> +|| total_size + size > pcms->max_fw_size) {
> error_report("combined size of system firmware exceeds "
> "%" PRIu64 " bytes",
> - FLASH_SIZE_LIMIT);
> + pcms->max_fw_size);
> exit(1);
> }
>
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index fe52e165b2..f7c8e7cbfe 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -43,6 +43,7 @@ struct PCMachineState {
> bool smbus
[PATCH 26/29] Revert "kernel-doc: Handle function typedefs that return pointers"
This reverts commit 19ab6044be0c55d529e875e3ee16fdd5c3b54d33.
We will replace the commit with the fix from Linux.
Signed-off-by: Paolo Bonzini
---
scripts/kernel-doc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 780aee4e92..d3a289628c 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1434,8 +1434,8 @@ sub dump_typedef($$) {
$x =~ s@/\*.*?\*/@@gos;# strip comments.
# Parse function prototypes
-if ($x =~ /typedef\s+(\w+\s*\**)\s*\(\*\s*(\w\S+)\s*\)\s*\((.*)\);/ ||
- $x =~ /typedef\s+(\w+\s*\**)\s*(\w\S+)\s*\s*\((.*)\);/) {
+if ($x =~ /typedef\s+(\w+)\s*\(\*\s*(\w\S+)\s*\)\s*\((.*)\);/ ||
+ $x =~ /typedef\s+(\w+)\s*(\w\S+)\s*\s*\((.*)\);/) {
# Function typedefs
$return_type = $1;
--
2.28.0
[PATCH 29/29] scripts: kernel-doc: use :c:union when needed
From: Mauro Carvalho Chehab
Sphinx C domain code after 3.2.1 will start complaning if :c:struct
would be used for an union type:
.../Documentation/gpu/drm-kms-helpers:352: ../drivers/video/hdmi.c:851:
WARNING: C 'identifier' cross-reference uses wrong tag: reference name is
'union hdmi_infoframe' but found name is 'struct hdmi_infoframe'. Full
reference name is 'union hdmi_infoframe'. Full found name is 'struct
hdmi_infoframe'.
So, let's address this issue too in advance, in order to
avoid future issues.
Signed-off-by: Mauro Carvalho Chehab
Link:
https://lore.kernel.org/r/6e4ec3eec914df62389a299797a3880ae4490f35.1603791716.git.mchehab+hua...@kernel.org
Signed-off-by: Jonathan Corbet
Signed-off-by: Paolo Bonzini
---
scripts/kernel-doc | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 524fc626ed..b95bae3654 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1092,7 +1092,11 @@ sub output_struct_rst(%) {
print "\n\n.. c:type:: " . $name . "\n\n";
} else {
my $name = $args{'struct'};
- print "\n\n.. c:struct:: " . $name . "\n\n";
+ if ($args{'type'} eq 'union') {
+ print "\n\n.. c:union:: " . $name . "\n\n";
+ } else {
+ print "\n\n.. c:struct:: " . $name . "\n\n";
+ }
}
print_lineno($declaration_start_line);
$lineprefix = " ";
--
2.28.0
[PATCH 22/29] scripts: kernel-doc: allow passing desired Sphinx C domain dialect
From: Mauro Carvalho Chehab
When kernel-doc is called via kerneldoc.py, there's no need to
auto-detect the Sphinx version, as the Sphinx module already
knows it. So, add an optional parameter to allow changing the
Sphinx dialect.
As kernel-doc can also be manually called, keep the auto-detection
logic if the parameter was not specified. On such case, emit
a warning if sphinx-build can't be found at PATH.
I ended using a suggestion from Joe for using a more readable
regex, instead of using a complex one with a hidden group like:
m/^(\d+)\.(\d+)(?:\.?(\d+)?)/
in order to get the optional argument.
Thanks-to: Joe Perches
Suggested-by: Jonathan Corbet
Signed-off-by: Mauro Carvalho Chehab
Signed-off-by: Paolo Bonzini
---
scripts/kernel-doc | 51 ++
1 file changed, 43 insertions(+), 8 deletions(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 478037f736..667ad3169c 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -56,6 +56,13 @@ Output format selection (mutually exclusive):
-rst Output reStructuredText format.
-noneDo not output documentation, only warnings.
+Output format selection modifier (affects only ReST output):
+
+ -sphinx-version Use the ReST C domain dialect compatible with an
+ specific Sphinx Version.
+ If not specified, kernel-doc will auto-detect using
+ the sphinx-build version found on PATH.
+
Output selection (mutually exclusive):
-export Only output documentation for symbols that have been
exported using EXPORT_SYMBOL() or EXPORT_SYMBOL_GPL()
@@ -270,7 +277,7 @@ if ($#ARGV == -1) {
}
my $kernelversion;
-my $sphinx_major;
+my ($sphinx_major, $sphinx_minor, $sphinx_patch);
my $dohighlight = "";
@@ -457,6 +464,23 @@ while ($ARGV[0] =~ m/^--?(.*)/) {
$enable_lineno = 1;
} elsif ($cmd eq 'show-not-found') {
$show_not_found = 1; # A no-op but don't fail
+} elsif ($cmd eq "sphinx-version") {
+ my $ver_string = shift @ARGV;
+ if ($ver_string =~ m/^(\d+)(\.\d+)?(\.\d+)?/) {
+ $sphinx_major = $1;
+ if (defined($2)) {
+ $sphinx_minor = substr($2,1);
+ } else {
+ $sphinx_minor = 0;
+ }
+ if (defined($3)) {
+ $sphinx_patch = substr($3,1)
+ } else {
+ $sphinx_patch = 0;
+ }
+ } else {
+ die "Sphinx version should either major.minor or major.minor.patch
format\n";
+ }
} else {
# Unknown argument
usage();
@@ -477,29 +501,37 @@ sub findprog($)
sub get_sphinx_version()
{
my $ver;
- my $major = 1;
my $cmd = "sphinx-build";
if (!findprog($cmd)) {
my $cmd = "sphinx-build3";
- return $major if (!findprog($cmd));
+ if (!findprog($cmd)) {
+ $sphinx_major = 1;
+ $sphinx_minor = 2;
+ $sphinx_patch = 0;
+ printf STDERR "Warning: Sphinx version not found. Using
default (Sphinx version %d.%d.%d)\n",
+ $sphinx_major, $sphinx_minor, $sphinx_patch;
+ return;
+ }
}
open IN, "$cmd --version 2>&1 |";
while () {
if (m/^\s*sphinx-build\s+([\d]+)\.([\d\.]+)(\+\/[\da-f]+)?$/) {
- $major=$1;
+ $sphinx_major = $1;
+ $sphinx_minor = $2;
+ $sphinx_patch = $3;
last;
}
# Sphinx 1.2.x uses a different format
if (m/^\s*Sphinx.*\s+([\d]+)\.([\d\.]+)$/) {
- $major=$1;
+ $sphinx_major = $1;
+ $sphinx_minor = $2;
+ $sphinx_patch = $3;
last;
}
}
close IN;
-
- return $major;
}
# get kernel version from env
@@ -2333,7 +2365,10 @@ sub process_file($) {
}
-$sphinx_major = get_sphinx_version();
+if ($output_mode eq "rst") {
+ get_sphinx_version() if (!$sphinx_major);
+}
+
$kernelversion = get_kernel_version();
# generate a sequence of code that will splice in highlighting information
--
2.28.0
[PATCH 21/29] scripts: kernel-doc: don't mangle with parameter list
From: Mauro Carvalho Chehab
While kernel-doc needs to parse parameters in order to
identify its name, it shouldn't be touching the type,
as parsing it is very difficult, and errors happen.
One current error is when parsing this parameter:
const u32 (*tab)[256]
Found at ./lib/crc32.c, on this function:
u32 __pure crc32_be_generic (u32 crc, unsigned char const *p, size_t
len, const u32 (*tab)[256], u32 polynomial);
The current logic mangles it, producing this output:
const u32 ( *tab
That's something that it is not recognizeable.
So, instead, let's push the argument as-is, and use it
when printing the function prototype and when describing
each argument.
Signed-off-by: Mauro Carvalho Chehab
Signed-off-by: Paolo Bonzini
---
scripts/kernel-doc | 26 ++
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 0c31e9ad66..478037f736 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -655,10 +655,10 @@ sub output_function_man(%) {
$type = $args{'parametertypes'}{$parameter};
if ($type =~ m/([^\(]*\(\*)\s*\)\s*\(([^\)]*)\)/) {
# pointer-to-function
- print ".BI \"" . $parenth . $1 . "\" " . $parameter . " \") (" . $2
. ")" . $post . "\"\n";
+ print ".BI \"" . $parenth . $1 . "\" " . " \") (" . $2 . ")" .
$post . "\"\n";
} else {
$type =~ s/([^\*])$/$1 /;
- print ".BI \"" . $parenth . $type . "\" " . $parameter . " \"" .
$post . "\"\n";
+ print ".BI \"" . $parenth . $type . "\" " . " \"" . $post . "\"\n";
}
$count++;
$parenth = "";
@@ -929,7 +929,7 @@ sub output_function_rst(%) {
# pointer-to-function
print $1 . $parameter . ") (" . $2 . ")";
} else {
- print $type . " " . $parameter;
+ print $type;
}
}
if ($args{'typedef'}) {
@@ -954,7 +954,7 @@ sub output_function_rst(%) {
$type = $args{'parametertypes'}{$parameter};
if ($type ne "") {
- print "``$type $parameter``\n";
+ print "``$type``\n";
} else {
print "``$parameter``\n";
}
@@ -1479,7 +1479,7 @@ sub create_parameterlist() {
# Treat preprocessor directive as a typeless variable just to fill
# corresponding data structures "correctly". Catch it later in
# output_* subs.
- push_parameter($arg, "", $file);
+ push_parameter($arg, "", "", $file);
} elsif ($arg =~ m/\(.+\)\s*\(/) {
# pointer-to-function
$arg =~ tr/#/,/;
@@ -1488,7 +1488,7 @@ sub create_parameterlist() {
$type = $arg;
$type =~ s/([^\(]+\(\*?)\s*$param/$1/;
save_struct_actual($param);
- push_parameter($param, $type, $file, $declaration_name);
+ push_parameter($param, $type, $arg, $file, $declaration_name);
} elsif ($arg) {
$arg =~ s/\s*:\s*/:/g;
$arg =~ s/\s*\[/\[/g;
@@ -1513,26 +1513,28 @@ sub create_parameterlist() {
foreach $param (@args) {
if ($param =~ m/^(\*+)\s*(.*)/) {
save_struct_actual($2);
- push_parameter($2, "$type $1", $file, $declaration_name);
+
+ push_parameter($2, "$type $1", $arg, $file,
$declaration_name);
}
elsif ($param =~ m/(.*?):(\d+)/) {
if ($type ne "") { # skip unnamed bit-fields
save_struct_actual($1);
- push_parameter($1, "$type:$2", $file, $declaration_name)
+ push_parameter($1, "$type:$2", $arg, $file,
$declaration_name)
}
}
else {
save_struct_actual($param);
- push_parameter($param, $type, $file, $declaration_name);
+ push_parameter($param, $type, $arg, $file,
$declaration_name);
}
}
}
}
}
-sub push_parameter() {
+sub push_parameter($) {
my $param = shift;
my $type = shift;
+ my $org_arg = shift;
my $file = shift;
my $declaration_name = shift;
@@ -1596,8 +1598,8 @@ sub push_parameter() {
# "[blah" in a parameter string;
###$param =~ s/\s*//g;
push @parameterlist, $param;
- $type =~ s/\s\s+/ /g;
- $parametertypes{$param} = $type;
+ $org_arg =~ s/\s\s+/ /g;
+ $parametertypes{$param} = $org_arg;
}
sub check_sections($) {
--
2.28.0
[PATCH 20/29] scripts: kernel-doc: fix typedef identification
From: Mauro Carvalho Chehab
Some typedef expressions are output as normal functions.
As we need to be clearer about the type with Sphinx 3.x,
detect such cases.
While here, fix a wrongly-indented block.
Signed-off-by: Mauro Carvalho Chehab
Signed-off-by: Paolo Bonzini
---
scripts/kernel-doc | 64 +-
1 file changed, 41 insertions(+), 23 deletions(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 35d60af834..0c31e9ad66 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1748,30 +1748,48 @@ sub dump_function($$) {
return;
}
- my $prms = join " ", @parameterlist;
- check_sections($file, $declaration_name, "function", $sectcheck, $prms);
-
-# This check emits a lot of warnings at the moment, because many
-# functions don't have a 'Return' doc section. So until the number
-# of warnings goes sufficiently down, the check is only performed in
-# verbose mode.
-# TODO: always perform the check.
-if ($verbose && !$noret) {
-check_return_section($file, $declaration_name, $return_type);
-}
+my $prms = join " ", @parameterlist;
+check_sections($file, $declaration_name, "function", $sectcheck, $prms);
+
+# This check emits a lot of warnings at the moment, because many
+# functions don't have a 'Return' doc section. So until the number
+# of warnings goes sufficiently down, the check is only performed in
+# verbose mode.
+# TODO: always perform the check.
+if ($verbose && !$noret) {
+ check_return_section($file, $declaration_name, $return_type);
+}
-output_declaration($declaration_name,
- 'function',
- {'function' => $declaration_name,
- 'module' => $modulename,
- 'functiontype' => $return_type,
- 'parameterlist' => \@parameterlist,
- 'parameterdescs' => \%parameterdescs,
- 'parametertypes' => \%parametertypes,
- 'sectionlist' => \@sectionlist,
- 'sections' => \%sections,
- 'purpose' => $declaration_purpose
- });
+# The function parser can be called with a typedef parameter.
+# Handle it.
+if ($return_type =~ /typedef/) {
+ output_declaration($declaration_name,
+ 'function',
+ {'function' => $declaration_name,
+ 'typedef' => 1,
+ 'module' => $modulename,
+ 'functiontype' => $return_type,
+ 'parameterlist' => \@parameterlist,
+ 'parameterdescs' => \%parameterdescs,
+ 'parametertypes' => \%parametertypes,
+ 'sectionlist' => \@sectionlist,
+ 'sections' => \%sections,
+ 'purpose' => $declaration_purpose
+ });
+} else {
+ output_declaration($declaration_name,
+ 'function',
+ {'function' => $declaration_name,
+ 'module' => $modulename,
+ 'functiontype' => $return_type,
+ 'parameterlist' => \@parameterlist,
+ 'parameterdescs' => \%parameterdescs,
+ 'parametertypes' => \%parametertypes,
+ 'sectionlist' => \@sectionlist,
+ 'sections' => \%sections,
+ 'purpose' => $declaration_purpose
+ });
+}
}
sub reset_state {
--
2.28.0
[PATCH 17/29] scripts: kernel-doc: use a less pedantic markup for funcs on Sphinx 3.x
From: Mauro Carvalho Chehab
Unfortunately, Sphinx 3.x parser for c functions is too pedantic:
https://github.com/sphinx-doc/sphinx/issues/8241
While it could be relaxed with some configurations, there are
several corner cases that it would make it hard to maintain,
and will require teaching conf.py about several macros.
So, let's instead use the :c:macro notation. This will
produce an output that it is not as nice as currently, but it
should still be acceptable, and will provide cross-references,
removing thousands of warnings when building with newer
versions of Sphinx.
Signed-off-by: Mauro Carvalho Chehab
Signed-off-by: Paolo Bonzini
---
scripts/kernel-doc | 34 --
1 file changed, 24 insertions(+), 10 deletions(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 771367a6ab..75ddd3b5e6 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -886,19 +886,29 @@ sub output_function_rst(%) {
my $oldprefix = $lineprefix;
my $start = "";
-if ($args{'typedef'}) {
- if ($sphinx_major < 3) {
+if ($sphinx_major < 3) {
+ if ($args{'typedef'}) {
print ".. c:type:: ". $args{'function'} . "\n\n";
+ print_lineno($declaration_start_line);
+ print " **Typedef**: ";
+ $lineprefix = "";
+ output_highlight_rst($args{'purpose'});
+ $start = "\n\n**Syntax**\n\n ``";
} else {
- print ".. c:function:: ". $args{'function'} . "\n\n";
+ print ".. c:function:: ";
}
- print_lineno($declaration_start_line);
- print " **Typedef**: ";
- $lineprefix = "";
- output_highlight_rst($args{'purpose'});
- $start = "\n\n**Syntax**\n\n ``";
} else {
- print ".. c:function:: ";
+ print ".. c:macro:: ". $args{'function'} . "\n\n";
+
+ if ($args{'typedef'}) {
+ print_lineno($declaration_start_line);
+ print " **Typedef**: ";
+ $lineprefix = "";
+ output_highlight_rst($args{'purpose'});
+ $start = "\n\n**Syntax**\n\n ``";
+ } else {
+ print "``";
+ }
}
if ($args{'functiontype'} ne "") {
$start .= $args{'functiontype'} . " " . $args{'function'} . " (";
@@ -925,7 +935,11 @@ sub output_function_rst(%) {
if ($args{'typedef'}) {
print ");``\n\n";
} else {
- print ")\n\n";
+ if ($sphinx_major < 3) {
+ print ")\n\n";
+ } else {
+ print ")``\n";
+ }
print_lineno($declaration_start_line);
$lineprefix = " ";
output_highlight_rst($args{'purpose'});
--
2.28.0
[PATCH 25/29] Revert "kernel-doc: Handle function typedefs without asterisks"
This reverts commit 3cd3c5193cde5242e243c25759f85802e267994f.
We will replace the commit with the fix from Linux.
Signed-off-by: Paolo Bonzini
---
scripts/kernel-doc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 2d56c46933..780aee4e92 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1434,7 +1434,7 @@ sub dump_typedef($$) {
$x =~ s@/\*.*?\*/@@gos;# strip comments.
# Parse function prototypes
-if ($x =~ /typedef\s+(\w+\s*\**)\s*\(\*?\s*(\w\S+)\s*\)\s*\((.*)\);/ ||
+if ($x =~ /typedef\s+(\w+\s*\**)\s*\(\*\s*(\w\S+)\s*\)\s*\((.*)\);/ ||
$x =~ /typedef\s+(\w+\s*\**)\s*(\w\S+)\s*\s*\((.*)\);/) {
# Function typedefs
--
2.28.0
[PATCH 18/29] scripts: kernel-doc: fix troubles with line counts
From: Mauro Carvalho Chehab
There's currently a bug with the way kernel-doc script
counts line numbers that can be seen with:
$ ./scripts/kernel-doc -rst -enable-lineno include/linux/math64.h >all
&& ./scripts/kernel-doc -rst -internal -enable-lineno include/linux/math64.h
>int && diff -U0 int all
--- int 2020-09-28 12:58:08.927486808 +0200
+++ all 2020-09-28 12:58:08.905486845 +0200
@@ -1 +1 @@
-#define LINENO 27
+#define LINENO 26
@@ -3 +3 @@
-#define LINENO 16
+#define LINENO 15
@@ -9 +9 @@
-#define LINENO 17
+#define LINENO 16
...
This is happening with perl version 5.30.3, but I'm not
so sure if this is a perl bug, or if this is due to something
else.
In any case, fixing it is easy. Basically, when "-internal"
parameter is used, the process_export_file() function opens the
handle "IN". This makes the line number to be incremented, as the
handler for the main open is also "IN".
Fix the problem by using a different handler for the
main open().
While here, add a missing close for it.
Signed-off-by: Mauro Carvalho Chehab
Signed-off-by: Paolo Bonzini
---
scripts/kernel-doc | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 75ddd3b5e6..f33a4b1cc7 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -2268,7 +2268,7 @@ sub process_file($) {
$file = map_filename($orig_file);
-if (!open(IN,"<$file")) {
+if (!open(IN_FILE,"<$file")) {
print STDERR "Error: Cannot open file $file\n";
++$errors;
return;
@@ -2277,9 +2277,9 @@ sub process_file($) {
$. = 1;
$section_counter = 0;
-while () {
+while () {
while (s/\\\s*$//) {
- $_ .= ;
+ $_ .= ;
}
# Replace tabs by spaces
while ($_ =~ s/\t+/' ' x (length($&) * 8 - length($`) % 8)/e) {};
@@ -2311,6 +2311,7 @@ sub process_file($) {
print STDERR "${file}:1: warning: no structured comments found\n";
}
}
+close IN_FILE;
}
--
2.28.0
[PATCH 24/29] scripts: kernel-doc: try to use c:function if possible
From: Mauro Carvalho Chehab
There are a few namespace clashes by using c:macro everywhere:
basically, when using it, we can't have something like:
.. c:struct:: pwm_capture
.. c:macro:: pwm_capture
So, we need to use, instead:
.. c:function:: int pwm_capture (struct pwm_device * pwm, struct
pwm_capture * result, unsigned long timeout)
for the function declaration.
The kernel-doc change was proposed by Jakob Lykke Andersen here:
https://github.com/jakobandersen/linux_docs/commit/6fd2076ec001cca7466857493cd678df4dfe4a65
Although I did a different implementation.
Signed-off-by: Mauro Carvalho Chehab
Signed-off-by: Paolo Bonzini
---
scripts/kernel-doc | 23 ++-
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 98752164eb..2d56c46933 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -917,6 +917,7 @@ sub output_function_rst(%) {
my ($parameter, $section);
my $oldprefix = $lineprefix;
my $start = "";
+my $is_macro = 0;
if ($sphinx_major < 3) {
if ($args{'typedef'}) {
@@ -926,11 +927,17 @@ sub output_function_rst(%) {
$lineprefix = "";
output_highlight_rst($args{'purpose'});
$start = "\n\n**Syntax**\n\n ``";
+ $is_macro = 1;
} else {
print ".. c:function:: ";
}
} else {
- print ".. c:macro:: ". $args{'function'} . "\n\n";
+ if ($args{'typedef'} || $args{'functiontype'} eq "") {
+ $is_macro = 1;
+ print ".. c:macro:: ". $args{'function'} . "\n\n";
+ } else {
+ print ".. c:function:: ";
+ }
if ($args{'typedef'}) {
print_lineno($declaration_start_line);
@@ -939,7 +946,7 @@ sub output_function_rst(%) {
output_highlight_rst($args{'purpose'});
$start = "\n\n**Syntax**\n\n ``";
} else {
- print "``";
+ print "``" if ($is_macro);
}
}
if ($args{'functiontype'} ne "") {
@@ -964,14 +971,12 @@ sub output_function_rst(%) {
print $type;
}
}
-if ($args{'typedef'}) {
- print ");``\n\n";
+if ($is_macro) {
+ print ")``\n\n";
} else {
- if ($sphinx_major < 3) {
- print ")\n\n";
- } else {
- print ")``\n";
- }
+ print ")\n\n";
+}
+if (!$args{'typedef'}) {
print_lineno($declaration_start_line);
$lineprefix = " ";
output_highlight_rst($args{'purpose'});
--
2.28.0
[PATCH 14/29] Revert "scripts/kerneldoc: For Sphinx 3 use c:macro for macros with arguments"
This reverts commit 92bb29f9b2c3d4a98eef5f0db935d4be291eec72.
We will replace the commit with the fix from Linux.
Signed-off-by: Paolo Bonzini
---
scripts/kernel-doc | 18 +-
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 073f72c7da..cb603532ed 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -860,23 +860,7 @@ sub output_function_rst(%) {
output_highlight_rst($args{'purpose'});
$start = "\n\n**Syntax**\n\n ``";
} else {
-if ((split(/\./, $sphinx_version))[0] >= 3) {
-# Sphinx 3 and later distinguish macros and functions and
-# complain if you use c:function with something that's not
-# syntactically valid as a function declaration.
-# We assume that anything with a return type is a function
-# and anything without is a macro.
-if ($args{'functiontype'} ne "") {
-print ".. c:function:: ";
-} else {
-print ".. c:macro:: ";
-}
-} else {
-# Older Sphinx don't support documenting macros that take
-# arguments with c:macro, and don't complain about the use
-# of c:function for this.
-print ".. c:function:: ";
-}
+ print ".. c:function:: ";
}
if ($args{'functiontype'} ne "") {
$start .= $args{'functiontype'} . " " . $args{'function'} . " (";
--
2.28.0
[PATCH 28/29] scripts: kernel-doc: split typedef complex regex
From: Mauro Carvalho Chehab
The typedef regex for function prototypes are very complex.
Split them into 3 separate regex and then join them using
qr.
Signed-off-by: Mauro Carvalho Chehab
Link:
https://lore.kernel.org/r/3a4af999a0d62d4ab9dfae1cdefdfcad93383356.1603792384.git.mchehab+hua...@kernel.org
Signed-off-by: Jonathan Corbet
Signed-off-by: Paolo Bonzini
---
scripts/kernel-doc | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 862b77686e..524fc626ed 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1427,17 +1427,21 @@ sub dump_enum($$) {
}
}
+my $typedef_type = qr { ((?:\s+[\w\*]+){1,8})\s* }x;
+my $typedef_ident = qr { \*?\s*(\w\S+)\s* }x;
+my $typedef_args = qr { \s*\((.*)\); }x;
+
+my $typedef1 = qr { typedef$typedef_type\($typedef_ident\)$typedef_args }x;
+my $typedef2 = qr { typedef$typedef_type$typedef_ident$typedef_args }x;
+
sub dump_typedef($$) {
my $x = shift;
my $file = shift;
$x =~ s@/\*.*?\*/@@gos;# strip comments.
-# Parse function prototypes
-if ($x =~
/typedef((?:\s+[\w\*]+){1,8})\s*\(\*?\s*(\w\S+)\s*\)\s*\((.*)\);/ ||
- $x =~ /typedef((?:\s+[\w\*]+\s+){1,8})\s*\*?(\w\S+)\s*\s*\((.*)\);/) {
-
- # Function typedefs
+# Parse function typedef prototypes
+if ($x =~ $typedef1 || $x =~ $typedef2) {
$return_type = $1;
$declaration_name = $2;
my $args = $3;
--
2.28.0
[PATCH 16/29] scripts: kernel-doc: make it more compatible with Sphinx 3.x
From: Mauro Carvalho Chehab
With Sphinx 3.x, the ".. c:type:" tag was changed to accept either:
.. c:type:: typedef-like declaration
.. c:type:: name
Using it for other types (including functions) don't work anymore.
So, there are newer tags for macro, enum, struct, union, and others,
which doesn't exist on older versions.
Add a check for the Sphinx version and change the produced tags
accordingly.
Signed-off-by: Mauro Carvalho Chehab
Signed-off-by: Paolo Bonzini
---
scripts/kernel-doc | 71 ++
1 file changed, 65 insertions(+), 6 deletions(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 60f75cd176..771367a6ab 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -271,6 +271,8 @@ if ($#ARGV == -1) {
}
my $kernelversion;
+my $sphinx_major;
+
my $dohighlight = "";
my $verbose = 0;
@@ -465,6 +467,43 @@ while ($ARGV[0] =~ m/^--?(.*)/) {
# continue execution near EOF;
+# The C domain dialect changed on Sphinx 3. So, we need to check the
+# version in order to produce the right tags.
+sub findprog($)
+{
+ foreach(split(/:/, $ENV{PATH})) {
+ return "$_/$_[0]" if(-x "$_/$_[0]");
+ }
+}
+
+sub get_sphinx_version()
+{
+ my $ver;
+ my $major = 1;
+
+ my $cmd = "sphinx-build";
+ if (!findprog($cmd)) {
+ my $cmd = "sphinx-build3";
+ return $major if (!findprog($cmd));
+ }
+
+ open IN, "$cmd --version 2>&1 |";
+ while () {
+ if (m/^\s*sphinx-build\s+([\d]+)\.([\d\.]+)(\+\/[\da-f]+)?$/) {
+ $major=$1;
+ last;
+ }
+ # Sphinx 1.2.x uses a different format
+ if (m/^\s*Sphinx.*\s+([\d]+)\.([\d\.]+)$/) {
+ $major=$1;
+ last;
+ }
+ }
+ close IN;
+
+ return $major;
+}
+
# get kernel version from env
sub get_kernel_version() {
my $version = 'unknown kernel version';
@@ -848,7 +887,11 @@ sub output_function_rst(%) {
my $start = "";
if ($args{'typedef'}) {
- print ".. c:type:: ". $args{'function'} . "\n\n";
+ if ($sphinx_major < 3) {
+ print ".. c:type:: ". $args{'function'} . "\n\n";
+ } else {
+ print ".. c:function:: ". $args{'function'} . "\n\n";
+ }
print_lineno($declaration_start_line);
print " **Typedef**: ";
$lineprefix = "";
@@ -938,9 +981,14 @@ sub output_enum_rst(%) {
my ($parameter);
my $oldprefix = $lineprefix;
my $count;
-my $name = "enum " . $args{'enum'};
-print "\n\n.. c:type:: " . $name . "\n\n";
+if ($sphinx_major < 3) {
+ my $name = "enum " . $args{'enum'};
+ print "\n\n.. c:type:: " . $name . "\n\n";
+} else {
+ my $name = $args{'enum'};
+ print "\n\n.. c:enum:: " . $name . "\n\n";
+}
print_lineno($declaration_start_line);
$lineprefix = " ";
output_highlight_rst($args{'purpose'});
@@ -966,8 +1014,13 @@ sub output_typedef_rst(%) {
my %args = %{$_[0]};
my ($parameter);
my $oldprefix = $lineprefix;
-my $name = "typedef " . $args{'typedef'};
+my $name;
+if ($sphinx_major < 3) {
+ $name = "typedef " . $args{'typedef'};
+} else {
+ $name = $args{'typedef'};
+}
print "\n\n.. c:type:: " . $name . "\n\n";
print_lineno($declaration_start_line);
$lineprefix = " ";
@@ -982,9 +1035,14 @@ sub output_struct_rst(%) {
my %args = %{$_[0]};
my ($parameter);
my $oldprefix = $lineprefix;
-my $name = $args{'type'} . " " . $args{'struct'};
-print "\n\n.. c:type:: " . $name . "\n\n";
+if ($sphinx_major < 3) {
+ my $name = $args{'type'} . " " . $args{'struct'};
+ print "\n\n.. c:type:: " . $name . "\n\n";
+} else {
+ my $name = $args{'struct'};
+ print "\n\n.. c:struct:: " . $name . "\n\n";
+}
print_lineno($declaration_start_line);
$lineprefix = " ";
output_highlight_rst($args{'purpose'});
@@ -2242,6 +2300,7 @@ sub process_file($) {
}
+$sphinx_major = get_sphinx_version();
$kernelversion = get_kernel_version();
# generate a sequence of code that will splice in highlighting information
--
2.28.0
[PATCH 15/29] Revert "kernel-doc: Use c:struct for Sphinx 3.0 and later"
This reverts commit 152d1967f650f67b7ece3a5dda77d48069d72647.
We will replace the commit with the fix from Linux.
Signed-off-by: Paolo Bonzini
---
docs/sphinx/kerneldoc.py | 1 -
scripts/kernel-doc | 16 +---
2 files changed, 1 insertion(+), 16 deletions(-)
diff --git a/docs/sphinx/kerneldoc.py b/docs/sphinx/kerneldoc.py
index 3ac277d162..0df6a0814d 100644
--- a/docs/sphinx/kerneldoc.py
+++ b/docs/sphinx/kerneldoc.py
@@ -99,7 +99,6 @@ class KernelDocDirective(Directive):
env.note_dependency(os.path.abspath(f))
cmd += ['-export-file', f]
-cmd += ['-sphinx-version', sphinx.__version__]
cmd += [filename]
try:
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index cb603532ed..60f75cd176 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -71,8 +71,6 @@ Output selection (mutually exclusive):
DOC: sections. May be specified multiple times.
Output selection modifiers:
- -sphinx-version VER Generate rST syntax for the specified Sphinx version.
-Only works with reStructuredTextFormat.
-no-doc-sections Do not output DOC: sections.
-enable-linenoEnable output of #define LINENO lines. Only works with
reStructuredText format.
@@ -294,7 +292,6 @@ use constant {
};
my $output_selection = OUTPUT_ALL;
my $show_not_found = 0;# No longer used
-my $sphinx_version = "0.0"; # if not specified, assume old
my @export_file_list;
@@ -460,8 +457,6 @@ while ($ARGV[0] =~ m/^--?(.*)/) {
$enable_lineno = 1;
} elsif ($cmd eq 'show-not-found') {
$show_not_found = 1; # A no-op but don't fail
-} elsif ($cmd eq 'sphinx-version') {
-$sphinx_version = shift @ARGV;
} else {
# Unknown argument
usage();
@@ -989,16 +984,7 @@ sub output_struct_rst(%) {
my $oldprefix = $lineprefix;
my $name = $args{'type'} . " " . $args{'struct'};
-# Sphinx 3.0 and up will emit warnings for "c:type:: struct Foo".
-# It wants to see "c:struct:: Foo" (and will add the word 'struct' in
-# the rendered output).
-if ((split(/\./, $sphinx_version))[0] >= 3) {
-my $sname = $name;
-$sname =~ s/^struct //;
-print "\n\n.. c:struct:: " . $sname . "\n\n";
-} else {
-print "\n\n.. c:type:: " . $name . "\n\n";
-}
+print "\n\n.. c:type:: " . $name . "\n\n";
print_lineno($declaration_start_line);
$lineprefix = " ";
output_highlight_rst($args{'purpose'});
--
2.28.0
[PATCH 11/29] kernel-doc: include line numbers for function prototypes
From: Mauro Carvalho Chehab
This should solve bad error reports like this one:
./include/linux/iio/iio.h:0: WARNING: Unknown target name: "devm".
Signed-off-by: Mauro Carvalho Chehab
Link:
https://lore.kernel.org/r/56eed0ba50cd726236acd12b11b55ce54854c5ea.1599660067.git.mchehab+hua...@kernel.org
Signed-off-by: Jonathan Corbet
Signed-off-by: Paolo Bonzini
---
scripts/kernel-doc | 2 ++
1 file changed, 2 insertions(+)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index eb635eb94c..3fd6f3925e 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1624,6 +1624,8 @@ sub dump_function($$) {
my $file = shift;
my $noret = 0;
+print_lineno($.);
+
$prototype =~ s/^static +//;
$prototype =~ s/^extern +//;
$prototype =~ s/^asmlinkage +//;
--
2.28.0
[PATCH 27/29] scripts: kernel-doc: fix typedef parsing
From: Mauro Carvalho Chehab
The include/linux/genalloc.h file defined this typedef:
typedef unsigned long (*genpool_algo_t)(unsigned long *map,unsigned
long size,unsigned long start,unsigned int nr,void *data, struct gen_pool
*pool, unsigned long start_addr);
Because it has a type composite of two words (unsigned long),
the parser gets the typedef name wrong:
.. c:macro:: long
**Typedef**: Allocation callback function type definition
Fix the regex in order to accept composite types when
defining a typedef for a function pointer.
Signed-off-by: Mauro Carvalho Chehab
Link:
https://lore.kernel.org/r/328e8018041cc44f7a1684e57f8d111230761c4f.1603792384.git.mchehab+hua...@kernel.org
Signed-off-by: Jonathan Corbet
Signed-off-by: Paolo Bonzini
---
scripts/kernel-doc | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index d3a289628c..862b77686e 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1434,13 +1434,14 @@ sub dump_typedef($$) {
$x =~ s@/\*.*?\*/@@gos;# strip comments.
# Parse function prototypes
-if ($x =~ /typedef\s+(\w+)\s*\(\*\s*(\w\S+)\s*\)\s*\((.*)\);/ ||
- $x =~ /typedef\s+(\w+)\s*(\w\S+)\s*\s*\((.*)\);/) {
+if ($x =~
/typedef((?:\s+[\w\*]+){1,8})\s*\(\*?\s*(\w\S+)\s*\)\s*\((.*)\);/ ||
+ $x =~ /typedef((?:\s+[\w\*]+\s+){1,8})\s*\*?(\w\S+)\s*\s*\((.*)\);/) {
# Function typedefs
$return_type = $1;
$declaration_name = $2;
my $args = $3;
+ $return_type =~ s/^\s+//;
create_parameterlist($args, ',', $file, $declaration_name);
--
2.28.0
[PATCH 12/29] kernel-doc: add support for ____cacheline_aligned attribute
From: Jonathan Cameron
Subroutine dump_struct uses type attributes to check if the struct
syntax is valid. Then, it removes all attributes before using it for
output. `cacheline_aligned` is an attribute that is
not included in both steps. Add it, since it is used by kernel structs.
Based on previous patch to add cacheline_aligned_in_smp.
Motivated by patches to reorder this attribute to before the
variable name. Whilst we could do that in all cases, that would
be a massive change and it is more common in the kernel to place
this particular attribute after the variable name. A quick grep
suggests approximately 400 instances of which 341 have this
attribute just before a semicolon and hence after the variable name.
Signed-off-by: Jonathan Cameron
Cc: Lee Jones
Link: https://lore.kernel.org/r/20200910185415.653139-1-ji...@kernel.org
Signed-off-by: Jonathan Corbet
Signed-off-by: Paolo Bonzini
---
scripts/kernel-doc | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 3fd6f3925e..c4c5640ded 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1113,7 +1113,7 @@ sub dump_struct($$) {
my $x = shift;
my $file = shift;
-if ($x =~
/(struct|union)\s+(\w+)\s*\{(.*)\}(\s*(__packed|__aligned|cacheline_aligned_in_smp|__attribute__\s*\(\([a-z0-9,_\s\(\)]*\)\)))*/)
{
+if ($x =~
/(struct|union)\s+(\w+)\s*\{(.*)\}(\s*(__packed|__aligned|cacheline_aligned_in_smp|cacheline_aligned|__attribute__\s*\(\([a-z0-9,_\s\(\)]*\)\)))*/)
{
my $decl_type = $1;
$declaration_name = $2;
my $members = $3;
@@ -1129,6 +1129,7 @@ sub dump_struct($$) {
$members =~ s/\s*__packed\s*/ /gos;
$members =~ s/\s*CRYPTO_MINALIGN_ATTR/ /gos;
$members =~ s/\s*cacheline_aligned_in_smp/ /gos;
+ $members =~ s/\s*cacheline_aligned/ /gos;
# replace DECLARE_BITMAP
$members =~
s/__ETHTOOL_DECLARE_LINK_MODE_MASK\s*\(([^\)]+)\)/DECLARE_BITMAP($1,
__ETHTOOL_LINK_MODE_MASK_NBITS)/gos;
--
2.28.0
