[Xen-devel] [qemu-mainline test] 142548: regressions - FAIL
flight 142548 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/142548/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-i386-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-amd64-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 140282 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-amd64-qemuu-nested-intel 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-amd64-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-amd64-qemuu-nested-amd 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-i386-qemuu-rhel6hvm-intel 10 redhat-install fail REGR. vs. 140282 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 140282 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-i386-qemuu-rhel6hvm-amd 10 redhat-install fail REGR. vs. 140282 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-i386-freebsd10-amd64 11 guest-start fail REGR. vs. 140282 test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 140282 test-amd64-i386-freebsd10-i386 11 guest-startfail REGR. vs. 140282 test-amd64-i386-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 140282 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 140282 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 140282 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 140282 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14
[Xen-devel] Commit moratorium for getting a push
Committers, Please don't push any new patch to staging because we want to have a push to master for 4.13-RC1. Another email will be sent once the moratorium is lifted. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen: Stop abusing DT of_dma_configure API
On Tue, 8 Oct 2019, Rob Herring wrote:
> As the removed comments say, these aren't DT based devices.
> of_dma_configure() is going to stop allowing a NULL DT node and calling
> it will no longer work.
>
> The comment is also now out of date as of commit 9ab91e7c5c51 ("arm64:
> default to the direct mapping in get_arch_dma_ops"). Direct mapping
> is now the default rather than dma_dummy_ops.
>
> According to Stefano and Oleksandr, the only other part needed is
> setting the DMA masks and there's no reason to restrict the masks to
> 32-bits. So set the masks to 64 bits.
>
> Cc: Robin Murphy
> Cc: Julien Grall
> Cc: Nicolas Saenz Julienne
> Cc: Oleksandr Andrushchenko
> Cc: Boris Ostrovsky
> Cc: Juergen Gross
> Cc: Stefano Stabellini
> Cc: Christoph Hellwig
> Cc: xen-devel@lists.xenproject.org
> Signed-off-by: Rob Herring
Reviewed-by: Stefano Stabellini
> ---
> v2:
> - Setup dma masks
> - Also fix xen_drm_front.c
>
> This can now be applied to the Xen tree independent of the coming
> of_dma_configure() changes.
>
> Rob
>
> drivers/gpu/drm/xen/xen_drm_front.c | 12 ++--
> drivers/xen/gntdev.c| 13 ++---
> 2 files changed, 4 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/xen/xen_drm_front.c
> b/drivers/gpu/drm/xen/xen_drm_front.c
> index ba1828acd8c9..4be49c1aef51 100644
> --- a/drivers/gpu/drm/xen/xen_drm_front.c
> +++ b/drivers/gpu/drm/xen/xen_drm_front.c
> @@ -718,17 +718,9 @@ static int xen_drv_probe(struct xenbus_device *xb_dev,
> struct device *dev = _dev->dev;
> int ret;
>
> - /*
> - * The device is not spawn from a device tree, so arch_setup_dma_ops
> - * is not called, thus leaving the device with dummy DMA ops.
> - * This makes the device return error on PRIME buffer import, which
> - * is not correct: to fix this call of_dma_configure() with a NULL
> - * node to set default DMA ops.
> - */
> - dev->coherent_dma_mask = DMA_BIT_MASK(32);
> - ret = of_dma_configure(dev, NULL, true);
> + ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
> if (ret < 0) {
> - DRM_ERROR("Cannot setup DMA ops, ret %d", ret);
> + DRM_ERROR("Cannot setup DMA mask, ret %d", ret);
> return ret;
> }
>
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index a446a7221e13..81401f386c9c 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -22,6 +22,7 @@
>
> #define pr_fmt(fmt) "xen:" KBUILD_MODNAME ": " fmt
>
> +#include
> #include
> #include
> #include
> @@ -34,9 +35,6 @@
> #include
> #include
> #include
> -#ifdef CONFIG_XEN_GRANT_DMA_ALLOC
> -#include
> -#endif
>
> #include
> #include
> @@ -625,14 +623,7 @@ static int gntdev_open(struct inode *inode, struct file
> *flip)
> flip->private_data = priv;
> #ifdef CONFIG_XEN_GRANT_DMA_ALLOC
> priv->dma_dev = gntdev_miscdev.this_device;
> -
> - /*
> - * The device is not spawn from a device tree, so arch_setup_dma_ops
> - * is not called, thus leaving the device with dummy DMA ops.
> - * Fix this by calling of_dma_configure() with a NULL node to set
> - * default DMA ops.
> - */
> - of_dma_configure(priv->dma_dev, NULL, true);
> + dma_coerce_mask_and_coherent(priv->dma_dev, DMA_BIT_MASK(64));
> #endif
> pr_debug("priv %p\n", priv);
>
> --
> 2.20.1
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC] Documentation formats, licenses and file system structure
On Thu, 10 Oct 2019, Lars Kurth wrote: > * Would we ever include API docs generated from GPLv2 code? E.g. for safety > use-cases? > @Stefano, @Artem: I guess this one is for you. > I suppose if we would have a similar issue for a safety manual > I am also assuming we would want to use sphinx docs and rst to generate a > future safety manual Hi Lars, Thanks for putting this email together. In terms of formats, I don't have a preference between rst and pandoc, but if we are going to use rst going forward, I'd say to try to use rst for everything, including converting all the old stuff. The fewer different formats, the better. As I mentioned during the FuSa call, I agree with you, Andrew, and others that it would be best to have the docs under a CC license. I do expect that we'll end up copy/pasting snippets of in-code comments into the docs, so I think it is important that we are allowed to do that from a license perspective. It is great that GPLv2 allows it (we need to be sure about this). Yes, I expect that some docs might be automatically generated, but from header files, not from source code. Especailly public/ header files, which are typically BSD, not GPLv2. I cannot come up with examples of docs we need to generated from GPLv2-only code at the moment, hopefully there won't be any. > I wasn't planning on reusing any of the markup, and wasn't expecting to > use much of the text either. I'm still considering the option of > defining that xen/public/* isn't the canonical description of the ABI, > because C is the wrong tool for the job. > > Its fine to provide a C set of headers implementing an ABI, but there is > a very deliberate reason why the canonical migration v2 spec is in a > text document. > > @Stefano: as you and I believe Brian will be spending time on improving the > ABI docs, I think we need to build some agreement here on what/how > to do it. I was assuming that generally the consensus was to have > docs close to the code in source, but this does not seem to be the case. > > But if we do have stuff separately, ideally we would have a tool that helps > point people editing headers to also look at the relevant docs. Otherwise it > will > be hard to keep them in sync. In general, it is a good idea to keep the docs close to the code to make it easier to keep them up to date. But there is no one-size-fits-all here. For public ABI descriptions, I agree with Andrew that ideally they should not be defined as C header files. But it is not an issue: any work that we do here won't be wasted. For instance, we could start by adding more comments to the current header files. Then, as a second step, take all the comments and turn them into a proper ABI description document without any C function declarations. It is easy to move English text around, as long as the license allows it -- that is the only potential blocker I can see. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [[PATCH for-4.13]] xen/arm: mm: Allow generic xen page-tables helpers to be called early
On Thu, 10 Oct 2019, Julien Grall wrote: > On 08/10/2019 23:24, Stefano Stabellini wrote: > > On Tue, 8 Oct 2019, Julien Grall wrote: > > > On 10/8/19 1:18 AM, Stefano Stabellini wrote: > > > > On Mon, 7 Oct 2019, Julien Grall wrote: > > > > > Hi, > > > > > > > > > > On 03/10/2019 02:02, Stefano Stabellini wrote: > > > > > > On Fri, 20 Sep 2019, Julien Grall wrote: > > > > > > > That's not correct. alloc_boot_pages() is actually here to allow > > > > > > > dynamic > > > > > > > allocation before the memory subsystem (and therefore the runtime > > > > > > > allocator) > > > > > > > is initialized. > > > > > > > > > > > > Let me change the question then: is the system_state == > > > > > > SYS_STATE_early_boot check strictly necessary? It looks like it is > > > > > > not: > > > > > > the patch would work even if it was just: > > > > > > > > > > I had a few thoughts about it. On Arm32, this only really works for > > > > > 32-bits machine address (it can go up to 40-bits). I haven't really > > > > > fully investigated what could go wrong, but it would be best to keep > > > > > it > > > > > only for early boot. > > > > > > > > > > Also, I don't really want to rely on this "workaround" after boot. > > > > > Maybe > > > > > we would want to keep them unmapped in the future. > > > > > > > > Yes, no problems, we agree on that. I am not asking in regards to the > > > > check system_state == SYS_STATE_early_boot with the goal of asking you > > > > to get rid of it. I am fine with keeping the check. (Maybe we want to > > > > add > > > > an `unlikely()' around the check.) > > > > > > > > I am trying to understand whether the code actually relies on > > > > system_state == SYS_STATE_early_boot, and, if so, why. The goal is to > > > > make sure that if there are some limitations that they are documented, > > > > or just to double-check that there are no limitations. > > > > > > The check is not strictly necessary. > > > > > > > > > > > In regards to your comment about only working for 32-bit addresses on > > > > Arm32, you have a point. At least we should be careful with the mfn to > > > > vaddr conversion because mfn_to_maddr returns a paddr_t which is 64-bit > > > > and vaddr_t is 32-bit. I imagine that theoretically, even with > > > > system_state == SYS_STATE_early_boot, it could get truncated with the > > > > wrong combination of mfn and phys_offset. > > > > > > > > If nothing else, maybe we should add a truncation check for safety? > > > > > > Except that phys_offset is not defined correctly, so your check below will > > > break some setup :(. Let's take the following example: > > > > > > Xen is loaded at PA 0x10 > > > > > > The boot offset is computed using 32-bit address (see head.S): > > > PA - VA = 0x10 - 0x20 > > > = 0xfff0 > > > > > > This value will be passed to C code as an unsigned long. But then we will > > > store it in a uint64_t/paddr_t (see phys_offset which is set in > > > setup_page_tables). Because this is a conversion from unsigned to > > > unsigned, > > > the "sign bit" will not be propagated. > > > > > > This means that phys_offset will be equal to 0xfff0 and not > > > 0xfff0! > > > > > > Therefore if we try to convert 0x10 (where Xen has been loaded) back > > > to > > > its VA, the resulting value will be 0x00200100. > > > > > > Looking at the code, I think pte_of_xenaddr() has also the exact problem. > > > :( > > > > One way to fix it would be to change phys_offset to register_t (or just > > declare it as unsigned long on arm32 and unsigned long long on arm64): > > sizeof (unsigned long) = 32 (resp. 64) on Arm32 (resp. arm64). This is what we > already rely on for boot_phys_offset (see setup_pagetables). So I am not sure > why phys_offset needs to be defined differently. > > An alternative is to use vaddr_t. Yes, I meant like vaddr_t or just "unsigned long" like boot_phys_offset. Even with your latest patch (https://marc.info/?l=xen-devel=157073004830894) which I like as a way to solve the original GRUB booting issue, it looks like we also need to change phys_addr to unsigned long to fix other arm32 problems. Are you going to send the patch for that too? ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.13] xen/arm: mm: Allow generic xen page-tables helpers to be called early
On Thu, 10 Oct 2019, Julien Grall wrote:
> The current implementations of xen_{map, unmap}_table() expect
> {map, unmap}_domain_page() to be usable. Those helpers are used to
> map/unmap page tables while update Xen page-tables.
>
> Since commit 022387ee1a "xen/arm: mm: Don't open-code Xen PT update in
> {set, clear}_fixmap()", setup_fixmap() will make use of the helpers
> mentioned above. When booting Xen using GRUB, setup_fixmap() may be used
> before map_domain_page() can be called. This will result to data abort:
>
> (XEN) Data Abort Trap. Syndrome=0x5
> (XEN) CPU0: Unexpected Trap: Data Abort
>
> [...]
>
> (XEN) Xen call trace:
> (XEN)[<0025ab6c>] mm.c#xen_pt_update+0x2b4/0x59c (PC)
> (XEN)[<0025ab20>] mm.c#xen_pt_update+0x268/0x59c (LR)
> (XEN)[<0025ae70>] set_fixmap+0x1c/0x2c
> (XEN)[<002a9c98>] copy_from_paddr+0x7c/0xdc
> (XEN)[<002a4ae0>] has_xsm_magic+0x18/0x34
> (XEN)[<002a5b5c>] bootfdt.c#early_scan_node+0x398/0x560
> (XEN)[<002a5de0>] device_tree_for_each_node+0xbc/0x144
> (XEN)[<002a5ed4>] boot_fdt_info+0x6c/0x260
> (XEN)[<002ac0d0>] start_xen+0x108/0xc74
> (XEN)[<0020044c>] arm64/head.o#paging+0x60/0x88
>
> During early boot, the page tables are either statically allocated in
> Xen binary or allocated via alloc_boot_pages().
>
> For statically allocated page-tables, they will already be mapped as
> part of Xen binary. So we can easily find the virtual address.
>
> For dynamically allocated page-tables, we need to rely
> map_domain_page() to be functionally working.
>
> For arm32, the call will be usable much before page can be dynamically
> allocated (see setup_pagetables()). For arm64, the call will be usable
> after setup_xenheap_mappings().
>
> In both cases, memory are given to the boot allocator afterwards. So we
> can rely on map_domain_page() for mapping page tables allocated
> dynamically.
>
> The helpers xen_{map, unmap}_table() are now updated to take into
> account the case where page-tables are part of Xen binary.
>
> Fixes: 022387ee1a ('xen/arm: mm: Don't open-code Xen PT update in {set,
> clear}_fixmap()')
> Signed-off-by: Julien Grall
> Release-acked-by: Juergen Gross
>
> ---
> Changes in v2:
> - Add RaB Juergen
> - Rework the check to avoid truncation
> ---
> xen/arch/arm/mm.c | 29 +
> 1 file changed, 29 insertions(+)
>
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index be23acfe26..a6637ce347 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -964,11 +964,40 @@ static int create_xen_table(lpae_t *entry)
>
> static lpae_t *xen_map_table(mfn_t mfn)
> {
> +/*
> + * We may require to map the page table before map_domain_page() is
> + * useable. The requirements here is it must be useable as soon as
> + * page-tables are allocated dynamically via alloc_boot_pages().
> + *
> + * We need to do the check on physical address rather than virtual
> + * address to avoid truncation on Arm32. Therefore is_kernel() cannot
> + * be used.
> + */
> +if ( system_state == SYS_STATE_early_boot )
> +{
> +const mfn_t mstart = virt_to_mfn(_start);
> +const mfn_t mend = virt_to_mfn(_end);
> +
> +if ( (mfn_x(mstart) <= mfn_x(mfn)) && (mfn_x(mfn) < mfn_x(mend)) )
> +{
The patch is good. Actually I noticed that we already have
is_xen_fixed_mfn(), looks like it is made for this. I think we should
use it here, except that is_xen_fixed_mfn has:
(mfn_to_maddr(mfn) <= virt_to_maddr(&_end)))
I think it should actually be:
(mfn_to_maddr(mfn) < virt_to_maddr(&_end)))
Maybe we could fix that at the same time in one patch? However, it is
the same definition as on x86, so I don't know what is going on there.
> +vaddr_t offset = mfn_to_maddr(mfn) - mfn_to_maddr(mstart);
> +return (lpae_t *)(XEN_VIRT_START + offset);
I know this is safe in this case thanks to the `if' above, so there are
no risks. But in general it is not a great idea to have a hidden 64-bit
to 32-bit cast (also it is not MISRA compliant.) I would add an explicit
cast:
vaddr_t offset = (vaddr_t)(mfn_to_maddr(mfn) - mfn_to_maddr(mstart));
I am not going to insist on this though.
> +}
> +}
> +
> return map_domain_page(mfn);
> }
>
> static void xen_unmap_table(const lpae_t *table)
> {
> +/*
> + * During early boot, xen_map_table() will not use map_domain_page()
> + * for page-tables residing in Xen binary. So skip the unmap part.
> + */
> +if ( system_state == SYS_STATE_early_boot && is_kernel(table) )
> +return;
> +
> unmap_domain_page(table);
> }
>
> --
> 2.11.0
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 142565: tolerable all pass - PUSHED
flight 142565 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/142565/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen fef8d99fbce1a5e7ddfd22b0f33940b8d6193ec8 baseline version: xen 521a1445510a30873aec471194045e7f4b5e8d75 Last test of basis 142557 2019-10-10 15:00:57 Z0 days Testing same since 142565 2019-10-10 20:06:10 Z0 days1 attempts People who touched revisions under test: Jan Beulich Paul Durrant Stefano Stabellini Stefano Stabellini jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 521a144551..fef8d99fbc fef8d99fbce1a5e7ddfd22b0f33940b8d6193ec8 -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.4 test] 142521: regressions - FAIL
flight 142521 linux-4.4 real [real] http://logs.test-lab.xenproject.org/osstest/logs/142521/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-pvshim 20 guest-start/debian.repeat fail REGR. vs. 139698 Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 7 xen-boot fail in 142470 pass in 142521 test-amd64-amd64-xl-pvshim 18 guest-localmigrate/x10 fail in 142470 pass in 142521 test-armhf-armhf-xl-vhd 10 debian-di-install fail in 142470 pass in 142521 test-amd64-amd64-qemuu-nested-amd 14 xen-boot/l1 fail pass in 142470 Tests which did not succeed, but are not blocking: test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail in 142470 never pass test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-arm64-arm64-xl-credit1 7 xen-boot fail never pass test-arm64-arm64-xl-xsm 7 xen-boot fail never pass test-arm64-arm64-xl-credit2 7 xen-boot fail never pass test-arm64-arm64-libvirt-xsm 7 xen-boot fail never pass test-arm64-arm64-xl-seattle 7 xen-boot fail never pass test-arm64-arm64-xl 7 xen-boot fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-xl-thunderx 7 xen-boot fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail never pass test-arm64-arm64-examine 8 reboot fail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-install
[Xen-devel] [libvirt test] 142535: regressions - FAIL
flight 142535 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/142535/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-arm64-arm64-libvirt-qcow2 16 guest-start.2 fail REGR. vs. 142252 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 142252 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 142252 test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 12 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 13 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass version targeted for testing: libvirt 8958b47fabe122a8c1dac14198a07906158191c9 baseline version: libvirt 2346b2f6564ae9f7ba35bc863cb0fab39cadeb12 Last test of basis 142252 2019-10-04 04:23:56 Z6 days Failing since142345 2019-10-06 04:19:12 Z4 days5 attempts Testing same since 142535 2019-10-10 04:25:36 Z0 days1 attempts People who touched revisions under test: Cole Robinson Collin Walling Daniel Henrique Barboza Daniel P. Berrangé Daniel Veillard Fabiano Fidêncio Jiri Denemark Jonathon Jongsma Ján Tomko Michal Privoznik Pavel Mores Peter Krempa jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass test-amd64-amd64-libvirt-xsm pass test-arm64-arm64-libvirt-xsm pass test-amd64-i386-libvirt-xsm pass test-amd64-amd64-libvirt pass test-arm64-arm64-libvirt pass test-armhf-armhf-libvirt pass test-amd64-i386-libvirt pass test-amd64-amd64-libvirt-pairpass test-amd64-i386-libvirt-pair pass test-arm64-arm64-libvirt-qcow2 fail test-armhf-armhf-libvirt-raw pass test-amd64-amd64-libvirt-vhd pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be
[Xen-devel] [ovmf test] 142533: regressions - FAIL
flight 142533 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/142533/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 142423 test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 142423 version targeted for testing: ovmf 976d0353a6ce48149039849b52bb67527be5b580 baseline version: ovmf 410c4d00d9f7e369d1ce183e9e8de98cb59c4d20 Last test of basis 142423 2019-10-08 01:39:34 Z2 days Failing since142455 2019-10-08 19:21:52 Z2 days3 attempts Testing same since 142495 2019-10-09 12:29:18 Z1 days2 attempts People who touched revisions under test: Ard Biesheuvel Laszlo Ersek Pete Batard jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 fail test-amd64-i386-xl-qemuu-ovmf-amd64 fail sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. (No revision log; it would be 827 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 142557: tolerable all pass - PUSHED
flight 142557 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/142557/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 521a1445510a30873aec471194045e7f4b5e8d75 baseline version: xen 4ca8eab5ce1893b3048b06921f12157d33ab60f7 Last test of basis 142556 2019-10-10 12:00:45 Z0 days Testing same since 142557 2019-10-10 15:00:57 Z0 days1 attempts People who touched revisions under test: Igor Druzhinin Julien Grall jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 4ca8eab5ce..521a144551 521a1445510a30873aec471194045e7f4b5e8d75 -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable test] 142518: regressions - trouble: fail/pass/starved
flight 142518 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/142518/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-libvirt-pair 22 guest-migrate/src_host/dst_host fail REGR. vs. 141822 test-amd64-i386-libvirt-pair 22 guest-migrate/src_host/dst_host fail REGR. vs. 141822 test-arm64-arm64-examine11 examine-serial/bootloader fail REGR. vs. 141822 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10 fail REGR. vs. 141822 test-armhf-armhf-xl-rtds16 guest-start/debian.repeat fail REGR. vs. 141822 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 141822 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 141822 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 141822 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 141822 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 141822 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 141822 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 141822 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 141822 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 141822 test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
Re: [Xen-devel] [RFC] Documentation formats, licenses and file system structure
On 10/10/2019, 18:05, "Andrew Cooper" wrote:
On 10/10/2019 13:34, Lars Kurth wrote:
> Hi all,
>
> following on from a discussion on IRC and on various other places, I
think we need to try and rationalize how we handle documentation.
>
> What we have now and what we may get in future
> * http://xenbits.xen.org/docs/unstable/ (GPL-2)
> * http://xenbits.xen.org/docs/sphinx-unstable-staging/ (CC-BY-4)
> * Additional API documentation (with a view to enabling safety)
> * Any future documentation related to safety (requirements, designs, test
cases, tracability)
>
> Desired licenses
> * There is a desire to keep
http://xenbits.xen.org/docs/sphinx-unstable-staging/ CC-BY-4 only
> * There is a desire to publish future documentation related to safety as
CC-BY-4
Its probably worth nothing that the
http://xenbits.xen.org/docs/sphinx-unstable-staging/ URL is only
transitional.
When Sphinx is more ready for primetime, I was thinking of using
http://xenbits.xen.org/docs/xen/, and using the Sphinx support for
multiple versions, which would end up becoming docs/xen/{4.13,...,latest}/
> Existing formats and licenses
> * Hypercall ABI Documentation generated from Xen public headers
> Format: kerndoc
> License: typically BSD-3-Clause (documentation is generated from public
headers)
Its homegrown markup, superimposed on what used to be doxygen in
the past.
Oh, I forgot
I wasn't planning on reusing any of the markup, and wasn't expecting to
use much of the text either. I'm still considering the option of
defining that xen/public/* isn't the canonical description of the ABI,
because C is the wrong tool for the job.
Its fine to provide a C set of headers implementing an ABI, but there is
a very deliberate reason why the canonical migration v2 spec is in a
text document.
@Stefano: as you and I believe Brian will be spending time on improving the
ABI docs, I think we need to build some agreement here on what/how
to do it. I was assuming that generally the consensus was to have
docs close to the code in source, but this does not seem to be the case.
But if we do have stuff separately, ideally we would have a tool that helps
point people editing headers to also look at the relevant docs. Otherwise it
will
be hard to keep them in sync.
> * docs/designs, docs/features, docs/specs
> Formats: primarily pandoc, with some files md
> License: GPL-2
> * docs/processs - covers internal processes
> Formats: txt, with some pandoc
> License: GPL-2
> * docs/figs
> Formats: misc
> License: GPL-2
> * docs/misc
> Formats: txt, with some large number of pandoc, some other docs
> License: GPL-2
> * docs/man
> Formats: pod
> License: GPL-2
> * Sphinx docs: docs, docs/guest-guide, docs/hypervisor-guide
> Formats: rst
> License: CC-BY-4
This is the intention, but hasn't taken effect while my series is still
pending. For now, strictly speaking it is still GPL-2.
I was basing this on the assumption the series will go in
> * Wiki:
> Formats: mediawiki markdown
> License: CC-BY-SA-3 which has an automatic update to CC-BY-SA-4
> (c) of Wiki contributions are kept by the authors
>
> This means that the 3 most common file formats in use are
> * pod
> * pandoc (with some md) - these are essentially identical
> * txt for legacy and old stuff
> * rst
>
> License compatibility
> * GPL-2 and CC-BY-4 are compatible, but mixing means that the complete
docset is GPL-2
> * GPL-2 and BSD-3-Clause are compatible, but mixing means that the
complete docset is GPL-2
> * BSD-3-Clause and CC-BY-4 I am not 100% sure, but should not be an issue
> * CC-BY-SA-4 is only one way compatible with GPLv3 (affecting content on
the wiki)
>
> The first question is whether we should convert pod to rst
> * https://metacpan.org/pod/pod2rst provides a conversion tool
> * man pages can be generated by rst2man
> Thus, technically this should be easy and should make contributions to
docs/man easier
> If we do this, we should add a CONTRIBUTING file, clarifying the license
in this directory
One thing I have done is put SPDX tags on every *.rst file. What I
haven't found is a nice way to insert one into the *.drawio.svg files,
but I should probably finish off some of my experimentation TODOs.
An easy way out is to just say "look at the SPDX tag", but then we end
up with a docset which is a mess of licenses, still can't be easily
built upon.
I think a per-directory approach is generally better + use SPDX tags
where it can easily be added.
And it's easy enough to do
> There are a set of related questions on what we would eventually merge
into the sphinx
> docset. I believe there is
[Xen-devel] [PATCH for-4.13] xen/arm: mm: Allow generic xen page-tables helpers to be called early
The current implementations of xen_{map, unmap}_table() expect
{map, unmap}_domain_page() to be usable. Those helpers are used to
map/unmap page tables while update Xen page-tables.
Since commit 022387ee1a "xen/arm: mm: Don't open-code Xen PT update in
{set, clear}_fixmap()", setup_fixmap() will make use of the helpers
mentioned above. When booting Xen using GRUB, setup_fixmap() may be used
before map_domain_page() can be called. This will result to data abort:
(XEN) Data Abort Trap. Syndrome=0x5
(XEN) CPU0: Unexpected Trap: Data Abort
[...]
(XEN) Xen call trace:
(XEN)[<0025ab6c>] mm.c#xen_pt_update+0x2b4/0x59c (PC)
(XEN)[<0025ab20>] mm.c#xen_pt_update+0x268/0x59c (LR)
(XEN)[<0025ae70>] set_fixmap+0x1c/0x2c
(XEN)[<002a9c98>] copy_from_paddr+0x7c/0xdc
(XEN)[<002a4ae0>] has_xsm_magic+0x18/0x34
(XEN)[<002a5b5c>] bootfdt.c#early_scan_node+0x398/0x560
(XEN)[<002a5de0>] device_tree_for_each_node+0xbc/0x144
(XEN)[<002a5ed4>] boot_fdt_info+0x6c/0x260
(XEN)[<002ac0d0>] start_xen+0x108/0xc74
(XEN)[<0020044c>] arm64/head.o#paging+0x60/0x88
During early boot, the page tables are either statically allocated in
Xen binary or allocated via alloc_boot_pages().
For statically allocated page-tables, they will already be mapped as
part of Xen binary. So we can easily find the virtual address.
For dynamically allocated page-tables, we need to rely
map_domain_page() to be functionally working.
For arm32, the call will be usable much before page can be dynamically
allocated (see setup_pagetables()). For arm64, the call will be usable
after setup_xenheap_mappings().
In both cases, memory are given to the boot allocator afterwards. So we
can rely on map_domain_page() for mapping page tables allocated
dynamically.
The helpers xen_{map, unmap}_table() are now updated to take into
account the case where page-tables are part of Xen binary.
Fixes: 022387ee1a ('xen/arm: mm: Don't open-code Xen PT update in {set,
clear}_fixmap()')
Signed-off-by: Julien Grall
Release-acked-by: Juergen Gross
---
Changes in v2:
- Add RaB Juergen
- Rework the check to avoid truncation
---
xen/arch/arm/mm.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index be23acfe26..a6637ce347 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -964,11 +964,40 @@ static int create_xen_table(lpae_t *entry)
static lpae_t *xen_map_table(mfn_t mfn)
{
+/*
+ * We may require to map the page table before map_domain_page() is
+ * useable. The requirements here is it must be useable as soon as
+ * page-tables are allocated dynamically via alloc_boot_pages().
+ *
+ * We need to do the check on physical address rather than virtual
+ * address to avoid truncation on Arm32. Therefore is_kernel() cannot
+ * be used.
+ */
+if ( system_state == SYS_STATE_early_boot )
+{
+const mfn_t mstart = virt_to_mfn(_start);
+const mfn_t mend = virt_to_mfn(_end);
+
+if ( (mfn_x(mstart) <= mfn_x(mfn)) && (mfn_x(mfn) < mfn_x(mend)) )
+{
+vaddr_t offset = mfn_to_maddr(mfn) - mfn_to_maddr(mstart);
+
+return (lpae_t *)(XEN_VIRT_START + offset);
+}
+}
+
return map_domain_page(mfn);
}
static void xen_unmap_table(const lpae_t *table)
{
+/*
+ * During early boot, xen_map_table() will not use map_domain_page()
+ * for page-tables residing in Xen binary. So skip the unmap part.
+ */
+if ( system_state == SYS_STATE_early_boot && is_kernel(table) )
+return;
+
unmap_domain_page(table);
}
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC] Documentation formats, licenses and file system structure
On 10/10/2019 13:34, Lars Kurth wrote:
> Hi all,
>
> following on from a discussion on IRC and on various other places, I think we
> need to try and rationalize how we handle documentation.
>
> What we have now and what we may get in future
> * http://xenbits.xen.org/docs/unstable/ (GPL-2)
> * http://xenbits.xen.org/docs/sphinx-unstable-staging/ (CC-BY-4)
> * Additional API documentation (with a view to enabling safety)
> * Any future documentation related to safety (requirements, designs, test
> cases, tracability)
>
> Desired licenses
> * There is a desire to keep
> http://xenbits.xen.org/docs/sphinx-unstable-staging/ CC-BY-4 only
> * There is a desire to publish future documentation related to safety as
> CC-BY-4
Its probably worth nothing that the
http://xenbits.xen.org/docs/sphinx-unstable-staging/ URL is only
transitional.
When Sphinx is more ready for primetime, I was thinking of using
http://xenbits.xen.org/docs/xen/, and using the Sphinx support for
multiple versions, which would end up becoming docs/xen/{4.13,...,latest}/
> Existing formats and licenses
> * Hypercall ABI Documentation generated from Xen public headers
> Format: kerndoc
> License: typically BSD-3-Clause (documentation is generated from public
> headers)
Its homegrown markup, superimposed on what used to be doxygen in
the past.
I wasn't planning on reusing any of the markup, and wasn't expecting to
use much of the text either. I'm still considering the option of
defining that xen/public/* isn't the canonical description of the ABI,
because C is the wrong tool for the job.
Its fine to provide a C set of headers implementing an ABI, but there is
a very deliberate reason why the canonical migration v2 spec is in a
text document.
> * docs/designs, docs/features, docs/specs
> Formats: primarily pandoc, with some files md
> License: GPL-2
> * docs/processs - covers internal processes
> Formats: txt, with some pandoc
> License: GPL-2
> * docs/figs
> Formats: misc
> License: GPL-2
> * docs/misc
> Formats: txt, with some large number of pandoc, some other docs
> License: GPL-2
> * docs/man
> Formats: pod
> License: GPL-2
> * Sphinx docs: docs, docs/guest-guide, docs/hypervisor-guide
> Formats: rst
> License: CC-BY-4
This is the intention, but hasn't taken effect while my series is still
pending. For now, strictly speaking it is still GPL-2.
>
> * Wiki:
> Formats: mediawiki markdown
> License: CC-BY-SA-3 which has an automatic update to CC-BY-SA-4
> (c) of Wiki contributions are kept by the authors
>
> This means that the 3 most common file formats in use are
> * pod
> * pandoc (with some md) - these are essentially identical
> * txt for legacy and old stuff
> * rst
>
> License compatibility
> * GPL-2 and CC-BY-4 are compatible, but mixing means that the complete docset
> is GPL-2
> * GPL-2 and BSD-3-Clause are compatible, but mixing means that the complete
> docset is GPL-2
> * BSD-3-Clause and CC-BY-4 I am not 100% sure, but should not be an issue
> * CC-BY-SA-4 is only one way compatible with GPLv3 (affecting content on the
> wiki)
>
> The first question is whether we should convert pod to rst
> * https://metacpan.org/pod/pod2rst provides a conversion tool
> * man pages can be generated by rst2man
> Thus, technically this should be easy and should make contributions to
> docs/man easier
> If we do this, we should add a CONTRIBUTING file, clarifying the license in
> this directory
One thing I have done is put SPDX tags on every *.rst file. What I
haven't found is a nice way to insert one into the *.drawio.svg files,
but I should probably finish off some of my experimentation TODOs.
An easy way out is to just say "look at the SPDX tag", but then we end
up with a docset which is a mess of licenses, still can't be easily
built upon.
> There are a set of related questions on what we would eventually merge into
> the sphinx
> docset. I believe there is agreement that most of what is in docs today is
> not really
> suitable, however there are a few possible exceptions
> * man pages - with a variety of different contributors from different orgs.
> Changing license would be hard
But certainly not impossible.
> * API docs generated from PUBLIC headers - changing license would be
> impossible, but would be BSD-3-Clause
The code, yes, but I'm expecting that to be orthogonal in the long run.
> * Some wiki content (e.g.
> https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches and friends)
>More than 95% of changes were from Citrix staff, so we could convert to
> CC-BY-4
>Most non-Citrix changes are one-line changes and could be covered by fair
> use
> * Possibly stuff such as
> https://xenbits.xen.org/docs/unstable/support-matrix.html (which is currently
> GPL-2,
>but we could relicense to say GPL-2 and CC-BY-4 if we had to)
> The implication is that the sphinx docs would not be fully CC-BY-4, but the
> bulk of the pages would be
Would be what?
~Andrew
>
> * Would we ever include API
Re: [Xen-devel] [[PATCH for-4.13]] xen/arm: mm: Allow generic xen page-tables helpers to be called early
Hi Stefano, On 08/10/2019 23:24, Stefano Stabellini wrote: On Tue, 8 Oct 2019, Julien Grall wrote: On 10/8/19 1:18 AM, Stefano Stabellini wrote: On Mon, 7 Oct 2019, Julien Grall wrote: Hi, On 03/10/2019 02:02, Stefano Stabellini wrote: On Fri, 20 Sep 2019, Julien Grall wrote: That's not correct. alloc_boot_pages() is actually here to allow dynamic allocation before the memory subsystem (and therefore the runtime allocator) is initialized. Let me change the question then: is the system_state == SYS_STATE_early_boot check strictly necessary? It looks like it is not: the patch would work even if it was just: I had a few thoughts about it. On Arm32, this only really works for 32-bits machine address (it can go up to 40-bits). I haven't really fully investigated what could go wrong, but it would be best to keep it only for early boot. Also, I don't really want to rely on this "workaround" after boot. Maybe we would want to keep them unmapped in the future. Yes, no problems, we agree on that. I am not asking in regards to the check system_state == SYS_STATE_early_boot with the goal of asking you to get rid of it. I am fine with keeping the check. (Maybe we want to add an `unlikely()' around the check.) I am trying to understand whether the code actually relies on system_state == SYS_STATE_early_boot, and, if so, why. The goal is to make sure that if there are some limitations that they are documented, or just to double-check that there are no limitations. The check is not strictly necessary. In regards to your comment about only working for 32-bit addresses on Arm32, you have a point. At least we should be careful with the mfn to vaddr conversion because mfn_to_maddr returns a paddr_t which is 64-bit and vaddr_t is 32-bit. I imagine that theoretically, even with system_state == SYS_STATE_early_boot, it could get truncated with the wrong combination of mfn and phys_offset. If nothing else, maybe we should add a truncation check for safety? Except that phys_offset is not defined correctly, so your check below will break some setup :(. Let's take the following example: Xen is loaded at PA 0x10 The boot offset is computed using 32-bit address (see head.S): PA - VA = 0x10 - 0x20 = 0xfff0 This value will be passed to C code as an unsigned long. But then we will store it in a uint64_t/paddr_t (see phys_offset which is set in setup_page_tables). Because this is a conversion from unsigned to unsigned, the "sign bit" will not be propagated. This means that phys_offset will be equal to 0xfff0 and not 0xfff0! Therefore if we try to convert 0x10 (where Xen has been loaded) back to its VA, the resulting value will be 0x00200100. Looking at the code, I think pte_of_xenaddr() has also the exact problem. :( One way to fix it would be to change phys_offset to register_t (or just declare it as unsigned long on arm32 and unsigned long long on arm64): sizeof (unsigned long) = 32 (resp. 64) on Arm32 (resp. arm64). This is what we already rely on for boot_phys_offset (see setup_pagetables). So I am not sure why phys_offset needs to be defined differently. An alternative is to use vaddr_t. diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index be23acfe26..c7ead39654 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -170,7 +170,7 @@ static DEFINE_PAGE_TABLE(xen_xenmap); /* Non-boot CPUs use this to find the correct pagetables. */ uint64_t init_ttbr; -static paddr_t phys_offset; +static register_t phys_offset; /* Limits of the Xen heap */ mfn_t xenheap_mfn_start __read_mostly = INVALID_MFN_INITIALIZER; It would work OK for virtual to physical conversions. This is because the addition will be done using vaddr_t (aka unsigned long) and then promote to 64-bit, right? Although, I am not too happy with making this assumption. I would much prefer if we keep phys_offset a paddr_t and make sure sign-bit is propagated. So this would cater user doing the conversion either using 64-bit or 32-bit. Physical to virtual is more challenging because physical could go up to 40bits. Fortunately, it doesn't look like we are using phys_offset to do many phys-to-virt conversions. The only example is the one in this patch, and dump_hyp_walk. I guess nobody tried to load Xen that low in memory on Arm32? But that's going to be definitely an issues with the memory rework I have in mind. I have some other important work to finish for Xen 4.13. So I am thinking to defer the problem I mention above for post Xen 4.13. Although, the GRUB issues would still need to be fix. Any opinions? I think for Xen 4.13 I would like to get in your patch to fix GRUB booting, with a little bit more attention to integer issues. Something like the following: paddr_t pa_end = _end + phys_offset; paddr_t pa_start = _start + phys_offset; This could be a good idea. Thinking a bit more, we don't need to use phys_offset here.
Re: [Xen-devel] [PATCH 2/2] libxl_pci: Fix guest shutdown with PCI PT attached
On 01/10/2019 12:35, Anthony PERARD wrote: > Rewrite of the commit message: > > Before the problematic commit, libxl used to ignore error when > destroying (force == true) a passthrough device, especially error that > happens when dealing with the DM. > > Since fae4880c45fe, if the DM failed to detach the pci device within > the allowed time, the timed out error raised skip part of > pci_remove_*, but also raise the error up to the caller of > libxl__device_pci_destroy_all, libxl__destroy_domid, and thus the > destruction of the domain fails. > > In this patch, if the DM didn't confirmed that the device is removed, > we will print a warning and keep going if force=true. The patch > reorder the functions so that pci_remove_timeout() calls > pci_remove_detatched() like it's done when DM calls are successful. > > We also clean the QMP states and associated timeouts earlier, as soon > as they are not needed anymore. > > Reported-by: Sander Eikelenboom > Fixes: fae4880c45fe015e567afa223f78bf17a6d98e1b > Signed-off-by: Anthony PERARD > Hi Anthony / Chao, I have to come back to this, a bit because perhaps there is an underlying issue. While it earlier occurred to me that the VM to which I passed through most pci-devices (8 to be exact) became very slow to shutdown, but I didn't investigate it further. But after you commit messages from this patch it kept nagging, so today I did some testing and bisecting. The difference in tear-down time at least from what the IOMMU code logs is quite large: xen-4.12.0 Setup: 7.452 s Tear-down: 7.626 s xen-unstable-ee7170822f1fc209f33feb47b268bab35541351d Setup: 7.468 s Tear-down: 50.239 s Bisection turned up: commit c4b1ef0f89aa6a74faa4618ce3efed1de246ec40 Author: Chao Gao Date: Fri Jul 19 10:24:08 2019 +0100 libxl_qmp: wait for completion of device removal Which makes me wonder if there is something going wrong in Qemu ? -- Sander xen-4.12.0 setup: (XEN) [2019-10-10 09:54:14.846] AMD-Vi: Disable: device id = 0x900, domain = 0, paging mode = 3 (XEN) [2019-10-10 09:54:14.846] AMD-Vi: Setup I/O page table: device id = 0x900, type = 0x1, root table = 0x4aa847000, domain = 1, paging mode = 3 (XEN) [2019-10-10 09:54:14.846] AMD-Vi: Re-assign :09:00.0 from dom0 to dom1 ... (XEN) [2019-10-10 09:54:22.298] AMD-Vi: Disable: device id = 0x907, domain = 0, paging mode = 3 (XEN) [2019-10-10 09:54:22.298] AMD-Vi: Setup I/O page table: device id = 0x907, type = 0x1, root table = 0x4aa847000, domain = 1, paging mode = 3 (XEN) [2019-10-10 09:54:22.298] AMD-Vi: Re-assign :09:00.7 from dom0 to dom1 xen-4.12.0 tear-down: (XEN) [2019-10-10 10:01:11.971] AMD-Vi: Disable: device id = 0x900, domain = 1, paging mode = 3 (XEN) [2019-10-10 10:01:11.971] AMD-Vi: Setup I/O page table: device id = 0x900, type = 0x1, root table = 0x53572c000, domain = 0, paging mode = 3 (XEN) [2019-10-10 10:01:11.971] AMD-Vi: Re-assign :09:00.0 from dom1 to dom0 ... (XEN) [2019-10-10 10:01:19.597] AMD-Vi: Disable: device id = 0x907, domain = 1, paging mode = 3 (XEN) [2019-10-10 10:01:19.597] AMD-Vi: Setup I/O page table: device id = 0x907, type = 0x1, root table = 0x53572c000, domain = 0, paging mode = 3 (XEN) [2019-10-10 10:01:19.597] AMD-Vi: Re-assign :09:00.7 from dom1 to dom0 xen-unstable-ee7170822f1fc209f33feb47b268bab35541351d setup: (XEN) [2019-10-10 10:21:38.549] d1: bind: m_gsi=47 g_gsi=36 dev=00.00.5 intx=0 (XEN) [2019-10-10 10:21:38.621] AMD-Vi: Disable: device id = 0x900, domain = 0, paging mode = 3 (XEN) [2019-10-10 10:21:38.621] AMD-Vi: Setup I/O page table: device id = 0x900, type = 0x1, root table = 0x4aa83b000, domain = 1, paging mode = 3 (XEN) [2019-10-10 10:21:38.621] AMD-Vi: Re-assign :09:00.0 from dom0 to dom1 ... (XEN) [2019-10-10 10:21:46.069] d1: bind: m_gsi=46 g_gsi=36 dev=00.01.4 intx=3 (XEN) [2019-10-10 10:21:46.089] AMD-Vi: Disable: device id = 0x907, domain = 0, paging mode = 3 (XEN) [2019-10-10 10:21:46.089] AMD-Vi: Setup I/O page table: device id = 0x907, type = 0x1, root table = 0x4aa83b000, domain = 1, paging mode = 3 (XEN) [2019-10-10 10:21:46.089] AMD-Vi: Re-assign :09:00.7 from dom0 to dom1 xen-unstable-ee7170822f1fc209f33feb47b268bab35541351d tear-down: (XEN) [2019-10-10 10:23:53.167] AMD-Vi: Disable: device id = 0x900, domain = 1, paging mode = 3 (XEN) [2019-10-10 10:23:53.167] AMD-Vi: Setup I/O page table: device id = 0x900, type = 0x1, root table = 0x5240f8000, domain = 0, paging mode = 3 (XEN) [2019-10-10 10:23:53.167] AMD-Vi: Re-assign :09:00.0 from dom1 to dom0 ... (XEN) [2019-10-10 10:24:43.406] AMD-Vi: Disable: device id = 0x907, domain = 1, paging mode = 3 (XEN) [2019-10-10 10:24:43.406]
Re: [Xen-devel] [PATCH v2] xen: Stop abusing DT of_dma_configure API
On 10/10/19 11:32 AM, Rob Herring wrote:
> On Thu, Oct 10, 2019 at 9:00 AM Boris Ostrovsky
> wrote:
>> On 10/9/19 7:42 AM, Oleksandr Andrushchenko wrote:
>>> On 10/8/19 10:41 PM, Rob Herring wrote:
As the removed comments say, these aren't DT based devices.
of_dma_configure() is going to stop allowing a NULL DT node and calling
it will no longer work.
The comment is also now out of date as of commit 9ab91e7c5c51 ("arm64:
default to the direct mapping in get_arch_dma_ops"). Direct mapping
is now the default rather than dma_dummy_ops.
According to Stefano and Oleksandr, the only other part needed is
setting the DMA masks and there's no reason to restrict the masks to
32-bits. So set the masks to 64 bits.
Cc: Robin Murphy
Cc: Julien Grall
Cc: Nicolas Saenz Julienne
Cc: Oleksandr Andrushchenko
Cc: Boris Ostrovsky
Cc: Juergen Gross
Cc: Stefano Stabellini
Cc: Christoph Hellwig
Cc: xen-devel@lists.xenproject.org
Signed-off-by: Rob Herring
>>> Acked-by: Oleksandr Andrushchenko
>>
>> Is this going to go via drm tree or should I pick it up for Xen tree?
> Please apply to the Xen tree.
Ok. FTR,
Reviewed-by: Boris Ostrovsky
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.13 v2] xen/arm: domain_build: Don't expose IOMMU specific properties to hwdom
On 10.10.19 18:32, Julien Grall wrote: Hi, Hi [1] https://lists.xenproject.org/archives/html/xen-devel/2019-10/msg00104.html --- xen/arch/arm/domain_build.c | 29 + 1 file changed, 29 insertions(+) diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 6d6dd52..a7321b8 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -480,10 +480,26 @@ static int __init write_properties(struct domain *d, struct kernel_info *kinfo, const struct dt_property *prop, *status = NULL; int res = 0; int had_dom0_bootargs = 0; + struct dt_device_node *iommu_node; if ( kinfo->cmdline && kinfo->cmdline[0] ) bootargs = >cmdline[0]; + /* + * If we skip the IOMMU device when creating DT for hwdom (even if + * the IOMMU device is not used by Xen), we should also skip the IOMMU + * specific properties of the master device behind it in order to avoid + * exposing an half complete IOMMU bindings to hwdom. + * Use "iommu_node" as an indicator of the master device which properties + * should be skipped. + */ + iommu_node = dt_parse_phandle(node, "iommus", 0); The code is slightly confusing to read. I don't think we should care of invalid DT here, so let's only consider valid one. Do you mean "the comment" is confusing to read? The code is confusing because "iommus" should always point to a IOMMU node, but then you check whether this is an IOMMU. So it is not clear if this is done for sanity check (or for a different reason). Got it. Will clarify a reason. -- Regards, Oleksandr Tyshchenko ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: add warning if memory modules overlap
Hi Brian,
Thank you for the patch.
On 10/9/19 8:47 PM, Brian Woods wrote:
It's possible for a misconfigured device tree to cause Xen to crash when
there are overlapping addresses in the memory modules. Add a warning
when printing the addresses to let the user know there's a possible
issue when DEBUG is enabled.
Signed-off-by: Brian Woods
---
sample output:
...
(XEN) MODULE[0]: 0140 - 0153b8f1 Xen
(XEN) MODULE[1]: 076d2000 - 076dc080 Device Tree
(XEN) MODULE[2]: 076df000 - 07fff364 Ramdisk
(XEN) MODULE[3]: 0008 - 0318 Kernel
(XEN) RESVD[0]: 076d2000 - 076dc000
(XEN) RESVD[1]: 076df000 - 07fff364
(XEN)
(XEN) WARNING: modules Xen and Kernel overlap
(XEN)
(XEN) Command line: console=dtuart dtuart=serial0 dom0_mem=1G bootscrub=0
maxcpus=1 timer_slop=0
...
xen/arch/arm/bootfdt.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index 08fb59f..3cb0c43 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -387,6 +387,23 @@ static void __init early_print_info(void)
mem_resv->bank[j].start + mem_resv->bank[j].size - 1);
}
printk("\n");
+
+#ifndef NDEBUG
+/*
+ * Assuming all combinations are checked, only the starting address
+ * has to be checked if it's in another memory module's range.
+ */
+for ( i = 0 ; i < mods->nr_mods; i++ )
+for ( j = 0 ; j < mods->nr_mods; j++ )
+if ( (i != j) &&
+ (mods->module[i].start >= mods->module[j].start) &&
+ (mods->module[i].start <
+ mods->module[j].start + mods->module[j].size) )
+printk("WARNING: modules %-12s and %-12s overlap\n",
+ boot_module_kind_as_string(mods->module[i].kind),
+ boot_module_kind_as_string(mods->module[j].kind));
I am not entirely happy with the double for-loop here.
As we already go through all the modules in add_boot_module(). Could you
look whether this check could be part of it?
This should also allow to have this check for non-debug build as well.
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen: Stop abusing DT of_dma_configure API
On Thu, Oct 10, 2019 at 9:00 AM Boris Ostrovsky
wrote:
>
> On 10/9/19 7:42 AM, Oleksandr Andrushchenko wrote:
> > On 10/8/19 10:41 PM, Rob Herring wrote:
> >> As the removed comments say, these aren't DT based devices.
> >> of_dma_configure() is going to stop allowing a NULL DT node and calling
> >> it will no longer work.
> >>
> >> The comment is also now out of date as of commit 9ab91e7c5c51 ("arm64:
> >> default to the direct mapping in get_arch_dma_ops"). Direct mapping
> >> is now the default rather than dma_dummy_ops.
> >>
> >> According to Stefano and Oleksandr, the only other part needed is
> >> setting the DMA masks and there's no reason to restrict the masks to
> >> 32-bits. So set the masks to 64 bits.
> >>
> >> Cc: Robin Murphy
> >> Cc: Julien Grall
> >> Cc: Nicolas Saenz Julienne
> >> Cc: Oleksandr Andrushchenko
> >> Cc: Boris Ostrovsky
> >> Cc: Juergen Gross
> >> Cc: Stefano Stabellini
> >> Cc: Christoph Hellwig
> >> Cc: xen-devel@lists.xenproject.org
> >> Signed-off-by: Rob Herring
> > Acked-by: Oleksandr Andrushchenko
>
>
> Is this going to go via drm tree or should I pick it up for Xen tree?
Please apply to the Xen tree.
Rob
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.13 v2] xen/arm: domain_build: Don't expose IOMMU specific properties to hwdom
Hi, On 10/10/19 4:27 PM, Oleksandr wrote: On 10.10.19 18:18, Julien Grall wrote: Hi, Hi Julien On 10/8/19 4:25 PM, Oleksandr Tyshchenko wrote: From: Oleksandr Tyshchenko We don't passthrough IOMMU device to hwdom even if it is not used by Xen. Therefore exposing the properties that describe relationship between master devices and IOMMUs does not make any sense. According to the: 1. Documentation/devicetree/bindings/iommu/iommu.txt 2. Documentation/devicetree/bindings/pci/pci-iommu.txt It is not entirely clear that documentation is from Linux. Will clarify. Signed-off-by: Oleksandr Tyshchenko --- Changes V1 [1] -> V2: - Only skip IOMMU specific properties of the master device if we skip the corresponding IOMMU device - Use "hwdom" over "Dom0" [1] https://lists.xenproject.org/archives/html/xen-devel/2019-10/msg00104.html --- xen/arch/arm/domain_build.c | 29 + 1 file changed, 29 insertions(+) diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 6d6dd52..a7321b8 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -480,10 +480,26 @@ static int __init write_properties(struct domain *d, struct kernel_info *kinfo, const struct dt_property *prop, *status = NULL; int res = 0; int had_dom0_bootargs = 0; + struct dt_device_node *iommu_node; if ( kinfo->cmdline && kinfo->cmdline[0] ) bootargs = >cmdline[0]; + /* + * If we skip the IOMMU device when creating DT for hwdom (even if + * the IOMMU device is not used by Xen), we should also skip the IOMMU + * specific properties of the master device behind it in order to avoid + * exposing an half complete IOMMU bindings to hwdom. + * Use "iommu_node" as an indicator of the master device which properties + * should be skipped. + */ + iommu_node = dt_parse_phandle(node, "iommus", 0); The code is slightly confusing to read. I don't think we should care of invalid DT here, so let's only consider valid one. Do you mean "the comment" is confusing to read? The code is confusing because "iommus" should always point to a IOMMU node, but then you check whether this is an IOMMU. So it is not clear if this is done for sanity check (or for a different reason). Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.13 v2] xen/arm: domain_build: Don't expose IOMMU specific properties to hwdom
On 10.10.19 18:18, Julien Grall wrote:
Hi,
Hi Julien
On 10/8/19 4:25 PM, Oleksandr Tyshchenko wrote:
From: Oleksandr Tyshchenko
We don't passthrough IOMMU device to hwdom even if it is not used by
Xen.
Therefore exposing the properties that describe relationship between
master devices and IOMMUs does not make any sense.
According to the:
1. Documentation/devicetree/bindings/iommu/iommu.txt
2. Documentation/devicetree/bindings/pci/pci-iommu.txt
It is not entirely clear that documentation is from Linux.
Will clarify.
Signed-off-by: Oleksandr Tyshchenko
---
Changes V1 [1] -> V2:
- Only skip IOMMU specific properties of the master device if we
skip the corresponding IOMMU device
- Use "hwdom" over "Dom0"
[1]
https://lists.xenproject.org/archives/html/xen-devel/2019-10/msg00104.html
---
xen/arch/arm/domain_build.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 6d6dd52..a7321b8 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -480,10 +480,26 @@ static int __init write_properties(struct
domain *d, struct kernel_info *kinfo,
const struct dt_property *prop, *status = NULL;
int res = 0;
int had_dom0_bootargs = 0;
+ struct dt_device_node *iommu_node;
if ( kinfo->cmdline && kinfo->cmdline[0] )
bootargs = >cmdline[0];
+ /*
+ * If we skip the IOMMU device when creating DT for hwdom (even if
+ * the IOMMU device is not used by Xen), we should also skip the
IOMMU
+ * specific properties of the master device behind it in order
to avoid
+ * exposing an half complete IOMMU bindings to hwdom.
+ * Use "iommu_node" as an indicator of the master device which
properties
+ * should be skipped.
+ */
+ iommu_node = dt_parse_phandle(node, "iommus", 0);
The code is slightly confusing to read. I don't think we should care
of invalid DT here, so let's only consider valid one.
Do you mean "the comment" is confusing to read?
For valid DT, any non-NULL return should point to an IOMMU. The
comment above suggests that all IOMMU will be skipped. However, the
check below (device_get_class(iommu_node)) will not return
DEVICE_IOMMU when there are not have a driver for the IOMMU.
So this needs to be clarified in the commit message.
Will do.
+ if ( iommu_node )
+ {
+ if ( device_get_class(iommu_node) != DEVICE_IOMMU )
+ iommu_node = NULL;
+ }
Could we gather the two conditions in one if?
Yes.
--
Regards,
Oleksandr Tyshchenko
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/2] x2APIC: translate IO-APIC entries when enabling the IOMMU
On Thu, Oct 10, 2019 at 03:46:45PM +0200, Jan Beulich wrote:
> On 10.10.2019 15:12, Roger Pau Monné wrote:
> > On Thu, Oct 10, 2019 at 02:55:02PM +0200, Jan Beulich wrote:
> >> On 10.10.2019 14:13, Roger Pau Monné wrote:
> >>> On Thu, Oct 10, 2019 at 01:54:06PM +0200, Jan Beulich wrote:
> On 10.10.2019 13:33, Roger Pau Monne wrote:
> > When interrupt remapping is enabled as part of enabling x2APIC the
>
> Perhaps "unmasked" instead of "the"?
>
> > IO-APIC entries also need to be translated to the new format and added
> > to the interrupt remapping table.
> >
> > This prevents IOMMU interrupt remapping faults when booting on
> > hardware that has unmasked IO-APIC pins.
>
> But in the end it only papers over whatever the spurious interrupts
> result form, doesn't it? Which isn't to say this isn't an
> improvement. Calling out the ExtInt case here may be worthwhile as
> well, as would be pointing out that this case still won't work on
> AMD IOMMUs.
> >>>
> >>> But the fix for the ExtINT AMD issue should be done in
> >>> amd_iommu_ioapic_update_ire then, so that it can properly handle
> >>> ExtINT delivery mode, not to this part of the code. I will look
> >>> into it, but I think it's kind of tangential to the issue here.
> >>
> >> I'm not talking of you working on fixing this right away. I'm merely
> >> asking that you mention here (a) the ExtInt special case and (b)
> >> that this special case will (continue to) not work in the AMD case.
> >>
> > --- a/xen/arch/x86/apic.c
> > +++ b/xen/arch/x86/apic.c
> > @@ -515,7 +515,7 @@ static void resume_x2apic(void)
> > iommu_enable_x2apic();
> > __enable_x2apic();
> >
> > -restore_IO_APIC_setup(ioapic_entries);
> > +restore_IO_APIC_setup(ioapic_entries, true);
> > unmask_8259A();
> >
> > out:
> > @@ -961,7 +961,12 @@ void __init x2apic_bsp_setup(void)
> > printk("Switched to APIC driver %s\n", genapic.name);
> >
> > restore_out:
> > -restore_IO_APIC_setup(ioapic_entries);
> > +/*
> > + * NB: do not use raw mode when restoring entries if the iommu has
> > been
> > + * enabled during the process, because the entries need to be
> > translated
> > + * and added to the remapping table in that case.
> > + */
> > +restore_IO_APIC_setup(ioapic_entries, !x2apic_enabled);
>
> How is this different in the resume_x2apic() case? The IOMMU gets
> enabled in the course of that as well. I.e. I'd expect you want
> to pass "false" there, not "true".
> >>>
> >>> In the resume_x2apic case interrupt remapping should already be
> >>> enabled or not, but that function is not going to enable interrupt
> >>> remapping if it wasn't enabled before, hence the IO-APIC entries
> >>> should already be using the interrupt remapping table and no
> >>> translation is needed.
> >>
> >> Who / what would have enabled the IOMMU in the resume case?
> >
> > I don't think the question is who enables interrupt remapping in the
> > resume case (which is resume_x2apic when calling iommu_enable_x2apic
> > AFAICT), the point here is that on resume the entries in the IO-APIC
> > will already match the state of interrupt remapping, so they shouldn't
> > be translated. If interrupt remapping was off before suspend it will
> > still be off after resume, and there won't be any translation needed.
> > The same is true if interrupt remapping is on before suspend.
>
> I disagree: save_IO_APIC_setup() gets called from resume_x2apic(),
> not prior to suspend.
Oh, so maybe that's a misunderstanding on my side. I don't seem to be
able to find a statement about the contents of the IO-APIC registers
(and more specifically the entries) when getting back from
suspension. Are all entries cleared and masked?
Are the values previous to suspension stored?
> Also how would "x2apic_enabled" reflect the transition? It may
> have been "true" already before entering the function here.
> >>>
> >>> If x2apic_enabled == true at the point where restore_IO_APIC_setup
> >>> is called interrupt remapping must be enabled, because AFAICT at this
> >>> point it's not possible to have x2apic_enabled == true and interrupt
> >>> remapping disabled.
> >>>
> >>> The issue I can see here is what happens if interrupt remapping was
> >>> already enabled by the hardware, and entries in the IO-APIC are
> >>> already using the remapping table. I would have to look into how to
> >>> detect that case properly, but I think the proposed change is an
> >>> improvement overall.
> >>
> >> Firmware handing off with the IOMMU (and hence interrupt remapping)
> >> already enabled is a case which - afaict - we can't currently cope
> >> with. Firmware handing off in x2APIC enabled state is typically
> >> with the IOMMU (and hence interrupt remapping) still disabled. This
> >> is not a forbidden
Re: [Xen-devel] [XEN PATCH for-4.13 v2 0/9] libxl memkb & pt defaulting
Ian Jackson writes ("[XEN PATCH for-4.13 v2 0/9] libxl memkb & pt defaulting"):
> This is v2 of my series to sort out the shadow/iommu memory and pci
> passthrough situation. I think this series will mask the bug which is
> currently blocking staging propagating to master.
I have pushed this to:
https://xenbits.xen.org/gitweb/?p=people/iwj/xen.git;a=summary
wip.libxl-memkb-ptcfg.v2
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.13 v2] xen/arm: domain_build: Don't expose IOMMU specific properties to hwdom
Hi,
On 10/8/19 4:25 PM, Oleksandr Tyshchenko wrote:
From: Oleksandr Tyshchenko
We don't passthrough IOMMU device to hwdom even if it is not used by Xen.
Therefore exposing the properties that describe relationship between
master devices and IOMMUs does not make any sense.
According to the:
1. Documentation/devicetree/bindings/iommu/iommu.txt
2. Documentation/devicetree/bindings/pci/pci-iommu.txt
It is not entirely clear that documentation is from Linux.
Signed-off-by: Oleksandr Tyshchenko
---
Changes V1 [1] -> V2:
- Only skip IOMMU specific properties of the master device if we
skip the corresponding IOMMU device
- Use "hwdom" over "Dom0"
[1] https://lists.xenproject.org/archives/html/xen-devel/2019-10/msg00104.html
---
xen/arch/arm/domain_build.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 6d6dd52..a7321b8 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -480,10 +480,26 @@ static int __init write_properties(struct domain *d,
struct kernel_info *kinfo,
const struct dt_property *prop, *status = NULL;
int res = 0;
int had_dom0_bootargs = 0;
+struct dt_device_node *iommu_node;
if ( kinfo->cmdline && kinfo->cmdline[0] )
bootargs = >cmdline[0];
+/*
+ * If we skip the IOMMU device when creating DT for hwdom (even if
+ * the IOMMU device is not used by Xen), we should also skip the IOMMU
+ * specific properties of the master device behind it in order to avoid
+ * exposing an half complete IOMMU bindings to hwdom.
+ * Use "iommu_node" as an indicator of the master device which properties
+ * should be skipped.
+ */
+iommu_node = dt_parse_phandle(node, "iommus", 0);
The code is slightly confusing to read. I don't think we should care of
invalid DT here, so let's only consider valid one.
For valid DT, any non-NULL return should point to an IOMMU. The comment
above suggests that all IOMMU will be skipped. However, the check below
(device_get_class(iommu_node)) will not return DEVICE_IOMMU when there
are not have a driver for the IOMMU.
So this needs to be clarified in the commit message.
+if ( iommu_node )
+{
+if ( device_get_class(iommu_node) != DEVICE_IOMMU )
+iommu_node = NULL;
+}
Could we gather the two conditions in one if?
+
dt_for_each_property_node (node, prop)
{
const void *prop_data = prop->value;
@@ -540,6 +556,19 @@ static int __init write_properties(struct domain *d,
struct kernel_info *kinfo,
continue;
}
+if ( iommu_node )
+{
+/* Don't expose IOMMU specific properties to hwdom */
+if ( dt_property_name_is_equal(prop, "iommus") )
+continue;
+
+if ( dt_property_name_is_equal(prop, "iommu-map") )
+continue;
+
+if ( dt_property_name_is_equal(prop, "iommu-map-mask") )
+continue;
+}
+
res = fdt_property(kinfo->fdt, prop->name, prop_data, prop_len);
if ( res )
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [XEN PATCH for-4.13 v2 9/9] libxl/xl: Overhaul passthrough setting logic
LIBXL_PASSTHROUGH_UNKNOWN (aka "ENABLED" in an earlier uncommitted
version of this code) is doing double duty. We actually need all of
the following to be specificable:
* default ("unknown"): enable PT iff we have devices to
pass through specified in the initial config file.
* "enabled" (and fail if the platform doesn't support it).
* "disabled" (and reject future PT hotplug).
* "share_pt"/"sync_pt": enable PT and set a specific PT mode.
Defaulting and error checking should be done in libxl. So, we make
several changes here.
We introduce "enabled". (And we document "unknown".)
We move all of the error checking and defaulting code from xl into
libxl. Now, libxl__domain_config_setdefault has all of the necessary
information to get this right. So we can do it all there, in one
place.
We can also arrange to have only one place each which calculates
(i) whether passthrough needs to be enabled because pt devices were
specified (ii) whether pt_share can be used.
xl now only has to parse the enum in the same way as it parses all
other enums.
This change fixes a regression from earlier 4.13-pre: until recent
changes, passthrough was only enabled by default if passthrough
devices were specified. We restore this behaviour.
This will hide, from the point of view of libvirt tests in osstest, a
separate hypervisor regression which prevents migration of a domain
with passthrough enabled but without actual PT devices.
Signed-off-by: Ian Jackson
---
v2: New patch in this version of the series.
---
tools/libxl/libxl_create.c | 57 ++
tools/libxl/libxl_types.idl | 5 ++--
tools/xl/xl_parse.c | 67 -
3 files changed, 53 insertions(+), 76 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 69971c97b6..fccb6a6271 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -57,18 +57,6 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
if (!c_info->ssidref)
c_info->ssidref = SECINITSID_DOMU;
-if (info->cap_hvm_directio &&
-(c_info->passthrough == LIBXL_PASSTHROUGH_UNKNOWN)) {
-c_info->passthrough = ((c_info->type == LIBXL_DOMAIN_TYPE_PV) ||
- !info->cap_iommu_hap_pt_share) ?
-LIBXL_PASSTHROUGH_SYNC_PT : LIBXL_PASSTHROUGH_SHARE_PT;
-} else if (!info->cap_hvm_directio) {
-c_info->passthrough = LIBXL_PASSTHROUGH_DISABLED;
-}
-
-/* An explicit setting should now have been chosen */
-assert(c_info->passthrough != LIBXL_PASSTHROUGH_UNKNOWN);
-
return 0;
}
@@ -904,6 +892,7 @@ int libxl__domain_config_setdefault(libxl__gc *gc,
libxl_ctx *ctx = libxl__gc_owner(gc);
int ret;
bool pod_enabled = false;
+libxl_domain_create_info *c_info = _config->c_info;
libxl_physinfo physinfo;
ret = libxl_get_physinfo(CTX, );
@@ -968,6 +957,50 @@ int libxl__domain_config_setdefault(libxl__gc *gc,
goto error_out;
}
+bool need_pt = d_config->num_pcidevs || d_config->num_dtdevs;
+if (c_info->passthrough == LIBXL_PASSTHROUGH_UNKNOWN) {
+c_info->passthrough = need_pt
+? LIBXL_PASSTHROUGH_ENABLED : LIBXL_PASSTHROUGH_DISABLED;
+}
+
+bool iommu_enabled = physinfo.cap_hvm_directio;
+if (c_info->passthrough != LIBXL_PASSTHROUGH_DISABLED && !iommu_enabled) {
+LOGD(ERROR, domid,
+ "ERROR: passthrough not supported on this platform\n");
+ret = ERROR_INVAL;
+goto error_out;
+}
+
+if (c_info->passthrough == LIBXL_PASSTHROUGH_DISABLED && need_pt) {
+LOGD(ERROR, domid,
+ "passthrough disabled but devices are specified");
+ret = ERROR_INVAL;
+goto error_out;
+}
+
+const char *whynot_pt_share =
+c_info->type == LIBXL_DOMAIN_TYPE_PV ? "not valid for PV domain" :
+!physinfo.cap_iommu_hap_pt_share ? "not supported on this platform" :
+NULL;
+
+if (c_info->passthrough == LIBXL_PASSTHROUGH_ENABLED) {
+assert(iommu_enabled);
+c_info->passthrough = whynot_pt_share
+? LIBXL_PASSTHROUGH_SYNC_PT : LIBXL_PASSTHROUGH_SHARE_PT;
+}
+
+if (c_info->passthrough == LIBXL_PASSTHROUGH_SHARE_PT && whynot_pt_share) {
+LOGD(ERROR, domid,
+ "ERROR: passthrough=\"share_pt\" %s\n",
+ whynot_pt_share);
+ret = ERROR_INVAL;
+goto error_out;
+}
+
+/* An explicit setting should now have been chosen */
+assert(c_info->passthrough != LIBXL_PASSTHROUGH_UNKNOWN);
+assert(c_info->passthrough != LIBXL_PASSTHROUGH_ENABLED);
+
/* If target_memkb is smaller than max_memkb, the subsequent call
* to libxc when building HVM domain will enable PoD mode.
*/
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 3ac9494b80..2441c0c233 100644
--- a/tools/libxl/libxl_types.idl
+++
[Xen-devel] [XEN PATCH for-4.13 v2 0/9] libxl memkb & pt defaulting
This is v2 of my series to sort out the shadow/iommu memory and pci passthrough situation. I think this series will mask the bug which is currently blocking staging propagating to master. I had some difficulty testing this, as the test box under my desk doesn't do PT and I didn't want to wait to book one out from the test lab. So I have executed only some of the new code in libxl. I would really appreciate review by a hypervisor expert on the main chunk of new code in libxl added in patch 9, which is where all the logic related to passthrough enabling and mode now resides. am 1 libxl: Offer API versions 0x040700 and 0x040800 r 2 xl: Pass libxl_domain_config to freemem(), instead of b_info r* 3 libxl: libxl__domain_config_setdefault: New function r* 4 libxl: libxl_domain_need_memory: Make it take a domain_config * 5 libxl: Move shadow_memkb and iommu_memkb defaulting into libxl a 6 libxl: Remove/deprecate libxl_get_required_*_memory from the API + 7 libxl: create: setdefault: Make libxl_physinfo info[1] + 8 libxl: create: setdefault: Move physinfo into config_setdefault + 9 libxl/xl: Overhaul passthrough setting logic a=acked; r=reviewed; m=message changed; *=patch changed; +=new patch tools/libxl/libxl.h | 24 +- tools/libxl/libxl_create.c | 174 ++- tools/libxl/libxl_dm.c | 7 +- tools/libxl/libxl_dom.c | 7 +- tools/libxl/libxl_internal.h | 13 +++- tools/libxl/libxl_mem.c | 69 + tools/libxl/libxl_types.idl | 5 +- tools/libxl/libxl_utils.c| 15 tools/libxl/libxl_utils.h| 2 +- tools/xl/xl_parse.c | 82 ++-- tools/xl/xl_vmcontrol.c | 6 +- 11 files changed, 254 insertions(+), 150 deletions(-) -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [XEN PATCH for-4.13 v2 7/9] libxl: create: setdefault: Make libxl_physinfo info[1]
No functional change. This will let us make it into a pointer without
textual change other than to the definition.
While we are here, fix some style errors (missing { }).
Signed-off-by: Ian Jackson
---
v2: New patch in this version of the series.
---
tools/libxl/libxl_create.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index b394312d98..9c56a914ca 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -30,10 +30,10 @@
int libxl__domain_create_info_setdefault(libxl__gc *gc,
libxl_domain_create_info *c_info)
{
-libxl_physinfo info;
+libxl_physinfo info[1];
int rc;
-rc = libxl_get_physinfo(CTX, );
+rc = libxl_get_physinfo(CTX, info);
if (rc)
return rc;
@@ -45,11 +45,11 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
libxl__arch_domain_create_info_setdefault(gc, c_info);
if (c_info->type != LIBXL_DOMAIN_TYPE_PV) {
-if (info.cap_hap)
+if (info->cap_hap) {
libxl_defbool_setdefault(_info->hap, true);
-else if (info.cap_shadow)
+} else if (info->cap_shadow) {
libxl_defbool_setdefault(_info->hap, false);
-else {
+} else {
LOG(ERROR, "neither hap nor shadow paging available");
return ERROR_INVAL;
}
@@ -63,12 +63,12 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
if (!c_info->ssidref)
c_info->ssidref = SECINITSID_DOMU;
-if (info.cap_hvm_directio &&
+if (info->cap_hvm_directio &&
(c_info->passthrough == LIBXL_PASSTHROUGH_UNKNOWN)) {
c_info->passthrough = ((c_info->type == LIBXL_DOMAIN_TYPE_PV) ||
- !info.cap_iommu_hap_pt_share) ?
+ !info->cap_iommu_hap_pt_share) ?
LIBXL_PASSTHROUGH_SYNC_PT : LIBXL_PASSTHROUGH_SHARE_PT;
-} else if (!info.cap_hvm_directio) {
+} else if (!info->cap_hvm_directio) {
c_info->passthrough = LIBXL_PASSTHROUGH_DISABLED;
}
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [XEN PATCH for-4.13 v2 3/9] libxl: libxl__domain_config_setdefault: New function
Break out this into a new function. We are going to want to call it
from a new call site.
Unfortunately not all of the defaults can be moved into the new
function without changing the order in which things are done. That
does not seem wise at this stage of the release. The effect is that
additional calls to libxl__domain_config_setdefault (which are going
to be introduced) do not quite set everything. But they will do what
is needed. After Xen 4.13 is done, we should move those settings into
the right order.
No functional change.
Signed-off-by: Ian Jackson
Reviewed-by: Anthony PERARD
---
v2: Add missing error check
---
tools/libxl/libxl_create.c | 41 +
tools/libxl/libxl_internal.h | 3 +++
2 files changed, 32 insertions(+), 12 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 099761a2d7..fd8bb22be9 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -862,22 +862,14 @@ static void domcreate_destruction_cb(libxl__egc *egc,
libxl__domain_destroy_state *dds,
int rc);
-static void initiate_domain_create(libxl__egc *egc,
- libxl__domain_create_state *dcs)
+int libxl__domain_config_setdefault(libxl__gc *gc,
+libxl_domain_config *d_config,
+uint32_t domid)
{
-STATE_AO_GC(dcs->ao);
libxl_ctx *ctx = libxl__gc_owner(gc);
-uint32_t domid;
-int i, ret;
+int ret;
bool pod_enabled = false;
-/* convenience aliases */
-libxl_domain_config *const d_config = dcs->guest_config;
-const int restore_fd = dcs->restore_fd;
-
-domid = dcs->domid_soft_reset;
-libxl__domain_build_state_init(>build_state);
-
if (d_config->c_info.ssid_label) {
char *s = d_config->c_info.ssid_label;
ret = libxl_flask_context_to_sid(ctx, s, strlen(s),
@@ -1008,6 +1000,28 @@ static void initiate_domain_create(libxl__egc *egc,
goto error_out;
}
+ret = 0;
+ error_out:
+return ret;
+}
+
+static void initiate_domain_create(libxl__egc *egc,
+ libxl__domain_create_state *dcs)
+{
+STATE_AO_GC(dcs->ao);
+uint32_t domid;
+int i, ret;
+
+/* convenience aliases */
+libxl_domain_config *const d_config = dcs->guest_config;
+const int restore_fd = dcs->restore_fd;
+
+domid = dcs->domid_soft_reset;
+libxl__domain_build_state_init(>build_state);
+
+ret = libxl__domain_config_setdefault(gc,d_config,domid);
+if (ret) goto error_out;
+
ret = libxl__domain_make(gc, d_config, >build_state, );
if (ret) {
LOGD(ERROR, domid, "cannot make domain: %d", ret);
@@ -1019,6 +1033,9 @@ static void initiate_domain_create(libxl__egc *egc,
dcs->guest_domid = domid;
dcs->sdss.dm.guest_domid = 0; /* means we haven't spawned */
+/* post-4.13 todo: move these next bits of defaulting to
+ * libxl__domain_config_setdefault */
+
/*
* Set the dm version quite early so that libxl doesn't have to pass the
* build info around just to know if the domain has a device model or not.
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index d2d5af746b..50ac7b64ed 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1440,6 +1440,9 @@ _hidden int libxl__resolve_domid(libxl__gc *gc, const
char *name,
* All libxl API functions are expected to have arranged for this
* to be called before using any values within these structures.
*/
+_hidden int libxl__domain_config_setdefault(libxl__gc *gc,
+libxl_domain_config *d_config,
+uint32_t domid /* logging only */);
_hidden int libxl__domain_create_info_setdefault(libxl__gc *gc,
libxl_domain_create_info *c_info);
_hidden int libxl__domain_build_info_setdefault(libxl__gc *gc,
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [XEN PATCH for-4.13 v2 1/9] libxl: Offer API versions 0x040700 and 0x040800
According to git log -G: 0x040700 was introduced in 304400459ef0 (aka 4.7.0-rc1~481) "tools/libxl: rename remus device to checkpoint device" 0x040800 was introduced in 57f8b13c7240 (aka 4.8.0-rc1~437) "libxl: memory size in kb requires 64 bit variable" It is surprising that no-one noticed this. Anyway, in the meantime, we should fix it. Backporting this is probably a good idea: it won't change the behaviour for existing callers but it will avoid errors for some older correct uses. Signed-off-by: Ian Jackson Acked-by: Anthony PERARD --- v2: Adjusted commit message slightly. --- tools/libxl/libxl.h | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index 2555e9cd3b..518fc9e47f 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -655,6 +655,7 @@ typedef struct libxl__ctx libxl_ctx; #ifdef LIBXL_API_VERSION #if LIBXL_API_VERSION != 0x040200 && LIBXL_API_VERSION != 0x040300 && \ LIBXL_API_VERSION != 0x040400 && LIBXL_API_VERSION != 0x040500 && \ +LIBXL_API_VERSION != 0x040700 && LIBXL_API_VERSION != 0x040800 && \ LIBXL_API_VERSION != 0x041300 #error Unknown LIBXL_API_VERSION #endif -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [XEN PATCH for-4.13 v2 6/9] libxl: Remove/deprecate libxl_get_required_*_memory from the API
These are now redundant because shadow_memkb and iommu_memkb are now
defaulted automatically by libxl_domain_need_memory and
libxl_domain_create etc. Callers should not now call these; instead,
they should just let libxl take care of it.
libxl_get_required_shadow_memory was introduced in f89f555827a6
"remove late (on-demand) construction of IOMMU page tables"
We can freely remove it because it was never in any release.
libxl_get_required_shadow_memory has been in libxl approximately
forever. It should probably not have survived the creation of
libxl_domain_create, but it seems the API awkwardnesses we see in
recent commits prevented this. So we have to keep it. It remains
functional but we can deprecate it. Hopefully we can get rid of it
completely before we find the need to change the calculation to use
additional information which its arguments do not currently supply.
Signed-off-by: Ian Jackson
Acked-by: Anthony PERARD
---
tools/libxl/libxl_create.c | 17 -
tools/libxl/libxl_utils.c | 15 ---
tools/libxl/libxl_utils.h | 2 +-
3 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index a1b00a8aef..b394312d98 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -888,6 +888,21 @@ static bool ok_to_default_memkb_in_create(libxl__gc *gc)
*/
}
+static unsigned long libxl__get_required_iommu_memory(unsigned long maxmem_kb)
+{
+unsigned long iommu_pages = 0, mem_pages = maxmem_kb / 4;
+unsigned int level;
+
+/* Assume a 4 level page table with 512 entries per level */
+for (level = 0; level < 4; level++)
+{
+mem_pages = DIV_ROUNDUP(mem_pages, 512);
+iommu_pages += mem_pages;
+}
+
+return iommu_pages * 4;
+}
+
int libxl__domain_config_setdefault(libxl__gc *gc,
libxl_domain_config *d_config,
uint32_t domid)
@@ -1011,7 +1026,7 @@ int libxl__domain_config_setdefault(libxl__gc *gc,
&& ok_to_default_memkb_in_create(gc))
d_config->b_info.iommu_memkb =
(d_config->c_info.passthrough == LIBXL_PASSTHROUGH_SYNC_PT)
-? libxl_get_required_iommu_memory(d_config->b_info.max_memkb)
+? libxl__get_required_iommu_memory(d_config->b_info.max_memkb)
: 0;
ret = libxl__domain_build_info_setdefault(gc, _config->b_info);
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 405733b7e1..f360f5e228 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -48,21 +48,6 @@ unsigned long libxl_get_required_shadow_memory(unsigned long
maxmem_kb, unsigned
return 4 * (256 * smp_cpus + 2 * (maxmem_kb / 1024));
}
-unsigned long libxl_get_required_iommu_memory(unsigned long maxmem_kb)
-{
-unsigned long iommu_pages = 0, mem_pages = maxmem_kb / 4;
-unsigned int level;
-
-/* Assume a 4 level page table with 512 entries per level */
-for (level = 0; level < 4; level++)
-{
-mem_pages = DIV_ROUNDUP(mem_pages, 512);
-iommu_pages += mem_pages;
-}
-
-return iommu_pages * 4;
-}
-
char *libxl_domid_to_name(libxl_ctx *ctx, uint32_t domid)
{
unsigned int len;
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 630ccbe28a..46918aea84 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -24,7 +24,7 @@ const
char *libxl_basename(const char *name); /* returns string from strdup */
unsigned long libxl_get_required_shadow_memory(unsigned long maxmem_kb,
unsigned int smp_cpus);
-unsigned long libxl_get_required_iommu_memory(unsigned long maxmem_kb);
+ /* deprecated; see LIBXL_HAVE_DOMAIN_NEED_MEMORY_CONFIG in libxl.h */
int libxl_name_to_domid(libxl_ctx *ctx, const char *name, uint32_t *domid);
int libxl_domain_qualifier_to_domid(libxl_ctx *ctx, const char *name, uint32_t
*domid);
char *libxl_domid_to_name(libxl_ctx *ctx, uint32_t domid);
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [XEN PATCH for-4.13 v2 5/9] libxl: Move shadow_memkb and iommu_memkb defaulting into libxl
Defaulting is supposed to be done by libxl. So these calculations
should be here in libxl. libxl__domain_config_setdefault has all the
necessary information including the values of max_memkb and max_vcpus.
The overall functional effect depends on the caller:
For xl, no change. The code moves from xl to libxl.
For callers who set one or both shadow_memkb and iommu_memkb (whether
from libxl_get_required_shadow_memory or otherwise) before calling
libxl_domain_need_memory (any version): the new code will leave their
setting(s) unchanged.
For callers who do not call libxl_domain_need_memory at all, and who
fail to set one of these memory values: now they are both are properly
set. The shadow and iommu memory to be properly accounted for as
intended.
For callers which call libxl_domain_need_memory and request the
current API (4.13) or which track libxl, the default values are also
now right and everything works as intended.
For callers which call libxl_domain_need_memory, and request an old
pre-4.13 libxl API, and which leave one of these memkb settings unset,
we take special measures to preserve the old behaviour.
This means that they don't get the additional iommu memory and are at
risk of the domain running out of memory as a result of f89f555827a6
"remove late (on-demand) construction of IOMMU page tables". But this
is no worse than the state just after f89f555827a6, which already
broke such callers in that way. This is perhaps justifiable because
of the API stability warning next to libxl_domain_need_memory.
An alternative would be to drop the special-casing of these callers.
That would cause a discrepancy between libxl_domain_need_memory and
libxl_domain_create: the former would not include the iommu memory and
the latter would. That seems worse, but it's debateable.
Signed-off-by: Ian Jackson
---
v2: Replace _Bool with bool
v2: Fix logic sense in ok_to_default_memkb_in_create
---
tools/libxl/libxl_create.c | 40
tools/libxl/libxl_internal.h | 3 +++
tools/libxl/libxl_mem.c | 4
tools/xl/xl_parse.c | 15 ++-
4 files changed, 49 insertions(+), 13 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index fd8bb22be9..a1b00a8aef 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -250,6 +250,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
switch (b_info->type) {
case LIBXL_DOMAIN_TYPE_HVM:
if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
+/* Normally defaulted in libxl__domain_create_info_setdefault */
b_info->shadow_memkb = 0;
if (b_info->u.hvm.mmio_hole_memkb == LIBXL_MEMKB_DEFAULT)
b_info->u.hvm.mmio_hole_memkb = 0;
@@ -395,6 +396,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
b_info->video_memkb = 0;
if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
+/* Normally defaulted in libxl__domain_create_info_setdefault */
b_info->shadow_memkb = 0;
if (b_info->u.pv.slack_memkb == LIBXL_MEMKB_DEFAULT)
b_info->u.pv.slack_memkb = 0;
@@ -862,6 +864,30 @@ static void domcreate_destruction_cb(libxl__egc *egc,
libxl__domain_destroy_state *dds,
int rc);
+static bool ok_to_default_memkb_in_create(libxl__gc *gc)
+{
+/*
+ * This is a fudge. We are trying to find whether the caller
+ * calls the old version of libxl_domain_need_memory. If they do
+ * then, because it only gets the b_info, and because it can't
+ * update the b_info (because it's const), it will base its
+ * calculations on defaulting shadow_memkb and iommu_memkb to 0
+ * In that case we probably shouldn't default them differently
+ * during libxl_domain_create.
+ *
+ * The result is that the behaviour with old callers is the same
+ * as in 4.13: no additional memory is allocated for shadow and
+ * iommu (unless the caller set shadow_memkb, eg from a call to
+ * libxl_get_required_shadow_memory).
+ */
+return !CTX->libxl_domain_need_memory_0x041200_called ||
+CTX->libxl_domain_need_memory_called;
+/*
+ * Treat mixed callers as new callers. Presumably they know what
+ * they are doing.
+ */
+}
+
int libxl__domain_config_setdefault(libxl__gc *gc,
libxl_domain_config *d_config,
uint32_t domid)
@@ -974,6 +1000,20 @@ int libxl__domain_config_setdefault(libxl__gc *gc,
goto error_out;
}
+if (d_config->b_info.shadow_memkb == LIBXL_MEMKB_DEFAULT
+&& ok_to_default_memkb_in_create(gc))
+d_config->b_info.shadow_memkb =
+libxl_get_required_shadow_memory(d_config->b_info.max_memkb,
+
[Xen-devel] [XEN PATCH for-4.13 v2 4/9] libxl: libxl_domain_need_memory: Make it take a domain_config
This should calculate the extra memory needed for shadow and iommu,
the defaults for which depend on values in c_info. So we need this to
have the complete domain config available.
And the defaults should actually be updated and stored. So make it
non-const.
We provide the usual kind of compatibility function for callers
expecting 4.12 and earlier. This function becomes responsible for the
clone-and-modify of the b_info.
No overall functional change for external libxl callers which use the
API version system to request a particular API version.
Other external libxl callers will need to update their calling code,
and will then find that the new version of this function fills in most
of the defaults in d_config. Because libxl__domain_config_setdefault
doesn't quite do all of the defaults, that's only partial. For
present purposes that doesn't matter because none of the missing
settings are used by the memory calculations. It does mean we need to
document in the API spec that the defaulting is only partial.
This lack of functional change is despite the fact that
numa_place_domain now no longer calls
libxl__domain_build_info_setdefault (via libxl_domain_need_memory).
That is OK because it's idempotent and numa_place_domain's one call
site is libxl__build_pre which is called from libxl__domain_build
which is called from domcreate_bootloader_done, well after the
defaults are set by initiate_domain_create.
Signed-off-by: Ian Jackson
Reviewed-by: Anthony PERARD
---
v2: Drop now-erroneous GC_FREE as well as the corresponding GC_INIT.
---
tools/libxl/libxl.h | 23 +++-
tools/libxl/libxl_dom.c | 7 +++--
tools/libxl/libxl_internal.h | 4 +++
tools/libxl/libxl_mem.c | 65 +++-
tools/xl/xl_vmcontrol.c | 2 +-
5 files changed, 83 insertions(+), 18 deletions(-)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 518fc9e47f..49b56fa1a3 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1245,6 +1245,20 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst,
const libxl_mac *src);
*/
#define LIBXL_HAVE_FN_USING_QMP_ASYNC 1
+/*
+ * LIBXL_HAVE_DOMAIN_NEED_MEMORY_CONFIG
+ *
+ * If this is set, libxl_domain_need_memory takes a
+ * libxl_domain_config* (non-const) and uint32_t domid_for_logging
+ * (instead of a const libxl_domain_build_info*).
+ *
+ * If this is set, there is no need to call
+ * libxl_get_required_shadow_memory and instead the caller should
+ * simply leave shadow_memkb set to LIBXL_MEMKB_DEFAULT and allow
+ * libxl to fill in a suitable default in the usual way.
+ */
+#define LIBXL_HAVE_DOMAIN_NEED_MEMORY_CONFIG
+
typedef char **libxl_string_list;
void libxl_string_list_dispose(libxl_string_list *sl);
int libxl_string_list_length(const libxl_string_list *sl);
@@ -1723,8 +1737,13 @@ int libxl_get_memory_target_0x040700(libxl_ctx *ctx,
uint32_t domid,
*/
/* how much free memory in the system a domain needs to be built */
int libxl_domain_need_memory(libxl_ctx *ctx,
- const libxl_domain_build_info *b_info_in,
+ libxl_domain_config *config
+ /* ^ will be partially defaulted */,
+ uint32_t domid_for_logging /* INVALID_DOMID ok */,
uint64_t *need_memkb);
+int libxl_domain_need_memory_0x041200(libxl_ctx *ctx,
+ const libxl_domain_build_info *b_info_in,
+ uint64_t *need_memkb);
int libxl_domain_need_memory_0x040700(libxl_ctx *ctx,
const libxl_domain_build_info *b_info_in,
uint32_t *need_memkb)
@@ -1754,6 +1773,8 @@ int libxl_wait_for_memory_target(libxl_ctx *ctx, uint32_t
domid, int wait_secs);
#define libxl_get_memory_target libxl_get_memory_target_0x040700
#define libxl_domain_need_memory libxl_domain_need_memory_0x040700
#define libxl_get_free_memory libxl_get_free_memory_0x040700
+#elif defined(LIBXL_API_VERSION) && LIBXL_API_VERSION < 0x041300
+#define libxl_domain_need_memory libxl_domain_need_memory_0x041200
#endif
int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t domid, int autopass);
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index c5685b061c..cdb294ab8d 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -140,8 +140,9 @@ static int numa_cmpf(const libxl__numa_candidate *c1,
/* The actual automatic NUMA placement routine */
static int numa_place_domain(libxl__gc *gc, uint32_t domid,
- libxl_domain_build_info *info)
+ libxl_domain_config *d_config)
{
+libxl_domain_build_info *info = _config->b_info;
int found;
libxl__numa_candidate candidate;
libxl_bitmap cpumap, cpupool_nodemap, *map;
@@ -195,7 +196,7 @@ static int numa_place_domain(libxl__gc *gc, uint32_t domid,
[Xen-devel] [XEN PATCH for-4.13 v2 8/9] libxl: create: setdefault: Move physinfo into config_setdefault
No functional change. This will let us refer to it in code we are
about to add to this function.
Signed-off-by: Ian Jackson
---
v2: New patch in this version of the series.
---
tools/libxl/libxl_create.c | 17 -
tools/libxl/libxl_dm.c | 7 ++-
tools/libxl/libxl_internal.h | 3 ++-
3 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 9c56a914ca..69971c97b6 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -28,15 +28,9 @@
#include
int libxl__domain_create_info_setdefault(libxl__gc *gc,
- libxl_domain_create_info *c_info)
+ libxl_domain_create_info *c_info,
+ const libxl_physinfo *info)
{
-libxl_physinfo info[1];
-int rc;
-
-rc = libxl_get_physinfo(CTX, info);
-if (rc)
-return rc;
-
if (!c_info->type) {
LOG(ERROR, "domain type unspecified");
return ERROR_INVAL;
@@ -911,6 +905,10 @@ int libxl__domain_config_setdefault(libxl__gc *gc,
int ret;
bool pod_enabled = false;
+libxl_physinfo physinfo;
+ret = libxl_get_physinfo(CTX, );
+if (ret) goto error_out;
+
if (d_config->c_info.ssid_label) {
char *s = d_config->c_info.ssid_label;
ret = libxl_flask_context_to_sid(ctx, s, strlen(s),
@@ -1009,7 +1007,8 @@ int libxl__domain_config_setdefault(libxl__gc *gc,
goto error_out;
}
-ret = libxl__domain_create_info_setdefault(gc, _config->c_info);
+ret = libxl__domain_create_info_setdefault(gc, _config->c_info,
+ );
if (ret) {
LOGD(ERROR, domid, "Unable to set domain create info defaults");
goto error_out;
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index c00356a2f1..e6a48974f8 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2167,7 +2167,12 @@ void libxl__spawn_stub_dm(libxl__egc *egc,
libxl__stub_dm_spawn_state *sdss)
dm_config->c_info.run_hotplug_scripts =
guest_config->c_info.run_hotplug_scripts;
-ret = libxl__domain_create_info_setdefault(gc, _config->c_info);
+libxl_physinfo physinfo;
+ret = libxl_get_physinfo(CTX, );
+if (ret) goto out;
+
+ret = libxl__domain_create_info_setdefault(gc, _config->c_info,
+ );
if (ret) goto out;
ret = libxl__domain_build_info_setdefault(gc, _config->b_info);
if (ret) goto out;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 0185b8ff01..6a614658c2 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1447,7 +1447,8 @@ _hidden int libxl__domain_config_setdefault(libxl__gc *gc,
libxl_domain_config *d_config,
uint32_t domid /* logging only */);
_hidden int libxl__domain_create_info_setdefault(libxl__gc *gc,
-libxl_domain_create_info *c_info);
+libxl_domain_create_info *c_info,
+const libxl_physinfo *info);
_hidden int libxl__domain_build_info_setdefault(libxl__gc *gc,
libxl_domain_build_info *b_info);
_hidden void libxl__rdm_setdefault(libxl__gc *gc,
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [XEN PATCH for-4.13 v2 2/9] xl: Pass libxl_domain_config to freemem(), instead of b_info
We are going to change the libxl API in a moment and this change will
make it simpler.
Signed-off-by: Ian Jackson
Reviewed-by: Anthony PERARD
---
tools/xl/xl_vmcontrol.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
index b20582e15b..d33c6b38c9 100644
--- a/tools/xl/xl_vmcontrol.c
+++ b/tools/xl/xl_vmcontrol.c
@@ -314,7 +314,7 @@ static int domain_wait_event(uint32_t domid, libxl_event
**event_r)
* Returns true in case there is already, or we manage to free it, enough
* memory, but also if autoballoon is false.
*/
-static bool freemem(uint32_t domid, libxl_domain_build_info *b_info)
+static bool freemem(uint32_t domid, libxl_domain_config *d_config)
{
int rc, retries = 3;
uint64_t need_memkb, free_memkb;
@@ -322,7 +322,7 @@ static bool freemem(uint32_t domid, libxl_domain_build_info
*b_info)
if (!autoballoon)
return true;
-rc = libxl_domain_need_memory(ctx, b_info, _memkb);
+rc = libxl_domain_need_memory(ctx, _config->b_info, _memkb);
if (rc < 0)
return false;
@@ -879,7 +879,7 @@ start:
goto error_out;
if (domid_soft_reset == INVALID_DOMID) {
-if (!freemem(domid, _config.b_info)) {
+if (!freemem(domid, _config)) {
fprintf(stderr, "failed to free memory for the domain\n");
ret = ERROR_FAIL;
goto error_out;
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.13] xen/xsm: flask: Prevent NULL deference in flask_assign_{, dt}device()
Hi,
On Thu, 2019-10-10 at 15:48 +0100, Julien Grall wrote:
>
> On 09/10/2019 12:57, Artem Mygaiev wrote:
> > Hi Julien
>
> Hi,
>
> > On Fri, 2019-10-04 at 17:42 +0100, Julien Grall wrote:
> > > flask_assign_{, dt}device() may be used to check whether you can test
> > > if
> > > a device is assigned. In this case, the domain will be NULL.
> > >
> > > However, flask_iommu_resource_use_perm() will be called and may end
> > > up
> > > to deference a NULL pointer. This can be prevented by moving the call
> > > after we check the validity for the domain pointer.
> > >
> > > Coverity-ID: 1486741
> >
> > The correct CID is 1486742
>
> Hmmm yes. The coverity report e-mail is a bit confusing to read.
>
> However, I have already committed the patch so we will have to leave with it
> :(.
>
I guess the solution could be to fix another one and make a proper
commit comment with cross-reference :)
> Cheers,
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RESEND][PATCH for-4.13] xen/arm: mm: Clear boot pagetables before bringing-up each secondary CPU
+Juergen
On 03/10/2019 02:22, Stefano Stabellini wrote:
On Sat, 21 Sep 2019, Julien Grall wrote:
At the moment, boot pagetables are only cleared once at boot. This means
when booting CPU2 (and onwards) then boot pagetables will not be
cleared.
To keep the interface exactly the same for all secondary CPU, the boot
pagetables are now cleared before bringing-up each secondary CPU.
Signed-off-by: Julien Grall
Reviewed-by: Stefano Stabellini
---
xen/arch/arm/mm.c | 27 ++-
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 1129dc28c8..e14ee76ff8 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -704,8 +704,20 @@ void __init setup_pagetables(unsigned long
boot_phys_offset)
switch_ttbr(ttbr);
-/* Clear the copy of the boot pagetables. Each secondary CPU
- * rebuilds these itself (see head.S) */
+xen_pt_enforce_wnx();
+
+#ifdef CONFIG_ARM_32
+per_cpu(xen_pgtable, 0) = cpu0_pgtable;
+per_cpu(xen_dommap, 0) = cpu0_dommap;
+#endif
+}
+
+static void clear_boot_pagetables(void)
+{
+/*
+ * Clear the copy of the boot pagetables. Each secondary CPU
+ * rebuilds these itself (see head.S)
+ */
clear_table(boot_pgtable);
#ifdef CONFIG_ARM_64
clear_table(boot_first);
@@ -713,18 +725,13 @@ void __init setup_pagetables(unsigned long
boot_phys_offset)
#endif
clear_table(boot_second);
clear_table(boot_third);
-
-xen_pt_enforce_wnx();
-
-#ifdef CONFIG_ARM_32
-per_cpu(xen_pgtable, 0) = cpu0_pgtable;
-per_cpu(xen_dommap, 0) = cpu0_dommap;
-#endif
}
#ifdef CONFIG_ARM_64
int init_secondary_pagetables(int cpu)
{
+clear_boot_pagetables();
+
/* Set init_ttbr for this CPU coming up. All CPus share a single setof
* pagetables, but rewrite it each time for consistency with 32 bit. */
init_ttbr = (uintptr_t) xen_pgtable + phys_offset;
@@ -767,6 +774,8 @@ int init_secondary_pagetables(int cpu)
per_cpu(xen_pgtable, cpu) = first;
per_cpu(xen_dommap, cpu) = domheap;
+clear_boot_pagetables();
+
/* Set init_ttbr for this CPU coming up */
init_ttbr = __pa(first);
clean_dcache(init_ttbr);
--
2.11.0
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.13] xen/xsm: flask: Prevent NULL deference in flask_assign_{, dt}device()
On 09/10/2019 12:57, Artem Mygaiev wrote:
Hi Julien
Hi,
On Fri, 2019-10-04 at 17:42 +0100, Julien Grall wrote:
flask_assign_{, dt}device() may be used to check whether you can test
if
a device is assigned. In this case, the domain will be NULL.
However, flask_iommu_resource_use_perm() will be called and may end
up
to deference a NULL pointer. This can be prevented by moving the call
after we check the validity for the domain pointer.
Coverity-ID: 1486741
The correct CID is 1486742
Hmmm yes. The coverity report e-mail is a bit confusing to read.
However, I have already committed the patch so we will have to leave with it :(.
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/docs: arm: Update dom0less binding and example
On 10.10.19 16:45, Julien Grall wrote:
Hi,
Juergen, would you be happy if this patch is committed for Xen 4.13?
Yes, you can add my:
Release-acked-by: Juergen Gross
Juergen
Cheers,
On 02/10/2019 23:27, Stefano Stabellini wrote:
On Tue, 13 Aug 2019, Julien Grall wrote:
The binding for the dom0less module does not match Xen implementation.
Any module should contain the compatible "multiboot,module" to be
recognized.
This was clearly an oversight as other examples with Xen code base
provide the compatible "multiboot,module".
So fix the binding and the example associated to it.
Signed-off-by: Julien Grall
Yes!
Reviewed-by: Stefano Stabellini
---
Cc: viktor_mi...@epam.com
We probably want to consolidate all the dom0less documentation in
one place rather than having to fix documation issues in a multiple
places one by one.
---
docs/misc/arm/device-tree/booting.txt | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/docs/misc/arm/device-tree/booting.txt
b/docs/misc/arm/device-tree/booting.txt
index 317a9e962a..0fafa01b5d 100644
--- a/docs/misc/arm/device-tree/booting.txt
+++ b/docs/misc/arm/device-tree/booting.txt
@@ -160,7 +160,7 @@ The kernel sub-node has the following properties:
- compatible
- "multiboot,kernel"
+ "multiboot,kernel", "multiboot,module"
- reg
@@ -175,7 +175,7 @@ The ramdisk sub-node has the following properties:
- compatible
- "multiboot,ramdisk"
+ "multiboot,ramdisk", "multiboot,module"
- reg
@@ -196,13 +196,13 @@ chosen {
vpl011;
module@0x4a00 {
- compatible = "multiboot,kernel";
+ compatible = "multiboot,kernel", "multiboot,module";
reg = <0x0 0x4a00 0xff>;
bootargs = "console=ttyAMA0 init=/bin/sh";
};
module@0x4b00 {
- compatible = "multiboot,ramdisk";
+ compatible = "multiboot,ramdisk", "multiboot,module";
reg = <0x0 0x4b00 0xff>;
};
};
@@ -215,13 +215,13 @@ chosen {
cpus = <1>;
module@0x4c00 {
- compatible = "multiboot,kernel";
+ compatible = "multiboot,kernel", "multiboot,module";
reg = <0x0 0x4c00 0xff>;
bootargs = "console=ttyAMA0 init=/bin/sh";
};
module@0x4d00 {
- compatible = "multiboot,ramdisk";
+ compatible = "multiboot,ramdisk", "multiboot,module";
reg = <0x0 0x4d00 0xff>;
};
};
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 1/3] x86/boot: Introduce the kernel_info
On 10/10/19 2:43 AM, Daniel Kiper wrote: > On Wed, Oct 09, 2019 at 05:43:31PM -0700, Randy Dunlap wrote: >> Hi, >> >> Questions and comments below... >> Thanks. >> >> On 10/9/19 3:53 AM, Daniel Kiper wrote: >> >>> Suggested-by: H. Peter Anvin >>> Signed-off-by: Daniel Kiper >>> Reviewed-by: Konrad Rzeszutek Wilk >>> Reviewed-by: Ross Philipson >>> --- >> >>> --- >>> Documentation/x86/boot.rst | 121 >>> + >>> arch/x86/boot/Makefile | 2 +- >>> arch/x86/boot/compressed/Makefile | 4 +- >>> arch/x86/boot/compressed/kernel_info.S | 17 + >>> arch/x86/boot/header.S | 1 + >>> arch/x86/boot/tools/build.c| 5 ++ >>> arch/x86/include/uapi/asm/bootparam.h | 1 + >>> 7 files changed, 148 insertions(+), 3 deletions(-) >>> create mode 100644 arch/x86/boot/compressed/kernel_info.S >>> >>> diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst >>> index 08a2f100c0e6..d5323a39f5e3 100644 >>> --- a/Documentation/x86/boot.rst >>> +++ b/Documentation/x86/boot.rst >>> @@ -68,8 +68,25 @@ Protocol 2.12(Kernel 3.8) Added the xloadflags field >>> and extension fields >>> Protocol 2.13 (Kernel 3.14) Support 32- and 64-bit flags being set in >>> xloadflags to support booting a 64-bit kernel from 32-bit >>> EFI >>> + >>> +Protocol 2.14: BURNT BY INCORRECT COMMIT >>> ae7e1238e68f2a472a125673ab506d49158c1889 >>> + (x86/boot: Add ACPI RSDP address to setup_header) >>> + DO NOT USE!!! ASSUME SAME AS 2.13. >>> + >>> +Protocol 2.15: (Kernel 5.5) Added the kernel_info. >>> = >>> >>> >>> +.. note:: >>> + The protocol version number should be changed only if the setup header >>> + is changed. There is no need to update the version number if >>> boot_params >>> + or kernel_info are changed. Additionally, it is recommended to use >>> + xloadflags (in this case the protocol version number should not be >>> + updated either) or kernel_info to communicate supported Linux kernel >>> + features to the boot loader. Due to very limited space available in >>> + the original setup header every update to it should be considered >>> + with great care. Starting from the protocol 2.15 the primary way to >>> + communicate things to the boot loader is the kernel_info. >>> + >>> >>> Memory Layout >>> = >>> @@ -207,6 +224,7 @@ Offset/Size Proto Name >>> Meaning >>> 0258/8 2.10+ pref_addressPreferred >>> loading address >>> 0260/4 2.10+ init_size Linear memory >>> required during initialization >>> 0264/4 2.11+ handover_offset Offset of >>> handover entry point >>> +0268/4 2.15+ kernel_info_offset Offset of the >>> kernel_info >>> ==== >>> >>> >>> .. note:: >>> @@ -855,6 +873,109 @@ Offset/size: 0x264/4 >>> >>>See EFI HANDOVER PROTOCOL below for more details. >>> >>> + == >>> +Field name:kernel_info_offset >>> +Type: read >>> +Offset/size: 0x268/4 >>> +Protocol: 2.15+ >>> + == >>> + >>> + This field is the offset from the beginning of the kernel image to the >>> + kernel_info. It is embedded in the Linux image in the uncompressed >> ^^ >>What does It refer to, please? > > s/It/The kernel_info structure/ Is it better? Yes. >>> + protected mode region. >>> + >>> + >>> +The kernel_info >>> +=== >>> + >>> +The relationships between the headers are analogous to the various data >>> +sections: >>> + >>> + setup_header = .data >>> + boot_params/setup_data = .bss >>> + >>> +What is missing from the above list? That's right: >>> + >>> + kernel_info = .rodata >>> + >>> +We have been (ab)using .data for things that could go into .rodata or .bss >>> for >>> +a long time, for lack of alternatives and -- especially early on -- >>> inertia. >>> +Also, the BIOS stub is responsible for creating boot_params, so it isn't >>> +available to a BIOS-based loader (setup_data is, though). >>> + >>> +setup_header is permanently limited to 144 bytes due to the reach of the >>> +2-byte jump field, which doubles as a length field for the structure, >>> combined >>> +with the size of the "hole" in struct boot_params that a protected-mode >>> loader >>> +or the BIOS stub has to copy it into. It is currently 119 bytes long, which >>> +leaves us with 25 very precious bytes. This isn't something that can be >>> fixed >>> +without revising the boot protocol entirely, breaking backwards >>> compatibility. >>> + >>> +boot_params proper is limited to 4096
Re: [Xen-devel] [PATCH v2] xen/arm: domain_build: Print the correct domain in dtb_load()
Hi,
Hmm, it looks like I forgot this patch before the freeze. Juergen, are you happy
with this to go in Xen 4.13?
Cheers,
On 15/08/2019 18:29, Julien Grall wrote:
dtb_load() can be called by other domain than dom0. To avoid confusion
in the log, print the correct domain.
Signed-off-by: Julien Grall
---
Changes in v2:
- Fix the second print in the function as well.
---
xen/arch/arm/domain_build.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 4e51e22927..126374f603 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1787,15 +1787,17 @@ static void __init dtb_load(struct kernel_info *kinfo)
{
unsigned long left;
-printk("Loading dom0 DTB to 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
- kinfo->dtb_paddr, kinfo->dtb_paddr + fdt_totalsize(kinfo->fdt));
+printk("Loading %pd DTB to 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
+ kinfo->d, kinfo->dtb_paddr,
+ kinfo->dtb_paddr + fdt_totalsize(kinfo->fdt));
left = copy_to_guest_phys_flush_dcache(kinfo->d, kinfo->dtb_paddr,
kinfo->fdt,
fdt_totalsize(kinfo->fdt));
if ( left != 0 )
-panic("Unable to copy the DTB to dom0 memory (left = %lu bytes)\n",
left);
+panic("Unable to copy the DTB to %pd memory (left = %lu bytes)\n",
+ kinfo->d, left);
xfree(kinfo->fdt);
}
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 142556: tolerable all pass - PUSHED
flight 142556 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/142556/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 4ca8eab5ce1893b3048b06921f12157d33ab60f7 baseline version: xen 1b00c16bdfbec98887731a40ea9f377f7dcac405 Last test of basis 142542 2019-10-10 08:04:32 Z0 days Testing same since 142556 2019-10-10 12:00:45 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Chao Gao Igor Druzhinin Jan Beulich Roger Pau Monné jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 1b00c16bdf..4ca8eab5ce 4ca8eab5ce1893b3048b06921f12157d33ab60f7 -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v1] libxl: Add DTB compatible list to config file
From: Oleksandr Grytsov
Some platforms need more compatible property values in device
tree root node in addition to "xen,xenvm-%d.%d" and "xen,xenvm"
values that are given by Xen by default.
Specify in domain configuration file which values should be added
by providing "dtb_compatible" list of strings separated by comas.
Signed-off-by: Iurii Konovalenko
Signed-off-by: Oleksandr Andrushchenko
---
tools/libxl/libxl_arm.c | 42 ++---
tools/libxl/libxl_types.idl | 1 +
tools/xl/xl_parse.c | 7 +++
3 files changed, 42 insertions(+), 8 deletions(-)
diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index bf31b9b3ca..b956a6356c 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -270,20 +270,46 @@ static int fdt_property_regs(libxl__gc *gc, void *fdt,
static int make_root_properties(libxl__gc *gc,
const libxl_version_info *vers,
-void *fdt)
+void *fdt,
+const libxl_domain_build_info *info)
{
-int res;
+const char *compat0 = GCSPRINTF("xen,xenvm-%d.%d",
+vers->xen_version_major,
+vers->xen_version_minor);
+const char *compat1 = "xen,xenvm";
+const char **compats;
+char *compat, *p;
+size_t sz = 0;
+int i, res, num_compats;
res = fdt_property_string(fdt, "model", GCSPRINTF("XENVM-%d.%d",
vers->xen_version_major,
vers->xen_version_minor));
if (res) return res;
-res = fdt_property_compat(gc, fdt, 2,
- GCSPRINTF("xen,xenvm-%d.%d",
-vers->xen_version_major,
-vers->xen_version_minor),
- "xen,xenvm");
+num_compats = 2 + libxl_string_list_length(>dt_compatible);
+compats = libxl__zalloc(gc, num_compats * sizeof(*compats));
+if (!compats)
+return -FDT_ERR_INTERNAL;
+
+compats[0] = compat0;
+compats[1] = compat1;
+sz = strlen(compat0) + strlen(compat1) + 2;
+for (i = 0; info->dt_compatible && info->dt_compatible[i] != NULL; i++) {
+compats[2 + i] = info->dt_compatible[i];
+sz += strlen(info->dt_compatible[i]) + 1;
+}
+
+p = compat = libxl__zalloc(gc, sz);
+if (!p)
+return -FDT_ERR_INTERNAL;
+
+for (i = 0; i < num_compats; i++) {
+strcpy(p, compats[i]);
+p += strlen(compats[i]) + 1;
+}
+
+res = fdt_property(fdt, "compatible", compat, sz);
if (res) return res;
res = fdt_property_cell(fdt, "interrupt-parent", GUEST_PHANDLE_GIC);
@@ -930,7 +956,7 @@ next_resize:
FDT( fdt_begin_node(fdt, "") );
-FDT( make_root_properties(gc, vers, fdt) );
+FDT( make_root_properties(gc, vers, fdt, info) );
FDT( make_chosen_node(gc, fdt, !!dom->modules[0].blob, state, info) );
FDT( make_cpus_node(gc, fdt, info->max_vcpus, ainfo) );
FDT( make_psci_node(gc, fdt) );
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 3ac9494b80..08ffb65904 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -544,6 +544,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
# Note that the partial device tree should avoid to use the phandle
# 65000 which is reserved by the toolstack.
("device_tree", string),
+("dt_compatible",libxl_string_list),
("acpi", libxl_defbool),
("bootloader", string),
("bootloader_args", libxl_string_list),
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 03a2c54dd2..db9821c765 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2408,6 +2408,13 @@ skip_vfb:
}
}
+e = xlu_cfg_get_list_as_string_list(config, "dt_compatible",
+_info->dt_compatible, 1);
+if (e && e != ESRCH) {
+fprintf(stderr,"xl: Unable to parse dt_compatible\n");
+exit(-ERROR_FAIL);
+}
+
if (!xlu_cfg_get_list(config, "usbctrl", , 0, 0)) {
d_config->num_usbctrls = 0;
d_config->usbctrls = NULL;
--
2.17.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen: Stop abusing DT of_dma_configure API
On 10/9/19 7:42 AM, Oleksandr Andrushchenko wrote:
> On 10/8/19 10:41 PM, Rob Herring wrote:
>> As the removed comments say, these aren't DT based devices.
>> of_dma_configure() is going to stop allowing a NULL DT node and calling
>> it will no longer work.
>>
>> The comment is also now out of date as of commit 9ab91e7c5c51 ("arm64:
>> default to the direct mapping in get_arch_dma_ops"). Direct mapping
>> is now the default rather than dma_dummy_ops.
>>
>> According to Stefano and Oleksandr, the only other part needed is
>> setting the DMA masks and there's no reason to restrict the masks to
>> 32-bits. So set the masks to 64 bits.
>>
>> Cc: Robin Murphy
>> Cc: Julien Grall
>> Cc: Nicolas Saenz Julienne
>> Cc: Oleksandr Andrushchenko
>> Cc: Boris Ostrovsky
>> Cc: Juergen Gross
>> Cc: Stefano Stabellini
>> Cc: Christoph Hellwig
>> Cc: xen-devel@lists.xenproject.org
>> Signed-off-by: Rob Herring
> Acked-by: Oleksandr Andrushchenko
Is this going to go via drm tree or should I pick it up for Xen tree?
-boris
>
> Unfortunately I cannot test this patch with real HW running Xen:
> I am still on 4.14 kernel which is dictated by the board's BSP and
> it is not possible to have more recent one at the moment.
> So, I hope the patch will work as intended.
>
> Thank you,
> Oleksandr
>> ---
>> v2:
>> - Setup dma masks
>> - Also fix xen_drm_front.c
>>
>> This can now be applied to the Xen tree independent of the coming
>> of_dma_configure() changes.
>>
>> Rob
>>
>> drivers/gpu/drm/xen/xen_drm_front.c | 12 ++--
>> drivers/xen/gntdev.c| 13 ++---
>> 2 files changed, 4 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xen/xen_drm_front.c
>> b/drivers/gpu/drm/xen/xen_drm_front.c
>> index ba1828acd8c9..4be49c1aef51 100644
>> --- a/drivers/gpu/drm/xen/xen_drm_front.c
>> +++ b/drivers/gpu/drm/xen/xen_drm_front.c
>> @@ -718,17 +718,9 @@ static int xen_drv_probe(struct xenbus_device *xb_dev,
>> struct device *dev = _dev->dev;
>> int ret;
>>
>> -/*
>> - * The device is not spawn from a device tree, so arch_setup_dma_ops
>> - * is not called, thus leaving the device with dummy DMA ops.
>> - * This makes the device return error on PRIME buffer import, which
>> - * is not correct: to fix this call of_dma_configure() with a NULL
>> - * node to set default DMA ops.
>> - */
>> -dev->coherent_dma_mask = DMA_BIT_MASK(32);
>> -ret = of_dma_configure(dev, NULL, true);
>> +ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
>> if (ret < 0) {
>> -DRM_ERROR("Cannot setup DMA ops, ret %d", ret);
>> +DRM_ERROR("Cannot setup DMA mask, ret %d", ret);
>> return ret;
>> }
>>
>> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
>> index a446a7221e13..81401f386c9c 100644
>> --- a/drivers/xen/gntdev.c
>> +++ b/drivers/xen/gntdev.c
>> @@ -22,6 +22,7 @@
>>
>> #define pr_fmt(fmt) "xen:" KBUILD_MODNAME ": " fmt
>>
>> +#include
>> #include
>> #include
>> #include
>> @@ -34,9 +35,6 @@
>> #include
>> #include
>> #include
>> -#ifdef CONFIG_XEN_GRANT_DMA_ALLOC
>> -#include
>> -#endif
>>
>> #include
>> #include
>> @@ -625,14 +623,7 @@ static int gntdev_open(struct inode *inode, struct file
>> *flip)
>> flip->private_data = priv;
>> #ifdef CONFIG_XEN_GRANT_DMA_ALLOC
>> priv->dma_dev = gntdev_miscdev.this_device;
>> -
>> -/*
>> - * The device is not spawn from a device tree, so arch_setup_dma_ops
>> - * is not called, thus leaving the device with dummy DMA ops.
>> - * Fix this by calling of_dma_configure() with a NULL node to set
>> - * default DMA ops.
>> - */
>> -of_dma_configure(priv->dma_dev, NULL, true);
>> +dma_coerce_mask_and_coherent(priv->dma_dev, DMA_BIT_MASK(64));
>> #endif
>> pr_debug("priv %p\n", priv);
>>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/2] x2APIC: translate IO-APIC entries when enabling the IOMMU
On 10.10.2019 15:29, Roger Pau Monné wrote: > On Thu, Oct 10, 2019 at 02:55:02PM +0200, Jan Beulich wrote: >> On 10.10.2019 14:13, Roger Pau Monné wrote: >>> On Thu, Oct 10, 2019 at 01:54:06PM +0200, Jan Beulich wrote: On 10.10.2019 13:33, Roger Pau Monne wrote: > When interrupt remapping is enabled as part of enabling x2APIC the Perhaps "unmasked" instead of "the"? > IO-APIC entries also need to be translated to the new format and added > to the interrupt remapping table. > > This prevents IOMMU interrupt remapping faults when booting on > hardware that has unmasked IO-APIC pins. But in the end it only papers over whatever the spurious interrupts result form, doesn't it? Which isn't to say this isn't an improvement. Calling out the ExtInt case here may be worthwhile as well, as would be pointing out that this case still won't work on AMD IOMMUs. >>> >>> But the fix for the ExtINT AMD issue should be done in >>> amd_iommu_ioapic_update_ire then, so that it can properly handle >>> ExtINT delivery mode, not to this part of the code. I will look >>> into it, but I think it's kind of tangential to the issue here. >> >> I'm not talking of you working on fixing this right away. I'm merely >> asking that you mention here (a) the ExtInt special case and (b) >> that this special case will (continue to) not work in the AMD case. > > Please bear with me, I've taken a look at the ExtINT handling for AMD > and I'm not sure I can spot what's missing. Xen seems to handle both > the EIntPass and IV fields of the DTE (see iommu_dte_add_device_entry > and amd_iommu_set_intremap_table), and that should be enough in order > to passthrough such interrupts. EIntPass gets set based on ACPI table information, not what we found in a particular RTE. That's hopefully fine, but you know how reliable firmware is especially in corner cases. > Is there some other handling that I'm missing? (maybe in the handling > of the interrupt itself?) Look at the update_intremap_entry_from_ioapic() -> update_intremap_entry() path: This converts the 3-bit delivery mode field into a 1-bit int_type one (the field is really 3 bits wide, but values 010..111 (binary) are documented as reserved; I can't exclude though that the documentation is wrong here). Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v1] Reset iomem's gfn to LIBXL_INVALID_GFN on reboot
From: Oleksandr Andrushchenko
During domain reboot its configuration is partially reused
to re-create a new domain, but iomem's GFN field for the
iomem is only restored for those memory ranges, which are
configured in form of [IOMEM_START,NUM_PAGES[@GFN], but not for
those in form of [IOMEM_START,NUM_PAGES], e.g. without GFN.
For the latter GFN is reset to 0, but while mapping ranges
to a domain during reboot there is a check that GFN treated
as valid if it is not equal to LIBXL_INVALID_GFN, thus making
Xen to map IOMEM_START to address 0 in the guest's address space.
Workaround it by resseting GFN to LIBXL_INVALID_GFN, so xl
can set proper values for mapping on reboot.
Signed-off-by: Oleksandr Andrushchenko
---
tools/libxl/libxl_domain.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/tools/libxl/libxl_domain.c b/tools/libxl/libxl_domain.c
index 9d0eb5aed1..0ae16a5b12 100644
--- a/tools/libxl/libxl_domain.c
+++ b/tools/libxl/libxl_domain.c
@@ -2120,6 +2120,15 @@ static void retrieve_domain_configuration_end(libxl__egc
*egc,
}
}
+/* reset IOMEM's GFN to initial value */
+{
+int i;
+
+for (i = 0; i < d_config->b_info.num_iomem; i++)
+if (d_config->b_info.iomem[i].gfn == 0)
+d_config->b_info.iomem[i].gfn = LIBXL_INVALID_GFN;
+}
+
/* Devices: disk, nic, vtpm, pcidev etc. */
/* The MERGE macro implements following logic:
--
2.17.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/2] x2APIC: translate IO-APIC entries when enabling the IOMMU
On 10.10.2019 15:12, Roger Pau Monné wrote:
> On Thu, Oct 10, 2019 at 02:55:02PM +0200, Jan Beulich wrote:
>> On 10.10.2019 14:13, Roger Pau Monné wrote:
>>> On Thu, Oct 10, 2019 at 01:54:06PM +0200, Jan Beulich wrote:
On 10.10.2019 13:33, Roger Pau Monne wrote:
> When interrupt remapping is enabled as part of enabling x2APIC the
Perhaps "unmasked" instead of "the"?
> IO-APIC entries also need to be translated to the new format and added
> to the interrupt remapping table.
>
> This prevents IOMMU interrupt remapping faults when booting on
> hardware that has unmasked IO-APIC pins.
But in the end it only papers over whatever the spurious interrupts
result form, doesn't it? Which isn't to say this isn't an
improvement. Calling out the ExtInt case here may be worthwhile as
well, as would be pointing out that this case still won't work on
AMD IOMMUs.
>>>
>>> But the fix for the ExtINT AMD issue should be done in
>>> amd_iommu_ioapic_update_ire then, so that it can properly handle
>>> ExtINT delivery mode, not to this part of the code. I will look
>>> into it, but I think it's kind of tangential to the issue here.
>>
>> I'm not talking of you working on fixing this right away. I'm merely
>> asking that you mention here (a) the ExtInt special case and (b)
>> that this special case will (continue to) not work in the AMD case.
>>
> --- a/xen/arch/x86/apic.c
> +++ b/xen/arch/x86/apic.c
> @@ -515,7 +515,7 @@ static void resume_x2apic(void)
> iommu_enable_x2apic();
> __enable_x2apic();
>
> -restore_IO_APIC_setup(ioapic_entries);
> +restore_IO_APIC_setup(ioapic_entries, true);
> unmask_8259A();
>
> out:
> @@ -961,7 +961,12 @@ void __init x2apic_bsp_setup(void)
> printk("Switched to APIC driver %s\n", genapic.name);
>
> restore_out:
> -restore_IO_APIC_setup(ioapic_entries);
> +/*
> + * NB: do not use raw mode when restoring entries if the iommu has
> been
> + * enabled during the process, because the entries need to be
> translated
> + * and added to the remapping table in that case.
> + */
> +restore_IO_APIC_setup(ioapic_entries, !x2apic_enabled);
How is this different in the resume_x2apic() case? The IOMMU gets
enabled in the course of that as well. I.e. I'd expect you want
to pass "false" there, not "true".
>>>
>>> In the resume_x2apic case interrupt remapping should already be
>>> enabled or not, but that function is not going to enable interrupt
>>> remapping if it wasn't enabled before, hence the IO-APIC entries
>>> should already be using the interrupt remapping table and no
>>> translation is needed.
>>
>> Who / what would have enabled the IOMMU in the resume case?
>
> I don't think the question is who enables interrupt remapping in the
> resume case (which is resume_x2apic when calling iommu_enable_x2apic
> AFAICT), the point here is that on resume the entries in the IO-APIC
> will already match the state of interrupt remapping, so they shouldn't
> be translated. If interrupt remapping was off before suspend it will
> still be off after resume, and there won't be any translation needed.
> The same is true if interrupt remapping is on before suspend.
I disagree: save_IO_APIC_setup() gets called from resume_x2apic(),
not prior to suspend.
Also how would "x2apic_enabled" reflect the transition? It may
have been "true" already before entering the function here.
>>>
>>> If x2apic_enabled == true at the point where restore_IO_APIC_setup
>>> is called interrupt remapping must be enabled, because AFAICT at this
>>> point it's not possible to have x2apic_enabled == true and interrupt
>>> remapping disabled.
>>>
>>> The issue I can see here is what happens if interrupt remapping was
>>> already enabled by the hardware, and entries in the IO-APIC are
>>> already using the remapping table. I would have to look into how to
>>> detect that case properly, but I think the proposed change is an
>>> improvement overall.
>>
>> Firmware handing off with the IOMMU (and hence interrupt remapping)
>> already enabled is a case which - afaict - we can't currently cope
>> with. Firmware handing off in x2APIC enabled state is typically
>> with the IOMMU (and hence interrupt remapping) still disabled. This
>> is not a forbidden mode, it's just that in such a configuration
>> interrupts can't be delivered to certain CPUs.
>>
>> In any event you need to properly distinguish x2APIC enabled state
>> (or the transition thereof) from IOMMU / interrupt remapping
>> enabled state (or the transition thereof). I.e. you want to avoid
>> raw mode restore if interrupt remapping state transitioned from
>> off to on in the process.
>
> Right, and that's why the call to restore_IO_APIC_setup in
> x2apic_bsp_setup uses !x2apic_enabled as it's second parameter. If
> interrupt
Re: [Xen-devel] [PATCH v2 1/2] x2APIC: translate IO-APIC entries when enabling the IOMMU
On Thu, Oct 10, 2019 at 02:55:02PM +0200, Jan Beulich wrote: > On 10.10.2019 14:13, Roger Pau Monné wrote: > > On Thu, Oct 10, 2019 at 01:54:06PM +0200, Jan Beulich wrote: > >> On 10.10.2019 13:33, Roger Pau Monne wrote: > >>> When interrupt remapping is enabled as part of enabling x2APIC the > >> > >> Perhaps "unmasked" instead of "the"? > >> > >>> IO-APIC entries also need to be translated to the new format and added > >>> to the interrupt remapping table. > >>> > >>> This prevents IOMMU interrupt remapping faults when booting on > >>> hardware that has unmasked IO-APIC pins. > >> > >> But in the end it only papers over whatever the spurious interrupts > >> result form, doesn't it? Which isn't to say this isn't an > >> improvement. Calling out the ExtInt case here may be worthwhile as > >> well, as would be pointing out that this case still won't work on > >> AMD IOMMUs. > > > > But the fix for the ExtINT AMD issue should be done in > > amd_iommu_ioapic_update_ire then, so that it can properly handle > > ExtINT delivery mode, not to this part of the code. I will look > > into it, but I think it's kind of tangential to the issue here. > > I'm not talking of you working on fixing this right away. I'm merely > asking that you mention here (a) the ExtInt special case and (b) > that this special case will (continue to) not work in the AMD case. Please bear with me, I've taken a look at the ExtINT handling for AMD and I'm not sure I can spot what's missing. Xen seems to handle both the EIntPass and IV fields of the DTE (see iommu_dte_add_device_entry and amd_iommu_set_intremap_table), and that should be enough in order to passthrough such interrupts. Is there some other handling that I'm missing? (maybe in the handling of the interrupt itself?) Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/2] x2APIC: translate IO-APIC entries when enabling the IOMMU
On Thu, Oct 10, 2019 at 02:55:02PM +0200, Jan Beulich wrote:
> On 10.10.2019 14:13, Roger Pau Monné wrote:
> > On Thu, Oct 10, 2019 at 01:54:06PM +0200, Jan Beulich wrote:
> >> On 10.10.2019 13:33, Roger Pau Monne wrote:
> >>> When interrupt remapping is enabled as part of enabling x2APIC the
> >>
> >> Perhaps "unmasked" instead of "the"?
> >>
> >>> IO-APIC entries also need to be translated to the new format and added
> >>> to the interrupt remapping table.
> >>>
> >>> This prevents IOMMU interrupt remapping faults when booting on
> >>> hardware that has unmasked IO-APIC pins.
> >>
> >> But in the end it only papers over whatever the spurious interrupts
> >> result form, doesn't it? Which isn't to say this isn't an
> >> improvement. Calling out the ExtInt case here may be worthwhile as
> >> well, as would be pointing out that this case still won't work on
> >> AMD IOMMUs.
> >
> > But the fix for the ExtINT AMD issue should be done in
> > amd_iommu_ioapic_update_ire then, so that it can properly handle
> > ExtINT delivery mode, not to this part of the code. I will look
> > into it, but I think it's kind of tangential to the issue here.
>
> I'm not talking of you working on fixing this right away. I'm merely
> asking that you mention here (a) the ExtInt special case and (b)
> that this special case will (continue to) not work in the AMD case.
>
> >>> --- a/xen/arch/x86/apic.c
> >>> +++ b/xen/arch/x86/apic.c
> >>> @@ -515,7 +515,7 @@ static void resume_x2apic(void)
> >>> iommu_enable_x2apic();
> >>> __enable_x2apic();
> >>>
> >>> -restore_IO_APIC_setup(ioapic_entries);
> >>> +restore_IO_APIC_setup(ioapic_entries, true);
> >>> unmask_8259A();
> >>>
> >>> out:
> >>> @@ -961,7 +961,12 @@ void __init x2apic_bsp_setup(void)
> >>> printk("Switched to APIC driver %s\n", genapic.name);
> >>>
> >>> restore_out:
> >>> -restore_IO_APIC_setup(ioapic_entries);
> >>> +/*
> >>> + * NB: do not use raw mode when restoring entries if the iommu has
> >>> been
> >>> + * enabled during the process, because the entries need to be
> >>> translated
> >>> + * and added to the remapping table in that case.
> >>> + */
> >>> +restore_IO_APIC_setup(ioapic_entries, !x2apic_enabled);
> >>
> >> How is this different in the resume_x2apic() case? The IOMMU gets
> >> enabled in the course of that as well. I.e. I'd expect you want
> >> to pass "false" there, not "true".
> >
> > In the resume_x2apic case interrupt remapping should already be
> > enabled or not, but that function is not going to enable interrupt
> > remapping if it wasn't enabled before, hence the IO-APIC entries
> > should already be using the interrupt remapping table and no
> > translation is needed.
>
> Who / what would have enabled the IOMMU in the resume case?
I don't think the question is who enables interrupt remapping in the
resume case (which is resume_x2apic when calling iommu_enable_x2apic
AFAICT), the point here is that on resume the entries in the IO-APIC
will already match the state of interrupt remapping, so they shouldn't
be translated. If interrupt remapping was off before suspend it will
still be off after resume, and there won't be any translation needed.
The same is true if interrupt remapping is on before suspend.
> >> Also how would "x2apic_enabled" reflect the transition? It may
> >> have been "true" already before entering the function here.
> >
> > If x2apic_enabled == true at the point where restore_IO_APIC_setup
> > is called interrupt remapping must be enabled, because AFAICT at this
> > point it's not possible to have x2apic_enabled == true and interrupt
> > remapping disabled.
> >
> > The issue I can see here is what happens if interrupt remapping was
> > already enabled by the hardware, and entries in the IO-APIC are
> > already using the remapping table. I would have to look into how to
> > detect that case properly, but I think the proposed change is an
> > improvement overall.
>
> Firmware handing off with the IOMMU (and hence interrupt remapping)
> already enabled is a case which - afaict - we can't currently cope
> with. Firmware handing off in x2APIC enabled state is typically
> with the IOMMU (and hence interrupt remapping) still disabled. This
> is not a forbidden mode, it's just that in such a configuration
> interrupts can't be delivered to certain CPUs.
>
> In any event you need to properly distinguish x2APIC enabled state
> (or the transition thereof) from IOMMU / interrupt remapping
> enabled state (or the transition thereof). I.e. you want to avoid
> raw mode restore if interrupt remapping state transitioned from
> off to on in the process.
Right, and that's why the call to restore_IO_APIC_setup in
x2apic_bsp_setup uses !x2apic_enabled as it's second parameter. If
interrupt remapping has been enabled by the call to
iommu_enable_x2apic x2apic_enabled must be true, and hence the entries
in the IO-APIC need to be translated to use the interrupt remapping
Re: [Xen-devel] [PATCH v2 1/2] x2APIC: translate IO-APIC entries when enabling the IOMMU
On 10.10.2019 14:13, Roger Pau Monné wrote:
> On Thu, Oct 10, 2019 at 01:54:06PM +0200, Jan Beulich wrote:
>> On 10.10.2019 13:33, Roger Pau Monne wrote:
>>> When interrupt remapping is enabled as part of enabling x2APIC the
>>
>> Perhaps "unmasked" instead of "the"?
>>
>>> IO-APIC entries also need to be translated to the new format and added
>>> to the interrupt remapping table.
>>>
>>> This prevents IOMMU interrupt remapping faults when booting on
>>> hardware that has unmasked IO-APIC pins.
>>
>> But in the end it only papers over whatever the spurious interrupts
>> result form, doesn't it? Which isn't to say this isn't an
>> improvement. Calling out the ExtInt case here may be worthwhile as
>> well, as would be pointing out that this case still won't work on
>> AMD IOMMUs.
>
> But the fix for the ExtINT AMD issue should be done in
> amd_iommu_ioapic_update_ire then, so that it can properly handle
> ExtINT delivery mode, not to this part of the code. I will look
> into it, but I think it's kind of tangential to the issue here.
I'm not talking of you working on fixing this right away. I'm merely
asking that you mention here (a) the ExtInt special case and (b)
that this special case will (continue to) not work in the AMD case.
>>> --- a/xen/arch/x86/apic.c
>>> +++ b/xen/arch/x86/apic.c
>>> @@ -515,7 +515,7 @@ static void resume_x2apic(void)
>>> iommu_enable_x2apic();
>>> __enable_x2apic();
>>>
>>> -restore_IO_APIC_setup(ioapic_entries);
>>> +restore_IO_APIC_setup(ioapic_entries, true);
>>> unmask_8259A();
>>>
>>> out:
>>> @@ -961,7 +961,12 @@ void __init x2apic_bsp_setup(void)
>>> printk("Switched to APIC driver %s\n", genapic.name);
>>>
>>> restore_out:
>>> -restore_IO_APIC_setup(ioapic_entries);
>>> +/*
>>> + * NB: do not use raw mode when restoring entries if the iommu has been
>>> + * enabled during the process, because the entries need to be
>>> translated
>>> + * and added to the remapping table in that case.
>>> + */
>>> +restore_IO_APIC_setup(ioapic_entries, !x2apic_enabled);
>>
>> How is this different in the resume_x2apic() case? The IOMMU gets
>> enabled in the course of that as well. I.e. I'd expect you want
>> to pass "false" there, not "true".
>
> In the resume_x2apic case interrupt remapping should already be
> enabled or not, but that function is not going to enable interrupt
> remapping if it wasn't enabled before, hence the IO-APIC entries
> should already be using the interrupt remapping table and no
> translation is needed.
Who / what would have enabled the IOMMU in the resume case?
>> Also how would "x2apic_enabled" reflect the transition? It may
>> have been "true" already before entering the function here.
>
> If x2apic_enabled == true at the point where restore_IO_APIC_setup
> is called interrupt remapping must be enabled, because AFAICT at this
> point it's not possible to have x2apic_enabled == true and interrupt
> remapping disabled.
>
> The issue I can see here is what happens if interrupt remapping was
> already enabled by the hardware, and entries in the IO-APIC are
> already using the remapping table. I would have to look into how to
> detect that case properly, but I think the proposed change is an
> improvement overall.
Firmware handing off with the IOMMU (and hence interrupt remapping)
already enabled is a case which - afaict - we can't currently cope
with. Firmware handing off in x2APIC enabled state is typically
with the IOMMU (and hence interrupt remapping) still disabled. This
is not a forbidden mode, it's just that in such a configuration
interrupts can't be delivered to certain CPUs.
In any event you need to properly distinguish x2APIC enabled state
(or the transition thereof) from IOMMU / interrupt remapping
enabled state (or the transition thereof). I.e. you want to avoid
raw mode restore if interrupt remapping state transitioned from
off to on in the process.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [RFC] Documentation formats, licenses and file system structure
Hi all, following on from a discussion on IRC and on various other places, I think we need to try and rationalize how we handle documentation. What we have now and what we may get in future * http://xenbits.xen.org/docs/unstable/ (GPL-2) * http://xenbits.xen.org/docs/sphinx-unstable-staging/ (CC-BY-4) * Additional API documentation (with a view to enabling safety) * Any future documentation related to safety (requirements, designs, test cases, tracability) Desired licenses * There is a desire to keep http://xenbits.xen.org/docs/sphinx-unstable-staging/ CC-BY-4 only * There is a desire to publish future documentation related to safety as CC-BY-4 Existing formats and licenses * Hypercall ABI Documentation generated from Xen public headers Format: kerndoc License: typically BSD-3-Clause (documentation is generated from public headers) * docs/designs, docs/features, docs/specs Formats: primarily pandoc, with some files md License: GPL-2 * docs/processs - covers internal processes Formats: txt, with some pandoc License: GPL-2 * docs/figs Formats: misc License: GPL-2 * docs/misc Formats: txt, with some large number of pandoc, some other docs License: GPL-2 * docs/man Formats: pod License: GPL-2 * Sphinx docs: docs, docs/guest-guide, docs/hypervisor-guide Formats: rst License: CC-BY-4 * Wiki: Formats: mediawiki markdown License: CC-BY-SA-3 which has an automatic update to CC-BY-SA-4 (c) of Wiki contributions are kept by the authors This means that the 3 most common file formats in use are * pod * pandoc (with some md) - these are essentially identical * txt for legacy and old stuff * rst License compatibility * GPL-2 and CC-BY-4 are compatible, but mixing means that the complete docset is GPL-2 * GPL-2 and BSD-3-Clause are compatible, but mixing means that the complete docset is GPL-2 * BSD-3-Clause and CC-BY-4 I am not 100% sure, but should not be an issue * CC-BY-SA-4 is only one way compatible with GPLv3 (affecting content on the wiki) The first question is whether we should convert pod to rst * https://metacpan.org/pod/pod2rst provides a conversion tool * man pages can be generated by rst2man Thus, technically this should be easy and should make contributions to docs/man easier If we do this, we should add a CONTRIBUTING file, clarifying the license in this directory There are a set of related questions on what we would eventually merge into the sphinx docset. I believe there is agreement that most of what is in docs today is not really suitable, however there are a few possible exceptions * man pages - with a variety of different contributors from different orgs. Changing license would be hard * API docs generated from PUBLIC headers - changing license would be impossible, but would be BSD-3-Clause * Some wiki content (e.g. https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches and friends) More than 95% of changes were from Citrix staff, so we could convert to CC-BY-4 Most non-Citrix changes are one-line changes and could be covered by fair use * Possibly stuff such as https://xenbits.xen.org/docs/unstable/support-matrix.html (which is currently GPL-2, but we could relicense to say GPL-2 and CC-BY-4 if we had to) The implication is that the sphinx docs would not be fully CC-BY-4, but the bulk of the pages would be * Would we ever include API docs generated from GPLv2 code? E.g. for safety use-cases? @Stefano, @Artem: I guess this one is for you. I suppose if we would have a similar issue for a safety manual I am also assuming we would want to use sphinx docs and rst to generate a future safety manual Other pages in docs that may be useful for the sphinx docs should essentially be re-written, so we would be fine from a licensing perspective. That means that over time, we could get rid of pandoc and text files in docs/misc, docs/designs, docs/features, docs/specs which have not really built a lot of traction. Related to this is the general question, whether we would ever copy code from source to docs and vice versa and to which degree. This is an unknown to me: I think in practice we have not seen much or even any of this in the past. On licensing, we should try and make the docs directory clean, e.g. * We should set the default to CC-BY-4 (e.g. through a contributing file in docs) * And specifically use GPL-2 for directories such as docs/misc, docs/man, ... In any case, this seems all a little bit of a mess at the moment and I think we need to agree on a foundation to get us to a better state. This mail is a start and intends to gather input and will eventually lead to a more concrete proposal. If I have missed anything, feel free to add Best Regards Lars ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.13 v2 2/3] x86/efi: properly handle 0 in pixel reserved bitmask
On 10/10/2019 08:13, Jan Beulich wrote:
> On 09.10.2019 22:40, Igor Druzhinin wrote:
>> --- a/xen/arch/x86/efi/efi-boot.h
>> +++ b/xen/arch/x86/efi/efi-boot.h
>> @@ -528,9 +528,15 @@ static void __init
>> efi_arch_video_init(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop,
>> bpp = set_color(mode_info->PixelInformation.BlueMask, bpp,
>> _console_info.u.vesa_lfb.blue_pos,
>> _console_info.u.vesa_lfb.blue_size);
>> -bpp = set_color(mode_info->PixelInformation.ReservedMask, bpp,
>> -_console_info.u.vesa_lfb.rsvd_pos,
>> -_console_info.u.vesa_lfb.rsvd_size);
>> +if ( !mode_info->PixelInformation.ReservedMask )
>> +{
>> +vga_console_info.u.vesa_lfb.rsvd_pos = 0;
>> +vga_console_info.u.vesa_lfb.rsvd_size = 0;
>> +}
>> +else
>> +bpp = set_color(mode_info->PixelInformation.ReservedMask, bpp,
>> +_console_info.u.vesa_lfb.rsvd_pos,
>> +_console_info.u.vesa_lfb.rsvd_size);
>
> Why not simply
>
> if ( mode_info->PixelInformation.ReservedMask )
> bpp = set_color(mode_info->PixelInformation.ReservedMask, bpp,
> _console_info.u.vesa_lfb.rsvd_pos,
> _console_info.u.vesa_lfb.rsvd_size);
>
> ? There's nothing I can see which might have changed
> vga_console_info.u.vesa_lfb.rsvd_{pos,size} from its zero-initialized
> value. With this adjustment (which could be done while committing) or
> with a reason supplied for the more complex code
> Reviewed-by: Jan Beulich
>
Didn't notice it was actually statically zero-initialized. Perfectly
fine with the suggested change.
Igor
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/2] x2APIC: translate IO-APIC entries when enabling the IOMMU
On Thu, Oct 10, 2019 at 01:54:06PM +0200, Jan Beulich wrote:
> On 10.10.2019 13:33, Roger Pau Monne wrote:
> > When interrupt remapping is enabled as part of enabling x2APIC the
>
> Perhaps "unmasked" instead of "the"?
>
> > IO-APIC entries also need to be translated to the new format and added
> > to the interrupt remapping table.
> >
> > This prevents IOMMU interrupt remapping faults when booting on
> > hardware that has unmasked IO-APIC pins.
>
> But in the end it only papers over whatever the spurious interrupts
> result form, doesn't it? Which isn't to say this isn't an
> improvement. Calling out the ExtInt case here may be worthwhile as
> well, as would be pointing out that this case still won't work on
> AMD IOMMUs.
But the fix for the ExtINT AMD issue should be done in
amd_iommu_ioapic_update_ire then, so that it can properly handle
ExtINT delivery mode, not to this part of the code. I will look
into it, but I think it's kind of tangential to the issue here.
> > --- a/xen/arch/x86/apic.c
> > +++ b/xen/arch/x86/apic.c
> > @@ -515,7 +515,7 @@ static void resume_x2apic(void)
> > iommu_enable_x2apic();
> > __enable_x2apic();
> >
> > -restore_IO_APIC_setup(ioapic_entries);
> > +restore_IO_APIC_setup(ioapic_entries, true);
> > unmask_8259A();
> >
> > out:
> > @@ -961,7 +961,12 @@ void __init x2apic_bsp_setup(void)
> > printk("Switched to APIC driver %s\n", genapic.name);
> >
> > restore_out:
> > -restore_IO_APIC_setup(ioapic_entries);
> > +/*
> > + * NB: do not use raw mode when restoring entries if the iommu has been
> > + * enabled during the process, because the entries need to be
> > translated
> > + * and added to the remapping table in that case.
> > + */
> > +restore_IO_APIC_setup(ioapic_entries, !x2apic_enabled);
>
> How is this different in the resume_x2apic() case? The IOMMU gets
> enabled in the course of that as well. I.e. I'd expect you want
> to pass "false" there, not "true".
In the resume_x2apic case interrupt remapping should already be
enabled or not, but that function is not going to enable interrupt
remapping if it wasn't enabled before, hence the IO-APIC entries
should already be using the interrupt remapping table and no
translation is needed.
> Also how would "x2apic_enabled" reflect the transition? It may
> have been "true" already before entering the function here.
If x2apic_enabled == true at the point where restore_IO_APIC_setup
is called interrupt remapping must be enabled, because AFAICT at this
point it's not possible to have x2apic_enabled == true and interrupt
remapping disabled.
The issue I can see here is what happens if interrupt remapping was
already enabled by the hardware, and entries in the IO-APIC are
already using the remapping table. I would have to look into how to
detect that case properly, but I think the proposed change is an
improvement overall.
Thanks, Roger.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 2/2] iommu: translate IO-APIC pins when enabling interrupt remapping
On 10.10.2019 13:33, Roger Pau Monne wrote: > On Intel hardware there's currently no translation of already enabled > IO-APIC pins when interrupt remapping is enabled on the IOMMU, hence > introduce a logic similar to the one used in x2apic_bsp_setup in order > to save and mask all IO-APIC pins, and then translate and restore them > after interrupt remapping has been enabled. > > With this change the AMD specific logic to deal with enabled pins > (amd_iommu_setup_ioapic_remapping) can be removed, thus unifying the > handling of IO-APIC when enabling interrupt remapping regardless of > the IOMMU vendor. > > Reported-by: Andrew Cooper > Signed-off-by: Roger Pau Monné The actual code change Reviewed-by: Jan Beulich but please mention here as well that the ExtInt case continues to be broken in the AMD case. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/2] x2APIC: translate IO-APIC entries when enabling the IOMMU
On 10.10.2019 13:33, Roger Pau Monne wrote:
> When interrupt remapping is enabled as part of enabling x2APIC the
Perhaps "unmasked" instead of "the"?
> IO-APIC entries also need to be translated to the new format and added
> to the interrupt remapping table.
>
> This prevents IOMMU interrupt remapping faults when booting on
> hardware that has unmasked IO-APIC pins.
But in the end it only papers over whatever the spurious interrupts
result form, doesn't it? Which isn't to say this isn't an
improvement. Calling out the ExtInt case here may be worthwhile as
well, as would be pointing out that this case still won't work on
AMD IOMMUs.
> --- a/xen/arch/x86/apic.c
> +++ b/xen/arch/x86/apic.c
> @@ -515,7 +515,7 @@ static void resume_x2apic(void)
> iommu_enable_x2apic();
> __enable_x2apic();
>
> -restore_IO_APIC_setup(ioapic_entries);
> +restore_IO_APIC_setup(ioapic_entries, true);
> unmask_8259A();
>
> out:
> @@ -961,7 +961,12 @@ void __init x2apic_bsp_setup(void)
> printk("Switched to APIC driver %s\n", genapic.name);
>
> restore_out:
> -restore_IO_APIC_setup(ioapic_entries);
> +/*
> + * NB: do not use raw mode when restoring entries if the iommu has been
> + * enabled during the process, because the entries need to be translated
> + * and added to the remapping table in that case.
> + */
> +restore_IO_APIC_setup(ioapic_entries, !x2apic_enabled);
How is this different in the resume_x2apic() case? The IOMMU gets
enabled in the course of that as well. I.e. I'd expect you want
to pass "false" there, not "true".
Also how would "x2apic_enabled" reflect the transition? It may
have been "true" already before entering the function here.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 1/2] x2APIC: translate IO-APIC entries when enabling the IOMMU
When interrupt remapping is enabled as part of enabling x2APIC the
IO-APIC entries also need to be translated to the new format and added
to the interrupt remapping table.
This prevents IOMMU interrupt remapping faults when booting on
hardware that has unmasked IO-APIC pins.
Reported-by: Andrew Cooper
Signed-off-by: Roger Pau Monné
---
Cc: Juergen Gross
---
Changes since v1:
- Remove the unneeded iommu_enabled local variable.
---
xen/arch/x86/apic.c | 9 +++--
xen/arch/x86/io_apic.c| 5 +++--
xen/include/asm-x86/io_apic.h | 3 ++-
3 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index 6cdb50cf41..8415d9f787 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -515,7 +515,7 @@ static void resume_x2apic(void)
iommu_enable_x2apic();
__enable_x2apic();
-restore_IO_APIC_setup(ioapic_entries);
+restore_IO_APIC_setup(ioapic_entries, true);
unmask_8259A();
out:
@@ -961,7 +961,12 @@ void __init x2apic_bsp_setup(void)
printk("Switched to APIC driver %s\n", genapic.name);
restore_out:
-restore_IO_APIC_setup(ioapic_entries);
+/*
+ * NB: do not use raw mode when restoring entries if the iommu has been
+ * enabled during the process, because the entries need to be translated
+ * and added to the remapping table in that case.
+ */
+restore_IO_APIC_setup(ioapic_entries, !x2apic_enabled);
unmask_8259A();
out:
diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index 5d25862bd8..37eabc16c9 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -379,7 +379,8 @@ void mask_IO_APIC_setup(struct IO_APIC_route_entry
**ioapic_entries)
/*
* Restore IO APIC entries which was saved in ioapic_entries.
*/
-int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries)
+int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries,
+ bool raw)
{
int apic, pin;
@@ -394,7 +395,7 @@ int restore_IO_APIC_setup(struct IO_APIC_route_entry
**ioapic_entries)
return -ENOMEM;
for (pin = 0; pin < nr_ioapic_entries[apic]; pin++)
- ioapic_write_entry(apic, pin, 1, ioapic_entries[apic][pin]);
+ ioapic_write_entry(apic, pin, raw, ioapic_entries[apic][pin]);
}
return 0;
diff --git a/xen/include/asm-x86/io_apic.h b/xen/include/asm-x86/io_apic.h
index 0b041f0565..998905186b 100644
--- a/xen/include/asm-x86/io_apic.h
+++ b/xen/include/asm-x86/io_apic.h
@@ -197,7 +197,8 @@ extern struct IO_APIC_route_entry
**alloc_ioapic_entries(void);
extern void free_ioapic_entries(struct IO_APIC_route_entry **ioapic_entries);
extern int save_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
extern void mask_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
-extern int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
+extern int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries,
+ bool raw);
unsigned highest_gsi(void);
--
2.23.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 2/2] iommu: translate IO-APIC pins when enabling interrupt remapping
On Intel hardware there's currently no translation of already enabled
IO-APIC pins when interrupt remapping is enabled on the IOMMU, hence
introduce a logic similar to the one used in x2apic_bsp_setup in order
to save and mask all IO-APIC pins, and then translate and restore them
after interrupt remapping has been enabled.
With this change the AMD specific logic to deal with enabled pins
(amd_iommu_setup_ioapic_remapping) can be removed, thus unifying the
handling of IO-APIC when enabling interrupt remapping regardless of
the IOMMU vendor.
Reported-by: Andrew Cooper
Signed-off-by: Roger Pau Monné
---
Cc: Juergen Gross
---
xen/drivers/passthrough/amd/iommu_init.c | 11 ++-
xen/drivers/passthrough/amd/iommu_intr.c | 90 +--
xen/drivers/passthrough/x86/iommu.c | 34 ++-
xen/include/asm-x86/hvm/svm/amd-iommu-proto.h | 1 +
4 files changed, 40 insertions(+), 96 deletions(-)
diff --git a/xen/drivers/passthrough/amd/iommu_init.c
b/xen/drivers/passthrough/amd/iommu_init.c
index 6f53c7ec08..3c244619b9 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -19,6 +19,7 @@
#include
#include
+#include
#include
#include
#include
@@ -1435,12 +1436,6 @@ int __init amd_iommu_init(bool xt)
if ( rc )
goto error_out;
-/* initialize io-apic interrupt remapping entries */
-if ( iommu_intremap )
-rc = amd_iommu_setup_ioapic_remapping();
-if ( rc )
-goto error_out;
-
/* Allocate and initialize device table(s). */
pci_init = !xt;
rc = iterate_ivrs_mappings(amd_iommu_setup_device_table);
@@ -1469,6 +1464,10 @@ int __init amd_iommu_init(bool xt)
goto error_out;
}
+if ( iommu_intremap )
+register_keyhandler('V', _iommu_dump_intremap_tables,
+"dump IOMMU intremap tables", 0);
+
return 0;
error_out:
diff --git a/xen/drivers/passthrough/amd/iommu_intr.c
b/xen/drivers/passthrough/amd/iommu_intr.c
index fb71073c84..1eed60f265 100644
--- a/xen/drivers/passthrough/amd/iommu_intr.c
+++ b/xen/drivers/passthrough/amd/iommu_intr.c
@@ -21,7 +21,6 @@
#include
#include
#include
-#include
#include
union irte32 {
@@ -79,8 +78,6 @@ unsigned long *shared_intremap_inuse;
static DEFINE_SPINLOCK(shared_intremap_lock);
unsigned int nr_ioapic_sbdf;
-static void dump_intremap_tables(unsigned char key);
-
#define intremap_page_order(irt) PFN_ORDER(virt_to_page(irt))
unsigned int amd_iommu_intremap_table_order(
@@ -354,91 +351,6 @@ static int update_intremap_entry_from_ioapic(
return 0;
}
-int __init amd_iommu_setup_ioapic_remapping(void)
-{
-struct IO_APIC_route_entry rte;
-unsigned long flags;
-union irte_ptr entry;
-int apic, pin;
-u8 delivery_mode, dest, vector, dest_mode;
-u16 seg, bdf, req_id;
-struct amd_iommu *iommu;
-spinlock_t *lock;
-unsigned int offset;
-
-/* Read ioapic entries and update interrupt remapping table accordingly */
-for ( apic = 0; apic < nr_ioapics; apic++ )
-{
-for ( pin = 0; pin < nr_ioapic_entries[apic]; pin++ )
-{
-unsigned int idx;
-
-rte = __ioapic_read_entry(apic, pin, 1);
-if ( rte.mask == 1 )
-continue;
-
-/* get device id of ioapic devices */
-idx = ioapic_id_to_index(IO_APIC_ID(apic));
-if ( idx == MAX_IO_APICS )
-return -EINVAL;
-
-bdf = ioapic_sbdf[idx].bdf;
-seg = ioapic_sbdf[idx].seg;
-iommu = find_iommu_for_device(seg, bdf);
-if ( !iommu )
-{
-AMD_IOMMU_DEBUG("Fail to find iommu for ioapic "
-"device id = %04x:%04x\n", seg, bdf);
-continue;
-}
-
-req_id = get_intremap_requestor_id(iommu->seg, bdf);
-lock = get_intremap_lock(iommu->seg, req_id);
-
-delivery_mode = rte.delivery_mode;
-vector = rte.vector;
-dest_mode = rte.dest_mode;
-dest = rte.dest.logical.logical_dest;
-
-if ( iommu->ctrl.xt_en )
-{
-/*
- * In x2APIC mode we have no way of discovering the high 24
- * bits of the destination of an already enabled interrupt.
- * We come here earlier than for xAPIC mode, so no interrupts
- * should have been set up before.
- */
-AMD_IOMMU_DEBUG("Unmasked IO-APIC#%u entry %u in x2APIC
mode\n",
-IO_APIC_ID(apic), pin);
-}
-
-spin_lock_irqsave(lock, flags);
-offset = alloc_intremap_entry(iommu, req_id, 1);
-BUG_ON(offset >= INTREMAP_MAX_ENTRIES);
-entry = get_intremap_entry(iommu, req_id, offset);
-update_intremap_entry(iommu, entry, vector,
-
[Xen-devel] [PATCH v2 0/2] iommu: fixes for interrupt remapping enabling
Hello, The following series contain two fixes for issues found when enabling interrupt remapping and the IO-APIC already has unmasked pins. While I'm not aware of any system malfunctioning (apart from reporting IOMMU interrupt remapping faults) I think it would be nice to have those in 4.13. The series can also be found at: git://xenbits.xen.org/people/royger/xen.git iommu_intr_v2 Thanks, Roger. Roger Pau Monne (2): x2APIC: translate IO-APIC entries when enabling the IOMMU iommu: translate IO-APIC pins when enabling interrupt remapping xen/arch/x86/apic.c | 9 +- xen/arch/x86/io_apic.c| 5 +- xen/drivers/passthrough/amd/iommu_init.c | 11 ++- xen/drivers/passthrough/amd/iommu_intr.c | 90 +-- xen/drivers/passthrough/x86/iommu.c | 34 ++- xen/include/asm-x86/hvm/svm/amd-iommu-proto.h | 1 + xen/include/asm-x86/io_apic.h | 3 +- 7 files changed, 52 insertions(+), 101 deletions(-) -- 2.23.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 142542: tolerable all pass - PUSHED
flight 142542 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/142542/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 1b00c16bdfbec98887731a40ea9f377f7dcac405 baseline version: xen 4fc32c22c0588a4972aa9cf82101cc6f7df71016 Last test of basis 142503 2019-10-09 16:01:05 Z0 days Testing same since 142542 2019-10-10 08:04:32 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 4fc32c22c0..1b00c16bdf 1b00c16bdfbec98887731a40ea9f377f7dcac405 -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 2/2] iommu: translate IO-APIC pins when enabling interrupt remapping
On Intel hardware there's currently no translation of already enabled
IO-APIC pins when interrupt remapping is enabled on the IOMMU, hence
introduce a logic similar to the one used in x2apic_bsp_setup in order
to save and mask all IO-APIC pins, and then translate and restore them
after interrupt remapping has been enabled.
With this change the AMD specific logic to deal with enabled pins
(amd_iommu_setup_ioapic_remapping) can be removed, thus unifying the
handling of IO-APIC when enabling interrupt remapping regardless of
the IOMMU vendor.
Reported-by: Andrew Cooper
Signed-off-by: Roger Pau Monné
---
Cc: Juergen Gross
---
xen/drivers/passthrough/amd/iommu_init.c | 11 ++-
xen/drivers/passthrough/amd/iommu_intr.c | 90 +--
xen/drivers/passthrough/x86/iommu.c | 34 ++-
xen/include/asm-x86/hvm/svm/amd-iommu-proto.h | 1 +
4 files changed, 40 insertions(+), 96 deletions(-)
diff --git a/xen/drivers/passthrough/amd/iommu_init.c
b/xen/drivers/passthrough/amd/iommu_init.c
index 6f53c7ec08..3c244619b9 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -19,6 +19,7 @@
#include
#include
+#include
#include
#include
#include
@@ -1435,12 +1436,6 @@ int __init amd_iommu_init(bool xt)
if ( rc )
goto error_out;
-/* initialize io-apic interrupt remapping entries */
-if ( iommu_intremap )
-rc = amd_iommu_setup_ioapic_remapping();
-if ( rc )
-goto error_out;
-
/* Allocate and initialize device table(s). */
pci_init = !xt;
rc = iterate_ivrs_mappings(amd_iommu_setup_device_table);
@@ -1469,6 +1464,10 @@ int __init amd_iommu_init(bool xt)
goto error_out;
}
+if ( iommu_intremap )
+register_keyhandler('V', _iommu_dump_intremap_tables,
+"dump IOMMU intremap tables", 0);
+
return 0;
error_out:
diff --git a/xen/drivers/passthrough/amd/iommu_intr.c
b/xen/drivers/passthrough/amd/iommu_intr.c
index fb71073c84..1eed60f265 100644
--- a/xen/drivers/passthrough/amd/iommu_intr.c
+++ b/xen/drivers/passthrough/amd/iommu_intr.c
@@ -21,7 +21,6 @@
#include
#include
#include
-#include
#include
union irte32 {
@@ -79,8 +78,6 @@ unsigned long *shared_intremap_inuse;
static DEFINE_SPINLOCK(shared_intremap_lock);
unsigned int nr_ioapic_sbdf;
-static void dump_intremap_tables(unsigned char key);
-
#define intremap_page_order(irt) PFN_ORDER(virt_to_page(irt))
unsigned int amd_iommu_intremap_table_order(
@@ -354,91 +351,6 @@ static int update_intremap_entry_from_ioapic(
return 0;
}
-int __init amd_iommu_setup_ioapic_remapping(void)
-{
-struct IO_APIC_route_entry rte;
-unsigned long flags;
-union irte_ptr entry;
-int apic, pin;
-u8 delivery_mode, dest, vector, dest_mode;
-u16 seg, bdf, req_id;
-struct amd_iommu *iommu;
-spinlock_t *lock;
-unsigned int offset;
-
-/* Read ioapic entries and update interrupt remapping table accordingly */
-for ( apic = 0; apic < nr_ioapics; apic++ )
-{
-for ( pin = 0; pin < nr_ioapic_entries[apic]; pin++ )
-{
-unsigned int idx;
-
-rte = __ioapic_read_entry(apic, pin, 1);
-if ( rte.mask == 1 )
-continue;
-
-/* get device id of ioapic devices */
-idx = ioapic_id_to_index(IO_APIC_ID(apic));
-if ( idx == MAX_IO_APICS )
-return -EINVAL;
-
-bdf = ioapic_sbdf[idx].bdf;
-seg = ioapic_sbdf[idx].seg;
-iommu = find_iommu_for_device(seg, bdf);
-if ( !iommu )
-{
-AMD_IOMMU_DEBUG("Fail to find iommu for ioapic "
-"device id = %04x:%04x\n", seg, bdf);
-continue;
-}
-
-req_id = get_intremap_requestor_id(iommu->seg, bdf);
-lock = get_intremap_lock(iommu->seg, req_id);
-
-delivery_mode = rte.delivery_mode;
-vector = rte.vector;
-dest_mode = rte.dest_mode;
-dest = rte.dest.logical.logical_dest;
-
-if ( iommu->ctrl.xt_en )
-{
-/*
- * In x2APIC mode we have no way of discovering the high 24
- * bits of the destination of an already enabled interrupt.
- * We come here earlier than for xAPIC mode, so no interrupts
- * should have been set up before.
- */
-AMD_IOMMU_DEBUG("Unmasked IO-APIC#%u entry %u in x2APIC
mode\n",
-IO_APIC_ID(apic), pin);
-}
-
-spin_lock_irqsave(lock, flags);
-offset = alloc_intremap_entry(iommu, req_id, 1);
-BUG_ON(offset >= INTREMAP_MAX_ENTRIES);
-entry = get_intremap_entry(iommu, req_id, offset);
-update_intremap_entry(iommu, entry, vector,
-
[Xen-devel] [PATCH 1/2] x2APIC: translate IO-APIC entries when enabling the IOMMU
When interrupt remapping is enabled as part of enabling x2APIC the
IO-APIC entries also need to be translated to the new format and added
to the interrupt remapping table.
This prevents IOMMU interrupt remapping faults when booting on
hardware that has unmasked IO-APIC pins.
Reported-by: Andrew Cooper
Signed-off-by: Roger Pau Monné
---
Cc: Juergen Gross
---
xen/arch/x86/apic.c | 12 ++--
xen/arch/x86/io_apic.c| 5 +++--
xen/include/asm-x86/io_apic.h | 3 ++-
3 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index 6cdb50cf41..9810de7473 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -515,7 +515,7 @@ static void resume_x2apic(void)
iommu_enable_x2apic();
__enable_x2apic();
-restore_IO_APIC_setup(ioapic_entries);
+restore_IO_APIC_setup(ioapic_entries, true);
unmask_8259A();
out:
@@ -887,6 +887,7 @@ void __init x2apic_bsp_setup(void)
{
struct IO_APIC_route_entry **ioapic_entries = NULL;
const char *orig_name;
+bool iommu_enabled = true;
if ( !cpu_has_x2apic )
return;
@@ -934,6 +935,7 @@ void __init x2apic_bsp_setup(void)
if ( !x2apic_enabled )
{
printk("Not enabling x2APIC (upon firmware request)\n");
+iommu_enabled = false;
goto restore_out;
}
/* fall through */
@@ -944,6 +946,7 @@ void __init x2apic_bsp_setup(void)
printk(XENLOG_ERR
"Failed to enable Interrupt Remapping: Will not enable
x2APIC.\n");
+iommu_enabled = false;
goto restore_out;
}
@@ -961,7 +964,12 @@ void __init x2apic_bsp_setup(void)
printk("Switched to APIC driver %s\n", genapic.name);
restore_out:
-restore_IO_APIC_setup(ioapic_entries);
+/*
+ * NB: do not use raw mode when restoring entries if the iommu has been
+ * enabled during the process, because the entries need to be translated
+ * and added to the remapping table in that case.
+ */
+restore_IO_APIC_setup(ioapic_entries, !iommu_enabled);
unmask_8259A();
out:
diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index 5d25862bd8..37eabc16c9 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -379,7 +379,8 @@ void mask_IO_APIC_setup(struct IO_APIC_route_entry
**ioapic_entries)
/*
* Restore IO APIC entries which was saved in ioapic_entries.
*/
-int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries)
+int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries,
+ bool raw)
{
int apic, pin;
@@ -394,7 +395,7 @@ int restore_IO_APIC_setup(struct IO_APIC_route_entry
**ioapic_entries)
return -ENOMEM;
for (pin = 0; pin < nr_ioapic_entries[apic]; pin++)
- ioapic_write_entry(apic, pin, 1, ioapic_entries[apic][pin]);
+ ioapic_write_entry(apic, pin, raw, ioapic_entries[apic][pin]);
}
return 0;
diff --git a/xen/include/asm-x86/io_apic.h b/xen/include/asm-x86/io_apic.h
index 0b041f0565..998905186b 100644
--- a/xen/include/asm-x86/io_apic.h
+++ b/xen/include/asm-x86/io_apic.h
@@ -197,7 +197,8 @@ extern struct IO_APIC_route_entry
**alloc_ioapic_entries(void);
extern void free_ioapic_entries(struct IO_APIC_route_entry **ioapic_entries);
extern int save_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
extern void mask_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
-extern int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
+extern int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries,
+ bool raw);
unsigned highest_gsi(void);
--
2.23.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 0/2] iommu: fixes for interrupt remapping enabling
Hello, The following series contain two fixes for issues found when enabling interrupt remapping and the IO-APIC already has unmasked pins. While I'm not aware of any system malfunctioning (apart from reporting IOMMU interrupt remapping faults) I think it would be nice to have those in 4.13. Thanks, Roger. Roger Pau Monne (2): x2APIC: translate IO-APIC entries when enabling the IOMMU iommu: translate IO-APIC pins when enabling interrupt remapping xen/arch/x86/apic.c | 12 ++- xen/arch/x86/io_apic.c| 5 +- xen/drivers/passthrough/amd/iommu_init.c | 11 ++- xen/drivers/passthrough/amd/iommu_intr.c | 90 +-- xen/drivers/passthrough/x86/iommu.c | 34 ++- xen/include/asm-x86/hvm/svm/amd-iommu-proto.h | 1 + xen/include/asm-x86/io_apic.h | 3 +- 7 files changed, 55 insertions(+), 101 deletions(-) -- 2.23.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] pci: clear {host/guest}_maskall field on assign
On Wed, Oct 09, 2019 at 07:13:45PM +0800, Chao Gao wrote:
> On Wed, Oct 09, 2019 at 10:33:21AM +0200, Roger Pau Monne wrote:
> >The current implementation of host_maskall makes it sticky across
> >assign and deassign calls, which means that once a guest forces Xen to
> >set host_maskall the maskall bit is not going to be cleared until a
> >call to PHYSDEVOP_prepare_msix is performed. Such call however
> >shouldn't be part of the normal flow when doing PCI passthrough, and
> >hence the flag needs to be cleared when assigning in order to prevent
> >host_maskall being carried over from previous assignations.
> >
> >Note that the entry maskbit is reset when the msix capability is
> >initialized, and the guest_maskall field is also cleared so that the
> >hardware value matches Xen's internal state (hardware maskall =
> >host_maskall | guest_maskall).
> >
> >Also note that doing the reset of host_maskall there would allow the
> >guest to reset such field by enabling and disabling MSIX, which is not
> >intended.
> >
> >Signed-off-by: Roger Pau Monné
> >---
> >Cc: Chao Gao
> >Cc: "Spassov, Stanislav"
> >Cc: Pasi Kärkkäinen
> >---
> >Chao, Stanislav, can you please check if this patch fixes your
> >issues?
>
> Tested-by: Chao Gao
>
> I got the assertion failure below when starting xencommons with the
> newest staging:
>
> Setting domain 0 name, domid and JSON config...
> xen-init-dom0: _libxl_types.c:2163: libxl_domain_build_info_init_type:
> Assertion `p->type == LIBXL_DOMAIN_TYPE_INVALID' failed.
> /etc/init.d/xencommons: line 54: 2006 Aborted (core dumped)
> ${LIBEXEC_BIN}/xen-init-dom0 ${XEN_DOM0_UUID}
What is your setup like?
Did you perhaps have some stale libraries?
Wei.
>
> It should be irrelated to this patch. So I apply this patch on
> cd93953538aac and it works.
>
> Thanks
> Chao
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 1/3] x86/boot: Introduce the kernel_info
On Wed, Oct 09, 2019 at 05:43:31PM -0700, Randy Dunlap wrote: > Hi, > > Questions and comments below... > Thanks. > > On 10/9/19 3:53 AM, Daniel Kiper wrote: > > > Suggested-by: H. Peter Anvin > > Signed-off-by: Daniel Kiper > > Reviewed-by: Konrad Rzeszutek Wilk > > Reviewed-by: Ross Philipson > > --- > > > --- > > Documentation/x86/boot.rst | 121 > > + > > arch/x86/boot/Makefile | 2 +- > > arch/x86/boot/compressed/Makefile | 4 +- > > arch/x86/boot/compressed/kernel_info.S | 17 + > > arch/x86/boot/header.S | 1 + > > arch/x86/boot/tools/build.c| 5 ++ > > arch/x86/include/uapi/asm/bootparam.h | 1 + > > 7 files changed, 148 insertions(+), 3 deletions(-) > > create mode 100644 arch/x86/boot/compressed/kernel_info.S > > > > diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst > > index 08a2f100c0e6..d5323a39f5e3 100644 > > --- a/Documentation/x86/boot.rst > > +++ b/Documentation/x86/boot.rst > > @@ -68,8 +68,25 @@ Protocol 2.12(Kernel 3.8) Added the xloadflags field > > and extension fields > > Protocol 2.13 (Kernel 3.14) Support 32- and 64-bit flags being set in > > xloadflags to support booting a 64-bit kernel from 32-bit > > EFI > > + > > +Protocol 2.14: BURNT BY INCORRECT COMMIT > > ae7e1238e68f2a472a125673ab506d49158c1889 > > + (x86/boot: Add ACPI RSDP address to setup_header) > > + DO NOT USE!!! ASSUME SAME AS 2.13. > > + > > +Protocol 2.15: (Kernel 5.5) Added the kernel_info. > > = > > > > > > +.. note:: > > + The protocol version number should be changed only if the setup header > > + is changed. There is no need to update the version number if > > boot_params > > + or kernel_info are changed. Additionally, it is recommended to use > > + xloadflags (in this case the protocol version number should not be > > + updated either) or kernel_info to communicate supported Linux kernel > > + features to the boot loader. Due to very limited space available in > > + the original setup header every update to it should be considered > > + with great care. Starting from the protocol 2.15 the primary way to > > + communicate things to the boot loader is the kernel_info. > > + > > > > Memory Layout > > = > > @@ -207,6 +224,7 @@ Offset/Size Proto Name > > Meaning > > 0258/8 2.10+ pref_addressPreferred > > loading address > > 0260/4 2.10+ init_size Linear memory > > required during initialization > > 0264/4 2.11+ handover_offset Offset of > > handover entry point > > +0268/4 2.15+ kernel_info_offset Offset of the > > kernel_info > > ==== > > > > > > .. note:: > > @@ -855,6 +873,109 @@ Offset/size: 0x264/4 > > > >See EFI HANDOVER PROTOCOL below for more details. > > > > + == > > +Field name:kernel_info_offset > > +Type: read > > +Offset/size: 0x268/4 > > +Protocol: 2.15+ > > + == > > + > > + This field is the offset from the beginning of the kernel image to the > > + kernel_info. It is embedded in the Linux image in the uncompressed > ^^ >What does It refer to, please? s/It/The kernel_info structure/ Is it better? > > + protected mode region. > > + > > + > > +The kernel_info > > +=== > > + > > +The relationships between the headers are analogous to the various data > > +sections: > > + > > + setup_header = .data > > + boot_params/setup_data = .bss > > + > > +What is missing from the above list? That's right: > > + > > + kernel_info = .rodata > > + > > +We have been (ab)using .data for things that could go into .rodata or .bss > > for > > +a long time, for lack of alternatives and -- especially early on -- > > inertia. > > +Also, the BIOS stub is responsible for creating boot_params, so it isn't > > +available to a BIOS-based loader (setup_data is, though). > > + > > +setup_header is permanently limited to 144 bytes due to the reach of the > > +2-byte jump field, which doubles as a length field for the structure, > > combined > > +with the size of the "hole" in struct boot_params that a protected-mode > > loader > > +or the BIOS stub has to copy it into. It is currently 119 bytes long, which > > +leaves us with 25 very precious bytes. This isn't something that can be > > fixed > > +without revising the boot protocol entirely, breaking backwards > > compatibility. > > + > > +boot_params proper is limited to 4096 bytes, but can be arbitrarily > > extended > > +by adding
Re: [Xen-devel] [PATCH v4] xen/arm: domain_build: harden make_cpus_node()
Hi Stefano,
On 10/10/19 1:42 AM, Stefano Stabellini wrote:
make_cpus_node() is using a static buffer to generate the FDT node name.
While mpdir_aff is a 64-bit integer, we only ever use the bits [23:0] as
only AFF{0, 1, 2} are supported for now.
To avoid any potential issues in the future, check that mpdir_aff has
only bits [23:0] set.
Take the opportunity to reduce the size of the buffer. Indeed, only 8
characters are needed to print a 32-bit hexadecimal number. So
sizeof("cpu@") + 8 + 1 (for '\0') = 13 characters is sufficient.
Fixes: c81a791d34 (xen/arm: Set 'reg' of cpu node for dom0 to match MPIDR's
affinity)
Signed-off-by: Stefano Stabellini
Reviewed-by: Julien Grall
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [qemu-mainline test] 142502: regressions - FAIL
flight 142502 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/142502/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-i386-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-amd64-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 140282 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-amd64-qemuu-nested-intel 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-amd64-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-amd64-qemuu-nested-amd 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-i386-qemuu-rhel6hvm-intel 10 redhat-install fail REGR. vs. 140282 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 140282 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-i386-qemuu-rhel6hvm-amd 10 redhat-install fail REGR. vs. 140282 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-i386-freebsd10-amd64 11 guest-start fail REGR. vs. 140282 test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 140282 test-amd64-i386-freebsd10-i386 11 guest-startfail REGR. vs. 140282 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-i386-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 140282 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 140282 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 140282 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 140282 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 140282 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14
Re: [Xen-devel] [PATCH] xen/grant-table: remove unnecessary printing
On 10.10.19 10:32, Fuqian Huang wrote: xen_auto_xlat_grant_frames.vaddr is definitely NULL in this case. So the address printing is unnecessary. Signed-off-by: Fuqian Huang Reviewed-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] pci: clear {host/guest}_maskall field on assign
On 09.10.19 11:49, Jan Beulich wrote: On 09.10.2019 10:33, Roger Pau Monne wrote: The current implementation of host_maskall makes it sticky across assign and deassign calls, which means that once a guest forces Xen to set host_maskall the maskall bit is not going to be cleared until a call to PHYSDEVOP_prepare_msix is performed. Such call however shouldn't be part of the normal flow when doing PCI passthrough, and hence the flag needs to be cleared when assigning in order to prevent host_maskall being carried over from previous assignations. Note that the entry maskbit is reset when the msix capability is initialized, and the guest_maskall field is also cleared so that the hardware value matches Xen's internal state (hardware maskall = host_maskall | guest_maskall). Also note that doing the reset of host_maskall there would allow the guest to reset such field by enabling and disabling MSIX, which is not intended. Signed-off-by: Roger Pau Monné Reviewed-by: Jan Beulich I'm also Cc-ing Jürgen for a possible release ack for 4.13, but I'd also like to point out that I'd prefer to wait a little with committing this to get at least one Tested-by. Release-acked-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen/grant-table: remove unnecessary printing
xen_auto_xlat_grant_frames.vaddr is definitely NULL in this case.
So the address printing is unnecessary.
Signed-off-by: Fuqian Huang
---
drivers/xen/grant-table.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 7ea6fb6a2e5d..49b381e104ef 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -1363,8 +1363,7 @@ static int gnttab_setup(void)
if (xen_feature(XENFEAT_auto_translated_physmap) && gnttab_shared.addr
== NULL) {
gnttab_shared.addr = xen_auto_xlat_grant_frames.vaddr;
if (gnttab_shared.addr == NULL) {
- pr_warn("gnttab share frames (addr=0x%08lx) is not
mapped!\n",
- (unsigned
long)xen_auto_xlat_grant_frames.vaddr);
+ pr_warn("gnttab share frames is not mapped!\n");
return -ENOMEM;
}
}
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-next test] 142487: tolerable FAIL
flight 142487 linux-next real [real] http://logs.test-lab.xenproject.org/osstest/logs/142487/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-i386-examine 8 reboot fail like 142431 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail like 142431 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail like 142431 test-amd64-i386-libvirt 7 xen-boot fail like 142431 test-amd64-i386-xl-qemut-win10-i386 7 xen-boot fail like 142431 test-amd64-i386-xl-qemuu-debianhvm-amd64 7 xen-boot fail like 142431 test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-bootfail like 142431 test-amd64-i386-xl-raw7 xen-boot fail like 142431 test-amd64-i386-xl-qemuu-win10-i386 7 xen-boot fail like 142431 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail like 142431 test-amd64-i386-libvirt-xsm 7 xen-boot fail like 142431 test-amd64-i386-xl7 xen-boot fail like 142431 test-amd64-i386-xl-xsm7 xen-boot fail like 142431 test-amd64-i386-xl-shadow 7 xen-boot fail like 142431 test-amd64-i386-freebsd10-i386 7 xen-bootfail like 142431 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail like 142431 test-amd64-i386-qemut-rhel6hvm-amd 7 xen-bootfail like 142431 test-amd64-i386-pair 10 xen-boot/src_hostfail like 142431 test-amd64-i386-pair 11 xen-boot/dst_hostfail like 142431 test-amd64-i386-qemut-rhel6hvm-intel 7 xen-boot fail like 142431 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail like 142431 test-amd64-i386-xl-qemut-ws16-amd64 7 xen-boot fail like 142431 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 7 xen-boot fail like 142431 test-amd64-i386-qemuu-rhel6hvm-intel 7 xen-boot fail like 142431 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail like 142431 test-amd64-i386-xl-qemut-debianhvm-amd64 7 xen-boot fail like 142431 test-amd64-i386-xl-qemuu-ws16-amd64 7 xen-boot fail like 142431 test-amd64-i386-xl-qemut-win7-amd64 7 xen-boot fail like 142431 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 7 xen-boot fail like 142431 test-amd64-i386-xl-pvshim 7 xen-boot fail like 142431 test-amd64-i386-xl-qemuu-win7-amd64 7 xen-boot fail like 142431 test-amd64-i386-freebsd10-amd64 7 xen-boot fail like 142431 test-amd64-i386-xl-qemuu-ovmf-amd64 7 xen-boot fail like 142431 test-arm64-arm64-examine 11 examine-serial/bootloaderfail like 142431 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 142431 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 142431 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 142431 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 142431 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 142431 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 142431 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never
Re: [Xen-devel] [PATCH for-4.13 v2 0/3] EFI GOP fixes
On 09.10.19 22:40, Igor Druzhinin wrote: Igor Druzhinin (3): efi/boot: add missing pointer dereference in set_color x86/efi: properly handle 0 in pixel reserved bitmask efi/boot: make sure graphics mode is set while booting through MB2 xen/arch/x86/efi/efi-boot.h | 12 +--- xen/common/efi/boot.c | 10 +++--- 2 files changed, 16 insertions(+), 6 deletions(-) For the series: Release-acked-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.13 v2 3/3] efi/boot: make sure graphics mode is set while booting through MB2
On 09.10.2019 22:40, Igor Druzhinin wrote: > If a bootloader is using native driver instead of EFI GOP it might > reset graphics mode to be different from what has been originally set > by firmware. While booting through MB2 Xen either need to parse video > setting passed by MB2 and use them instead of what GOP reports or > reset the mode to synchronise it with firmware - prefer the latter. > > Observed while booting Xen using MB2 with EFI GRUB2 compiled with > all possible video drivers where native drivers take priority over firmware. > > Signed-off-by: Igor Druzhinin Reviewed-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.13 v2 2/3] x86/efi: properly handle 0 in pixel reserved bitmask
On 09.10.2019 22:40, Igor Druzhinin wrote:
> --- a/xen/arch/x86/efi/efi-boot.h
> +++ b/xen/arch/x86/efi/efi-boot.h
> @@ -528,9 +528,15 @@ static void __init
> efi_arch_video_init(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop,
> bpp = set_color(mode_info->PixelInformation.BlueMask, bpp,
> _console_info.u.vesa_lfb.blue_pos,
> _console_info.u.vesa_lfb.blue_size);
> -bpp = set_color(mode_info->PixelInformation.ReservedMask, bpp,
> -_console_info.u.vesa_lfb.rsvd_pos,
> -_console_info.u.vesa_lfb.rsvd_size);
> +if ( !mode_info->PixelInformation.ReservedMask )
> +{
> +vga_console_info.u.vesa_lfb.rsvd_pos = 0;
> +vga_console_info.u.vesa_lfb.rsvd_size = 0;
> +}
> +else
> +bpp = set_color(mode_info->PixelInformation.ReservedMask, bpp,
> +_console_info.u.vesa_lfb.rsvd_pos,
> +_console_info.u.vesa_lfb.rsvd_size);
Why not simply
if ( mode_info->PixelInformation.ReservedMask )
bpp = set_color(mode_info->PixelInformation.ReservedMask, bpp,
_console_info.u.vesa_lfb.rsvd_pos,
_console_info.u.vesa_lfb.rsvd_size);
? There's nothing I can see which might have changed
vga_console_info.u.vesa_lfb.rsvd_{pos,size} from its zero-initialized
value. With this adjustment (which could be done while committing) or
with a reason supplied for the more complex code
Reviewed-by: Jan Beulich
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.13 v2 1/3] efi/boot: add missing pointer dereference in set_color
On 09.10.2019 22:40, Igor Druzhinin wrote: > Signed-off-by: Igor Druzhinin Reviewed-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/hvm: Fix the use of "hap=0" following c/s c0902a9a143a
On 09.10.2019 20:56, Paul Durrant wrote: > On Wed, 9 Oct 2019 at 19:21, Andrew Cooper wrote: >> >> c/s c0902a9a143a refactored hvm_enable() a little, but dropped the logic >> which >> cleared hap_supported in the case that the user had asked for it off. >> >> This results in Xen booting up, claiming: >> >> (XEN) HVM: Hardware Assisted Paging (HAP) detected but disabled >> >> but with HAP advertised via sysctl, and XEN_DOMCTL_CDF_hap being accepted in >> domain_create(). >> >> Signed-off-by: Andrew Cooper > > That should have been largely code movement, so I don't know how I > managed to drop that. > > Reviewed-by: Paul Durrant Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4] xen/arm: domain_build: harden make_cpus_node()
On 10.10.19 02:42, Stefano Stabellini wrote:
make_cpus_node() is using a static buffer to generate the FDT node name.
While mpdir_aff is a 64-bit integer, we only ever use the bits [23:0] as
only AFF{0, 1, 2} are supported for now.
To avoid any potential issues in the future, check that mpdir_aff has
only bits [23:0] set.
Take the opportunity to reduce the size of the buffer. Indeed, only 8
characters are needed to print a 32-bit hexadecimal number. So
sizeof("cpu@") + 8 + 1 (for '\0') = 13 characters is sufficient.
Fixes: c81a791d34 (xen/arm: Set 'reg' of cpu node for dom0 to match MPIDR's
affinity)
Signed-off-by: Stefano Stabellini
Release-acked-by: Juergen Gross
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v7 1/3] AMD/IOMMU: allocate one device table per PCI segment
On 10.10.19 07:57, Jan Beulich wrote:
On 04.10.2019 19:28, Andrew Cooper wrote:
On 04/10/2019 14:30, Jan Beulich wrote:
On 04.10.2019 15:18, Andrew Cooper wrote:
On 26/09/2019 15:28, Jan Beulich wrote:
@@ -1068,8 +1067,29 @@ static void * __init allocate_ppr_log(st
IOMMU_PPR_LOG_DEFAULT_ENTRIES, "PPR Log");
}
+/*
+ * Within ivrs_mappings[] we allocate an extra array element to store
+ * - segment number,
+ * - device table.
+ */
+#define IVRS_MAPPINGS_SEG(m) (m)[ivrs_bdf_entries].dte_requestor_id
+#define IVRS_MAPPINGS_DEVTAB(m) (m)[ivrs_bdf_entries].intremap_table
+
+static void __init free_ivrs_mapping(void *ptr)
+{
+const struct ivrs_mappings *ivrs_mappings = ptr;
How absolutely certain are we that ptr will never be NULL?
As certain as we can be by never installing a NULL pointer into the
radix tree, and by observing that neither radix_tree_destroy() nor
radix_tree_node_destroy() would ever call the callback for a NULL
node.
It might be better to rename this to radix_tree_free_ivrs_mappings() to
make it clear who calls it, and also provide a hint as to why the
parameter is void.
I'm not happy to add a radix_tree_ prefix; I'd be fine with adding
e.g. do_ instead, in case this provides enough of a hint for your
taste that this is actually a callback function.
How about a _callback() suffix? I'm looking to make it obvious that you
code shouldn't simply call it directly.
As indicated I've done this.
@@ -1082,13 +1102,15 @@ static int __init amd_iommu_init_one(str
if ( intr && !set_iommu_interrupt_handler(iommu) )
goto error_out;
-/* To make sure that device_table.buffer has been successfully allocated */
-if ( device_table.buffer == NULL )
+/* Make sure that the device table has been successfully allocated. */
+ivrs_mappings = get_ivrs_mappings(iommu->seg);
+if ( !IVRS_MAPPINGS_DEVTAB(ivrs_mappings) )
This is still going to crash with a NULL pointer deference in the case
described by the comment. (Then again, it may not crash, and hit
userspace at the 64M mark.)
You absolutely need to check ivrs_mappings being non NULL before using
IVRS_MAPPINGS_DEVTAB(), or perhaps roll the check into the macro.
I can only repeat what I've said in reply to your respective v6 remark:
We won't come here for an IOMMU which didn't have its ivrs_mappings
successfully allocated.
Right, but to a first approximation, I don't care. I can picture
exactly what Coverity will say about this, in that radix_tree_lookup()
may return NULL, and it is used here unconditionally where in most other
contexts, the pointer gets checked before use.
Just one more word on top of the prior discussion: Would you also
insist on an explicit check here (when ...
You also seem to be mixing up this and the
device table allocation - the comment refers to the latter, while your
NULL deref concern is about the former. (If you go through the code
you'll find that we have numerous other places utilizing the fact that
get_ivrs_mappings() can't fail in cases like the one above.)
The existing code being terrible isn't a reasonable justification for
adding to the mess.
It appears we have:
1x assert not null
14x blind use
3x check
... none exists on basically all similar paths elsewhere) if the
IVRS mappings array hung off of struct amd_iommu as a plain pointer,
rather than being taken from a guaranteed populated (by this point
in time) radix tree slot?
Seeing as we are pushed to the deadline for 4.13, begrudgingly A-by
(preferably with the _callback() suffix), but I'm still not happy with
the overall quality of the code. At least it isn't getting
substantially worse as a consequence here.
Juergen, since I didn't hear back from Andrew, would you be willing
to give a release ack on this series, as at this point I don't see
any good alternative to using the "begrudgingly A-by" give above?
Release-acked-by: Juergen Gross
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
