Re: [Xen-devel] [PATCH v7 1/3] AMD/IOMMU: allocate one device table per PCI segment
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?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-linus test] 142485: regressions - FAIL
flight 142485 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/142485/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-freebsd10-i386 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 133580 test-amd64-i386-examine 8 reboot fail REGR. vs. 133580 test-amd64-i386-qemuu-rhel6hvm-intel 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-libvirt 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-qemut-rhel6hvm-intel 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-freebsd10-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-ovmf-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-debianhvm-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemut-win7-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-qemut-rhel6hvm-amd 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 133580 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 133580 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-libvirt-xsm 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-pvshim 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-xsm7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemut-ws16-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemut-debianhvm-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemut-win10-i386 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-ws16-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-win7-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 133580 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 133580 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-shadow 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-win10-i386 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-xl-pvshim 18 guest-localmigrate/x10 fail REGR. vs. 133580 test-arm64-arm64-libvirt-xsm 16 guest-start/debian.repeat fail REGR. vs. 133580 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 7 xen-boot fail baseline untested test-amd64-i386-xl-qemut-debianhvm-i386-xsm 7 xen-boot fail baseline untested test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 133580 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 133580 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 133580 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 133580 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 133580 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 133580 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-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 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-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 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-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
[Xen-devel] [qemu-mainline bisection] complete test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict
branch xen-unstable xenbranch xen-unstable job test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict testid debian-hvm-install Tree: linux git://xenbits.xen.org/linux-pvops.git Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://git.qemu.org/qemu.git Tree: seabios git://xenbits.xen.org/osstest/seabios.git Tree: xen git://xenbits.xen.org/xen.git *** Found and reproduced problem changeset *** Bug is in tree: qemuu git://git.qemu.org/qemu.git Bug introduced: 6105683da35babad9ae168a72d1e89e63e9d6974 Bug not present: e1b3d47751a420835cb0560fd029c39fea961a79 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/142534/ commit 6105683da35babad9ae168a72d1e89e63e9d6974 Author: Laurent Vivier Date: Fri Sep 6 10:38:12 2019 +0200 ui: add an embedded Barrier client This allows to receive mouse and keyboard events from a Barrier server. This is enabled by adding the following parameter on the command line ... -object input-barrier,id=$id,name=$name ... Where $name is the name declared in the screens section of barrier.conf The barrier server (barriers) must be configured and must run on the local host. For instance: section: screens localhost: ... VM-1: ... end section: links localhost: right = VM-1 VM-1: left = localhost end Then on the QEMU command line: ... -object input-barrier,id=barrie0,name=VM-1 ... When the mouse will move out of the screen of the local host on the right, the mouse and the keyboard will be grabbed and all related events will be send to the guest OS. This is usefull when qemu is configured without emulated graphic card but with a VFIO attached graphic card. More information about Barrier can be found at: https://github.com/debauchee/barrier This avoids to install the Barrier server in the guest OS, for instance when it is not supported or during the installation. Signed-off-by: Laurent Vivier Message-id: 20190906083812.29487-1-laur...@vivier.eu Signed-off-by: Gerd Hoffmann For bisection revision-tuple graph see: http://logs.test-lab.xenproject.org/osstest/results/bisect/qemu-mainline/test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict.debian-hvm-install.html Revision IDs in each graph node refer, respectively, to the Trees above. Running cs-bisection-step --graph-out=/home/logs/results/bisect/qemu-mainline/test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict.debian-hvm-install --summary-out=tmp/142534.bisection-summary --basis-template=140282 --blessings=real,real-bisect qemu-mainline test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict debian-hvm-install Searching for failure / basis pass: 142450 fail [host=huxelrebe1] / 141466 [host=huxelrebe0] 141434 [host=baroque1] 141377 [host=godello0] 141348 [host=elbling1] 141320 [host=italia0] 141285 [host=debina0] 141259 [host=fiano0] 141243 [host=italia1] 141204 [host=pinot1] 141179 [host=albana0] 141087 [host=albana1] 141058 [host=baroque0] 141015 ok. Failure / basis pass flights: 142450 / 141015 (tree with no url: minios) Tree: linux git://xenbits.xen.org/linux-pvops.git Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://git.qemu.org/qemu.git Tree: seabios git://xenbits.xen.org/osstest/seabios.git Tree: xen git://xenbits.xen.org/xen.git Latest 42327896f194f256e5a361e0069985bc8d209b42 c530a75c1e6a472b0eb9558310b518f0dfcd8860 d19040804afb2bdd60f18e8aef7da78028575fe6 d0d8ad39ecb51cd7497cd524484fe09f50876798 14d40ab1d55e54a87350d44769152dd7a59a7b42 43f5df79dad6738d52ea79d072de2b56eb96a91f f93abf0315efef861270c25d83c8047fd6a54ec4 Basis pass 3ffe1e79c174b2093f7ee3df589a7705572c9620 c530a75c1e6a472b0eb9558310b518f0dfcd8860 48d49ea507e571c5ace752077832ab23917ab9cd d0d8ad39ecb51cd7497cd524484fe09f50876798 a8b5ad8e1faef0d1bb3e550530328e8ec76fe87c 43f5df79dad6738d52ea79d072de2b56eb96a91f 6c9639a72f0ca3a9430ef75f375877182281fdef Generating revisions with ./adhoc-revtuple-generator git://xenbits.xen.org/linux-pvops.git#3ffe1e79c174b2093f7ee3df589a7705572c9620-42327896f194f256e5a361e0069985bc8d209b42 git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860 git://xenbits.xen.org/osstest/ovmf.git#48d49ea507e571c5ace752077832ab23917ab9cd-d19040804afb2bdd60f18e8aef7da78028575fe6
Re: [Xen-devel] [PATCH] x86/hvm: Fix the use of "hap=0" following c/s c0902a9a143a
On 09.10.19 20: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 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] [ovmf test] 142495: regressions - FAIL
flight 142495 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/142495/ 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 Z1 days2 attempts Testing same since 142495 2019-10-09 12:29:18 Z0 days1 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
Re: [Xen-devel] [PATCH v3 1/3] x86/boot: Introduce the kernel_info
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? > + 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 setup_data entries. It cannot be used to communicate properties of > +the kernel image, because it is .bss and has no image-provided content. > + > +kernel_info solves this by providing an extensible place for information > about > +the kernel image. It is readonly, because the kernel cannot rely on a > +bootloader copying its contents anywhere, but that
[Xen-devel] [PATCH v4] xen/arm: domain_build: harden make_cpus_node()
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
---
Changes in v4:
- commit message
- in-code comments
Changes in v3:
- make sure only [23:0] bits are used in mpidr_aff
- clarify that we only need 32bit for buf writes
Changes in v2:
- patch added
---
xen/arch/arm/domain_build.c | 19 +--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 921b054520..38adb6e954 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -788,8 +788,8 @@ static int __init make_cpus_node(const struct domain *d,
void *fdt)
unsigned int cpu;
const void *compatible = NULL;
u32 len;
-/* Placeholder for cpu@ + a 32-bit number + \0 */
-char buf[15];
+/* Placeholder for cpu@ + a 32-bit hexadecimal number + \0 */
+char buf[13];
u32 clock_frequency;
bool clock_valid;
uint64_t mpidr_aff;
@@ -847,11 +847,26 @@ static int __init make_cpus_node(const struct domain *d,
void *fdt)
* the MPIDR's affinity bits. We will use AFF0 and AFF1 when
* constructing the reg value of the guest at the moment, for it
* is enough for the current max vcpu number.
+ *
+ * We only deal with AFF{0, 1, 2} stored in bits [23:0] at the
+ * moment.
*/
mpidr_aff = vcpuid_to_vaffinity(cpu);
+if ( (mpidr_aff & ~GENMASK_ULL(23, 0)) != 0 )
+{
+printk(XENLOG_ERR "Unable to handle MPIDR AFFINITY 0x%"PRIx64"\n",
+ mpidr_aff);
+return -EINVAL;
+}
+
dt_dprintk("Create cpu@%"PRIx64" (logical CPUID: %d) node\n",
mpidr_aff, cpu);
+/*
+ * We use PRIx64 because mpidr_aff is a 64bit integer. However,
+ * only bits [23:0] are used, thus, we are sure it will fit in
+ * buf.
+ */
snprintf(buf, sizeof(buf), "cpu@%"PRIx64, mpidr_aff);
res = fdt_begin_node(fdt, buf);
if ( res )
--
2.17.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.13 v3] xen/arm: fix buf size in make_cpus_node
On Wed, 9 Oct 2019, Julien Grall wrote:
> Hi Stefano,
>
> On 09/10/2019 00:12, Stefano Stabellini wrote:
> > The size of buf is calculated wrongly: the number is printed as a
> > hexadecimal number, so we need 8 bytes for 32bit, not 10 bytes.
> >
> > As a result, it should be sizeof("cpu@") + 8 bytes for a 32-bit number +
> > 1 byte for \0. Total = 13.
> >
> > mpidr_aff is 64-bit, however, only bits [0-23] are used. Add a check for
> > that.
>
> I am not entirely happy with the commit message. There are no real issue with
> the current code (the buffer is big enough) as mpdir_aff can only have [23:0]
> set in the current code.
>
> The patch is only hardening the code and that should be reflected in the
> commit message.
>
> So how about:
>
> xen/arm: domain_build: Harden make_cpus_node()
>
> 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 issue in the future, check that mpdir_aff has only bits
> [23:0] set.
>
> At the same time, take the opportunity to reduce the size of the buffer.
> Indeed, only 8 characters is useful to generate an 32-bit hexadecimal number.
> So sizeof("cpu@") + 8 = 13 characters is sufficient here.
Ok, thanks for providing the commit message. I'll use it.
> > 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
> > ---
> > Changes in v3:
> > - make sure only [23:0] bits are used in mpidr_aff
> > - clarify that we only need 32bit for buf writes
> >
> > Changes in v2:
> > - patch added
> > ---
> > xen/arch/arm/domain_build.c | 12 +++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 921b054520..d5ee639548 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -789,7 +789,7 @@ static int __init make_cpus_node(const struct domain *d,
> > void *fdt)
> > const void *compatible = NULL;
> > u32 len;
> > /* Placeholder for cpu@ + a 32-bit number + \0 */
>
> I think you want to update the comment to say "32-bit hexa number".
OK
> > -char buf[15];
> > +char buf[13];
>
> This is a confusing code to read because above you mention this is a 32-bit
> number, but below you are using PRIx64. It takes a bit of time to figure out
> that mpdir_aff will always have bits above 32-bit zeroed.
>
> I would prefer to use a temporary variable for the register, but I would be
> happy to consider a suitable comment in code.
I'll go with the comment
> > u32 clock_frequency;
> > bool clock_valid;
> > uint64_t mpidr_aff;
> > @@ -847,8 +847,18 @@ static int __init make_cpus_node(const struct domain
> > *d, void *fdt)
> >* the MPIDR's affinity bits. We will use AFF0 and AFF1 when
> >* constructing the reg value of the guest at the moment, for it
> >* is enough for the current max vcpu number.
> > + *
> > + * We only deal with AFF{0, 1, 2} stored in bits [23:0] at the
> > + * moment.
> >*/
> > mpidr_aff = vcpuid_to_vaffinity(cpu);
> > +if ( (mpidr_aff & ~GENMASK_ULL(23, 0)) != 0 )
> > +{
> > +printk(XENLOG_ERR "Unable to handle MPIDR AFFINITY
> > 0x%"PRIx64"\n",
> > + mpidr_aff);
> > +return -EINVAL;
> > +}
> > +
> > dt_dprintk("Create cpu@%"PRIx64" (logical CPUID: %d) node\n",
> > mpidr_aff, cpu);
> >
>
> Cheers,
>
> --
> Julien Grall
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it
On Wed, Oct 09, 2019 at 03:31:49PM +0200, Jan Beulich wrote: > On 09.10.2019 14:27, Marek Marczykowski-Górecki wrote: > > But then, Linux won't have EFI system table pointer either, no? I don't > > see Xen passing it over in any way. > > Making the system table pointer available e.g. to kexec userspace > (so it can pass it in whatever suitable way) would be an easy > addition. Use of SetVirtualAddressMap(), otoh, would have been a > hard to undo step if I had made Xen's EFI boot path rely on it. > The kexec situation wrt EFI was very much in flux back then, and > hence I didn't want to take unnecessary risks of future > complications. Any step changing the current state of affairs > wants to provide assurance that it doesn't close roads which we > may need to go at some point. I don't agree with the above being a problem - as we can see, expanding the kexec mechanism to pass EFI system table isn't really necessary to make it usable for Linux doing crash dump (this is the use case of kexec here, right?). But even if it would be, we're talking about some new (possibly Linux-specific) mechanism - in that case, I don't see why it couldn't also pass over memory map for the runtime services (as set via SetVirtualAddressMap()) - similar as Linux->Linux kexec does. On the other hand, lack of SetVirtualAddressMap() do cause real problems now, making Xen unbootable on some machines. Or noticeably limited (with efi=no-rs workaround) - while RS aren't that useful for the crash kernel, they are useful for the main system. Anyway, it's your call, as the maintainer. The patch series I've sent implements the approach by your preference. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? signature.asc Description: PGP signature ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.4 test] 142470: regressions - FAIL
flight 142470 linux-4.4 real [real] http://logs.test-lab.xenproject.org/osstest/logs/142470/ 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 in 142430 REGR. vs. 139698 Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail in 142430 pass in 142470 test-amd64-amd64-xl-qemuu-ovmf-amd64 7 xen-boot fail in 142430 pass in 142470 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 7 xen-boot fail pass in 142430 test-amd64-amd64-xl-pvshim 18 guest-localmigrate/x10 fail pass in 142430 test-armhf-armhf-xl-vhd 10 debian-di-install fail pass in 142430 Tests which did not succeed, but are not blocking: test-armhf-armhf-xl-vhd 12 migrate-support-check fail in 142430 never pass test-armhf-armhf-xl-vhd 13 saverestore-support-check fail in 142430 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-qemuu-nested-amd 17 debian-hvm-install/l1/l2 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-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 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-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-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-qemut-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemuu-win10-i386 10
[Xen-devel] [xen-unstable test] 142462: regressions - FAIL
flight 142462 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/142462/ 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 Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-pvshim 16 guest-localmigrate fail in 142417 pass in 142462 test-amd64-i386-libvirt 7 xen-boot fail pass in 142417 test-amd64-amd64-pair10 xen-boot/src_host fail pass in 142417 test-amd64-amd64-pair11 xen-boot/dst_host fail pass in 142417 test-arm64-arm64-examine 11 examine-serial/bootloader fail pass in 142417 test-arm64-arm64-xl-thunderx 7 xen-boot fail pass in 142417 test-amd64-i386-qemuu-rhel6hvm-intel 12 guest-start/redhat.repeat fail pass in 142417 test-armhf-armhf-xl-rtds 12 guest-startfail pass in 142417 Regressions which are regarded as allowable (not blocking): test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat fail in 142417 REGR. vs. 141822 Tests which did not succeed, but are not blocking: test-amd64-i386-libvirt 13 migrate-support-check fail in 142417 never pass test-arm64-arm64-xl-thunderx 13 migrate-support-check fail in 142417 never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-check fail in 142417 never pass test-armhf-armhf-xl-rtds13 migrate-support-check fail in 142417 never pass test-armhf-armhf-xl-rtds 14 saverestore-support-check fail in 142417 never pass 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-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-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 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-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-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
[Xen-devel] [PATCH for-4.13 v2 0/3] EFI GOP fixes
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(-) -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-4.13 v2 1/3] efi/boot: add missing pointer dereference in set_color
Signed-off-by: Igor Druzhinin --- xen/common/efi/boot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c index 9a89414..6cef429 100644 --- a/xen/common/efi/boot.c +++ b/xen/common/efi/boot.c @@ -1116,7 +1116,7 @@ static int __init __maybe_unused set_color(u32 mask, int bpp, u8 *pos, u8 *sz) return -EINVAL; for ( *pos = 0; !(mask & 1); ++*pos ) mask >>= 1; - for ( *sz = 0; mask & 1; ++sz) + for ( *sz = 0; mask & 1; ++*sz) mask >>= 1; if ( mask ) return -EINVAL; -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-4.13 v2 3/3] efi/boot: make sure graphics mode is set while booting through MB2
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 --- xen/common/efi/boot.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c index 6cef429..6b069c4 100644 --- a/xen/common/efi/boot.c +++ b/xen/common/efi/boot.c @@ -1051,8 +1051,12 @@ static void __init efi_set_gop_mode(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop, UINTN gop EFI_STATUS status; UINTN info_size; -/* Set graphics mode. */ -if ( gop_mode < gop->Mode->MaxMode && gop_mode != gop->Mode->Mode ) +/* + * Set graphics mode to a selected one and reset it if we didn't come + * directly from EFI loader as video settings might have been already modified. + */ +if ( gop_mode < gop->Mode->MaxMode && + (gop_mode != gop->Mode->Mode || !efi_enabled(EFI_LOADER)) ) gop->SetMode(gop, gop_mode); /* Get graphics and frame buffer info. */ -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-4.13 v2 2/3] x86/efi: properly handle 0 in pixel reserved bitmask
In some graphics modes firmware is allowed to return 0 in pixel reserved
bitmask which doesn't go against UEFI Spec 2.8 (12.9 Graphics Output Protocol).
Without this change non-TrueColor modes won't work which will cause
GOP init to fail - observed while trying to boot EFI Xen with Cirrus VGA.
Signed-off-by: Igor Druzhinin
---
xen/arch/x86/efi/efi-boot.h | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index a0737f9..4af6314 100644
--- 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);
if ( bpp > 0 )
break;
/* fall through */
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [libvirt test] 142476: regressions - FAIL
flight 142476 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/142476/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-arm64-arm64-libvirt-qcow2 10 debian-di-install 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-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 412cc0f4038875aa1b0a3c176c309a7e777e9c3f baseline version: libvirt 2346b2f6564ae9f7ba35bc863cb0fab39cadeb12 Last test of basis 142252 2019-10-04 04:23:56 Z5 days Failing since142345 2019-10-06 04:19:12 Z3 days4 attempts Testing same since 142476 2019-10-09 04:18:43 Z0 days1 attempts People who touched revisions under test: Collin Walling Daniel P. Berrangé Daniel Veillard Fabiano Fidêncio 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 found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. (No revision log; it would be 715 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org
[Xen-devel] [PATCH] xen/arm: add warning if memory modules overlap
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));
+#endif
+
for ( i = 0 ; i < cmds->nr_mods; i++ )
printk("CMDLINE[%"PRIpaddr"]:%s %s\n", cmds->cmdline[i].start,
cmds->cmdline[i].dt_name,
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 142503: tolerable all pass - PUSHED
flight 142503 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/142503/ 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 4fc32c22c0588a4972aa9cf82101cc6f7df71016 baseline version: xen 98d1dac88f82c2b79d528faabe5e3fda8133e8bb Last test of basis 142459 2019-10-08 22:01:41 Z0 days Testing same since 142503 2019-10-09 16:01:05 Z0 days1 attempts People who touched revisions under test: Oleksandr Tyshchenko 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 98d1dac88f..4fc32c22c0 4fc32c22c0588a4972aa9cf82101cc6f7df71016 -> smoke ___ 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 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 Reviewed-by: Wei Liu ___ 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 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
> ---
> CC: Jan Beulich
> CC: Wei Liu
> CC: Roger Pau Monné
> CC: Paul Durrant
> CC: Juergen Gross
>
> This is a regression from 4.12, so should be fixed before 4.13 ships.
> ---
> xen/arch/x86/hvm/hvm.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index c22cb39cf3..9acd359c99 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -142,7 +142,7 @@ static struct notifier_block cpu_nfb = {
> .notifier_call = cpu_callback
> };
>
> -static bool __init hap_supported(const struct hvm_function_table *fns)
> +static bool __init hap_supported(struct hvm_function_table *fns)
> {
> if ( !fns->hap_supported )
> {
> @@ -152,6 +152,7 @@ static bool __init hap_supported(const struct
> hvm_function_table *fns)
>
> if ( !opt_hap_enabled )
> {
> +fns->hap_supported = 0;
> printk("HVM: Hardware Assisted Paging (HAP) detected but
> disabled\n");
> return false;
> }
> @@ -175,7 +176,7 @@ static int __init hvm_enable(void)
> hvm_enabled = 1;
>
> printk("HVM: %s enabled\n", fns->name);
> -if ( !hap_supported(fns) )
> +if ( !hap_supported(_funcs) )
> clear_iommu_hap_pt_share();
> else
> {
> --
> 2.11.0
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-linus bisection] complete test-amd64-i386-xl-qemuu-win10-i386
branch xen-unstable xenbranch xen-unstable job test-amd64-i386-xl-qemuu-win10-i386 testid xen-boot Tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: seabios git://xenbits.xen.org/osstest/seabios.git Tree: xen git://xenbits.xen.org/xen.git *** Found and reproduced problem changeset *** Bug is in tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git Bug introduced: eda57a0e42998d1d403187844faa86c9a3ab2fd0 Bug not present: 223cea6a4f0552b86fb25e3b8bbd00469816cd7a Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/142510/ (Revision log too long, omitted.) For bisection revision-tuple graph see: http://logs.test-lab.xenproject.org/osstest/results/bisect/linux-linus/test-amd64-i386-xl-qemuu-win10-i386.xen-boot.html Revision IDs in each graph node refer, respectively, to the Trees above. Running cs-bisection-step --graph-out=/home/logs/results/bisect/linux-linus/test-amd64-i386-xl-qemuu-win10-i386.xen-boot --summary-out=tmp/142510.bisection-summary --basis-template=133580 --blessings=real,real-bisect linux-linus test-amd64-i386-xl-qemuu-win10-i386 xen-boot Searching for failure / basis pass: 142431 fail [host=italia1] / 138849 ok. Failure / basis pass flights: 142431 / 138849 (tree with no url: minios) Tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: seabios git://xenbits.xen.org/osstest/seabios.git Tree: xen git://xenbits.xen.org/xen.git Latest eda57a0e42998d1d403187844faa86c9a3ab2fd0 c530a75c1e6a472b0eb9558310b518f0dfcd8860 d19040804afb2bdd60f18e8aef7da78028575fe6 d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef 43f5df79dad6738d52ea79d072de2b56eb96a91f f93abf0315efef861270c25d83c8047fd6a54ec4 Basis pass 223cea6a4f0552b86fb25e3b8bbd00469816cd7a c530a75c1e6a472b0eb9558310b518f0dfcd8860 d031fc07eb83c9d13bff3ebac25da458d5a47917 d0d8ad39ecb51cd7497cd524484fe09f50876798 9cca02d8ffc23e9688a971d858e4ffdff5389b11 30f1e41f04fb4c715d27f987f003cfc31c9ff4f3 843cec0de800a5f925f8071a7f58f3fb1c6b6eb6 Generating revisions with ./adhoc-revtuple-generator git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git#223cea6a4f0552b86fb25e3b8bbd00469816cd7a-eda57a0e42998d1d403187844faa86c9a3ab2fd0 git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860 git://xenbits.xen.org/osstest/ovmf.git#d031fc07eb83c9d13bff3ebac25da458d5a47917-d19040804afb2bdd60f18e8aef7da78028575fe6 git://xenbits.xen.org/qemu-xen-traditional.\ git#d0d8ad39ecb51cd7497cd524484fe09f50876798-d0d8ad39ecb51cd7497cd524484fe09f50876798 git://xenbits.xen.org/qemu-xen.git#9cca02d8ffc23e9688a971d858e4ffdff5389b11-933ebad2470a169504799a1d95b8e410bd9847ef git://xenbits.xen.org/osstest/seabios.git#30f1e41f04fb4c715d27f987f003cfc31c9ff4f3-43f5df79dad6738d52ea79d072de2b56eb96a91f git://xenbits.xen.org/xen.git#843cec0de800a5f925f8071a7f58f3fb1c6b6eb6-f93abf0315efef861270c25d83c8047fd6a54ec4 adhoc-revtuple-generator: tree discontiguous: linux-2.6 adhoc-revtuple-generator: tree discontiguous: qemu-xen Loaded 3003 nodes in revision graph Searching for test results: 138780 pass irrelevant 138813 pass irrelevant 138849 pass 223cea6a4f0552b86fb25e3b8bbd00469816cd7a c530a75c1e6a472b0eb9558310b518f0dfcd8860 d031fc07eb83c9d13bff3ebac25da458d5a47917 d0d8ad39ecb51cd7497cd524484fe09f50876798 9cca02d8ffc23e9688a971d858e4ffdff5389b11 30f1e41f04fb4c715d27f987f003cfc31c9ff4f3 843cec0de800a5f925f8071a7f58f3fb1c6b6eb6 138878 fail irrelevant 138902 fail irrelevant 138962 fail irrelevant 139003 fail irrelevant 139068 fail irrelevant 139134 fail irrelevant 139237 fail irrelevant 139223 fail irrelevant 139257 fail irrelevant 139324 fail irrelevant 139306 fail irrelevant 139286 fail irrelevant 139338 fail irrelevant 139361 fail irrelevant 139383 fail irrelevant 139408 fail irrelevant 139478 fail irrelevant 139532 fail irrelevant 139584 fail irrelevant 139555 fail irrelevant 139687 fail irrelevant 139616 fail irrelevant 139669 fail irrelevant 139711 fail irrelevant 139761 pass irrelevant 139763 pass irrelevant 139768 pass irrelevant 139748 pass 223cea6a4f0552b86fb25e3b8bbd00469816cd7a c530a75c1e6a472b0eb9558310b518f0dfcd8860 d031fc07eb83c9d13bff3ebac25da458d5a47917 d0d8ad39ecb51cd7497cd524484fe09f50876798 9cca02d8ffc23e9688a971d858e4ffdff5389b11 30f1e41f04fb4c715d27f987f003cfc31c9ff4f3
[Xen-devel] [PATCH] x86/hvm: Fix the use of "hap=0" following c/s c0902a9a143a
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
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
CC: Paul Durrant
CC: Juergen Gross
This is a regression from 4.12, so should be fixed before 4.13 ships.
---
xen/arch/x86/hvm/hvm.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index c22cb39cf3..9acd359c99 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -142,7 +142,7 @@ static struct notifier_block cpu_nfb = {
.notifier_call = cpu_callback
};
-static bool __init hap_supported(const struct hvm_function_table *fns)
+static bool __init hap_supported(struct hvm_function_table *fns)
{
if ( !fns->hap_supported )
{
@@ -152,6 +152,7 @@ static bool __init hap_supported(const struct
hvm_function_table *fns)
if ( !opt_hap_enabled )
{
+fns->hap_supported = 0;
printk("HVM: Hardware Assisted Paging (HAP) detected but disabled\n");
return false;
}
@@ -175,7 +176,7 @@ static int __init hvm_enable(void)
hvm_enabled = 1;
printk("HVM: %s enabled\n", fns->name);
-if ( !hap_supported(fns) )
+if ( !hap_supported(_funcs) )
clear_iommu_hap_pt_share();
else
{
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen/arm: platform: fix Raspberry Pi compatible string
On Wednesday, October 9, 2019 11:30 AM, Julien Grall wrote: >On 04/10/2019 01:47, Stewart Hildebrand wrote: >> Both upstream [1] and downstream [2] Linux kernels use "brcm,bcm2711" >> as the compatible string for Raspberry Pi 4. Add this string to our >> platform compatible list. > >Did the RPI foundation released any kernel with the old binding? Sure, I see the following tags in their linux tree since initial RPi4 support until the binding was updated: raspberrypi-kernel_1.20190709-1 raspberrypi-kernel_1.20190718-1 raspberrypi-kernel_1.20190819-1 raspberrypi-kernel_1.20190925-1 These correspond with their binary releases at [3], except the binary releases also have an earlier 1.20190620+1 tag with RPi4 support. However, even with Xen looking for bcm2838, you wouldn't be able to grab one of those releases and boot without running into other issues. You'd still need a couple of additional patches at [4]. Currently the only way that I'm aware of to successfully boot into dom0 and launch domU is to build the dom0 kernel from source with the extra patches applied found at [4]. >If so, I am ok if we don't support the compatible in Xen (we don't have a >release with it yet!), but it would be worth mentioning in the commit message >that this is going to break for some users (TBD) so they need to upgrade. See below for suggestion. >@Juergen: I would like to consider this patch for Xen 4.13. This is limited to >RPI4 and would avoid us to ship it with a compatible that is going to >disappear. > >> >> The brcm,bcm2838 convention is abandoned. Remove it. >> >> Rename the variables within the file to a rpi4_* prefix since the file >> is meant to cover the Raspberry Pi 4 platform. "If you are using a device tree with the old compatible string brcm,bcm2838, you will need to upgrade your device tree to one that has the new brcm,bcm2711 compatible string." >> >> [1] https://patchwork.kernel.org/patch/11165407/ >> [2] >> https://github.com/raspberrypi/linux/commit/53fdd7b8c8cb9c87190caab4fd459f89e1b4a7f8 >> >> Signed-off-by: Stewart Hildebrand [3] https://github.com/raspberrypi/firmware [4] https://github.com/dornerworks/xen-rpi4-builder/tree/master/patches/linux Stew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [freebsd-master test] 142488: regressions - trouble: blocked/fail
flight 142488 freebsd-master real [real] http://logs.test-lab.xenproject.org/osstest/logs/142488/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-freebsd 7 freebsd-buildfail REGR. vs. 141501 Tests which did not succeed, but are not blocking: build-amd64-xen-freebsd 1 build-check(1) blocked n/a build-amd64-freebsd-again 1 build-check(1) blocked n/a version targeted for testing: freebsd 6bf933c43450e1a221ace3e2eb24d40c3ca0ae47 baseline version: freebsd 14aef6dfca96006e52b8fb920bde7c612ba58b79 Last test of basis 141501 2019-09-20 09:19:51 Z 19 days Failing since141701 2019-09-23 09:19:41 Z 16 days7 attempts Testing same since 142488 2019-10-09 09:19:48 Z0 days1 attempts People who touched revisions under test: 0mp <0...@freebsd.org> alc allanjude andrew asomers avg bapt brooks cem cperciva cy dab daichi dchagin dim dougm emaste erj gallatin gjb glebius gonzo grembo hrs hselasky ian imp jhb jhibbits jilles jkim jtl kaktus kan karels kevans kib lwhsu manu markj mav mckusick mhorne mjg mm mmacy olivier oshogbo Piotr Pietruszewski ray rmacklem royger rrs rstone schweikh sef sjg tijl trasz tsoome tuexen vangyzen vmaffione yuripv jobs: build-amd64-freebsd-againblocked build-amd64-freebsd fail build-amd64-xen-freebsd blocked 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 6423 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] How PV frontend and backend initializes?
On Tue, Oct 08, 2019 at 10:39:11AM +0200, Roger Pau Monné wrote: > On Sat, Oct 05, 2019 at 10:35:11AM +, tosher 1 wrote: > > 3. How xenbus directories are created? What is the hierarchy of the > > directories? > > They are created by the toolstack during domain creation: xl/libxl. > There are documents in the public headers that describe the expected > and optional xenstore nodes for each device, see: > > http://xenbits.xen.org/gitweb/?p=xen.git;a=tree;f=xen/include/public/io The hierarchy of the directories can be found in this other document: https://xenbits.xenproject.org/docs/unstable/misc/xenstore-paths.html -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for Xen 4.13] iommu/arm: Remove arch_iommu_populate_page_table() completely
On 08/10/2019 18:21, Oleksandr wrote: Hi, all Hi, Gentle reminder... Sorry this fell through the cracks. I have now committed 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 v2] xen/arm: platform: fix Raspberry Pi compatible string
Hi Stewart,
Sorry for the delay in answer.
On 04/10/2019 01:47, Stewart Hildebrand wrote:
Both upstream [1] and downstream [2] Linux kernels use "brcm,bcm2711"
as the compatible string for Raspberry Pi 4. Add this string to our
platform compatible list.
Did the RPI foundation released any kernel with the old binding?
If so, I am ok if we don't support the compatible in Xen (we don't have a
release with it yet!), but it would be worth mentioning in the commit message
that this is going to break for some users (TBD) so they need to upgrade.
@Juergen: I would like to consider this patch for Xen 4.13. This is limited to
RPI4 and would avoid us to ship it with a compatible that is going to disappear.
The brcm,bcm2838 convention is abandoned. Remove it.
Rename the variables within the file to a rpi4_* prefix since the file
is meant to cover the Raspberry Pi 4 platform.
[1] https://patchwork.kernel.org/patch/11165407/
[2]
https://github.com/raspberrypi/linux/commit/53fdd7b8c8cb9c87190caab4fd459f89e1b4a7f8
Signed-off-by: Stewart Hildebrand
---
xen/arch/arm/platforms/brcm-raspberry-pi.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/xen/arch/arm/platforms/brcm-raspberry-pi.c
b/xen/arch/arm/platforms/brcm-raspberry-pi.c
index e22d2b3184..b697fa2c6c 100644
--- a/xen/arch/arm/platforms/brcm-raspberry-pi.c
+++ b/xen/arch/arm/platforms/brcm-raspberry-pi.c
@@ -19,13 +19,13 @@
#include
-static const char *const brcm_bcm2838_dt_compat[] __initconst =
+static const char *const rpi4_dt_compat[] __initconst =
{
-"brcm,bcm2838",
+"brcm,bcm2711",
NULL
};
-static const struct dt_device_match brcm_bcm2838_blacklist_dev[] __initconst =
+static const struct dt_device_match rpi4_blacklist_dev[] __initconst =
{
/*
* The aux SPIs share an IRQ and a page with the aux UART.
@@ -40,9 +40,9 @@ static const struct dt_device_match
brcm_bcm2838_blacklist_dev[] __initconst =
{ /* sentinel */ },
};
-PLATFORM_START(brcm_bcm2838, "Raspberry Pi 4")
-.compatible = brcm_bcm2838_dt_compat,
-.blacklist_dev = brcm_bcm2838_blacklist_dev,
+PLATFORM_START(rpi4, "Raspberry Pi 4")
+.compatible = rpi4_dt_compat,
+.blacklist_dev = rpi4_blacklist_dev,
PLATFORM_END
/*
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/3] Free allocated resource on error
Hi Artem,
On 09/10/2019 15:20, Artem Mygaiev wrote:
Also do not set mark device as SMMU protected when there are no more
SMMU resources left
This is a sanity check on the content of the node, not a lack of resource in
this case.
TBH, the current placement is probably not correct. But I am not convinced the
new placement is better.
Xen 4.12 and earlier will ignore any failure and continue, so we cannot use this
device at all. Indeed, Xen will configure the SMMU to deny any transaction. If
you fail to initialize the device, then it will be in an unusable state. In this
case, we don't want a domain (including Dom0) to use it at all. This could be
achieved by calling dt_device_set_protected.
Xen 4.13 will stop booting if any of the SMMU fails to configure (this include
Master Device cannot be assigned). So there are no difference there.
My preference is to cater for all Xen versions so we can consider the patch for
a backport and potentially revert of the Xen 4.13 behavior (i.e. crashing when
one IOMMU is not correctly setup). So we would need to move the call at the
beginning of the function.
Coverity-ID: 1381862
Signed-off-by: Artem Mygaiev
---
xen/drivers/passthrough/arm/smmu.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/xen/drivers/passthrough/arm/smmu.c
b/xen/drivers/passthrough/arm/smmu.c
index f151b9f5b5..cf42335eed 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -804,9 +804,6 @@ static int register_smmu_master(struct arm_smmu_device
*smmu,
master->of_node = masterspec->np;
master->cfg.num_streamids= masterspec->args_count;
- /* Xen: Let Xen know that the device is protected by an SMMU */
- dt_device_set_protected(masterspec->np);
-
for (i = 0; i < master->cfg.num_streamids; ++i) {
u16 streamid = masterspec->args[i];
@@ -815,10 +812,15 @@ static int register_smmu_master(struct arm_smmu_device *smmu,
dev_err(dev,
"stream ID for master device %s greater than maximum
allowed (%d)\n",
masterspec->np->name, smmu->num_mapping_groups);
+ devm_free(master);
return -ERANGE;
}
master->cfg.streamids[i] = streamid;
}
+
+/* Xen: Let Xen know that the device is protected by an SMMU */
+dt_device_set_protected(masterspec->np);
This code is using Linux coding style, so it should be hard tab here.
+
return insert_smmu_master(smmu, master);
}
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] HPET interrupt remapping during boot
On 09.10.2019 15:56, Roger Pau Monné wrote: > On Wed, Oct 09, 2019 at 02:03:12PM +0200, Jan Beulich wrote: >> On 09.10.2019 13:29, Roger Pau Monné wrote: >>> Right, then I guess we either mask all the io-apic pins or we setup >>> proper remapping entries for non-masked pins? (in order to avoid iommu >>> faults) >> >> Making the ExtInt entry is at least worth an experiment, to >> (hopefully) confirm that this would take care of the IOMMU >> fault. But I'm afraid (as per above) it's not an option in >> general. What I could see us doing is mask the entry if all >> legacy IRQs are handled through the IO-APIC. This would take >> care of spurious interrupts, as these are the only ones >> which can make it through when the PIC mask bits are all set. >> However, maybe it is legitimate to mask the ExtInt entry >> when an IOMMU comes into play. > > That was my thinking, ie: make sure every io-apic pin is masked before > enabling iommu interrupt remapping. Nothing useful can happen of > having io-apic pins unmasked, as the remapping is not setup anyway. > If/when those pins get used a proper remapping entry is going to be > setup, and the pin would then be unmasked. Well, this isn't the only option. Another would be to transform all unmasked entries to be translated. >> As to "proper" remapping entries: I'll have to look at the >> spec what they say about this. There's only one IRT index >> that we can put in the RTE, yet this would need to serve all >> 15 IRQs potentially coming through the PIC. Recall that the >> vector gets supplied by the PIC in the ExtInt case, not by >> the IO-APIC RTE. > > You can set the delivery mode of the IRTE to ExtINT, much like how this > is done on the io-apic, and then poke the PIC to figure out which IRQ > triggered? Hmm, yes - it didn't even occur to me that VT-d might allow ExtInt as delivery mode; too much AMD IOMMU work recently, where the only way to deal with ExtInt is a "don't remap" flag in the device table entry. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [qemu-mainline test] 142450: regressions - FAIL
flight 142450 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/142450/ 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-i386-xl-qemuu-debianhvm-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-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 2/3] Remove useless ASSERT condition
Hi Artem, On 09/10/2019 15:20, Artem Mygaiev wrote: cnt is unsigned, so always >=0 Coverity-ID: 1381848 Signed-off-by: Artem Mygaiev Acked-by: Julien Grall --- xen/drivers/char/scif-uart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c index fa0b8274ca..9d3f66b55b 100644 --- a/xen/drivers/char/scif-uart.c +++ b/xen/drivers/char/scif-uart.c @@ -205,7 +205,7 @@ static int scif_uart_tx_ready(struct serial_port *port) /* Check number of data bytes stored in TX FIFO */ cnt = scif_readw(uart, SCIF_SCFDR) >> 8; -ASSERT( cnt >= 0 && cnt <= params->fifo_size ); +ASSERT( cnt <= params->fifo_size ); return (params->fifo_size - cnt); } 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 v3] xen/arm: fix buf size in make_cpus_node
Hi Stefano,
On 09/10/2019 00:12, Stefano Stabellini wrote:
The size of buf is calculated wrongly: the number is printed as a
hexadecimal number, so we need 8 bytes for 32bit, not 10 bytes.
As a result, it should be sizeof("cpu@") + 8 bytes for a 32-bit number +
1 byte for \0. Total = 13.
mpidr_aff is 64-bit, however, only bits [0-23] are used. Add a check for
that.
I am not entirely happy with the commit message. There are no real issue with
the current code (the buffer is big enough) as mpdir_aff can only have [23:0]
set in the current code.
The patch is only hardening the code and that should be reflected in the commit
message.
So how about:
xen/arm: domain_build: Harden make_cpus_node()
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 issue in the future, check that mpdir_aff has only bits
[23:0] set.
At the same time, take the opportunity to reduce the size of the buffer. Indeed,
only 8 characters is useful to generate an 32-bit hexadecimal number. So
sizeof("cpu@") + 8 = 13 characters is sufficient here.
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
---
Changes in v3:
- make sure only [23:0] bits are used in mpidr_aff
- clarify that we only need 32bit for buf writes
Changes in v2:
- patch added
---
xen/arch/arm/domain_build.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 921b054520..d5ee639548 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -789,7 +789,7 @@ static int __init make_cpus_node(const struct domain *d,
void *fdt)
const void *compatible = NULL;
u32 len;
/* Placeholder for cpu@ + a 32-bit number + \0 */
I think you want to update the comment to say "32-bit hexa number".
-char buf[15];
+char buf[13];
This is a confusing code to read because above you mention this is a 32-bit
number, but below you are using PRIx64. It takes a bit of time to figure out
that mpdir_aff will always have bits above 32-bit zeroed.
I would prefer to use a temporary variable for the register, but I would be
happy to consider a suitable comment in code.
u32 clock_frequency;
bool clock_valid;
uint64_t mpidr_aff;
@@ -847,8 +847,18 @@ static int __init make_cpus_node(const struct domain *d,
void *fdt)
* the MPIDR's affinity bits. We will use AFF0 and AFF1 when
* constructing the reg value of the guest at the moment, for it
* is enough for the current max vcpu number.
+ *
+ * We only deal with AFF{0, 1, 2} stored in bits [23:0] at the
+ * moment.
*/
mpidr_aff = vcpuid_to_vaffinity(cpu);
+if ( (mpidr_aff & ~GENMASK_ULL(23, 0)) != 0 )
+{
+printk(XENLOG_ERR "Unable to handle MPIDR AFFINITY 0x%"PRIx64"\n",
+ mpidr_aff);
+return -EINVAL;
+}
+
dt_dprintk("Create cpu@%"PRIx64" (logical CPUID: %d) node\n",
mpidr_aff, cpu);
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] x86/mm: don't needlessly veto migration
On 10/9/19 3:28 PM, Jan Beulich wrote: > On 09.10.2019 16:14, George Dunlap wrote: >> On 10/9/19 11:23 AM, Jürgen Groß wrote: Regardless of the merits of the change Andy wants to see, it's not a one that should be made during a feature freeze. >>> >>> Indeed. So either we take this patch or we have to revert the patch(es) >>> introducing the regression. >> >> Actually, just chatting with Ian -- the worse issue ATM, AFAICT, is that >> the IOMMU is enabled for a guest which has neither asked for PCI >> devices, nor explicitly enabled it; and he's currently working on a fix >> for that. Once that issue is fixed, then osstest should become >> unblocked again. >> >> It is, arguably, not ideal to refuse to migrate a VM with IOMMU enabled >> but no devices attached; but if it only affected guests who had >> specifically requested the IOMMU be enabled, that wouldn't be so >> terrible. (And in fact it has highlighted the other, more important issue.) >> >> In summary, this patch is not strictly needed to get a push to osstest. >> >> That said, the behavior in 4.12 was, as far as I can tell: >> >> 1. If a guest had never had a PCI device assigned, Xen will allow >> logdirty to be enabled. >> >> 2. If a guest has a PCI device assigned, Xen will not allow logdirty to >> be enabled (blocking migration). >> >> 3. If a guest had a PCI device assigned in the past but does not have >> one now, Xen will also not allow logdirty to be enabled (blocking >> migration). > > No - the connection previously was to whether IOMMU page tables > had been set up; these page tables would have been torn down > upon de-assignment of the last device, allowing migration again. > People actually use this behavior afaik, using a bond of a > passed through SR-IOV NIC and netfront provided device. To > migrate the VM, the SR-IOV NIC is taken away without the domain > losing network access, and a new one might then be assigned > again after migration. > > The "IOMMU page tables set up" property was previously identical > to "has devices assigned", i.e. even before we could have used > has_arch_pdevs() instead of is_iommu_enabled(). I didn't realize that. So yes, this patch fixes a regression over and above the toolstack fix Ian is working on. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] x86/mm: don't needlessly veto migration
On 09.10.2019 16:14, George Dunlap wrote: > On 10/9/19 11:23 AM, Jürgen Groß wrote: >>> Regardless of the merits of the change Andy wants to see, it's not a one >>> that should be made during a feature freeze. >> >> Indeed. So either we take this patch or we have to revert the patch(es) >> introducing the regression. > > Actually, just chatting with Ian -- the worse issue ATM, AFAICT, is that > the IOMMU is enabled for a guest which has neither asked for PCI > devices, nor explicitly enabled it; and he's currently working on a fix > for that. Once that issue is fixed, then osstest should become > unblocked again. > > It is, arguably, not ideal to refuse to migrate a VM with IOMMU enabled > but no devices attached; but if it only affected guests who had > specifically requested the IOMMU be enabled, that wouldn't be so > terrible. (And in fact it has highlighted the other, more important issue.) > > In summary, this patch is not strictly needed to get a push to osstest. > > That said, the behavior in 4.12 was, as far as I can tell: > > 1. If a guest had never had a PCI device assigned, Xen will allow > logdirty to be enabled. > > 2. If a guest has a PCI device assigned, Xen will not allow logdirty to > be enabled (blocking migration). > > 3. If a guest had a PCI device assigned in the past but does not have > one now, Xen will also not allow logdirty to be enabled (blocking > migration). No - the connection previously was to whether IOMMU page tables had been set up; these page tables would have been torn down upon de-assignment of the last device, allowing migration again. People actually use this behavior afaik, using a bond of a passed through SR-IOV NIC and netfront provided device. To migrate the VM, the SR-IOV NIC is taken away without the domain losing network access, and a new one might then be assigned again after migration. The "IOMMU page tables set up" property was previously identical to "has devices assigned", i.e. even before we could have used has_arch_pdevs() instead of is_iommu_enabled(). Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 1/3] Consistent use for lock variable
... for both lock and unlock
Coverity-ID: 1381840
Signed-off-by: Artem Mygaiev
---
xen/xsm/flask/avc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/xsm/flask/avc.c b/xen/xsm/flask/avc.c
index 87ea38b7a0..3a9507f62a 100644
--- a/xen/xsm/flask/avc.c
+++ b/xen/xsm/flask/avc.c
@@ -320,7 +320,7 @@ static inline int avc_reclaim_node(void)
head = _cache.slots[hvalue];
lock = _cache.slots_lock[hvalue];
-spin_lock_irqsave(_cache.slots_lock[hvalue], flags);
+spin_lock_irqsave(lock, flags);
rcu_read_lock(_rcu_lock);
hlist_for_each_entry(node, next, head, list)
{
--
2.20.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 3/3] Free allocated resource on error
Also do not set mark device as SMMU protected when there are no more
SMMU resources left
Coverity-ID: 1381862
Signed-off-by: Artem Mygaiev
---
xen/drivers/passthrough/arm/smmu.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/xen/drivers/passthrough/arm/smmu.c
b/xen/drivers/passthrough/arm/smmu.c
index f151b9f5b5..cf42335eed 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -804,9 +804,6 @@ static int register_smmu_master(struct arm_smmu_device
*smmu,
master->of_node = masterspec->np;
master->cfg.num_streamids = masterspec->args_count;
- /* Xen: Let Xen know that the device is protected by an SMMU */
- dt_device_set_protected(masterspec->np);
-
for (i = 0; i < master->cfg.num_streamids; ++i) {
u16 streamid = masterspec->args[i];
@@ -815,10 +812,15 @@ static int register_smmu_master(struct arm_smmu_device
*smmu,
dev_err(dev,
"stream ID for master device %s greater than
maximum allowed (%d)\n",
masterspec->np->name, smmu->num_mapping_groups);
+ devm_free(master);
return -ERANGE;
}
master->cfg.streamids[i] = streamid;
}
+
+/* Xen: Let Xen know that the device is protected by an SMMU */
+dt_device_set_protected(masterspec->np);
+
return insert_smmu_master(smmu, master);
}
--
2.20.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 2/3] Remove useless ASSERT condition
cnt is unsigned, so always >=0 Coverity-ID: 1381848 Signed-off-by: Artem Mygaiev --- xen/drivers/char/scif-uart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c index fa0b8274ca..9d3f66b55b 100644 --- a/xen/drivers/char/scif-uart.c +++ b/xen/drivers/char/scif-uart.c @@ -205,7 +205,7 @@ static int scif_uart_tx_ready(struct serial_port *port) /* Check number of data bytes stored in TX FIFO */ cnt = scif_readw(uart, SCIF_SCFDR) >> 8; -ASSERT( cnt >= 0 && cnt <= params->fifo_size ); +ASSERT( cnt <= params->fifo_size ); return (params->fifo_size - cnt); } -- 2.20.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 0/3] Minor Coverity fixes
From: Artem Mygaiev There is a big amount of insignificant or false positives reported by Coverity, fixing some of them can at least make code more consistent or at least bit more readable. Artem Mygaiev (3): Consistent use for lock variables Remove useless ASSERT condition Free allocated resource on error xen/drivers/char/scif-uart.c | 2 +- xen/drivers/passthrough/arm/smmu.c | 8 +--- xen/xsm/flask/avc.c| 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) -- 2.20.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] x86/mm: don't needlessly veto migration
On 10/9/19 11:23 AM, Jürgen Groß wrote: >> Regardless of the merits of the change Andy wants to see, it's not a one >> that should be made during a feature freeze. > > Indeed. So either we take this patch or we have to revert the patch(es) > introducing the regression. Actually, just chatting with Ian -- the worse issue ATM, AFAICT, is that the IOMMU is enabled for a guest which has neither asked for PCI devices, nor explicitly enabled it; and he's currently working on a fix for that. Once that issue is fixed, then osstest should become unblocked again. It is, arguably, not ideal to refuse to migrate a VM with IOMMU enabled but no devices attached; but if it only affected guests who had specifically requested the IOMMU be enabled, that wouldn't be so terrible. (And in fact it has highlighted the other, more important issue.) In summary, this patch is not strictly needed to get a push to osstest. That said, the behavior in 4.12 was, as far as I can tell: 1. If a guest had never had a PCI device assigned, Xen will allow logdirty to be enabled. 2. If a guest has a PCI device assigned, Xen will not allow logdirty to be enabled (blocking migration). 3. If a guest had a PCI device assigned in the past but does not have one now, Xen will also not allow logdirty to be enabled (blocking migration). At the moment, once Ian fixes the default, the behavior will be: 1 If a guest never had PCI device assigned, and a. the IOMMU was not explicitly enabled, Xen will allow logdirty to be enabled b. the IOMMU was explicitly enabled, xen will not allow logdirty 1a, 2, and 3 are the same, and 1b didn't exist in 4.12, so arguably there's no regression. This patch changes things so that migration will work in the 1b and 3 cases (if I'm reading it right). This is arguably better, but also arguably an unnecessary change post-freeze. And of course there's the option Andy is proposing, of having Xen allow logdirty to be enabled in all cases, and having the toolstack keep track of whether there are devices assigned and refuse migration if so; that's a technical change which should be avoided post-freeze. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] HPET interrupt remapping during boot
On Wed, Oct 09, 2019 at 02:03:12PM +0200, Jan Beulich wrote:
> On 09.10.2019 13:29, Roger Pau Monné wrote:
> > On Wed, Oct 09, 2019 at 12:41:05PM +0200, Jan Beulich wrote:
> >> On 09.10.2019 12:11, Roger Pau Monné wrote:
> >>> And it does print the following when setting up the iommu:
> >>>
> >>> (XEN) ioapic 0 pin 0 not masked
> >>> (XEN) vec=00 delivery=ExINT dest=P status=0 polarity=0 irr=0 trig=E
> >>> mask=0 dest_id:0001
> >>>
> >>> I wonder, shouldn't all pins of all the io-apics be masked at boot?
> >>
> >> I think you might get different answers here depending on whether
> >> you ask firmware or OS people. In fact there are cases where the
> >> IO-APIC needs to be left in this state, I think, but such would
> >> likely need properly reflecting in ACPI tables (albeit I don't
> >> know/recall how this would be done; looking at the code ). This goes back
> >> to times
> >> when IO-APICs were new and OSes would not even know about them,
> >> yet they wouldn't get any interrupts to work if fiddling with
> >> only the PIC (sitting behind IO-APIC pin 0).
> >>
> >> See enable_IO_APIC(), where we actually use this property to
> >> determine the pin behind which the 8259 sits.
> >>
> >> I've seen quite many systems where in the BIOS setup you have an
> >> option to select whether you have an "ACPI OS" (wording of course
> >> varies). I've never checked whether this may e.g. reflect itself
> >> in the handover state of the GSI 0 RTE.
> >>
> >> In your testing patch, could you also log the PIC mask bytes?
> >> There ought to be at least one unmasked; or wait - there actually
> >> is a spurious interrupt there (right before IOMMU initialization):
> >>
> >> (XEN) spurious 8259A interrupt: IRQ7.
> >
> > So I've added a log of the PIC masks just before checking the ioapic
> > masks:
> >
> > (XEN) 8259A-1 mask: fe 8259A-2 mask: ff
> >
> > AFAICT IRQ7 seems to be unmasked? Sorry my knowledge of PICs is quite
> > limited since I've never had to deal with them.
>
> That's IRQ0 then which is unmasked. As said the spurious one
> (IRQ7) can't be masked (at the PIC); only the "normal" IRQ7 can
> be.
>
> > The line I've added is:
> >
> > printk("8259A-1 mask: %x 8259A-2 mask: %x\n", inb(0x21), inb(0xA1));
> >
> > I wonder why does Xen even has any code to deal with the PICs,
> > shouldn't we rely on io-apics only for legacy delivery?
>
> There are (were?) systems where things wouldn't work without.
>
> >> Hence I wonder if there's not possibly a 2nd one once the IOMMU
> >> has been set up.
> >
> > Right, then I guess we either mask all the io-apic pins or we setup
> > proper remapping entries for non-masked pins? (in order to avoid iommu
> > faults)
>
> Making the ExtInt entry is at least worth an experiment, to
> (hopefully) confirm that this would take care of the IOMMU
> fault. But I'm afraid (as per above) it's not an option in
> general. What I could see us doing is mask the entry if all
> legacy IRQs are handled through the IO-APIC. This would take
> care of spurious interrupts, as these are the only ones
> which can make it through when the PIC mask bits are all set.
> However, maybe it is legitimate to mask the ExtInt entry
> when an IOMMU comes into play.
That was my thinking, ie: make sure every io-apic pin is masked before
enabling iommu interrupt remapping. Nothing useful can happen of
having io-apic pins unmasked, as the remapping is not setup anyway.
If/when those pins get used a proper remapping entry is going to be
setup, and the pin would then be unmasked.
> As to "proper" remapping entries: I'll have to look at the
> spec what they say about this. There's only one IRT index
> that we can put in the RTE, yet this would need to serve all
> 15 IRQs potentially coming through the PIC. Recall that the
> vector gets supplied by the PIC in the ExtInt case, not by
> the IO-APIC RTE.
You can set the delivery mode of the IRTE to ExtINT, much like how this
is done on the io-apic, and then poke the PIC to figure out which IRQ
triggered?
Roger.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts
On 09.10.2019 14:52, Roger Pau Monne wrote: > When using posted interrupts and the guest migrates MSI from vCPUs Xen > needs to flush any pending PIRR vectors on the previous vCPU, or else > those vectors could get wrongly injected at a later point when the MSI > fields are already updated. > > Rename sync_pir_to_irr to vlapic_sync_pir_to_irr and export it so it > can be called when updating the posted interrupt descriptor field in > pi_update_irte. While there also remove the unlock_out from > pi_update_irte, it's used in a single goto and removing it makes the > function smaller. > > Note that PIRR is synced to IRR both in pt_irq_destroy_bind and > pt_irq_create_bind when the interrupt delivery data is being updated. > > Also store the vCPU ID in multi-destination mode when using posted > interrupts and the interrupt is bound to a single vCPU in order for > posted interrupts to be used. > > While there guard pi_update_irte with CONFIG_HVM since it's only used > with HVM guests. > > Reported-by: Joe Jin > Signed-off-by: Roger Pau Monné Reviewed-by: Jan Beulich Like for the other patch I'd prefer to wait a little with committing (even if the VT-d ack appeared quickly) until hopefully a Tested-by could be provided. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it
On 09.10.2019 14:27, Marek Marczykowski-Górecki wrote: > On Wed, Oct 09, 2019 at 02:24:31PM +0200, Jan Beulich wrote: >> On 09.10.2019 14:21, Marek Marczykowski-Górecki wrote: >>> On Wed, Oct 09, 2019 at 02:07:05PM +0200, Jan Beulich wrote: On 09.10.2019 13:52, Marek Marczykowski-Górecki wrote: > I'm talking about Xen->Xen transition here. How system table pointer is > passed from old Xen to new Xen instance? And how the new Xen instance > deals with boot services being not available anymore? It doesn't. I should better have said "* -> Xen transitions" in my earlier reply. I simply can't see how this can all work with EFI underneath without some extra conveying of data from the old to the new instance. >>> >>> Does it mean the whole discussion about SetVirtualAddressMap() being >>> incompatible with kexec is moot, because runtime services (including >>> SetVirtualAddressMap()) are not used by Xen after kexec anyway? If I >>> understand correctly, you just said the Xen after kexec don't have >>> runtime services pointer. >> >> The concern is about kexec-ing to Linux (based on what I recall >> from when I wrote this code; as said the situation may have >> changed for modern Linux). > > But then, Linux won't have EFI system table pointer either, no? I don't > see Xen passing it over in any way. Making the system table pointer available e.g. to kexec userspace (so it can pass it in whatever suitable way) would be an easy addition. Use of SetVirtualAddressMap(), otoh, would have been a hard to undo step if I had made Xen's EFI boot path rely on it. The kexec situation wrt EFI was very much in flux back then, and hence I didn't want to take unnecessary risks of future complications. Any step changing the current state of affairs wants to provide assurance that it doesn't close roads which we may need to go at some point. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts
When using posted interrupts and the guest migrates MSI from vCPUs Xen
needs to flush any pending PIRR vectors on the previous vCPU, or else
those vectors could get wrongly injected at a later point when the MSI
fields are already updated.
Rename sync_pir_to_irr to vlapic_sync_pir_to_irr and export it so it
can be called when updating the posted interrupt descriptor field in
pi_update_irte. While there also remove the unlock_out from
pi_update_irte, it's used in a single goto and removing it makes the
function smaller.
Note that PIRR is synced to IRR both in pt_irq_destroy_bind and
pt_irq_create_bind when the interrupt delivery data is being updated.
Also store the vCPU ID in multi-destination mode when using posted
interrupts and the interrupt is bound to a single vCPU in order for
posted interrupts to be used.
While there guard pi_update_irte with CONFIG_HVM since it's only used
with HVM guests.
Reported-by: Joe Jin
Signed-off-by: Roger Pau Monné
---
Cc: Joe Jin
Cc: Juergen Gross
---
I would like to see a bug fix for this issue in 4.13. The fix here only
affects posted interrupts, hence I think the risk of breaking anything
else is low.
---
Changes since v1:
- Store the vcpu id also in multi-dest mode if the interrupt is bound
to a vcpu for posted delivery.
- s/#if/#ifdef/.
---
xen/arch/x86/hvm/vlapic.c | 6 +++---
xen/drivers/passthrough/io.c | 13 ++---
xen/drivers/passthrough/vtd/intremap.c | 15 ---
xen/include/asm-x86/hvm/vlapic.h | 2 ++
xen/include/asm-x86/iommu.h| 2 +-
5 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 9466258d6f..d255ad8db7 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -106,7 +106,7 @@ static void vlapic_clear_irr(int vector, struct vlapic
*vlapic)
vlapic_clear_vector(vector, >regs->data[APIC_IRR]);
}
-static void sync_pir_to_irr(struct vcpu *v)
+void vlapic_sync_pir_to_irr(struct vcpu *v)
{
if ( hvm_funcs.sync_pir_to_irr )
alternative_vcall(hvm_funcs.sync_pir_to_irr, v);
@@ -114,7 +114,7 @@ static void sync_pir_to_irr(struct vcpu *v)
static int vlapic_find_highest_irr(struct vlapic *vlapic)
{
-sync_pir_to_irr(vlapic_vcpu(vlapic));
+vlapic_sync_pir_to_irr(vlapic_vcpu(vlapic));
return vlapic_find_highest_vector(>regs->data[APIC_IRR]);
}
@@ -1493,7 +1493,7 @@ static int lapic_save_regs(struct vcpu *v,
hvm_domain_context_t *h)
if ( !has_vlapic(v->domain) )
return 0;
-sync_pir_to_irr(v);
+vlapic_sync_pir_to_irr(v);
return hvm_save_entry(LAPIC_REGS, v->vcpu_id, h, vcpu_vlapic(v)->regs);
}
diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c
index b292e79382..5bf1877726 100644
--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/io.c
@@ -341,7 +341,7 @@ int pt_irq_create_bind(
{
uint8_t dest, delivery_mode;
bool dest_mode;
-int dest_vcpu_id;
+int dest_vcpu_id, prev_vcpu_id = -1;
const struct vcpu *vcpu;
uint32_t gflags = pt_irq_bind->u.msi.gflags &
~XEN_DOMCTL_VMSI_X86_UNMASKED;
@@ -411,6 +411,7 @@ int pt_irq_create_bind(
pirq_dpci->gmsi.gvec = pt_irq_bind->u.msi.gvec;
pirq_dpci->gmsi.gflags = gflags;
+prev_vcpu_id = pirq_dpci->gmsi.dest_vcpu_id;
}
}
/* Calculate dest_vcpu_id for MSI-type pirq migration. */
@@ -432,7 +433,10 @@ int pt_irq_create_bind(
vcpu = vector_hashing_dest(d, dest, dest_mode,
pirq_dpci->gmsi.gvec);
if ( vcpu )
+{
pirq_dpci->gmsi.posted = true;
+pirq_dpci->gmsi.dest_vcpu_id = vcpu->vcpu_id;
+}
}
if ( vcpu && is_iommu_enabled(d) )
hvm_migrate_pirq(pirq_dpci, vcpu);
@@ -440,7 +444,8 @@ int pt_irq_create_bind(
/* Use interrupt posting if it is supported. */
if ( iommu_intpost )
pi_update_irte(vcpu ? >arch.hvm.vmx.pi_desc : NULL,
- info, pirq_dpci->gmsi.gvec);
+ info, pirq_dpci->gmsi.gvec,
+ prev_vcpu_id >= 0 ? d->vcpu[prev_vcpu_id] : NULL);
if ( pt_irq_bind->u.msi.gflags & XEN_DOMCTL_VMSI_X86_UNMASKED )
{
@@ -729,7 +734,9 @@ int pt_irq_destroy_bind(
what = "bogus";
}
else if ( pirq_dpci && pirq_dpci->gmsi.posted )
-pi_update_irte(NULL, pirq, 0);
+pi_update_irte(NULL, pirq, 0,
+ pirq_dpci->gmsi.dest_vcpu_id >= 0
+ ? d->vcpu[pirq_dpci->gmsi.dest_vcpu_id] : NULL);
if ( pirq_dpci && (pirq_dpci->flags & HVM_IRQ_DPCI_MAPPED) &&
list_empty(_dpci->digl_list) )
diff --git a/xen/drivers/passthrough/vtd/intremap.c
Re: [Xen-devel] [PATCH -tip v4 0/4] x86: kprobes: Prohibit kprobes on Xen/KVM emulate prefixes
On Tue, Sep 17, 2019 at 03:14:03PM +0900, Masami Hiramatsu wrote: > Hi Peter, > > Could you review this version? These look good to me; shall I merge them or what was the plan? ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [ovmf test] 142455: regressions - FAIL
flight 142455 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/142455/ 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 2de1f611be06ded3a59726a4052a9039be7d459b baseline version: ovmf 410c4d00d9f7e369d1ce183e9e8de98cb59c4d20 Last test of basis 142423 2019-10-08 01:39:34 Z1 days Testing same since 142455 2019-10-08 19:21:52 Z0 days1 attempts People who touched revisions under test: 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. commit 2de1f611be06ded3a59726a4052a9039be7d459b Author: Pete Batard Date: Wed Sep 25 23:50:05 2019 +0800 MdeModulePkg/BdsDxe: Also call PlatformBootManagerWaitCallback on 0 The existing loop is set to call PlatformBootManagerWaitCallback every second except the last one. We believe this is a mistake as it prevents the called code from performing timeout expiration tasks such as, for instance, ensuring that the last segment of a progress bar is displayed before continuing (which is a current issue for the RPi3 platform). Signed-off-by: Pete Batard Reviewed-by: Liming Gao Reviewed-by: Ray Ni ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it
On Wed, Oct 09, 2019 at 02:24:31PM +0200, Jan Beulich wrote: > On 09.10.2019 14:21, Marek Marczykowski-Górecki wrote: > > On Wed, Oct 09, 2019 at 02:07:05PM +0200, Jan Beulich wrote: > >> On 09.10.2019 13:52, Marek Marczykowski-Górecki wrote: > >>> I'm talking about Xen->Xen transition here. How system table pointer is > >>> passed from old Xen to new Xen instance? And how the new Xen instance > >>> deals with boot services being not available anymore? > >> > >> It doesn't. I should better have said "* -> Xen transitions" in > >> my earlier reply. I simply can't see how this can all work with > >> EFI underneath without some extra conveying of data from the old > >> to the new instance. > > > > Does it mean the whole discussion about SetVirtualAddressMap() being > > incompatible with kexec is moot, because runtime services (including > > SetVirtualAddressMap()) are not used by Xen after kexec anyway? If I > > understand correctly, you just said the Xen after kexec don't have > > runtime services pointer. > > The concern is about kexec-ing to Linux (based on what I recall > from when I wrote this code; as said the situation may have > changed for modern Linux). But then, Linux won't have EFI system table pointer either, no? I don't see Xen passing it over in any way. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? signature.asc Description: PGP signature ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it
On 09.10.2019 14:21, Marek Marczykowski-Górecki wrote: > On Wed, Oct 09, 2019 at 02:07:05PM +0200, Jan Beulich wrote: >> On 09.10.2019 13:52, Marek Marczykowski-Górecki wrote: >>> I'm talking about Xen->Xen transition here. How system table pointer is >>> passed from old Xen to new Xen instance? And how the new Xen instance >>> deals with boot services being not available anymore? >> >> It doesn't. I should better have said "* -> Xen transitions" in >> my earlier reply. I simply can't see how this can all work with >> EFI underneath without some extra conveying of data from the old >> to the new instance. > > Does it mean the whole discussion about SetVirtualAddressMap() being > incompatible with kexec is moot, because runtime services (including > SetVirtualAddressMap()) are not used by Xen after kexec anyway? If I > understand correctly, you just said the Xen after kexec don't have > runtime services pointer. The concern is about kexec-ing to Linux (based on what I recall from when I wrote this code; as said the situation may have changed for modern Linux). Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it
On Wed, Oct 09, 2019 at 02:07:05PM +0200, Jan Beulich wrote: > On 09.10.2019 13:52, Marek Marczykowski-Górecki wrote: > > On Wed, Oct 09, 2019 at 01:48:38PM +0200, Jan Beulich wrote: > >> On 09.10.2019 13:00, Marek Marczykowski-Górecki wrote: > >>> On Wed, Oct 09, 2019 at 12:50:09PM +0200, Jan Beulich wrote: > On 09.10.2019 12:31, Marek Marczykowski-Górecki wrote: > > BTW How runtime services work after kexec? I don't see EFI handles > > handed over kexec, are they somehow re-discovered? > > What EFI handles are you talking about? For runtime services > what a consumer needs is a table pointer, which is a field > in the system table, which in turn is an argument passed to > the EFI application's entry point. > >>> > >>> Yes, I'm talking about those pointers (system table specifically). > >>> > I didn't think there are > provisions in the spec for either of these pointers being NULL. > >>> > >>> But I don't see kexec using EFI application entry point. Am I missing > >>> something? > >> > >> Can we stop thinking about a Linux -> Xen transition on this > >> thread please? > > > > I'm talking about Xen->Xen transition here. How system table pointer is > > passed from old Xen to new Xen instance? And how the new Xen instance > > deals with boot services being not available anymore? > > It doesn't. I should better have said "* -> Xen transitions" in > my earlier reply. I simply can't see how this can all work with > EFI underneath without some extra conveying of data from the old > to the new instance. Does it mean the whole discussion about SetVirtualAddressMap() being incompatible with kexec is moot, because runtime services (including SetVirtualAddressMap()) are not used by Xen after kexec anyway? If I understand correctly, you just said the Xen after kexec don't have runtime services pointer. If not, can you explain what exactly is the case when second call to SetVirtualAddressMap() could happen (being the reason for #ifdef-ing it out in efi/boot.c)? -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? signature.asc Description: PGP signature ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.9 test] 142443: tolerable FAIL - PUSHED
flight 142443 linux-4.9 real [real] http://logs.test-lab.xenproject.org/osstest/logs/142443/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 142318 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 142318 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 142318 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 142318 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 142318 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-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-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail 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-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 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-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 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-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 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-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail 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-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-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-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-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-amd64-i386-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-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-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass version targeted for testing: linux
Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it
On 09.10.2019 13:52, Marek Marczykowski-Górecki wrote: > On Wed, Oct 09, 2019 at 01:48:38PM +0200, Jan Beulich wrote: >> On 09.10.2019 13:00, Marek Marczykowski-Górecki wrote: >>> On Wed, Oct 09, 2019 at 12:50:09PM +0200, Jan Beulich wrote: On 09.10.2019 12:31, Marek Marczykowski-Górecki wrote: > BTW How runtime services work after kexec? I don't see EFI handles > handed over kexec, are they somehow re-discovered? What EFI handles are you talking about? For runtime services what a consumer needs is a table pointer, which is a field in the system table, which in turn is an argument passed to the EFI application's entry point. >>> >>> Yes, I'm talking about those pointers (system table specifically). >>> I didn't think there are provisions in the spec for either of these pointers being NULL. >>> >>> But I don't see kexec using EFI application entry point. Am I missing >>> something? >> >> Can we stop thinking about a Linux -> Xen transition on this >> thread please? > > I'm talking about Xen->Xen transition here. How system table pointer is > passed from old Xen to new Xen instance? And how the new Xen instance > deals with boot services being not available anymore? It doesn't. I should better have said "* -> Xen transitions" in my earlier reply. I simply can't see how this can all work with EFI underneath without some extra conveying of data from the old to the new instance. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] HPET interrupt remapping during boot
On 09.10.2019 13:29, Roger Pau Monné wrote:
> On Wed, Oct 09, 2019 at 12:41:05PM +0200, Jan Beulich wrote:
>> On 09.10.2019 12:11, Roger Pau Monné wrote:
>>> And it does print the following when setting up the iommu:
>>>
>>> (XEN) ioapic 0 pin 0 not masked
>>> (XEN) vec=00 delivery=ExINT dest=P status=0 polarity=0 irr=0 trig=E mask=0
>>> dest_id:0001
>>>
>>> I wonder, shouldn't all pins of all the io-apics be masked at boot?
>>
>> I think you might get different answers here depending on whether
>> you ask firmware or OS people. In fact there are cases where the
>> IO-APIC needs to be left in this state, I think, but such would
>> likely need properly reflecting in ACPI tables (albeit I don't
>> know/recall how this would be done; looking at the code ). This goes back to
>> times
>> when IO-APICs were new and OSes would not even know about them,
>> yet they wouldn't get any interrupts to work if fiddling with
>> only the PIC (sitting behind IO-APIC pin 0).
>>
>> See enable_IO_APIC(), where we actually use this property to
>> determine the pin behind which the 8259 sits.
>>
>> I've seen quite many systems where in the BIOS setup you have an
>> option to select whether you have an "ACPI OS" (wording of course
>> varies). I've never checked whether this may e.g. reflect itself
>> in the handover state of the GSI 0 RTE.
>>
>> In your testing patch, could you also log the PIC mask bytes?
>> There ought to be at least one unmasked; or wait - there actually
>> is a spurious interrupt there (right before IOMMU initialization):
>>
>> (XEN) spurious 8259A interrupt: IRQ7.
>
> So I've added a log of the PIC masks just before checking the ioapic
> masks:
>
> (XEN) 8259A-1 mask: fe 8259A-2 mask: ff
>
> AFAICT IRQ7 seems to be unmasked? Sorry my knowledge of PICs is quite
> limited since I've never had to deal with them.
That's IRQ0 then which is unmasked. As said the spurious one
(IRQ7) can't be masked (at the PIC); only the "normal" IRQ7 can
be.
> The line I've added is:
>
> printk("8259A-1 mask: %x 8259A-2 mask: %x\n", inb(0x21), inb(0xA1));
>
> I wonder why does Xen even has any code to deal with the PICs,
> shouldn't we rely on io-apics only for legacy delivery?
There are (were?) systems where things wouldn't work without.
>> Hence I wonder if there's not possibly a 2nd one once the IOMMU
>> has been set up.
>
> Right, then I guess we either mask all the io-apic pins or we setup
> proper remapping entries for non-masked pins? (in order to avoid iommu
> faults)
Making the ExtInt entry is at least worth an experiment, to
(hopefully) confirm that this would take care of the IOMMU
fault. But I'm afraid (as per above) it's not an option in
general. What I could see us doing is mask the entry if all
legacy IRQs are handled through the IO-APIC. This would take
care of spurious interrupts, as these are the only ones
which can make it through when the PIC mask bits are all set.
However, maybe it is legitimate to mask the ExtInt entry
when an IOMMU comes into play.
As to "proper" remapping entries: I'll have to look at the
spec what they say about this. There's only one IRT index
that we can put in the RTE, yet this would need to serve all
15 IRQs potentially coming through the PIC. Recall that the
vector gets supplied by the PIC in the ExtInt case, not by
the IO-APIC RTE.
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] xen/xsm: flask: Prevent NULL deference in flask_assign_{, dt}device()
Hi Julien
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
> Fixes: 71e617a6b8 ('use is_iommu_enabled() where appropriate...')
> Signed-off-by: Julien Grall <
> julien.gr...@arm.com
> >
> ---
> xen/xsm/flask/hooks.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> index 3b30827764..cf7f25cda2 100644
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -1296,11 +1296,13 @@ static int flask_assign_device(struct domain
> *d, uint32_t machine_bdf)
> u32 dsid, rsid;
> int rc = -EPERM;
> struct avc_audit_data ad;
> -u32 dperm = flask_iommu_resource_use_perm(d);
> +u32 dperm;
>
> if ( !d )
> return flask_test_assign_device(machine_bdf);
>
> +dperm = flask_iommu_resource_use_perm(d);
> +
> rc = current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__ADD);
> if ( rc )
> return rc;
> @@ -1355,11 +1357,13 @@ static int flask_assign_dtdevice(struct
> domain *d, const char *dtpath)
> u32 dsid, rsid;
> int rc = -EPERM;
> struct avc_audit_data ad;
> -u32 dperm = flask_iommu_resource_use_perm(d);
> +u32 dperm;
>
> if ( !d )
> return flask_test_assign_dtdevice(dtpath);
>
> +dperm = flask_iommu_resource_use_perm(d);
> +
> rc = current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__ADD);
> if ( rc )
> return rc;
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it
On Wed, Oct 09, 2019 at 01:48:38PM +0200, Jan Beulich wrote: > On 09.10.2019 13:00, Marek Marczykowski-Górecki wrote: > > On Wed, Oct 09, 2019 at 12:50:09PM +0200, Jan Beulich wrote: > >> On 09.10.2019 12:31, Marek Marczykowski-Górecki wrote: > >>> BTW How runtime services work after kexec? I don't see EFI handles > >>> handed over kexec, are they somehow re-discovered? > >> > >> What EFI handles are you talking about? For runtime services > >> what a consumer needs is a table pointer, which is a field > >> in the system table, which in turn is an argument passed to > >> the EFI application's entry point. > > > > Yes, I'm talking about those pointers (system table specifically). > > > >> I didn't think there are > >> provisions in the spec for either of these pointers being NULL. > > > > But I don't see kexec using EFI application entry point. Am I missing > > something? > > Can we stop thinking about a Linux -> Xen transition on this > thread please? I'm talking about Xen->Xen transition here. How system table pointer is passed from old Xen to new Xen instance? And how the new Xen instance deals with boot services being not available anymore? -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? signature.asc Description: PGP signature ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it
On 09.10.2019 13:00, Marek Marczykowski-Górecki wrote: > On Wed, Oct 09, 2019 at 12:50:09PM +0200, Jan Beulich wrote: >> On 09.10.2019 12:31, Marek Marczykowski-Górecki wrote: >>> BTW How runtime services work after kexec? I don't see EFI handles >>> handed over kexec, are they somehow re-discovered? >> >> What EFI handles are you talking about? For runtime services >> what a consumer needs is a table pointer, which is a field >> in the system table, which in turn is an argument passed to >> the EFI application's entry point. > > Yes, I'm talking about those pointers (system table specifically). > >> I didn't think there are >> provisions in the spec for either of these pointers being NULL. > > But I don't see kexec using EFI application entry point. Am I missing > something? Can we stop thinking about a Linux -> Xen transition on this thread please? It only serves to confuse matters imo. Kexec _is_ different, and if anyone wants to get that transition working, then they'll have to properly investigate what it takes. Here we're concerned only about not breaking kexec-ing _from_ Xen. Jan ___ 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/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
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] HPET interrupt remapping during boot
On Wed, Oct 09, 2019 at 12:41:05PM +0200, Jan Beulich wrote:
> On 09.10.2019 12:11, Roger Pau Monné wrote:
> > And it does print the following when setting up the iommu:
> >
> > (XEN) ioapic 0 pin 0 not masked
> > (XEN) vec=00 delivery=ExINT dest=P status=0 polarity=0 irr=0 trig=E mask=0
> > dest_id:0001
> >
> > I wonder, shouldn't all pins of all the io-apics be masked at boot?
>
> I think you might get different answers here depending on whether
> you ask firmware or OS people. In fact there are cases where the
> IO-APIC needs to be left in this state, I think, but such would
> likely need properly reflecting in ACPI tables (albeit I don't
> know/recall how this would be done; looking at the code ). This goes back to
> times
> when IO-APICs were new and OSes would not even know about them,
> yet they wouldn't get any interrupts to work if fiddling with
> only the PIC (sitting behind IO-APIC pin 0).
>
> See enable_IO_APIC(), where we actually use this property to
> determine the pin behind which the 8259 sits.
>
> I've seen quite many systems where in the BIOS setup you have an
> option to select whether you have an "ACPI OS" (wording of course
> varies). I've never checked whether this may e.g. reflect itself
> in the handover state of the GSI 0 RTE.
>
> In your testing patch, could you also log the PIC mask bytes?
> There ought to be at least one unmasked; or wait - there actually
> is a spurious interrupt there (right before IOMMU initialization):
>
> (XEN) spurious 8259A interrupt: IRQ7.
So I've added a log of the PIC masks just before checking the ioapic
masks:
(XEN) 8259A-1 mask: fe 8259A-2 mask: ff
AFAICT IRQ7 seems to be unmasked? Sorry my knowledge of PICs is quite
limited since I've never had to deal with them.
The line I've added is:
printk("8259A-1 mask: %x 8259A-2 mask: %x\n", inb(0x21), inb(0xA1));
I wonder why does Xen even has any code to deal with the PICs,
shouldn't we rely on io-apics only for legacy delivery?
> Hence I wonder if there's not possibly a 2nd one once the IOMMU
> has been set up.
Right, then I guess we either mask all the io-apic pins or we setup
proper remapping entries for non-masked pins? (in order to avoid iommu
faults)
Thanks, Roger.
___
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 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}
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] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it
On Wed, Oct 09, 2019 at 12:50:09PM +0200, Jan Beulich wrote: > On 09.10.2019 12:31, Marek Marczykowski-Górecki wrote: > > On Wed, Oct 09, 2019 at 10:56:56AM +0200, Jan Beulich wrote: > >> On 08.10.2019 18:29, Marek Marczykowski-Górecki wrote: > >>> On Tue, Oct 08, 2019 at 04:19:13PM +0200, Jan Beulich wrote: > On 08.10.2019 15:52, Marek Marczykowski-Górecki wrote: > > In Linux case, it looks like it passes around the EFI memory map using > > some Linux-specific mechanism, but I don't find it particularly > > appealing option. > >> > >> ... that this would require Xen following a Linux protocol. > >> This is nothing that can work building on EFI interfaces alone. > > > > Actually, there is something that could be used: presence of boot > > services. If the call to SetVirtualAddressMap() is bound to initial > > presence of boot services, then it surely won't happen after kexec, as > > boot services are not available anymore. In fact the patch I've sent > > does exactly that - call SetVirtualAddressMap() directly after > > ExitBootServices(), but I've realized this property only now. In this > > case, maybe kconfig option is not needed anymore? > > I'm unaware of a property telling an EFI application whether > boot services are available. By the definition I know they're > available up and until ExitBootServices() gets called. Regardless of how it is done, calling SetVirtualAddressMap() directly after ExitBootServices() should be fine. If for some reason Xen would try to call ExitBootServices() when boot services are already gone, it is a bug elsewhere (and probably will crash Xen before it event gets to SetVirtualAddressMap() call). I'm simply relying on this property, regardless of how it is technically done. > > BTW How runtime services work after kexec? I don't see EFI handles > > handed over kexec, are they somehow re-discovered? > > What EFI handles are you talking about? For runtime services > what a consumer needs is a table pointer, which is a field > in the system table, which in turn is an argument passed to > the EFI application's entry point. Yes, I'm talking about those pointers (system table specifically). > I didn't think there are > provisions in the spec for either of these pointers being NULL. But I don't see kexec using EFI application entry point. Am I missing something? -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? signature.asc Description: PGP signature ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 0/3] x86/boot: Introduce the kernel_info et consortes
Hi, Due to very limited space in the setup_header this patch series introduces new kernel_info struct which will be used to convey information from the kernel to the bootloader. This way the boot protocol can be extended regardless of the setup_header limitations. Additionally, the patch series introduces some convenience features like the setup_indirect struct and the kernel_info.setup_type_max field. Daniel Documentation/x86/boot.rst | 168 ++ arch/x86/boot/Makefile | 2 +- arch/x86/boot/compressed/Makefile | 4 +- arch/x86/boot/compressed/kaslr.c | 12 ++ arch/x86/boot/compressed/kernel_info.S | 22 +++ arch/x86/boot/header.S | 3 +- arch/x86/boot/tools/build.c| 5 +++ arch/x86/include/uapi/asm/bootparam.h | 16 +++- arch/x86/kernel/e820.c | 11 ++ arch/x86/kernel/kdebugfs.c | 20 -- arch/x86/kernel/ksysfs.c | 30 ++ arch/x86/kernel/setup.c| 4 ++ arch/x86/mm/ioremap.c | 11 ++ 13 files changed, 292 insertions(+), 16 deletions(-) Daniel Kiper (3): x86/boot: Introduce the kernel_info x86/boot: Introduce the kernel_info.setup_type_max x86/boot: Introduce the setup_indirect ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 2/3] x86/boot: Introduce the kernel_info.setup_type_max
This field contains maximal allowed type for setup_data. Now bump the setup_header version in arch/x86/boot/header.S. Suggested-by: H. Peter Anvin Signed-off-by: Daniel Kiper Reviewed-by: Konrad Rzeszutek Wilk Reviewed-by: Ross Philipson --- Documentation/x86/boot.rst | 9 - arch/x86/boot/compressed/kernel_info.S | 5 + arch/x86/boot/header.S | 2 +- arch/x86/include/uapi/asm/bootparam.h | 3 +++ 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst index d5323a39f5e3..4c536bc8816d 100644 --- a/Documentation/x86/boot.rst +++ b/Documentation/x86/boot.rst @@ -73,7 +73,7 @@ Protocol 2.14:BURNT BY INCORRECT COMMIT ae7e1238e68f2a472a125673ab506d49158c188 (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. +Protocol 2.15: (Kernel 5.5) Added the kernel_info and kernel_info.setup_type_max. = .. note:: @@ -976,6 +976,13 @@ Offset/size: 0x0008/4 This field contains the size of the kernel_info including kernel_info.header and kernel_info.kernel_info_var_len_data. + == +Field name:setup_type_max +Offset/size: 0x0008/4 + == + + This field contains maximal allowed type for setup_data and setup_indirect structs. + The Image Checksum == diff --git a/arch/x86/boot/compressed/kernel_info.S b/arch/x86/boot/compressed/kernel_info.S index 8ea6f6e3feef..f818ee8fba38 100644 --- a/arch/x86/boot/compressed/kernel_info.S +++ b/arch/x86/boot/compressed/kernel_info.S @@ -1,5 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0 */ +#include + .section ".rodata.kernel_info", "a" .global kernel_info @@ -12,6 +14,9 @@ kernel_info: /* Size total. */ .long kernel_info_end - kernel_info + /* Maximal allowed type for setup_data and setup_indirect structs. */ + .long SETUP_TYPE_MAX + kernel_info_var_len_data: /* Empty for time being... */ kernel_info_end: diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S index 22dcecaaa898..97d9b6d6c1af 100644 --- a/arch/x86/boot/header.S +++ b/arch/x86/boot/header.S @@ -300,7 +300,7 @@ _start: # Part 2 of the header, from the old setup.S .ascii "HdrS" # header signature - .word 0x020d # header version number (>= 0x0105) + .word 0x020f # header version number (>= 0x0105) # or else old loadlin-1.5 will fail) .globl realmode_swtch realmode_swtch:.word 0, 0# default_switch, SETUPSEG diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h index a1ebcd7a991c..dbb41128e5a0 100644 --- a/arch/x86/include/uapi/asm/bootparam.h +++ b/arch/x86/include/uapi/asm/bootparam.h @@ -11,6 +11,9 @@ #define SETUP_APPLE_PROPERTIES 5 #define SETUP_JAILHOUSE6 +/* max(SETUP_*) */ +#define SETUP_TYPE_MAX SETUP_JAILHOUSE + /* ram_size flags */ #define RAMDISK_IMAGE_START_MASK 0x07FF #define RAMDISK_PROMPT_FLAG0x8000 -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 3/3] x86/boot: Introduce the setup_indirect
The setup_data is a bit awkward to use for extremely large data objects,
both because the setup_data header has to be adjacent to the data object
and because it has a 32-bit length field. However, it is important that
intermediate stages of the boot process have a way to identify which
chunks of memory are occupied by kernel data. Thus we introduce an uniform
way to specify such indirect data as setup_indirect struct and
SETUP_INDIRECT type.
Suggested-by: H. Peter Anvin
Signed-off-by: Daniel Kiper
Acked-by: Konrad Rzeszutek Wilk
Reviewed-by: Ross Philipson
---
v3 - suggestions/fixes:
- add setup_indirect mapping/KASLR avoidance/etc. code
(suggested by H. Peter Anvin),
- the SETUP_INDIRECT sets most significant bit right now;
this way it is possible to differentiate regular setup_data
and setup_indirect objects in the debugfs filesystem.
v2 - suggestions/fixes:
- add setup_indirect usage example
(suggested by Eric Snowberg and Ross Philipson).
---
Documentation/x86/boot.rst| 40 +++
arch/x86/boot/compressed/kaslr.c | 12 +++
arch/x86/include/uapi/asm/bootparam.h | 16 +++---
arch/x86/kernel/e820.c| 11 ++
arch/x86/kernel/kdebugfs.c| 20 ++
arch/x86/kernel/ksysfs.c | 30 --
arch/x86/kernel/setup.c | 4
arch/x86/mm/ioremap.c | 11 ++
8 files changed, 130 insertions(+), 14 deletions(-)
diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
index 4c536bc8816d..d6d03b00b594 100644
--- a/Documentation/x86/boot.rst
+++ b/Documentation/x86/boot.rst
@@ -827,6 +827,46 @@ Protocol: 2.09+
sure to consider the case where the linked list already contains
entries.
+ The setup_data is a bit awkward to use for extremely large data objects,
+ both because the setup_data header has to be adjacent to the data object
+ and because it has a 32-bit length field. However, it is important that
+ intermediate stages of the boot process have a way to identify which
+ chunks of memory are occupied by kernel data.
+
+ Thus setup_indirect struct and SETUP_INDIRECT type were introduced in
+ protocol 2.15.
+
+ struct setup_indirect {
+__u32 type;
+__u32 reserved; /* Reserved, must be set to zero. */
+__u64 len;
+__u64 addr;
+ };
+
+ The type member is a SETUP_INDIRECT | SETUP_* type. However, it cannot be
+ SETUP_INDIRECT itself since making the setup_indirect a tree structure
+ could require a lot of stack space in something that needs to parse it
+ and stack space can be limited in boot contexts.
+
+ Let's give an example how to point to SETUP_E820_EXT data using
setup_indirect.
+ In this case setup_data and setup_indirect will look like this:
+
+ struct setup_data {
+__u64 next = 0 or ;
+__u32 type = SETUP_INDIRECT;
+__u32 len = sizeof(setup_data);
+__u8 data[sizeof(setup_indirect)] = struct setup_indirect {
+ __u32 type = SETUP_INDIRECT | SETUP_E820_EXT;
+ __u32 reserved = 0;
+ __u64 len = ;
+ __u64 addr = ;
+}
+ }
+
+ Note: SETUP_INDIRECT | SETUP_NONE objects cannot be properly distinguished
+from SETUP_INDIRECT itself. So, this kind of objects cannot be provided
+by the bootloaders.
+
Field name:pref_address
Type: read (reloc)
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 2e53c056ba20..bb9bfef174ae 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -459,6 +459,18 @@ static bool mem_avoid_overlap(struct mem_vector *img,
is_overlapping = true;
}
+ if (ptr->type == SETUP_INDIRECT &&
+ ((struct setup_indirect *)ptr->data)->type !=
SETUP_INDIRECT) {
+ avoid.start = ((struct setup_indirect
*)ptr->data)->addr;
+ avoid.size = ((struct setup_indirect *)ptr->data)->len;
+
+ if (mem_overlaps(img, ) && (avoid.start <
earliest)) {
+ *overlap = avoid;
+ earliest = overlap->start;
+ is_overlapping = true;
+ }
+ }
+
ptr = (struct setup_data *)(unsigned long)ptr->next;
}
diff --git a/arch/x86/include/uapi/asm/bootparam.h
b/arch/x86/include/uapi/asm/bootparam.h
index dbb41128e5a0..949066b5398a 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -2,7 +2,7 @@
#ifndef _ASM_X86_BOOTPARAM_H
#define _ASM_X86_BOOTPARAM_H
-/* setup_data types */
+/* setup_data/setup_indirect types */
#define SETUP_NONE 0
#define SETUP_E820_EXT 1
#define SETUP_DTB 2
@@ -11,8 +11,10 @@
#define
[Xen-devel] [PATCH v3 1/3] x86/boot: Introduce 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 setup_data entries. It cannot be used to communicate properties of the kernel image, because it is .bss and has no image-provided content. kernel_info solves this by providing an extensible place for information about the kernel image. It is readonly, because the kernel cannot rely on a bootloader copying its contents anywhere, but that is OK; if it becomes necessary it can still contain data items that an enabled bootloader would be expected to copy into a setup_data chunk. This patch does not bump setup_header version in arch/x86/boot/header.S because it will be followed by additional changes coming into the Linux/x86 boot protocol. Suggested-by: H. Peter Anvin Signed-off-by: Daniel Kiper Reviewed-by: Konrad Rzeszutek Wilk Reviewed-by: Ross Philipson --- v3 - suggestions/fixes: - split kernel_info data into fixed and variable sized regions, (suggested by H. Peter Anvin), - change kernel_info.header value to "LToP" (0x506f544c), (suggested by H. Peter Anvin), - improve the comments, - improve the documentation. v2 - suggestions/fixes: - rename setup_header2 to kernel_info, (suggested by H. Peter Anvin), - change kernel_info.header value to "InfO" (0x4f666e49), - new kernel_info description in Documentation/x86/boot.rst, (suggested by H. Peter Anvin), - drop kernel_info_offset_update() as an overkill and update kernel_info offset directly from main(), (suggested by Eric Snowberg), - new commit message (suggested by H. Peter Anvin), - fix some commit message misspellings (suggested by Eric Snowberg). --- 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 NameMeaning 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
Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it
On 09.10.2019 12:31, Marek Marczykowski-Górecki wrote: > On Wed, Oct 09, 2019 at 10:56:56AM +0200, Jan Beulich wrote: >> On 08.10.2019 18:29, Marek Marczykowski-Górecki wrote: >>> On Tue, Oct 08, 2019 at 04:19:13PM +0200, Jan Beulich wrote: On 08.10.2019 15:52, Marek Marczykowski-Górecki wrote: > In Linux case, it looks like it passes around the EFI memory map using > some Linux-specific mechanism, but I don't find it particularly > appealing option. >> >> ... that this would require Xen following a Linux protocol. >> This is nothing that can work building on EFI interfaces alone. > > Actually, there is something that could be used: presence of boot > services. If the call to SetVirtualAddressMap() is bound to initial > presence of boot services, then it surely won't happen after kexec, as > boot services are not available anymore. In fact the patch I've sent > does exactly that - call SetVirtualAddressMap() directly after > ExitBootServices(), but I've realized this property only now. In this > case, maybe kconfig option is not needed anymore? I'm unaware of a property telling an EFI application whether boot services are available. By the definition I know they're available up and until ExitBootServices() gets called. > BTW How runtime services work after kexec? I don't see EFI handles > handed over kexec, are they somehow re-discovered? What EFI handles are you talking about? For runtime services what a consumer needs is a table pointer, which is a field in the system table, which in turn is an argument passed to the EFI application's entry point. I didn't think there are provisions in the spec for either of these pointers being NULL. 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] x86/passthrough: fix migration of MSI when using posted interrupts
On 09.10.2019 12:20, Roger Pau Monné wrote: > On Wed, Oct 09, 2019 at 12:08:50PM +0200, Jan Beulich wrote: >> Considering that hvm_girq_dest_2_vcpu_id() isn't really inexpensive, >> subsequent cleanup may then involve avoiding to call this function >> if we'd overwrite .dest_vcpu_id as per above anyway. > > Hm, I see. I would leave that for a further optimization. Sure, hence me saying "subsequent cleanup". Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] HPET interrupt remapping during boot
On 09.10.2019 12:11, Roger Pau Monné wrote: > And it does print the following when setting up the iommu: > > (XEN) ioapic 0 pin 0 not masked > (XEN) vec=00 delivery=ExINT dest=P status=0 polarity=0 irr=0 trig=E mask=0 > dest_id:0001 > > I wonder, shouldn't all pins of all the io-apics be masked at boot? I think you might get different answers here depending on whether you ask firmware or OS people. In fact there are cases where the IO-APIC needs to be left in this state, I think, but such would likely need properly reflecting in ACPI tables (albeit I don't know/recall how this would be done; looking at the code ). This goes back to times when IO-APICs were new and OSes would not even know about them, yet they wouldn't get any interrupts to work if fiddling with only the PIC (sitting behind IO-APIC pin 0). See enable_IO_APIC(), where we actually use this property to determine the pin behind which the 8259 sits. I've seen quite many systems where in the BIOS setup you have an option to select whether you have an "ACPI OS" (wording of course varies). I've never checked whether this may e.g. reflect itself in the handover state of the GSI 0 RTE. In your testing patch, could you also log the PIC mask bytes? There ought to be at least one unmasked; or wait - there actually is a spurious interrupt there (right before IOMMU initialization): (XEN) spurious 8259A interrupt: IRQ7. Hence I wonder if there's not possibly a 2nd one once the IOMMU has been set up. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it
On Wed, Oct 09, 2019 at 10:56:56AM +0200, Jan Beulich wrote: > On 08.10.2019 18:29, Marek Marczykowski-Górecki wrote: > > On Tue, Oct 08, 2019 at 04:19:13PM +0200, Jan Beulich wrote: > >> On 08.10.2019 15:52, Marek Marczykowski-Górecki wrote: > >>> Regardless of SetVirtualAddressMap() discussion, I propose to > >>> automatically map boot services code/data, to make Xen work on more > >>> machines (even if _we_ consider those buggy). > >> > >> I remain on my prior position: Adding command line triggerable > >> workarounds for such cases is fine. Defaulting to assume buggy > >> firmware is acceptable only if this means no extra penalty to > >> systems with conforming firmware. Hence, for the case at hand, > >> I object to doing this automatically; we already have the > >> /mapbs workaround in place to deal with the case when xen.efi > >> is used. Judging from the title here there may need to be an > >> addition to also allow triggering this from the MB2 boot path. > > > > What about mirroring Linux behavior? I.e. mapping those regions for the > > SetVirtualAddressMap() time (when enabled) and unmapping after - unless > > tagged with EFI_MEMORY_RUNTIME? > > Similarly to Andrew, I'd really prefer for Xen to work out of the box, > > with as little as possible manual tweaks needed. > > If there's going to be a config where SetVirtualAddressMap() is to > be called - why not? But the same logic doesn't make sense when > such a call won#t happen in the first place. See my other email, I think I've found a better (simple and working!) solution. > >>> What if Xen was kexec'ed from Linux? > > Honestly - I'm getting tired. You said yourself ... > > >>> In Linux case, it looks like it passes around the EFI memory map using > >>> some Linux-specific mechanism, but I don't find it particularly > >>> appealing option. > > ... that this would require Xen following a Linux protocol. > This is nothing that can work building on EFI interfaces alone. Actually, there is something that could be used: presence of boot services. If the call to SetVirtualAddressMap() is bound to initial presence of boot services, then it surely won't happen after kexec, as boot services are not available anymore. In fact the patch I've sent does exactly that - call SetVirtualAddressMap() directly after ExitBootServices(), but I've realized this property only now. In this case, maybe kconfig option is not needed anymore? BTW How runtime services work after kexec? I don't see EFI handles handed over kexec, are they somehow re-discovered? > >>> What about something in between: make this SetVirtualAddressMap() call > >>> compile-time option (kconfig), depending on !CONFIG_KEXEC ? And when > >>> enabled, properly handle SetVirtualAddressMap() failure. > >> > >> What is "proper handling" here? > > > > Logging the error and either panic or disabling runtime services (I tend > > to the latter). > > Hmm, yes, disabling runtime services in this case makes sense. > But are you sure a SetVirtualAddressMap() failure doesn't incur > other potential issues later on? (Calling panic() is what I'd > rather not call "proper handling", but rather "emergency > handling".) Well, as for being sure, one could say calling ExitBootServices() but not SetVirtualAddressMap() definitely won't cause any troubles. I can't say anything about UEFI for sure. But UEFI spec doesn't mention any side effect of a failed call. BTW Linux panic on failed SetVirtualAddressMap(). But on kexec, if didn't received address map for EFI RS calls, it simply disable RS. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? signature.asc Description: PGP signature ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] x86/mm: don't needlessly veto migration
On 09.10.19 12:21, George Dunlap wrote: On 10/9/19 11:16 AM, Jan Beulich wrote: On 08.10.2019 18:38, Andrew Cooper wrote: On 08/10/2019 17:10, Jan Beulich wrote: From: Paul Durrant Now that xl.cfg has an option to explicitly enable IOMMU mappings for a domain, migration may be needlessly vetoed due to the check of is_iommu_enabled() in paging_log_dirty_enable(). There is actually no need to prevent logdirty from being enabled unless devices are assigned to a domain. NOTE: While in the neighbourhood, the bool_t parameter type in paging_log_dirty_enable() is replaced with a bool and the format of the comment in assign_device() is fixed. Signed-off-by: Paul Durrant Signed-off-by: Jan Beulich Release-acked-by: Juergen Gross Seriously FFS. Why am I having to repeat myself? What if any way unclear on the previous threads? NACK NACK NACK. With George having given his R-b I'm now in an awkward position: I shouldn't ignore this triple NACK and commit the patch, but there's also no sensible way forward here which would allow the regression to be taken care of without committing the patch in its current shape. Are you willing to take back all three of the NACKs, allowing us to continue discussion on the controversial part of Paul's patch while also allowing the non-controversial part to go in right away? Regardless of the merits of the change Andy wants to see, it's not a one that should be made during a feature freeze. Indeed. So either we take this patch or we have to revert the patch(es) introducing the regression. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] x86/mm: don't needlessly veto migration
On 10/9/19 11:16 AM, Jan Beulich wrote: > On 08.10.2019 18:38, Andrew Cooper wrote: >> On 08/10/2019 17:10, Jan Beulich wrote: >>> From: Paul Durrant >>> >>> Now that xl.cfg has an option to explicitly enable IOMMU mappings for a >>> domain, migration may be needlessly vetoed due to the check of >>> is_iommu_enabled() in paging_log_dirty_enable(). >>> There is actually no need to prevent logdirty from being enabled unless >>> devices are assigned to a domain. >>> >>> NOTE: While in the neighbourhood, the bool_t parameter type in >>> paging_log_dirty_enable() is replaced with a bool and the format >>> of the comment in assign_device() is fixed. >>> >>> Signed-off-by: Paul Durrant >>> Signed-off-by: Jan Beulich >>> Release-acked-by: Juergen Gross >> >> Seriously FFS. Why am I having to repeat myself? What if any way >> unclear on the previous threads? >> >> NACK NACK NACK. > > With George having given his R-b I'm now in an awkward position: > I shouldn't ignore this triple NACK and commit the patch, but > there's also no sensible way forward here which would allow the > regression to be taken care of without committing the patch in > its current shape. Are you willing to take back all three of the > NACKs, allowing us to continue discussion on the controversial > part of Paul's patch while also allowing the non-controversial > part to go in right away? Regardless of the merits of the change Andy wants to see, it's not a one that should be made during a feature freeze. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.13] x86/passthrough: fix migration of MSI when using posted interrupts
On Wed, Oct 09, 2019 at 12:08:50PM +0200, Jan Beulich wrote:
> On 09.10.2019 11:05, Roger Pau Monne wrote:
> > @@ -411,6 +411,7 @@ int pt_irq_create_bind(
> >
> > pirq_dpci->gmsi.gvec = pt_irq_bind->u.msi.gvec;
> > pirq_dpci->gmsi.gflags = gflags;
> > +prev_vcpu_id = pirq_dpci->gmsi.dest_vcpu_id;
>
> If this and ...
>
> > @@ -440,7 +441,8 @@ int pt_irq_create_bind(
> > /* Use interrupt posting if it is supported. */
> > if ( iommu_intpost )
> > pi_update_irte(vcpu ? >arch.hvm.vmx.pi_desc : NULL,
> > - info, pirq_dpci->gmsi.gvec);
> > + info, pirq_dpci->gmsi.gvec,
> > + prev_vcpu_id >= 0 ? d->vcpu[prev_vcpu_id] :
> > NULL);
>
> ... this are to be reliable, then - as explained to Joe already
> in the earlier discussion - I think you need to update
> pirq_dpci->gmsi.dest_vcpu_id in a code section a few lines up
> from here (such that it would be reliable the next time we come
> here) like this:
>
> @@ -xxx,7 +yyy,10 @@
> vcpu = vector_hashing_dest(d, dest, dest_mode,
> pirq_dpci->gmsi.gvec);
> if ( vcpu )
> +{
> pirq_dpci->gmsi.posted = true;
> +pirq_dpci->gmsi.dest_vcpu_id = vcpu->vcpu_id;
> +}
> }
> if ( vcpu && is_iommu_enabled(d) )
> hvm_migrate_pirq(pirq_dpci, vcpu);
>
> This ought to be fine because so far .dest_vcpu_id has a consumer
> only in the non-posted case (in hvm_migrate_pirq()).
Oh, I see. dest_vcpu_id is only valid for non-multidest, but when
using posted interrupts Xen selects a fixed destination vcpu even in
multidest, so we can take advantage of posted interrupts at the price
of always delivering to the same vcpu.
>
> Considering that hvm_girq_dest_2_vcpu_id() isn't really inexpensive,
> subsequent cleanup may then involve avoiding to call this function
> if we'd overwrite .dest_vcpu_id as per above anyway.
Hm, I see. I would leave that for a further optimization.
Thanks, Roger.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-coverity test] 142486: all pass - PUSHED
flight 142486 xen-unstable-coverity real [real] http://logs.test-lab.xenproject.org/osstest/logs/142486/ Perfect :-) All tests in this flight passed as required version targeted for testing: xen 98d1dac88f82c2b79d528faabe5e3fda8133e8bb baseline version: xen f4049b2a9850c847b06ec6ad1cec1c7c2c303b94 Last test of basis 142352 2019-10-06 09:20:41 Z3 days Testing same since 142486 2019-10-09 09:18:55 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Daniel De Graaf Juergen Gross Julien Grall Lars Kurth Stefano Stabellini Stefano Stabellini Wei Liu jobs: coverity-amd64 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 f4049b2a98..98d1dac88f 98d1dac88f82c2b79d528faabe5e3fda8133e8bb -> coverity-tested/smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] read grubenv and set default from saved_entry or next_entry [and 1 more messages]
Lars Kurth writes ("Re: [Xen-devel] [PATCH] read grubenv and set default from
saved_entry or next_entry [and 1 more messages]"):
> I am assuming there is no chance that this will make 4.13 with the freeze
> having passed.
Assuming we can get good quality patches, I would probably support a
freeze exception. Failing that, this would be a strong candidate for
a backport.
> But in any case, I wasn't sure whether Michael strictly will need it
> as the change can presumably be carried in a fedora patch q for Xen
> packages until it ends up upstream
That's not really enough, because pygrub runs in the host but
interprets the guest filesystem. Since we want to be able to boot
Fedora guests on non-Fedora hosts, that means that every host must
have this fix.
This is an inherent consequence of pygrub's design approach, and means
that I aggressively backport pygrub changes to older releases, so that
older hosts can boot newer guests.
But in the meantime putting this patch in the Fedora host packages is
not a terrible idea. (There is a bit of a risk that if we deploy in
Fedora one algorith, and then upstream a different algorithm, we may
end up stuck with divergence, but I doubt this will be a big problem
here.)
thanks,
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] x86/mm: don't needlessly veto migration
On 08.10.2019 18:38, Andrew Cooper wrote: > On 08/10/2019 17:10, Jan Beulich wrote: >> From: Paul Durrant >> >> Now that xl.cfg has an option to explicitly enable IOMMU mappings for a >> domain, migration may be needlessly vetoed due to the check of >> is_iommu_enabled() in paging_log_dirty_enable(). >> There is actually no need to prevent logdirty from being enabled unless >> devices are assigned to a domain. >> >> NOTE: While in the neighbourhood, the bool_t parameter type in >> paging_log_dirty_enable() is replaced with a bool and the format >> of the comment in assign_device() is fixed. >> >> Signed-off-by: Paul Durrant >> Signed-off-by: Jan Beulich >> Release-acked-by: Juergen Gross > > Seriously FFS. Why am I having to repeat myself? What if any way > unclear on the previous threads? > > NACK NACK NACK. With George having given his R-b I'm now in an awkward position: I shouldn't ignore this triple NACK and commit the patch, but there's also no sensible way forward here which would allow the regression to be taken care of without committing the patch in its current shape. Are you willing to take back all three of the NACKs, allowing us to continue discussion on the controversial part of Paul's patch while also allowing the non-controversial part to go in right away? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] HPET interrupt remapping during boot
On Wed, Oct 09, 2019 at 11:31:59AM +0200, Jan Beulich wrote:
> On 08.10.2019 20:30, Andrew Cooper wrote:
> > Hello,
> >
> > I have no idea if this is a regression or not. I suspect it might not
> > be, and has always been broken.
> >
> > Either way, I'm seeing occasional single interrupt remapping errors when
> > booting a range of Intel systems
> >
> > (XEN) x2APIC mode is already enabled by BIOS.
> > (XEN) Using APIC driver x2apic_cluster
> > ...
> > (XEN) Platform timer is 23.999MHz HPET
> > (XEN) Detected 2194.922 MHz processor.
> > ...
> > (XEN) HVM: HAP page sizes: 4kB, 2MB, 1GB
> > (XEN) alt table 82d08047a070 -> 82d080486c6c
> > (XEN) [VT-D]INTR-REMAP: Request device [:f0:1f.0] fault index 0,
> > iommu reg = 82c00072d000
> > (XEN) [VT-D]INTR-REMAP: reason 22 - Present field in the IRTE entry is clear
> > (XEN) microcode: CPU2 updated from revision 0x521 to 0x52b, date
> > = 2019-08-12
> >
> > From other debugging, I know that this happens after CPU 1 (which is a
> > hyperthread) has passed through start_secondary().
> >
> > f0:1f.0 is one of the IO-APICs, and if I've cross referenced the DMAR
> > and APIC tables properly, is the IO-APIC on the PCH, making the
> > problematic IRQ GSI0.
> >
> > This suggests that we have an error setting up the timer IRQ (as the
> > HPET isn't MSI-capable), but we have already allegedly used it
> > successfully earlier on boot.
>
> Wait - is this really a system with the timer at GSI 0, i.e. without
> an interrupt source override like this (as seen in Linux logs):
>
> ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
I also have such a system, and I do have and entry like:
(XEN) ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
On the Xen log. I've added some more debug info to my build with the
following patch:
diff --git a/xen/drivers/passthrough/vtd/iommu.c
b/xen/drivers/passthrough/vtd/iommu.c
index f08eec070d..0d7abcd8fa 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -993,6 +993,7 @@ static void iommu_page_fault(int irq, void *dev_id,
* specs since a new interrupt won't be generated until we clear all
* the faults that caused this one to happen.
*/
+WARN();
tasklet_schedule(_fault_tasklet);
}
diff --git a/xen/drivers/passthrough/x86/iommu.c
b/xen/drivers/passthrough/x86/iommu.c
index 59905629e1..2dbc7a3607 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -23,12 +23,59 @@
#include
#include
+#include
+
const struct iommu_init_ops *__initdata iommu_init_ops;
struct iommu_ops __read_mostly iommu_ops;
+static const char * delivery_mode_2_str(
+const enum ioapic_irq_destination_types mode)
+{
+switch ( mode )
+{
+case dest_Fixed: return "Fixed";
+case dest_LowestPrio: return "LoPri";
+case dest_SMI: return "SMI";
+case dest_NMI: return "NMI";
+case dest_INIT: return "INIT";
+case dest_ExtINT: return "ExINT";
+case dest__reserved_1:
+case dest__reserved_2: return "Resvd";
+default: return "INVAL";
+}
+}
+
int __init iommu_hardware_setup(void)
{
int rc;
+int apic;
+
+for(apic = 0; apic < nr_ioapics; apic++)
+{
+int pin;
+/* See if any of the pins is in ExtINT mode */
+for (pin = 0; pin < nr_ioapic_entries[apic]; pin++) {
+struct IO_APIC_route_entry rte = __ioapic_read_entry(apic, pin, 0);
+
+/* If the interrupt line is enabled and in ExtInt mode
+ * I have found the pin where the i8259 is connected.
+ */
+if (!rte.mask)
+{
+printk("ioapic %d pin %d not masked\n", apic, pin);
+printk("vec=%02x delivery=%-5s dest=%c status=%d "
+ "polarity=%d irr=%d trig=%c mask=%d dest_id:%0*x\n",
+ rte.vector, delivery_mode_2_str(rte.delivery_mode),
+ rte.dest_mode ? 'L' : 'P',
+ rte.delivery_status, rte.polarity, rte.irr,
+ rte.trigger ? 'L' : 'E', rte.mask,
+ x2apic_enabled ? 8 : 2,
+ x2apic_enabled ? rte.dest.dest32
+ : rte.dest.logical.logical_dest);
+}
+}
+}
+
if ( !iommu_init_ops )
return -ENODEV;
And it does print the following when setting up the iommu:
(XEN) ioapic 0 pin 0 not masked
(XEN) vec=00 delivery=ExINT dest=P status=0 polarity=0 irr=0 trig=E mask=0
dest_id:0001
I wonder, shouldn't all pins of all the io-apics be masked at boot?
Full Xen boot log below.
Thanks, Roger.
---
(XEN) Xen version 4.13-unstable (root@xenrtcloud) (gcc (Debian 6.3.0-18+deb9u1)
6.3.0 20170516) debug=y Wed Oct 9 12:03:37 CEST 2019
(XEN) Latest ChangeSet: Copyright (C) 1994-2013 H. Peter Anvin et al
(XEN) build-id: 52e9ffb079497b12ad075c4d09e1c9070151bfdf
(XEN) Console output is synchronous.
(XEN) Bootloader:
Re: [Xen-devel] [PATCH for-4.13] x86/passthrough: fix migration of MSI when using posted interrupts
On 09.10.2019 11:05, Roger Pau Monne wrote:
> @@ -411,6 +411,7 @@ int pt_irq_create_bind(
>
> pirq_dpci->gmsi.gvec = pt_irq_bind->u.msi.gvec;
> pirq_dpci->gmsi.gflags = gflags;
> +prev_vcpu_id = pirq_dpci->gmsi.dest_vcpu_id;
If this and ...
> @@ -440,7 +441,8 @@ int pt_irq_create_bind(
> /* Use interrupt posting if it is supported. */
> if ( iommu_intpost )
> pi_update_irte(vcpu ? >arch.hvm.vmx.pi_desc : NULL,
> - info, pirq_dpci->gmsi.gvec);
> + info, pirq_dpci->gmsi.gvec,
> + prev_vcpu_id >= 0 ? d->vcpu[prev_vcpu_id] : NULL);
... this are to be reliable, then - as explained to Joe already
in the earlier discussion - I think you need to update
pirq_dpci->gmsi.dest_vcpu_id in a code section a few lines up
from here (such that it would be reliable the next time we come
here) like this:
@@ -xxx,7 +yyy,10 @@
vcpu = vector_hashing_dest(d, dest, dest_mode,
pirq_dpci->gmsi.gvec);
if ( vcpu )
+{
pirq_dpci->gmsi.posted = true;
+pirq_dpci->gmsi.dest_vcpu_id = vcpu->vcpu_id;
+}
}
if ( vcpu && is_iommu_enabled(d) )
hvm_migrate_pirq(pirq_dpci, vcpu);
This ought to be fine because so far .dest_vcpu_id has a consumer
only in the non-posted case (in hvm_migrate_pirq()).
Considering that hvm_girq_dest_2_vcpu_id() isn't really inexpensive,
subsequent cleanup may then involve avoiding to call this function
if we'd overwrite .dest_vcpu_id as per above anyway.
> --- a/xen/drivers/passthrough/vtd/intremap.c
> +++ b/xen/drivers/passthrough/vtd/intremap.c
> @@ -946,12 +946,13 @@ void intel_iommu_disable_eim(void)
> disable_qinval(drhd->iommu);
> }
>
> +#if CONFIG_HVM
#ifdef please.
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] x86/microcode: Drop trailing whitespace in printk()
On 09.10.19 11:35, Jan Beulich wrote: On 08.10.2019 21:27, Andrew Cooper wrote: This has actually been present since c/s bd7c09c0 in 2008, and survived through all of the recent microcode refactoring. Signed-off-by: Andrew Cooper Acked-by: Jan Beulich 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 v3] x86/mm: don't needlessly veto migration
On 10/8/19 5:10 PM, Jan Beulich wrote: > From: Paul Durrant > > Now that xl.cfg has an option to explicitly enable IOMMU mappings for a > domain, migration may be needlessly vetoed due to the check of > is_iommu_enabled() in paging_log_dirty_enable(). > There is actually no need to prevent logdirty from being enabled unless > devices are assigned to a domain. > > NOTE: While in the neighbourhood, the bool_t parameter type in > paging_log_dirty_enable() is replaced with a bool and the format > of the comment in assign_device() is fixed. > > Signed-off-by: Paul Durrant > Signed-off-by: Jan Beulich > Release-acked-by: Juergen Gross Reviewed-by: George Dunlap ___ 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.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. 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] x86/microcode: Drop trailing whitespace in printk()
On 08.10.2019 21:27, Andrew Cooper wrote: > This has actually been present since c/s bd7c09c0 in 2008, and survived > through all of the recent microcode refactoring. > > Signed-off-by: Andrew Cooper Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] HPET interrupt remapping during boot
On 08.10.2019 20:30, Andrew Cooper wrote: > Hello, > > I have no idea if this is a regression or not. I suspect it might not > be, and has always been broken. > > Either way, I'm seeing occasional single interrupt remapping errors when > booting a range of Intel systems > > (XEN) x2APIC mode is already enabled by BIOS. > (XEN) Using APIC driver x2apic_cluster > ... > (XEN) Platform timer is 23.999MHz HPET > (XEN) Detected 2194.922 MHz processor. > ... > (XEN) HVM: HAP page sizes: 4kB, 2MB, 1GB > (XEN) alt table 82d08047a070 -> 82d080486c6c > (XEN) [VT-D]INTR-REMAP: Request device [:f0:1f.0] fault index 0, > iommu reg = 82c00072d000 > (XEN) [VT-D]INTR-REMAP: reason 22 - Present field in the IRTE entry is clear > (XEN) microcode: CPU2 updated from revision 0x521 to 0x52b, date > = 2019-08-12 > > From other debugging, I know that this happens after CPU 1 (which is a > hyperthread) has passed through start_secondary(). > > f0:1f.0 is one of the IO-APICs, and if I've cross referenced the DMAR > and APIC tables properly, is the IO-APIC on the PCH, making the > problematic IRQ GSI0. > > This suggests that we have an error setting up the timer IRQ (as the > HPET isn't MSI-capable), but we have already allegedly used it > successfully earlier on boot. Wait - is this really a system with the timer at GSI 0, i.e. without an interrupt source override like this (as seen in Linux logs): ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl) Otherwise isn't this rather an ExtInt arriving through the PIC? We mask all PIC interrupts first thing, but I'm not sure there aren't paths where we'd unmask any. Additionally I seem to vaguely recall that the spurious interrupt can't be masked at the PIC, and I do recall seeing (randomly, like you) spurious interrupts during boot (can't tell offhand whether this was on AMD and/or Intel, and perhaps on IOMMU-less systems only). I've never seen though what you describe here. Then again the log message saying "fault index 0" doesn't tell us what the GSI is, or what IO-APIC pin the IRQ can through. All not yet set up IO-APIC RTEs would specify a remapping index of zero. Of course all such IO-APIC entries ought to have their mask bit set - question is whether the system comes out of boot with one (perhaps indeed an ExtInt one) not masked. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-4.13] x86/passthrough: fix migration of MSI when using posted interrupts
When using posted interrupts and the guest migrates MSI from vCPUs Xen
needs to flush any pending PIRR vectors on the previous vCPU, or else
those vectors could get wrongly injected at a later point when the MSI
fields are already updated.
Rename sync_pir_to_irr to vlapic_sync_pir_to_irr and export it so it
can be called when updating the posted interrupt descriptor field in
pi_update_irte. While there also remove the unlock_out from
pi_update_irte, it's used in a single goto and removing it makes the
function smaller.
Note that PIRR is synced to IRR both in pt_irq_destroy_bind and
pt_irq_create_bind when the interrupt delivery data is being updated.
While there guard pi_update_irte with CONFIG_HVM since it's only used
with HVM guests.
Reported-by: Joe Jin
Signed-off-by: Roger Pau Monné
---
Cc: Joe Jin
Cc: Juergen Gross
---
I would like to see a bug fix for this issue in 4.13. The fix here only
affects posted interrupts, hence I think the risk of breaking anything
else is low.
---
xen/arch/x86/hvm/vlapic.c | 6 +++---
xen/drivers/passthrough/io.c | 10 +++---
xen/drivers/passthrough/vtd/intremap.c | 15 ---
xen/include/asm-x86/hvm/vlapic.h | 2 ++
xen/include/asm-x86/iommu.h| 2 +-
5 files changed, 21 insertions(+), 14 deletions(-)
diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 9466258d6f..d255ad8db7 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -106,7 +106,7 @@ static void vlapic_clear_irr(int vector, struct vlapic
*vlapic)
vlapic_clear_vector(vector, >regs->data[APIC_IRR]);
}
-static void sync_pir_to_irr(struct vcpu *v)
+void vlapic_sync_pir_to_irr(struct vcpu *v)
{
if ( hvm_funcs.sync_pir_to_irr )
alternative_vcall(hvm_funcs.sync_pir_to_irr, v);
@@ -114,7 +114,7 @@ static void sync_pir_to_irr(struct vcpu *v)
static int vlapic_find_highest_irr(struct vlapic *vlapic)
{
-sync_pir_to_irr(vlapic_vcpu(vlapic));
+vlapic_sync_pir_to_irr(vlapic_vcpu(vlapic));
return vlapic_find_highest_vector(>regs->data[APIC_IRR]);
}
@@ -1493,7 +1493,7 @@ static int lapic_save_regs(struct vcpu *v,
hvm_domain_context_t *h)
if ( !has_vlapic(v->domain) )
return 0;
-sync_pir_to_irr(v);
+vlapic_sync_pir_to_irr(v);
return hvm_save_entry(LAPIC_REGS, v->vcpu_id, h, vcpu_vlapic(v)->regs);
}
diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c
index b292e79382..88188d2d7f 100644
--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/io.c
@@ -341,7 +341,7 @@ int pt_irq_create_bind(
{
uint8_t dest, delivery_mode;
bool dest_mode;
-int dest_vcpu_id;
+int dest_vcpu_id, prev_vcpu_id = -1;
const struct vcpu *vcpu;
uint32_t gflags = pt_irq_bind->u.msi.gflags &
~XEN_DOMCTL_VMSI_X86_UNMASKED;
@@ -411,6 +411,7 @@ int pt_irq_create_bind(
pirq_dpci->gmsi.gvec = pt_irq_bind->u.msi.gvec;
pirq_dpci->gmsi.gflags = gflags;
+prev_vcpu_id = pirq_dpci->gmsi.dest_vcpu_id;
}
}
/* Calculate dest_vcpu_id for MSI-type pirq migration. */
@@ -440,7 +441,8 @@ int pt_irq_create_bind(
/* Use interrupt posting if it is supported. */
if ( iommu_intpost )
pi_update_irte(vcpu ? >arch.hvm.vmx.pi_desc : NULL,
- info, pirq_dpci->gmsi.gvec);
+ info, pirq_dpci->gmsi.gvec,
+ prev_vcpu_id >= 0 ? d->vcpu[prev_vcpu_id] : NULL);
if ( pt_irq_bind->u.msi.gflags & XEN_DOMCTL_VMSI_X86_UNMASKED )
{
@@ -729,7 +731,9 @@ int pt_irq_destroy_bind(
what = "bogus";
}
else if ( pirq_dpci && pirq_dpci->gmsi.posted )
-pi_update_irte(NULL, pirq, 0);
+pi_update_irte(NULL, pirq, 0,
+ pirq_dpci->gmsi.dest_vcpu_id >= 0
+ ? d->vcpu[pirq_dpci->gmsi.dest_vcpu_id] : NULL);
if ( pirq_dpci && (pirq_dpci->flags & HVM_IRQ_DPCI_MAPPED) &&
list_empty(_dpci->digl_list) )
diff --git a/xen/drivers/passthrough/vtd/intremap.c
b/xen/drivers/passthrough/vtd/intremap.c
index bf846195c4..0f364e81b0 100644
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -946,12 +946,13 @@ void intel_iommu_disable_eim(void)
disable_qinval(drhd->iommu);
}
+#if CONFIG_HVM
/*
* This function is used to update the IRTE for posted-interrupt
* when guest changes MSI/MSI-X information.
*/
int pi_update_irte(const struct pi_desc *pi_desc, const struct pirq *pirq,
-const uint8_t gvec)
+const uint8_t gvec, struct vcpu *prev)
{
struct irq_desc *desc;
struct msi_desc *msi_desc;
@@ -964,8 +965,8 @@ int pi_update_irte(const struct pi_desc *pi_desc, const
struct pirq *pirq,
msi_desc = desc->msi_desc;
if ( !msi_desc )
{
-rc =
Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it
On 08.10.2019 18:29, Marek Marczykowski-Górecki wrote: > On Tue, Oct 08, 2019 at 04:19:13PM +0200, Jan Beulich wrote: >> On 08.10.2019 15:52, Marek Marczykowski-Górecki wrote: >>> Regardless of SetVirtualAddressMap() discussion, I propose to >>> automatically map boot services code/data, to make Xen work on more >>> machines (even if _we_ consider those buggy). >> >> I remain on my prior position: Adding command line triggerable >> workarounds for such cases is fine. Defaulting to assume buggy >> firmware is acceptable only if this means no extra penalty to >> systems with conforming firmware. Hence, for the case at hand, >> I object to doing this automatically; we already have the >> /mapbs workaround in place to deal with the case when xen.efi >> is used. Judging from the title here there may need to be an >> addition to also allow triggering this from the MB2 boot path. > > What about mirroring Linux behavior? I.e. mapping those regions for the > SetVirtualAddressMap() time (when enabled) and unmapping after - unless > tagged with EFI_MEMORY_RUNTIME? > Similarly to Andrew, I'd really prefer for Xen to work out of the box, > with as little as possible manual tweaks needed. If there's going to be a config where SetVirtualAddressMap() is to be called - why not? But the same logic doesn't make sense when such a call won#t happen in the first place. > Then I've tried a different approach: call SetVirtualAddressMap(), but > with an address map that tries to pretend physical addressing (the code > under #ifndef USE_SET_VIRTUAL_ADDRESS_MAP). This mostly worked, I needed > only few changes: > - set VirtualStart back to PhysicalStart in that memory map (it was set >to directmap) > - map boot services (at least for the SetVirtualAddressMap() call time, >but haven't tried unmapping it later) > - call SetVirtualAddressMap() with that "1:1" map in place, using >efi_rs_enter/efi_rs_leave. > > This fixed the issue for me, now runtime services do work even without > disabling ExitBootServices() call. And without any extra > platform-specific command line arguments. And I think it also shouldn't > break > kexec, since it uses 1:1-like map, but I haven't tried. One should > simply ignore EFI_UNSUPPORTED return code (I don't know how to avoid the > call at all after kexec). That's the point - it can't be avoided, and hence it failing is not an option. Or else there needs to be a protocol telling kexec what it is to expect, and that it in particular can't change the virtual address map to its liking. Back at the time when I put together the EFI booting code, no such protocol existed, and hence calling SetVirtualAddressMap() was not an option. I have no idea whether things have changed in the meantime. >>> >>> Hmm, how is it different from the current situation? Not calling >>> SetVirtualAddressMap() means UEFI will not be notified about new address >>> map. It does _not_ mean it will use 1:1 map, it will use what was >>> previously set. >> >> What do you mean by "previously set"? SetVirtualAddressMap() can be >> invoked only once during a given session (i.e. without intervening >> boot). How would other than a 1:1 map have got set? > > Like, in the very next sentence of my answer: > >>> What if Xen was kexec'ed from Linux? Honestly - I'm getting tired. You said yourself ... >>> In Linux case, it looks like it passes around the EFI memory map using >>> some Linux-specific mechanism, but I don't find it particularly >>> appealing option. ... that this would require Xen following a Linux protocol. This is nothing that can work building on EFI interfaces alone. >>> What about something in between: make this SetVirtualAddressMap() call >>> compile-time option (kconfig), depending on !CONFIG_KEXEC ? And when >>> enabled, properly handle SetVirtualAddressMap() failure. >> >> What is "proper handling" here? > > Logging the error and either panic or disabling runtime services (I tend > to the latter). Hmm, yes, disabling runtime services in this case makes sense. But are you sure a SetVirtualAddressMap() failure doesn't incur other potential issues later on? (Calling panic() is what I'd rather not call "proper handling", but rather "emergency handling".) Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] x86/mm: don't needlessly veto migration
On Wed, 9 Oct 2019 at 09:49, Jan Beulich wrote: > > On 08.10.2019 18:38, Andrew Cooper wrote: > > On 08/10/2019 17:10, Jan Beulich wrote: > >> From: Paul Durrant > >> > >> Now that xl.cfg has an option to explicitly enable IOMMU mappings for a > >> domain, migration may be needlessly vetoed due to the check of > >> is_iommu_enabled() in paging_log_dirty_enable(). > >> There is actually no need to prevent logdirty from being enabled unless > >> devices are assigned to a domain. > >> > >> NOTE: While in the neighbourhood, the bool_t parameter type in > >> paging_log_dirty_enable() is replaced with a bool and the format > >> of the comment in assign_device() is fixed. > >> > >> Signed-off-by: Paul Durrant > >> Signed-off-by: Jan Beulich > >> Release-acked-by: Juergen Gross > > > > Seriously FFS. Why am I having to repeat myself? What if any way > > unclear on the previous threads? > > > > NACK NACK NACK. Xen is, and has always been, the wrong place to have > > any logic, because IT DOESN'T HAVE ENOUGH INFORMATION TO MAKE THE > > DECISION CORRECTLY. > > > > The toolstack does. > > > > Therefore, the toolstack is the only level capable decide whether it is > > safe to migration/suspend/resume/checkpoint the VM. > > > > If I have to write the patches myself, I will, but this patch in this > > form is frankly unacceptable. > > You're kidding, aren't you? By taking only part of Paul's original > patch, we should be able to get rid of two of the current osstest > reported regressions. At the same time this _does not_ exclude an > incremental subsequent patch to also add the other half (see my > reply to him yesterday suggesting this split). The two steps > shouldn't have been merged into a single patch anyway imo: The > part here fixes a regression, while the other part changes original > behavior, and continues to be (irrespective of your wording, which > once again suggests that in certain cases you aren't willing to > tolerate thinking that's different from yours) controversial. > > If it helps, I can change the title (and perhaps description) to > make it look less like the original patch, and to put focus on the > regression. I just didn't want to massage it more than absolutely > needed. Agreed. Given where we are w.r.t. regressions and a release schedule, I think we need to be pragmatic. Realistically I'm not going get a Xen dev. environment up and running for maybe a week so I can't work on this myself at the moment. I am happy for Jan to fix the regressions and then we can move on after 4.13 is out the door. Paul > > Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-linus test] 142431: regressions - FAIL
flight 142431 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/142431/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-freebsd10-i386 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 133580 test-amd64-i386-examine 8 reboot fail REGR. vs. 133580 test-amd64-i386-qemuu-rhel6hvm-intel 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-libvirt 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-qemut-rhel6hvm-intel 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemut-win7-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-freebsd10-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-ovmf-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-debianhvm-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-qemut-rhel6hvm-amd 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 133580 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 133580 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-libvirt-xsm 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-xsm7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemut-ws16-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemut-debianhvm-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemut-win10-i386 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-pvshim 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-ws16-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-win7-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 133580 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 133580 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-shadow 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-win10-i386 7 xen-boot fail REGR. vs. 133580 test-arm64-arm64-examine11 examine-serial/bootloader fail REGR. vs. 133580 test-amd64-amd64-xl-pvshim 20 guest-start/debian.repeat fail REGR. vs. 133580 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 7 xen-boot fail baseline untested test-amd64-i386-xl-qemut-debianhvm-i386-xsm 7 xen-boot fail baseline untested test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 133580 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 133580 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 133580 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 133580 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 133580 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 133580 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-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-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-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 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-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail
Re: [Xen-devel] [PATCH v3] x86/mm: don't needlessly veto migration
On 08.10.2019 18:38, Andrew Cooper wrote: > On 08/10/2019 17:10, Jan Beulich wrote: >> From: Paul Durrant >> >> Now that xl.cfg has an option to explicitly enable IOMMU mappings for a >> domain, migration may be needlessly vetoed due to the check of >> is_iommu_enabled() in paging_log_dirty_enable(). >> There is actually no need to prevent logdirty from being enabled unless >> devices are assigned to a domain. >> >> NOTE: While in the neighbourhood, the bool_t parameter type in >> paging_log_dirty_enable() is replaced with a bool and the format >> of the comment in assign_device() is fixed. >> >> Signed-off-by: Paul Durrant >> Signed-off-by: Jan Beulich >> Release-acked-by: Juergen Gross > > Seriously FFS. Why am I having to repeat myself? What if any way > unclear on the previous threads? > > NACK NACK NACK. Xen is, and has always been, the wrong place to have > any logic, because IT DOESN'T HAVE ENOUGH INFORMATION TO MAKE THE > DECISION CORRECTLY. > > The toolstack does. > > Therefore, the toolstack is the only level capable decide whether it is > safe to migration/suspend/resume/checkpoint the VM. > > If I have to write the patches myself, I will, but this patch in this > form is frankly unacceptable. You're kidding, aren't you? By taking only part of Paul's original patch, we should be able to get rid of two of the current osstest reported regressions. At the same time this _does not_ exclude an incremental subsequent patch to also add the other half (see my reply to him yesterday suggesting this split). The two steps shouldn't have been merged into a single patch anyway imo: The part here fixes a regression, while the other part changes original behavior, and continues to be (irrespective of your wording, which once again suggests that in certain cases you aren't willing to tolerate thinking that's different from yours) controversial. If it helps, I can change the title (and perhaps description) to make it look less like the original patch, and to put focus on the regression. I just didn't want to massage it more than absolutely needed. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2] pci: clear {host/guest}_maskall field on assign
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?
---
Changes since v1:
- Also set guest_maskall.
- Place the code in a helper function in x86/msi.c
- Add a comment to describe the expected state.
- Test that maskall is not set on hardware.
---
xen/arch/x86/msi.c| 22 ++
xen/drivers/passthrough/pci.c | 5 +
xen/include/asm-x86/msi.h | 1 +
3 files changed, 28 insertions(+)
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index 76d4034c4f..c239a00fb1 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -1249,6 +1249,28 @@ void pci_cleanup_msi(struct pci_dev *pdev)
msi_free_irqs(pdev);
}
+int pci_reset_msix_state(struct pci_dev *pdev)
+{
+unsigned int pos = pci_find_cap_offset(pdev->seg, pdev->bus,
pdev->sbdf.dev,
+ pdev->sbdf.fn, PCI_CAP_ID_MSIX);
+
+ASSERT(pos);
+/*
+ * Xen expects the device state to be the after reset one, and hence
+ * host_maskall = guest_maskall = false and all entries should have the
+ * mask bit set. Test that the maskall bit is not set, having it set could
+ * signal that the device hasn't been reset properly.
+ */
+if ( pci_conf_read16(pdev->sbdf, msix_control_reg(pos)) &
+ PCI_MSIX_FLAGS_MASKALL )
+return -EBUSY;
+
+pdev->msix->host_maskall = false;
+pdev->msix->guest_maskall = false;
+
+return 0;
+}
+
int pci_msi_conf_write_intercept(struct pci_dev *pdev, unsigned int reg,
unsigned int size, uint32_t *data)
{
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 90ccb8370b..bdcc482d81 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1505,7 +1505,12 @@ static int assign_device(struct domain *d, u16 seg, u8
bus, u8 devfn, u32 flag)
}
if ( pdev->msix )
+{
+rc = pci_reset_msix_state(pdev);
+if ( rc )
+goto done;
msixtbl_init(d);
+}
pdev->fault.count = 0;
diff --git a/xen/include/asm-x86/msi.h b/xen/include/asm-x86/msi.h
index d0b0045d0d..6e35713ec7 100644
--- a/xen/include/asm-x86/msi.h
+++ b/xen/include/asm-x86/msi.h
@@ -92,6 +92,7 @@ extern int __setup_msi_irq(struct irq_desc *, struct msi_desc
*,
extern void teardown_msi_irq(int irq);
extern int msi_free_vector(struct msi_desc *entry);
extern int pci_restore_msi_state(struct pci_dev *pdev);
+extern int pci_reset_msix_state(struct pci_dev *pdev);
struct msi_desc {
struct msi_attrib {
--
2.23.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] /sys/hypervisor entries for Xen (Domain-0, PV, PVH and HVM)
On Wed, 9 Oct 2019, Steven Haigh wrote: > For now, the only big issue that remains is that the current pygrub will > always boot the second image in the list due to pygrub incorrectly parsing the > failover sections of the Fedora grub.cfg where the failover will set > 'default=1' causing this behaviour. I did post a very hacky patch to improve this to xen-devel and should have a more acceptable set of patches for this soon. Michael Young ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] read grubenv and set default from saved_entry or next_entry [and 1 more messages]
> On 7 Oct 2019, at 11:01, Ian Jackson wrote:
>
> Hi. Thanks for the message.
>
> Just one thing I wanted to reply to in your mail:
>
> YOUNG, MICHAEL A. writes ("Re: [Xen-devel] [PATCH] read grubenv and set
> default from saved_entry or next_entry [and 1 more messages]"):
>> On Wed, 11 Sep 2019, Ian Jackson wrote:
>>> I find this filename hackery rather unprincipled. I'm not entirely
>>> sure I can see a better way, given the way cfg_list is constructed.
>>> Can you think of a less hacky approach ?
> ...
>> One option would be to do a fresh search for grubenv in the same places
>> we looked for grub.cfg.
>>
>> Alternatively, it should be possible to do a more precise edit using
>> f.rpartition("/").
>
> I don't feel I fully understand the implications, but either of these
> seems like they might be good strategies to me. I guess the former
> risks finding an unrelated leftover grubenv somewhere which is
> probably not desirable.
>
> Anyway, I will leave this to your judgement.
>
> Thanks for the rest of your comments. I'll look forward to a revised
> set of patches - thanks.
Hi all,
I am assuming there is no chance that this will make 4.13 with the freeze
having passed.
But in any case, I wasn't sure whether Michael strictly will need it as the
change can presumably be carried in a fedora patch q for Xen packages until it
ends up upstream
Lars
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] /sys/hypervisor entries for Xen (Domain-0, PV, PVH and HVM)
Thanks Michael, In the meantime, we're looking at just disabling BLS by default in the grub packages within Fedora when its run on a Xen guest. This means we should at least be at a point where Fedora guests will work reliably again as Xen guests. It seems to be agreed that this will stay in place until such point where pygrub understands BLS and this no longer becomes an issue - and likely there'd be some overlap to let the updated pygrub spread as far as possible before yanking out this workaround. For now, the only big issue that remains is that the current pygrub will always boot the second image in the list due to pygrub incorrectly parsing the failover sections of the Fedora grub.cfg where the failover will set 'default=1' causing this behaviour. Assuming that the Fedora side is resolved, and we always get a non-BLS grub.cfg in a Fedora guest, is there a simpler fix that could be included before Xen 4.13 gets launched (and hopefully backported)? I'm not sure if the proposed changes to Fedora makes this a little simpler in fixing the entire issue. (apologies for top posting, Geary doesn't seem to like letting me bottom post!) Steven Haigh net...@crc.id.au https://www.crc.id.au +613 9001 6090 +614 1293 5897 On Wed, Oct 9, 2019 at 09:00, M A Young wrote: On Wed, 9 Oct 2019, Steven Haigh wrote: Hi all, I'm working on fixing up the grub packages for Fedora in deducing the new BLS logic in Fedora and disabling it in non-compatible environments. BZ Report: https://bugzilla.redhat.com/show_bug.cgi?id=1703700 Currently, it seems that we can deduce the following two scenarios: in /sys/hypervisor: 1) type == xen && uuid == all zeros, then this is BLS safe (the Domain-0). 2) type == xen && uuid != all zeros, then this is BLS *unsafe* (covers PV, HVM and PVH guests). Is there any other variables that come into effect that could cause a variation in the above checks as to enable or disable BLS? Right now, I'm proposing that we try to disable the new BLS behaviour in Fedora for PV, HVM and PVH guests - as pygrub is not up to the task of booting them. We included HVM as it may be common for users to switch between HVM and PVH configurations for the same installed VM. I do have a long term plan to try to get pygrub to handle BLS, though I don't expect to have it working soon. Michael Young ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] /sys/hypervisor entries for Xen (Domain-0, PV, PVH and HVM)
On Wed, 9 Oct 2019, Steven Haigh wrote: > Hi all, > > I'm working on fixing up the grub packages for Fedora in deducing the new BLS > logic in Fedora and disabling it in non-compatible environments. > > BZ Report: > https://bugzilla.redhat.com/show_bug.cgi?id=1703700 > > Currently, it seems that we can deduce the following two scenarios: > > in /sys/hypervisor: > > 1) type == xen && uuid == all zeros, then this is BLS safe (the Domain-0). > 2) type == xen && uuid != all zeros, then this is BLS *unsafe* (covers PV, HVM > and PVH guests). > > Is there any other variables that come into effect that could cause a > variation in the above checks as to enable or disable BLS? > > Right now, I'm proposing that we try to disable the new BLS behaviour in > Fedora for PV, HVM and PVH guests - as pygrub is not up to the task of booting > them. We included HVM as it may be common for users to switch between HVM and > PVH configurations for the same installed VM. I do have a long term plan to try to get pygrub to handle BLS, though I don't expect to have it working soon. Michael Young ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.19 test] 142436: tolerable FAIL - PUSHED
flight 142436 linux-4.19 real [real] http://logs.test-lab.xenproject.org/osstest/logs/142436/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 142323 test-amd64-i386-xl-pvshim12 guest-start 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-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-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-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-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail 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 13 migrate-support-checkfail never pass test-arm64-arm64-xl 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-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail 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-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail 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-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-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail 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-amd64-i386-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-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail 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-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop 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 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass version targeted for testing: linux58fce20645303bee01d74144ec00e405be43b1ca baseline version: linux6cad9d0cf87b95b10f3f4d7826c2c15e45e2a277 Last test of basis 142323 2019-10-05 11:40:01 Z3 days Testing same since 142407 2019-10-07 17:10:56 Z1 days2 attempts People who
