Re: [Intel-gfx] [patch 27/30] xen/events: Only force affinity mask for percpu interrupts
On 11.12.20 00:20, boris.ostrov...@oracle.com wrote: On 12/10/20 2:26 PM, Thomas Gleixner wrote: All event channel setups bind the interrupt on CPU0 or the target CPU for percpu interrupts and overwrite the affinity mask with the corresponding cpumask. That does not make sense. The XEN implementation of irqchip::irq_set_affinity() already picks a single target CPU out of the affinity mask and the actual target is stored in the effective CPU mask, so destroying the user chosen affinity mask which might contain more than one CPU is wrong. Change the implementation so that the channel is bound to CPU0 at the XEN level and leave the affinity mask alone. At startup of the interrupt affinity will be assigned out of the affinity mask and the XEN binding will be updated. If that's the case then I wonder whether we need this call at all and instead bind at startup time. After some discussion with Thomas on IRC and xen-devel archaeology the result is: this will be needed especially for systems running on a single vcpu (e.g. small guests), as the .irq_set_affinity() callback won't be called in this case when starting the irq. Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: application/pgp-keys OpenPGP_signature Description: OpenPGP digital signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [patch 27/30] xen/events: Only force affinity mask for percpu interrupts
On 11.12.20 11:13, Thomas Gleixner wrote:
On Fri, Dec 11 2020 at 07:17, Jürgen Groß wrote:
On 11.12.20 00:20, boris.ostrov...@oracle.com wrote:
On 12/10/20 2:26 PM, Thomas Gleixner wrote:
All event channel setups bind the interrupt on CPU0 or the target CPU for
percpu interrupts and overwrite the affinity mask with the corresponding
cpumask. That does not make sense.
The XEN implementation of irqchip::irq_set_affinity() already picks a
single target CPU out of the affinity mask and the actual target is stored
in the effective CPU mask, so destroying the user chosen affinity mask
which might contain more than one CPU is wrong.
Change the implementation so that the channel is bound to CPU0 at the XEN
level and leave the affinity mask alone. At startup of the interrupt
affinity will be assigned out of the affinity mask and the XEN binding will
be updated.
If that's the case then I wonder whether we need this call at all and instead
bind at startup time.
This binding to cpu0 was introduced with commit 97253eeeb792d61ed2
and I have no reason to believe the underlying problem has been
eliminated.
"The kernel-side VCPU binding was not being correctly set for newly
allocated or bound interdomain events. In ARM guests where 2-level
events were used, this would result in no interdomain events being
handled because the kernel-side VCPU masks would all be clear.
x86 guests would work because the irq affinity was set during irq
setup and this would set the correct kernel-side VCPU binding."
I'm not convinced that this is really correctly analyzed because affinity
setting is done at irq startup.
switch (__irq_startup_managed(desc, aff, force)) {
case IRQ_STARTUP_NORMAL:
ret = __irq_startup(desc);
irq_setup_affinity(desc);
break;
which is completely architecture agnostic. So why should this magically
work on x86 and not on ARM if both are using the same XEN irqchip with
the same irqchip callbacks.
I think this might be related to _initial_ cpu binding of events and
changing the binding later. This might be handled differently in the
hypervisor.
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys
OpenPGP_signature
Description: OpenPGP digital signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [patch 27/30] xen/events: Only force affinity mask for percpu interrupts
On 11.12.20 00:20, boris.ostrov...@oracle.com wrote: On 12/10/20 2:26 PM, Thomas Gleixner wrote: All event channel setups bind the interrupt on CPU0 or the target CPU for percpu interrupts and overwrite the affinity mask with the corresponding cpumask. That does not make sense. The XEN implementation of irqchip::irq_set_affinity() already picks a single target CPU out of the affinity mask and the actual target is stored in the effective CPU mask, so destroying the user chosen affinity mask which might contain more than one CPU is wrong. Change the implementation so that the channel is bound to CPU0 at the XEN level and leave the affinity mask alone. At startup of the interrupt affinity will be assigned out of the affinity mask and the XEN binding will be updated. If that's the case then I wonder whether we need this call at all and instead bind at startup time. This binding to cpu0 was introduced with commit 97253eeeb792d61ed2 and I have no reason to believe the underlying problem has been eliminated. Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: application/pgp-keys OpenPGP_signature Description: OpenPGP digital signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 0/5] Fixes and improvements for Xen pvdrm
On 13.08.20 17:10, Oleksandr Andrushchenko wrote: On 8/13/20 6:02 PM, Jürgen Groß wrote: On 13.08.20 08:21, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko Series pushed to: xen/tip.git for-linus-5.9 The top patch has strange title though: "Subject: [PATCH v2 5/5] drm/xen-front: Pass dumb buffer data offset to the backend" Oh, indeed. I had to repair it manually as it seems some mail system (probably on my end) mangled it a little bit. Will repair it. Juergen ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 0/5] Fixes and improvements for Xen pvdrm
On 13.08.20 08:21, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko Series pushed to: xen/tip.git for-linus-5.9 Juergen ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 0/5] Fixes and improvements for Xen pvdrm
On 13.08.20 08:32, Oleksandr Andrushchenko wrote: Juergen, Boris, can we please merge these via Xen Linux tree as I have collected enough Ack/R-b? The series has DRM patches, but those anyway are Xen related, so I think this should be fine from DRI point of view. Yes, fine with me. Juergen Thank you, Oleksandr On 8/13/20 9:21 AM, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko Hello, This series contains an assorted set of fixes and improvements for the Xen para-virtualized display driver and grant device driver which I have collected over the last couple of months: 1. Minor fixes to grant device driver and drm/xen-front. 2. New format (YUYV) added to the list of the PV DRM supported formats which allows the driver to be used in zero-copying use-cases when a camera device is the source of the dma-bufs. 3. Synchronization with the latest para-virtualized protocol definition in Xen [1]. 4. SGT offset is now propagated to the backend: while importing a dmabuf it is possible that the data of the buffer is put with offset which is indicated by the SGT offset. This is needed for some GPUs which have non-zero offset. Thank you, Oleksandr Andrushchenko [1] https://urldefense.com/v3/__https://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=c27a184225eab54d20435c8cab5ad0ef384dc2c0__;!!GF_29dbcQIUBPA!iAHOdk4M167VNM1AypMGVmyKJu-iqC9e5cXyu6N595Np3iyIZDnZl0MIBX3IROJSD1GSMX_GfQ$ [xenbits[.]xen[.]org] Changes since v1: = 1. Removed patch which adds EDID to PV DRM as it needs more time for review: "5. Version 2 of the Xen displif protocol adds XENDISPL_OP_GET_EDID request which allows frontends to request EDID structure per connector. This request is optional and if not supported by the backend then visible area is still defined by the relevant XenStore's "resolution" property. If backend provides EDID with XENDISPL_OP_GET_EDID request then its values must take precedence over the resolutions defined in XenStore." I will send this as a dedicated patch. 2. Added missing CC stable for the patches with fixes Oleksandr Andrushchenko (5): xen/gntdev: Fix dmabuf import with non-zero sgt offset drm/xen-front: Fix misused IS_ERR_OR_NULL checks drm/xen-front: Add YUYV to supported formats xen: Sync up with the canonical protocol definition in Xen drm/xen-front: Pass dumb buffer data offset to the backend drivers/gpu/drm/xen/xen_drm_front.c | 10 +-- drivers/gpu/drm/xen/xen_drm_front.h | 2 +- drivers/gpu/drm/xen/xen_drm_front_conn.c | 1 + drivers/gpu/drm/xen/xen_drm_front_gem.c | 11 +-- drivers/gpu/drm/xen/xen_drm_front_kms.c | 2 +- drivers/xen/gntdev-dmabuf.c | 8 +++ include/xen/interface/io/displif.h | 91 +++- 7 files changed, 111 insertions(+), 14 deletions(-) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/6] drm/xen-front: Fix misused IS_ERR_OR_NULL checks
On 04.08.20 08:35, Oleksandr Andrushchenko wrote:
On 8/4/20 9:12 AM, Jürgen Groß wrote:
On 31.07.20 14:51, Oleksandr Andrushchenko wrote:
From: Oleksandr Andrushchenko
The patch c575b7eeb89f: "drm/xen-front: Add support for Xen PV
display frontend" from Apr 3, 2018, leads to the following static
checker warning:
drivers/gpu/drm/xen/xen_drm_front_gem.c:140 xen_drm_front_gem_create()
warn: passing zero to 'ERR_CAST'
drivers/gpu/drm/xen/xen_drm_front_gem.c
133 struct drm_gem_object *xen_drm_front_gem_create(struct drm_device
*dev,
134 size_t size)
135 {
136 struct xen_gem_object *xen_obj;
137
138 xen_obj = gem_create(dev, size);
139 if (IS_ERR_OR_NULL(xen_obj))
140 return ERR_CAST(xen_obj);
Fix this and the rest of misused places with IS_ERR_OR_NULL in the
driver.
Fixes: c575b7eeb89f: "drm/xen-front: Add support for Xen PV display frontend"
Again forgot to Cc stable?
I was just not sure if these minor fixes need to go the stable, but I will add
I'm fine both ways.
Its just a reflex when I'm seeing a Fixes: tag but no Cc: stable. :-)
Juergen
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/6] xen: Sync up with the canonical protocol definition in Xen
On 31.07.20 14:51, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko This is the sync up with the canonical definition of the display protocol in Xen. 1. Add protocol version as an integer Version string, which is in fact an integer, is hard to handle in the code that supports different protocol versions. To simplify that also add the version as an integer. 2. Pass buffer offset with XENDISPL_OP_DBUF_CREATE There are cases when display data buffer is created with non-zero offset to the data start. Handle such cases and provide that offset while creating a display buffer. 3. Add XENDISPL_OP_GET_EDID command Add an optional request for reading Extended Display Identification Data (EDID) structure which allows better configuration of the display connectors over the configuration set in XenStore. With this change connectors may have multiple resolutions defined with respect to detailed timing definitions and additional properties normally provided by displays. If this request is not supported by the backend then visible area is defined by the relevant XenStore's "resolution" property. If backend provides extended display identification data (EDID) with XENDISPL_OP_GET_EDID request then EDID values must take precedence over the resolutions defined in XenStore. 4. Bump protocol version to 2. Signed-off-by: Oleksandr Andrushchenko Reviewed-by: Juergen Gross Juergen ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/6] drm/xen-front: Fix misused IS_ERR_OR_NULL checks
On 31.07.20 14:51, Oleksandr Andrushchenko wrote:
From: Oleksandr Andrushchenko
The patch c575b7eeb89f: "drm/xen-front: Add support for Xen PV
display frontend" from Apr 3, 2018, leads to the following static
checker warning:
drivers/gpu/drm/xen/xen_drm_front_gem.c:140 xen_drm_front_gem_create()
warn: passing zero to 'ERR_CAST'
drivers/gpu/drm/xen/xen_drm_front_gem.c
133 struct drm_gem_object *xen_drm_front_gem_create(struct drm_device *dev,
134 size_t size)
135 {
136 struct xen_gem_object *xen_obj;
137
138 xen_obj = gem_create(dev, size);
139 if (IS_ERR_OR_NULL(xen_obj))
140 return ERR_CAST(xen_obj);
Fix this and the rest of misused places with IS_ERR_OR_NULL in the
driver.
Fixes: c575b7eeb89f: "drm/xen-front: Add support for Xen PV display frontend"
Again forgot to Cc stable?
Juergen
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/6] xen/gntdev: Fix dmabuf import with non-zero sgt offset
On 31.07.20 14:51, Oleksandr Andrushchenko wrote:
From: Oleksandr Andrushchenko
It is possible that the scatter-gather table during dmabuf import has
non-zero offset of the data, but user-space doesn't expect that.
Fix this by failing the import, so user-space doesn't access wrong data.
Fixes: 37ccb44d0b00 ("xen/gntdev: Implement dma-buf import functionality")
I can't find this commit in the tree. Did you mean bf8dc55b1358?
And don't you want to Cc stable for this patch, too?
Signed-off-by: Oleksandr Andrushchenko
Acked-by: Juergen Gross
Juergen
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 13/15] xen/gntdev-dmabuf: Ditch dummy map functions
On 18.11.19 11:35, Daniel Vetter wrote: There's no in-kernel users for the k(un)map stuff. And the mmap one is actively harmful - return 0 and then _not_ actually mmaping can't end well. Signed-off-by: Daniel Vetter Cc: Boris Ostrovsky Cc: Juergen Gross Cc: Stefano Stabellini Cc: xen-de...@lists.xenproject.org Reviewed-by: Juergen Gross -- Ack for merging this through drm trees very much appreciated. Yes, I'm fine with that. Juergen ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
