Re: [PATCH 19/30] panic: Add the panic hypervisor notifier list
Hi Guilherme, +Desmond On 2022-05-17 09:42, Guilherme G. Piccoli wrote: On 17/05/2022 10:57, Petr Mladek wrote: [...] --- a/drivers/misc/bcm-vk/bcm_vk_dev.c +++ b/drivers/misc/bcm-vk/bcm_vk_dev.c @@ -1446,7 +1446,7 @@ static int bcm_vk_probe(struct pci_dev *pdev, const struct pci_device_id *ent) [... snip ...] It seems to reset some hardware or so. IMHO, it should go into the pre-reboot list. Mixed feelings here, I'm looping Broadcom maintainers to comment. (CC Scott and Broadcom list) I'm afraid it breaks kdump if this device is not reset beforehand - it's a doorbell write, so not high risk I think... But in case the not-reset device can be probed normally in kdump kernel, then I'm fine in moving this to the reboot list! I don't have the HW to test myself. Good question. Well, it if has to be called before kdump then even "hypervisor" list is a wrong place because is not always called before kdump. Agreed! I'll defer that to Scott and Broadcom folks to comment. If it's not strictly necessary, I'll happily move it to the reboot list. If necessary, we could use the machine_crash_kexec() approach, but we'll fall into the case arm64 doesn't support it and I'm not sure if this device is available for arm - again a question for the maintainers. We register to the panic notifier so that we can kill the VK card ASAP to stop DMAing things over to the host side. If it is not notified then memory may not be frozen when kdump is occurring. Notifying the card on panic is also needed to allow for any type of reset to occur. So, the only thing preventing moving the notifier later is the chance that memory is modified while kdump is occurring. Or, if DMA is disabled before kdump already then this wouldn't be an issue and the notification to the card (to allow for clean resets) can be done later. [...] --- a/drivers/power/reset/ltc2952-poweroff.c +++ b/drivers/power/reset/ltc2952-poweroff.c [...] This is setting a variable only, and once it's set (data->kernel_panic is the bool's name), it just bails out the IRQ handler and a timer setting - this timer seems kinda tricky, so bailing out ASAP makes sense IMHO. IMHO, the timer informs the hardware that the system is still alive in the middle of panic(). If the timer is not working then the hardware (chip) will think that the system frozen in panic() and will power off the system. See the comments in drivers/power/reset/ltc2952-poweroff.c: [ snip ...] IMHO, we really have to keep it alive until we reach the reboot stage. Another question is how it actually works when the interrupts are disabled during panic() and the timer callbacks are not handled. Agreed here! Guess I can move this one the reboot list, fine by me. Unless PM folks think otherwise. [...] Disagree here, I'm CCing Florian for information. This notifier preserves RAM so it's *very interesting* if we have kmsg_dump() for example, but maybe might be also relevant in case kdump kernel is configured to store something in a persistent RAM (then, without this notifier, after kdump reboots the system data would be lost). I see. It is actually similar problem as with drivers/firmware/google/gsmi.c. I does similar things like kmsg_dump() so it should be called in the same location (after info notifier list and before kdump). A solution might be to put it at these notifiers at the very end of the "info" list or make extra "dump" notifier list. Here I still disagree. I've commented in the other response thread (about Google gsmi) about the semantics of the hypervisor list, but again: this list should contain callbacks that (a) Should run early, _by default_ before a kdump; (b) Communicate with the firmware/hypervisor in a "low-risk" way; Imagine a scenario where users configure kdump kernel to save something in a persistent form in DRAM - it'd be like a late pstore, in the next kernel. This callback enables that, it's meant to inform FW "hey, panic happened, please from now on don't clear the RAM in the next FW-reboot". I don't see a reason to postpone that - let's see if the maintainers have an opinion. Cheers, Guilherme smime.p7s Description: S/MIME Cryptographic Signature
[ovmf test] 170559: regressions - FAIL
flight 170559 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170559/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 2189c71026cba9dab768776a2f8ebf689b484c56 baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 79 days Failing since168258 2022-03-01 01:55:31 Z 79 days 1094 attempts Testing same since 170557 2022-05-19 03:10:24 Z0 days3 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kun Qin Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Liu, Zhiguang Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min M Xu Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Tom Lendacky Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhiguang Liu Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 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 6713 lines long.)
[qemu-mainline test] 170542: tolerable FAIL - PUSHED
flight 170542 qemu-mainline real [real] flight 170555 qemu-mainline real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/170542/ http://logs.test-lab.xenproject.org/osstest/logs/170555/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-install fail pass in 170555-retest test-armhf-armhf-libvirt-qcow2 13 guest-start fail pass in 170555-retest Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail in 170555 like 170499 test-armhf-armhf-libvirt-qcow2 14 migrate-support-check fail in 170555 never pass test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 170499 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 170499 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 170499 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 170499 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 170499 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 170499 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 170499 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: qemuubcf0a3a422cd5d1b1c3c09c0e161205837dbe131 baseline version: qemuu
[xen-unstable-smoke test] 170553: tolerable all pass - PUSHED
flight 170553 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/170553/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass version targeted for testing: xen 43aa3f6e72d340a85d3943b86350f6196a87289c baseline version: xen c8040aefe66edb9a9f9a54cc0541a77b4103c9f9 Last test of basis 170543 2022-05-18 15:03:19 Z0 days Testing same since 170553 2022-05-19 01:03:09 Z0 days1 attempts People who touched revisions under test: Bertrand Marquis Julien Grall Michal Orzel Stefano Stabellini Stefano Stabellini Tamas K Lengyel 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 c8040aefe6..43aa3f6e72 43aa3f6e72d340a85d3943b86350f6196a87289c -> smoke
[ovmf test] 170558: regressions - FAIL
flight 170558 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170558/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 2189c71026cba9dab768776a2f8ebf689b484c56 baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 79 days Failing since168258 2022-03-01 01:55:31 Z 79 days 1093 attempts Testing same since 170557 2022-05-19 03:10:24 Z0 days2 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kun Qin Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Liu, Zhiguang Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min M Xu Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Tom Lendacky Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhiguang Liu Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 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 6713 lines long.)
RE: [PATCH v3 8/9] xen/x86: add detection of memory interleaves for different nodes
Hi Jan,
> -Original Message-
> From: Jan Beulich
> Sent: 2022年5月18日 21:31
> To: Wei Chen
> Cc: nd ; Andrew Cooper ; Roger Pau
> Monné ; Wei Liu ; Jiamei Xie
> ; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH v3 8/9] xen/x86: add detection of memory interleaves
> for different nodes
>
> On 11.05.2022 03:46, Wei Chen wrote:
> > --- a/xen/arch/x86/srat.c
> > +++ b/xen/arch/x86/srat.c
> > @@ -42,6 +42,12 @@ static struct node node_memblk_range[NR_NODE_MEMBLKS];
> > static nodeid_t memblk_nodeid[NR_NODE_MEMBLKS];
> > static __initdata DECLARE_BITMAP(memblk_hotplug, NR_NODE_MEMBLKS);
> >
> > +enum conflicts {
> > + NO_CONFLICT = 0,
>
> No need for the "= 0".
>
Ack.
> > + ERR_OVERLAP,
>
> While this at least can be an error (the self-overlap case is merely
> warned about), ...
>
> > + ERR_INTERLEAVE,
>
> ... I don't think this is, and hence I'd recommend to drop "ERR_".
>
Oh, yes. I all drop it for all above enumerations.
> > @@ -119,20 +125,43 @@ int valid_numa_range(paddr_t start, paddr_t end,
> nodeid_t node)
> > return 0;
> > }
> >
> > -static __init int conflicting_memblks(paddr_t start, paddr_t end)
> > +static enum conflicts __init
> > +conflicting_memblks(nodeid_t nid, paddr_t start, paddr_t end,
> > + paddr_t nd_start, paddr_t nd_end, int *mblkid)
>
> Why "int"? Can the value passed back be negative?
>
The caller "acpi_numa_memory_affinity_init" defines int for node memory
block id, and as a return value at the same time. So I haven't changed it.
As we don't use this "int" for return value any more, I agree, it will
never be negative, I would fix it.
> > {
> > int i;
> >
> > + /*
> > +* Scan all recorded nodes' memory blocks to check conflicts:
> > +* Overlap or interleave.
> > +*/
> > for (i = 0; i < num_node_memblks; i++) {
> > struct node *nd = _memblk_range[i];
> > + *mblkid = i;
>
> Style: Please maintain a blank line between declaration(s) and
> statement(s).
>
Ok.
> > @@ -310,42 +342,67 @@ acpi_numa_memory_affinity_init(const struct
> acpi_srat_mem_affinity *ma)
> > bad_srat();
> > return;
> > }
> > +
> > + /*
> > +* For the node that already has some memory blocks, we will
> > +* expand the node memory range temporarily to check memory
> > +* interleaves with other nodes. We will not use this node
> > +* temp memory range to check overlaps, because it will mask
> > +* the overlaps in same node.
> > +*
> > +* Node with 0 bytes memory doesn't need this expandsion.
> > +*/
> > + nd_start = start;
> > + nd_end = end;
> > + nd = [node];
> > + if (nd->start != nd->end) {
> > + if (nd_start > nd->start)
> > + nd_start = nd->start;
> > +
> > + if (nd_end < end)
>
> Did you mean nd->end here on the right side of < ? By intentionally
Oh! thanks for pointing out this one! Yes, right side should be nd->end.
> not adding "default:" in the body, you then also allow the compiler
> to point out that addition of a new enumerator also needs handling
> here.
>
Did you mean, we need to add if ... else ... in this block? If yes,
is it ok to update this block like:
if (nd->start != nd->end) {
nd_start = min(nd_start, nd->start);
nd_end = max(nd_end, nd->end);
}
?
> > + nd_end = nd->end;
> > + }
> > +
> > /* It is fine to add this area to the nodes data it will be used
> later*/
> > - i = conflicting_memblks(start, end);
> > - if (i < 0)
> > - /* everything fine */;
> > - else if (memblk_nodeid[i] == node) {
> > - bool mismatch = !(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) !=
> > - !test_bit(i, memblk_hotplug);
> > -
> > - printk("%sSRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr") overlaps with
> itself (%"PRIpaddr"-%"PRIpaddr")\n",
> > - mismatch ? KERN_ERR : KERN_WARNING, pxm, start, end,
> > - node_memblk_range[i].start, node_memblk_range[i].end);
> > - if (mismatch) {
> > + status = conflicting_memblks(node, start, end, nd_start, nd_end, );
> > + if (status == ERR_OVERLAP) {
>
> Please use switch(status) when checking enumerated values.
>
Ok, I will do it.
> > + if (memblk_nodeid[i] == node) {
> > + bool mismatch = !(ma->flags &
> > + ACPI_SRAT_MEM_HOT_PLUGGABLE) !=
> > + !test_bit(i, memblk_hotplug);
>
> Style: The middle line wants indenting by two more characters.
>
Yes, I will do it.
> > +
> > + printk("%sSRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr")
> overlaps with itself (%"PRIpaddr"-%"PRIpaddr")\n",
> > + mismatch ? KERN_ERR : KERN_WARNING, pxm, start,
> > + end, node_memblk_range[i].start,
> > + node_memblk_range[i].end);
> > +
[ovmf test] 170557: regressions - FAIL
flight 170557 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170557/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 2189c71026cba9dab768776a2f8ebf689b484c56 baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 79 days Failing since168258 2022-03-01 01:55:31 Z 79 days 1092 attempts Testing same since 170557 2022-05-19 03:10:24 Z0 days1 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kun Qin Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Liu, Zhiguang Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min M Xu Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Tom Lendacky Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhiguang Liu Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 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 6713 lines long.)
[ovmf test] 170556: regressions - FAIL
flight 170556 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170556/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 708620d29db89d03e822b8d17dc75fbac865c6dc baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 79 days Failing since168258 2022-03-01 01:55:31 Z 79 days 1091 attempts Testing same since 170392 2022-05-13 15:40:22 Z5 days 110 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kun Qin Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Liu, Zhiguang Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min M Xu Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Tom Lendacky Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhiguang Liu Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 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 6662 lines long.)
RE: [PATCH v3 9/9] xen/x86: use INFO level for node's without memory log message
Hi Jan, > -Original Message- > From: Jan Beulich > Sent: 2022年5月18日 21:34 > To: Wei Chen > Cc: nd ; Andrew Cooper ; Roger Pau > Monné ; Wei Liu ; xen- > de...@lists.xenproject.org > Subject: Re: [PATCH v3 9/9] xen/x86: use INFO level for node's without > memory log message > > On 11.05.2022 03:46, Wei Chen wrote: > > In previous code, Xen was using KERN_WARNING for log message > > when Xen found a node without memory. Xen will print this > > warning message, and said that this may be an BIOS Bug or > > mis-configured hardware. But actually, this warning is bogus, > > because in an NUMA setting, nodes can only have processors, > > and with 0 bytes memory. So it is unreasonable to warn about > > BIOS or hardware corruption based on the detection of node > > with 0 bytes memory. > > > > So in this patch, we remove the warning messages, but just > > keep an info message to info users that there is one or more > > nodes with 0 bytes memory in the system. > > > > Signed-off-by: Wei Chen > > Reviewed-by: Jan Beulich > preferably with ... > > > --- a/xen/arch/x86/srat.c > > +++ b/xen/arch/x86/srat.c > > @@ -549,8 +549,7 @@ int __init acpi_scan_nodes(paddr_t start, paddr_t > end) > > uint64_t size = nodes[i].end - nodes[i].start; > > > > if ( size == 0 ) > > - printk(KERN_WARNING "SRAT: Node %u has no memory. " > > - "BIOS Bug or mis-configured hardware?\n", i); > > + printk(KERN_INFO "SRAT: Node %u has no memory.\n", i); > > ... the full stop also dropped (and maybe the upper-case N converted to > lower-case). > Ok, I will do it in next version. > Jan
RE: [PATCH v3 2/9] xen: reuse x86 EFI stub functions for Arm
Hi Jan, > -Original Message- > From: Jan Beulich > Sent: 2022年5月18日 21:05 > To: Wei Chen > Cc: nd ; Stefano Stabellini ; Julien > Grall ; Bertrand Marquis ; > Volodymyr Babchuk ; Andrew Cooper > ; Roger Pau Monné ; Wei > Liu ; Jiamei Xie ; xen- > de...@lists.xenproject.org > Subject: Re: [PATCH v3 2/9] xen: reuse x86 EFI stub functions for Arm > > On 11.05.2022 03:46, Wei Chen wrote: > > x86 is using compiler feature testing to decide EFI build > > enable or not. When EFI build is disabled, x86 will use an > > efi/stub.c file to replace efi/runtime.c for build objects. > > Following this idea, we introduce a stub file for Arm, but > > use CONFIG_ARM_EFI to decide EFI build enable or not. > > > > And the most functions in x86 EFI stub.c can be reused for > > other architectures, like Arm. So we move them to common > > and keep the x86 specific function in x86/efi/stub.c. > > > > To avoid the symbol link conflict error when linking common > > stub files to x86/efi. We add a regular file check in efi > > stub files' link script. Depends on this check we can bypass > > the link behaviors for existed stub files in x86/efi. > > > > As there is no Arm specific EFI stub function for Arm in > > current stage, Arm still can use the existed symbol link > > method for EFI stub files. > > Wouldn't it be better to mandate that every arch has its stub.c, > and in the Arm one all you'd do (for now) is #include the common > one? (But see also below.) > Personally, I don't like to include a C file into another C file. But I am OK as long as the Arm maintainers agree. @Stefano Stabellini @Bertrand Marquis @Julien Grall > > --- a/xen/arch/arm/Makefile > > +++ b/xen/arch/arm/Makefile > > @@ -1,6 +1,5 @@ > > obj-$(CONFIG_ARM_32) += arm32/ > > obj-$(CONFIG_ARM_64) += arm64/ > > -obj-$(CONFIG_ARM_64) += efi/ > > obj-$(CONFIG_ACPI) += acpi/ > > obj-$(CONFIG_HAS_PCI) += pci/ > > ifneq ($(CONFIG_NO_PLAT),y) > > @@ -20,6 +19,7 @@ obj-y += domain.o > > obj-y += domain_build.init.o > > obj-y += domctl.o > > obj-$(CONFIG_EARLY_PRINTK) += early_printk.o > > +obj-y += efi/ > > obj-y += gic.o > > obj-y += gic-v2.o > > obj-$(CONFIG_GICV3) += gic-v3.o > > diff --git a/xen/arch/arm/efi/Makefile b/xen/arch/arm/efi/Makefile > > index 4313c39066..dffe72e589 100644 > > --- a/xen/arch/arm/efi/Makefile > > +++ b/xen/arch/arm/efi/Makefile > > @@ -1,4 +1,12 @@ > > include $(srctree)/common/efi/efi-common.mk > > > > +ifeq ($(CONFIG_ARM_EFI),y) > > obj-y += $(EFIOBJ-y) > > obj-$(CONFIG_ACPI) += efi-dom0.init.o > > +else > > +# Add stub.o to EFIOBJ-y to re-use the clean-files in > > +# efi-common.mk. Otherwise the link of stub.c in arm/efi > > +# will not be cleaned in "make clean". > > +EFIOBJ-y += stub.o > > +obj-y += stub.o > > +endif > > I realize Stefano indicated he's happy with the Arm side, but I still > wonder: What use is the stub on Arm32? Even further - once you have a > config option (rather than x86'es build-time check plus x86'es dual- > purposing of all object files), why do you need a stub in the first > place? You ought to be able to deal with things via inline functions > and macros, I would think. > We will use efi_enabled() on some common codes of Arm. In the last version, I had used static inline function, but that will need an CONFIG_EFI in xen/efi.h to gate the definitions of EFI functions, otherwise we just can implement the efi_enabled in non-static-inline way. Or use another name to wrapper efi_enabled. (patch#20, 21) But as x86 has its own way to decide EFI build or not, the CONFIG_EFI has been rejected. In this case, we use CONFIG_ARM_EFI for Arm itself. For CONFIG_ARM_EFI, it's impossible to be used in xen/efi.h to gate definitions. So if I want to use macros or static-inline functions, I need to use #ifdef CONFIG_ARM_EFI in everywhere to gate xen/efi.h. Or use another header file to warpper xen/efi.h. > > --- a/xen/common/efi/efi-common.mk > > +++ b/xen/common/efi/efi-common.mk > > @@ -9,7 +9,8 @@ CFLAGS-y += -iquote $(srcdir) > > # e.g.: It transforms "dir/foo/bar" into successively > > # "dir foo bar", ".. .. ..", "../../.." > > $(obj)/%.c: $(srctree)/common/efi/%.c FORCE > > - $(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst > /, ,$(obj/source/common/efi/$( > + $(Q)test -f $@ || \ > > + ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst > /, ,$(obj/source/common/efi/$( > Please can you indent the "ln" to match "test", such that it's easily > visible (without paying attention to line continuation characters) > that these two lines are a single command? > Yeah, of course, I will do it. > Jan
[ovmf test] 170554: regressions - FAIL
flight 170554 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170554/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 708620d29db89d03e822b8d17dc75fbac865c6dc baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 79 days Failing since168258 2022-03-01 01:55:31 Z 79 days 1090 attempts Testing same since 170392 2022-05-13 15:40:22 Z5 days 109 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kun Qin Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Liu, Zhiguang Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min M Xu Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Tom Lendacky Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhiguang Liu Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 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 6662 lines long.)
[ovmf test] 170552: regressions - FAIL
flight 170552 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170552/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 708620d29db89d03e822b8d17dc75fbac865c6dc baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 79 days Failing since168258 2022-03-01 01:55:31 Z 78 days 1089 attempts Testing same since 170392 2022-05-13 15:40:22 Z5 days 108 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kun Qin Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Liu, Zhiguang Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min M Xu Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Tom Lendacky Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhiguang Liu Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 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 6662 lines long.)
Re: [PATCH V2 5/7] dt-bindings: Add xen,dev-domid property description for xen-grant DMA ops
On Thu, 19 May 2022, Oleksandr wrote:
> > On Wed, May 18, 2022 at 5:06 PM Oleksandr wrote:
> > > On 18.05.22 17:32, Arnd Bergmann wrote:
> > > > On Sat, May 7, 2022 at 7:19 PM Oleksandr Tyshchenko
> > > > wrote:
> > > >This would mean having a device
> > > > node for the grant-table mechanism that can be referred to using the
> > > > 'iommus'
> > > > phandle property, with the domid as an additional argument.
> > > I assume, you are speaking about something like the following?
> > >
> > >
> > > xen_dummy_iommu {
> > > compatible = "xen,dummy-iommu";
> > > #iommu-cells = <1>;
> > > };
> > >
> > > virtio@3000 {
> > > compatible = "virtio,mmio";
> > > reg = <0x3000 0x100>;
> > > interrupts = <41>;
> > >
> > > /* The device is located in Xen domain with ID 1 */
> > > iommus = <_dummy_iommu 1>;
> > > };
> > Right, that's that's the idea,
>
> thank you for the confirmation
>
>
>
> > except I would not call it a 'dummy'.
> > From the perspective of the DT, this behaves just like an IOMMU,
> > even if the exact mechanism is different from most hardware IOMMU
> > implementations.
>
> well, agree
>
>
> >
> > > > It does not quite fit the model that Linux currently uses for iommus,
> > > > as that has an allocator for dma_addr_t space
> > > yes (# 3/7 adds grant-table based allocator)
> > >
> > >
> > > > , but it would think it's
> > > > conceptually close enough that it makes sense for the binding.
> > > Interesting idea. I am wondering, do we need an extra actions for this
> > > to work in Linux guest (dummy IOMMU driver, etc)?
> > It depends on how closely the guest implementation can be made to
> > resemble a normal iommu. If you do allocate dma_addr_t addresses,
> > it may actually be close enough that you can just turn the grant-table
> > code into a normal iommu driver and change nothing else.
>
> Unfortunately, I failed to find a way how use grant references at the
> iommu_ops level (I mean to fully pretend that we are an IOMMU driver). I am
> not too familiar with that, so what is written below might be wrong or at
> least not precise.
>
> The normal IOMMU driver in Linux doesn’t allocate DMA addresses by itself, it
> just maps (IOVA-PA) what was requested to be mapped by the upper layer. The
> DMA address allocation is done by the upper layer (DMA-IOMMU which is the glue
> layer between DMA API and IOMMU API allocates IOVA for PA?). But, all what we
> need here is just to allocate our specific grant-table based DMA addresses
> (DMA address = grant reference + offset in the page), so let’s say we need an
> entity to take a physical address as parameter and return a DMA address (what
> actually commit #3/7 is doing), and that’s all. So working at the dma_ops
> layer we get exactly what we need, with the minimal changes to guest
> infrastructure. In our case the Xen itself acts as an IOMMU.
>
> Assuming that we want to reuse the IOMMU infrastructure somehow for our needs.
> I think, in that case we will likely need to introduce a new specific IOVA
> allocator (alongside with a generic one) to be hooked up by the DMA-IOMMU
> layer if we run on top of Xen. But, even having the specific IOVA allocator to
> return what we indeed need (DMA address = grant reference + offset in the
> page) we will still need the specific minimal required IOMMU driver to be
> present in the system anyway in order to track the mappings(?) and do nothing
> with them, returning a success (this specific IOMMU driver should have all
> mandatory callbacks implemented).
>
> I completely agree, it would be really nice to reuse generic IOMMU bindings
> rather than introducing Xen specific property if what we are trying to
> implement in current patch series fits in the usage of "iommus" in Linux
> more-less. But, if we will have to add more complexity/more components to the
> code for the sake of reusing device tree binding, this raises a question
> whether that’s worthwhile.
>
> Or I really missed something?
I think Arnd was primarily suggesting to reuse the IOMMU Device Tree
bindings, not necessarily the IOMMU drivers framework in Linux (although
that would be an added bonus.)
I know from previous discussions with you that making the grant table
fit in the existing IOMMU drivers model is difficult, but just reusing
the Device Tree bindings seems feasible?
Re: [PATCH 2/2] x86/vmx: implement Notify VM Exit
On 17/05/2022 14:21, Roger Pau Monne wrote:
> Under certain conditions guests can get the CPU stuck in an infinite
> loop without the possibility of an interrupt window to occur.
instruction boundary.
It's trivial to create an infinite loop without an interrupt window :)
Also, I'd probably phrase that as an unbounded loop, because not all
problem cases are truly infinite.
> This
> was the case with the scenarios described in XSA-156.
Case in point, both of these can be broken by something else (another
vCPU, or coherent DMA write) editing the IDT and e.g. making the #AC/#DB
vectors not present, which will yield #NP instead.
>
> Make use of the Notify VM Exit mechanism, that will trigger a VM Exit
> if no interrupt window occurs for a specified amount of time. Note
> that using the Notify VM Exit avoids having to trap #AC and #DB
> exceptions, as Xen is guaranteed to get a VM Exit even if the guest
> puts the CPU in a loop without an interrupt window, as such disable
> the intercepts if the feature is available and enabled.
>
> Setting the notify VM exit window to 0 is safe because there's a
> threshold added by the hardware in order to have a sane window value.
>
> Suggested-by: Andrew Cooper
> Signed-off-by: Roger Pau Monné
> ---
> This change enables the notify VM exit by default, KVM however doesn't
> seem to enable it by default, and there's the following note in the
> commit message:
>
> "- There's a possibility, however small, that a notify VM exit happens
>with VM_CONTEXT_INVALID set in exit qualification. In this case, the
>vcpu can no longer run. To avoid killing a well-behaved guest, set
>notify window as -1 to disable this feature by default."
>
> It's not obviously clear to me whether the comment was meant to be:
> "There's a possibility, however small, that a notify VM exit _wrongly_
> happens with VM_CONTEXT_INVALID".
TBH, I read that as a get-out clause for "we have no idea what to set
for a default window", and it's not a decision reasonable to defer to
users, because they have even less of an idea than us.
All CPUs with Notify VM Exit have the TSC crystal information in CPUID,
so I'd suggest that we trust CPUID to be accurate, and program for maybe
10us? That's 1/3 of a default timeslice.
> It's also not clear whether such wrong hardware behavior only affects
> a specific set of hardware, in a way that we could avoid enabling
> notify VM exit there.
>
> There's a discussion in one of the Linux patches that 128K might be
> the safer value in order to prevent false positives, but I have no
> formal confirmation about this. Maybe our Intel maintainers can
> provide some more feedback on a suitable notify VM exit window
> value.
>
> I've tested with 0 (the proposed default in the patch) and I don't
> seem to be able to trigger notify VM exits under normal guest
> operation. Note that even in that case the guest won't be destroyed
> unless the context is corrupt.
Huh... There's nothing in the manual about that, but obviously hardware
has some minimum safe value if 0 appears to work in practice.
> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> index d388e6729c..5685a5523e 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -67,6 +67,9 @@ integer_param("ple_gap", ple_gap);
> static unsigned int __read_mostly ple_window = 4096;
> integer_param("ple_window", ple_window);
>
> +static int __read_mostly vm_notify_window;
> +integer_param("vm-notify-window", vm_notify_window);
Part of me is loath to keep on adding new top-level options for this.
I was about to suggest having a vmx= option, but I've just noticed that
ple_{window,gap} are wired up to cmdline options on Intel, and fixed
constants on AMD.
Thoughts on a suitable name?
> @@ -1333,6 +1338,19 @@ static int construct_vmcs(struct vcpu *v)
> rc = vmx_add_msr(v, MSR_FLUSH_CMD, FLUSH_CMD_L1D,
> VMX_MSR_GUEST_LOADONLY);
>
> +if ( cpu_has_vmx_notify_vm_exiting && vm_notify_window >= 0 )
> +{
> +__vmwrite(NOTIFY_WINDOW, vm_notify_window);
> +/*
> + * Disable #AC and #DB interception: by using VM Notify Xen is
> + * guaranteed to get a VM exit even if the guest manages to lock the
> + * CPU.
> + */
> +v->arch.hvm.vmx.exception_bitmap &= ~((1U << TRAP_debug) |
> + (1U << TRAP_alignment_check));
> +vmx_update_exception_bitmap(v);
IIRC, it's not quite this easy. There are conditions, e.g. attaching
gdbsx, where #DB interception wants turning on/off dynamically, and the
logic got simplified to nothing following XSA-156, so will need
reintroducing.
AMD Milan (Zen3) actually has NoNestedDataBp in CPUID.8021.eax[0]
which allows us to not intercept #DB, so perhaps that might offer an
easier way of adjusting the interception logic. (Or maybe not. I can't
remember).
> +}
> +
> out:
>
Re: [PATCH V2 5/7] dt-bindings: Add xen,dev-domid property description for xen-grant DMA ops
On 18.05.22 21:59, Rob Herring wrote:
Hello Rob, Arnd
On Wed, May 18, 2022 at 03:32:27PM +0100, Arnd Bergmann wrote:
On Sat, May 7, 2022 at 7:19 PM Oleksandr Tyshchenko wrote:
diff --git a/Documentation/devicetree/bindings/virtio/mmio.yaml
b/Documentation/devicetree/bindings/virtio/mmio.yaml
index 10c22b5..29a0932 100644
--- a/Documentation/devicetree/bindings/virtio/mmio.yaml
+++ b/Documentation/devicetree/bindings/virtio/mmio.yaml
@@ -13,6 +13,9 @@ description:
See https://www.oasis-open.org/committees/tc_home.php?wg_abbrev=virtio for
more details.
+allOf:
+ - $ref: /schemas/arm/xen,dev-domid.yaml#
+
properties:
compatible:
const: virtio,mmio
@@ -33,6 +36,10 @@ properties:
description: Required for devices making accesses thru an IOMMU.
maxItems: 1
+ xen,dev-domid:
+description: Required when Xen grant mappings need to be enabled for
device.
+$ref: /schemas/types.yaml#/definitions/uint32
+
required:
- compatible
- reg
Sorry for joining the discussion late. Have you considered using the
generic iommu
binding here instead of a custom property? This would mean having a device
node for the grant-table mechanism that can be referred to using the 'iommus'
phandle property, with the domid as an additional argument.
It does not quite fit the model that Linux currently uses for iommus,
as that has an allocator for dma_addr_t space, but it would think it's
conceptually close enough that it makes sense for the binding.
Something common is almost always better.
agree
That may also have the issue that fw_devlink will make the 'iommu'
driver a dependency to probe.
Looks like I ran into it while experimenting. I generated the following
nodes in guest DT using Xen toolstack:
[snip]
xen_dummy_iommu {
compatible = "xen,dummy-iommu";
#iommu-cells = <0x01>;
phandle = <0xfde9>;
};
virtio@200 {
compatible = "virtio,mmio";
reg = <0x00 0x200 0x00 0x200>;
interrupts = <0x00 0x01 0xf01>;
interrupt-parent = <0xfde8>;
dma-coherent;
iommus = <0xfde9 0x01>;
};
[snip]
And got:
virtio-mmio 200.virtio: deferred probe timeout, ignoring dependency
Rob
--
Regards,
Oleksandr Tyshchenko
Re: [PATCH V2 5/7] dt-bindings: Add xen,dev-domid property description for xen-grant DMA ops
On 18.05.22 19:39, Arnd Bergmann wrote:
Hello Arnd
On Wed, May 18, 2022 at 5:06 PM Oleksandr wrote:
On 18.05.22 17:32, Arnd Bergmann wrote:
On Sat, May 7, 2022 at 7:19 PM Oleksandr Tyshchenko wrote:
This would mean having a device
node for the grant-table mechanism that can be referred to using the 'iommus'
phandle property, with the domid as an additional argument.
I assume, you are speaking about something like the following?
xen_dummy_iommu {
compatible = "xen,dummy-iommu";
#iommu-cells = <1>;
};
virtio@3000 {
compatible = "virtio,mmio";
reg = <0x3000 0x100>;
interrupts = <41>;
/* The device is located in Xen domain with ID 1 */
iommus = <_dummy_iommu 1>;
};
Right, that's that's the idea,
thank you for the confirmation
except I would not call it a 'dummy'.
From the perspective of the DT, this behaves just like an IOMMU,
even if the exact mechanism is different from most hardware IOMMU
implementations.
well, agree
It does not quite fit the model that Linux currently uses for iommus,
as that has an allocator for dma_addr_t space
yes (# 3/7 adds grant-table based allocator)
, but it would think it's
conceptually close enough that it makes sense for the binding.
Interesting idea. I am wondering, do we need an extra actions for this
to work in Linux guest (dummy IOMMU driver, etc)?
It depends on how closely the guest implementation can be made to
resemble a normal iommu. If you do allocate dma_addr_t addresses,
it may actually be close enough that you can just turn the grant-table
code into a normal iommu driver and change nothing else.
Unfortunately, I failed to find a way how use grant references at the
iommu_ops level (I mean to fully pretend that we are an IOMMU driver). I
am not too familiar with that, so what is written below might be wrong
or at least not precise.
The normal IOMMU driver in Linux doesn’t allocate DMA addresses by
itself, it just maps (IOVA-PA) what was requested to be mapped by the
upper layer. The DMA address allocation is done by the upper layer
(DMA-IOMMU which is the glue layer between DMA API and IOMMU API
allocates IOVA for PA?). But, all what we need here is just to allocate
our specific grant-table based DMA addresses (DMA address = grant
reference + offset in the page), so let’s say we need an entity to take
a physical address as parameter and return a DMA address (what actually
commit #3/7 is doing), and that’s all. So working at the dma_ops layer
we get exactly what we need, with the minimal changes to guest
infrastructure. In our case the Xen itself acts as an IOMMU.
Assuming that we want to reuse the IOMMU infrastructure somehow for our
needs. I think, in that case we will likely need to introduce a new
specific IOVA allocator (alongside with a generic one) to be hooked up
by the DMA-IOMMU layer if we run on top of Xen. But, even having the
specific IOVA allocator to return what we indeed need (DMA address =
grant reference + offset in the page) we will still need the specific
minimal required IOMMU driver to be present in the system anyway in
order to track the mappings(?) and do nothing with them, returning a
success (this specific IOMMU driver should have all mandatory callbacks
implemented).
I completely agree, it would be really nice to reuse generic IOMMU
bindings rather than introducing Xen specific property if what we are
trying to implement in current patch series fits in the usage of
"iommus" in Linux more-less. But, if we will have to add more
complexity/more components to the code for the sake of reusing device
tree binding, this raises a question whether that’s worthwhile.
Or I really missed something?
Arnd
--
Regards,
Oleksandr Tyshchenko
Re: [PATCH 1/2] x86/vmx: implement Bus Lock detection
On 17/05/2022 14:21, Roger Pau Monne wrote:
> Add support for enabling Bus Lock Detection on Intel systems. Such
> detection works by triggering a vmexit, which is enough of a pause to
> prevent a guest from abusing of the Bus Lock.
"which is enough of a" is a bit firmer than ideal. "which Andy says
will be ok" is perhaps more accurate.
Perhaps "which ought to be enough" ?
A buslock here or there is no problem, and non-malicious software
appears to be devoid of buslocks (hardly surprising - it would be a hard
error on other architectures), but a malicious piece of userspace can
trivially cripple the system.
Forcing a vmexit on every buslock limits an attacker to one buslock per
however long a vmentry/exit cycle takes.
> Add an extra perf counter to track the number of Bus Locks detected.
extra Xen perf counter.
Because other hypervisors use actual perf counters to emulate this
ability on current hardware. Maybe something we should consider...
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index d03e78bf0d..02cc7a2023 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -4053,6 +4053,16 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>
> if ( unlikely(exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY) )
> return vmx_failed_vmentry(exit_reason, regs);
> +if ( unlikely(exit_reason & VMX_EXIT_REASONS_BUS_LOCK) )
> +{
> +/*
> + * Delivery of Bus Lock VM exit was pre-empted by a higher priority
> VM
> + * exit.
> + */
> +exit_reason &= ~VMX_EXIT_REASONS_BUS_LOCK;
> +if ( exit_reason != EXIT_REASON_BUS_LOCK )
> +perfc_incr(buslock);
I'm pretty sure you can drop the if, and do the perfc_incr()
unconditionally. You won't get EXIT_REASON_BUS_LOCK |
VMX_EXIT_REASONS_BUS_LOCK given that wording in the ISE.
To test, Intel has PENDING_DBG which interferes with most easy attempts
to create the condition, but how about this.
Load an LDT, misaligned across a cacheline boundary, and set #DB's %cs
to LDT[0] with a clear access bit, then execute an `icebp` instruction.
The atomic write to set the access bit is a 4-byte access typically.
This should cause the #DB intercept to trigger on the same instantaneous
boundary that generated the buslock.
Otherwise, LGTM.
~Andrew
[ovmf test] 170549: regressions - FAIL
flight 170549 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170549/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 708620d29db89d03e822b8d17dc75fbac865c6dc baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 79 days Failing since168258 2022-03-01 01:55:31 Z 78 days 1088 attempts Testing same since 170392 2022-05-13 15:40:22 Z5 days 107 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kun Qin Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Liu, Zhiguang Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min M Xu Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Tom Lendacky Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhiguang Liu Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 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 6662 lines long.)
[xen-unstable-smoke test] 170543: tolerable all pass - PUSHED
flight 170543 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/170543/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass version targeted for testing: xen c8040aefe66edb9a9f9a54cc0541a77b4103c9f9 baseline version: xen 25c160a74f4489f031ac79a24078cc12efd5c96b Last test of basis 170535 2022-05-18 10:01:52 Z0 days Testing same since 170543 2022-05-18 15:03:19 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Christian Lindig Luca Fancellu Roger Pau Monne Roger Pau Monné Stefano Stabellini jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 25c160a74f..c8040aefe6 c8040aefe66edb9a9f9a54cc0541a77b4103c9f9 -> smoke
Re: [XEN][RFC PATCH v3 11/14] xen/arm: Implement device tree node addition functionalities
Hi Vikram,
On 08/03/2022 19:47, Vikram Garhwal wrote:
Update sysctl XEN_SYSCTL_dt_overlay to enable support for dtbo nodes addition
using device tree overlay.
xl overlay add file.dtbo:
Each time overlay nodes are added using .dtbo, a new fdt(memcpy of
device_tree_flattened) is created and updated with overlay nodes. This
updated fdt is further unflattened to a dt_host_new. Next, it checks if any
of the overlay nodes already exists in the dt_host. If overlay nodes
doesn't
exist then find the overlay nodes in dt_host_new, find the overlay node's
parent in dt_host and add the nodes as child under their parent in the
dt_host. The node is attached as the last node under target parent.
Finally, add IRQs, add device to IOMMUs, set permissions and map MMIO for
the
overlay node.
When a node is added using overlay, a new entry is allocated in the
overlay_track to keep the track of memory allocation due to addition of overlay
node. This is helpful for freeing the memory allocated when a device tree node
is removed.
Signed-off-by: Vikram Garhwal
---
xen/common/dt_overlay.c | 324
xen/include/public/sysctl.h | 1 +
2 files changed, 325 insertions(+)
diff --git a/xen/common/dt_overlay.c b/xen/common/dt_overlay.c
index fcb31de495..01aed62d74 100644
--- a/xen/common/dt_overlay.c
+++ b/xen/common/dt_overlay.c
@@ -82,6 +82,64 @@ static int dt_overlay_remove_node(struct dt_device_node
*device_node)
return 0;
}
+static int dt_overlay_add_node(struct dt_device_node *device_node,
+ const char *parent_node_path)
+{
+struct dt_device_node *parent_node;
+struct dt_device_node *np;
+struct dt_device_node *next_node;
+struct dt_device_node *new_node;
+
+parent_node = dt_find_node_by_path(parent_node_path);
+
+new_node = device_node;
You only use device_node to set new_node. So the local variable is a bit
pointless. I don't mind whether you want to rename the parameter
new_node or keep the existing name.
+
+if ( new_node == NULL )
OOI, how could it be NULL?
+return -EINVAL;
+
+if ( parent_node == NULL )
+{
+dt_dprintk("Node not found. Partial dtb will not be added");
+return -EINVAL;
+}
+
+/*
+ * If node is found. We can attach the device_node as a child of the
Below you use new_node rather than device_node
+ * parent node.
+ */
+
+/* If parent has no child. */
+if ( parent_node->child == NULL )
+{
+next_node = parent_node->allnext;
+new_node->parent = parent_node;
+parent_node->allnext = new_node;
+parent_node->child = new_node;
+/* Now plug next_node at the end of device_node. */
Same.
+new_node->allnext = next_node;
+} else {
Coding style:
The } and { should be on there own line. I.e:
}
else
{
+/* If parent has at least one child node. */
+
+/*
+ * Iterate to the last child node of parent.
+ */
+for ( np = parent_node->child; np->sibling != NULL; np = np->sibling )
+{
+}
+
+next_node = np->allnext;
+new_node->parent = parent_node;
+np->sibling = new_node;
+np->allnext = new_node;
+/* Now plug next_node at the end of device_node. */
Same remark about device node.
+new_node->sibling = next_node;
+new_node->allnext = next_node;
+np->sibling->sibling = NULL;
+}
+
+return 0;
+}
+
/* Basic sanity check for the dtbo tool stack provided to Xen. */
static int check_overlay_fdt(const void *overlay_fdt, uint32_t
overlay_fdt_size)
{
@@ -377,6 +435,267 @@ out:
return rc;
}
+/*
+ * Adds device tree nodes under target node.
+ * We use dt_host_new to unflatten the updated device_tree_flattened. This is
+ * done to avoid the removal of device_tree generation, iomem regions mapping
to
+ * hardware domain done by handle_node().
+ */
+static long handle_add_overlay_nodes(void *overlay_fdt,
+ uint32_t overlay_fdt_size)
+{
+int rc = 0;
+struct dt_device_node *overlay_node;
+char **nodes_full_path = NULL;
+int **nodes_irq = NULL;
+int *node_num_irq = NULL;
+void *fdt = NULL;
+struct dt_device_node *dt_host_new = NULL;
+struct domain *d = hardware_domain;
+struct overlay_track *tr = NULL;
+unsigned int naddr;
+unsigned int num_irq;
+unsigned int i, j, k;
+unsigned int num_overlay_nodes;
+u64 addr, size;
+
+fdt = xmalloc_bytes(fdt_totalsize(device_tree_flattened));
+if ( fdt == NULL )
+return -ENOMEM;
+
+num_overlay_nodes = overlay_node_count(overlay_fdt);
+if ( num_overlay_nodes == 0 )
+{
+xfree(fdt);
+return -ENOMEM;
+}
+
+spin_lock(_lock);
+
+memcpy(fdt, device_tree_flattened, fdt_totalsize(device_tree_flattened));
+
+rc = check_overlay_fdt(overlay_fdt,
Re: [PATCH V2 5/7] dt-bindings: Add xen,dev-domid property description for xen-grant DMA ops
On Wed, May 18, 2022 at 03:32:27PM +0100, Arnd Bergmann wrote: > On Sat, May 7, 2022 at 7:19 PM Oleksandr Tyshchenko > wrote: > > > > diff --git a/Documentation/devicetree/bindings/virtio/mmio.yaml > > b/Documentation/devicetree/bindings/virtio/mmio.yaml > > index 10c22b5..29a0932 100644 > > --- a/Documentation/devicetree/bindings/virtio/mmio.yaml > > +++ b/Documentation/devicetree/bindings/virtio/mmio.yaml > > @@ -13,6 +13,9 @@ description: > >See https://www.oasis-open.org/committees/tc_home.php?wg_abbrev=virtio > > for > >more details. > > > > +allOf: > > + - $ref: /schemas/arm/xen,dev-domid.yaml# > > + > > properties: > >compatible: > > const: virtio,mmio > > @@ -33,6 +36,10 @@ properties: > > description: Required for devices making accesses thru an IOMMU. > > maxItems: 1 > > > > + xen,dev-domid: > > +description: Required when Xen grant mappings need to be enabled for > > device. > > +$ref: /schemas/types.yaml#/definitions/uint32 > > + > > required: > >- compatible > >- reg > > Sorry for joining the discussion late. Have you considered using the > generic iommu > binding here instead of a custom property? This would mean having a device > node for the grant-table mechanism that can be referred to using the 'iommus' > phandle property, with the domid as an additional argument. > > It does not quite fit the model that Linux currently uses for iommus, > as that has an allocator for dma_addr_t space, but it would think it's > conceptually close enough that it makes sense for the binding. Something common is almost always better. That may also have the issue that fw_devlink will make the 'iommu' driver a dependency to probe. Rob
Re: [XEN][RFC PATCH v3 10/14] xen/arm: Implement device tree node removal functionalities
Hi Vikram,
On 08/03/2022 19:47, Vikram Garhwal wrote:
Introduce sysctl XEN_SYSCTL_dt_overlay to remove device-tree nodes added using
device tree overlay.
xl overlay remove file.dtbo:
Removes all the nodes in a given dtbo.
First, removes IRQ permissions and MMIO accesses. Next, it finds the nodes
in dt_host and delete the device node entries from dt_host.
The nodes get removed only if it is not used by any of dom0 or domio.
Also, added overlay_track struct to keep the track of added node through device
tree overlay. overlay_track has dt_host_new which is unflattened form of updated
fdt and name of overlay nodes. When a node is removed, we also free the memory
used by overlay_track for the particular overlay node.
Signed-off-by: Vikram Garhwal
---
xen/common/Makefile | 1 +
xen/common/dt_overlay.c | 447 +++
xen/common/sysctl.c | 10 +
xen/include/public/sysctl.h | 18 ++
xen/include/xen/dt_overlay.h | 47
5 files changed, 523 insertions(+)
create mode 100644 xen/common/dt_overlay.c
create mode 100644 xen/include/xen/dt_overlay.h
diff --git a/xen/common/Makefile b/xen/common/Makefile
index dc8d3a13f5..2eb5734f8e 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -54,6 +54,7 @@ obj-y += wait.o
obj-bin-y += warning.init.o
obj-$(CONFIG_XENOPROF) += xenoprof.o
obj-y += xmalloc_tlsf.o
+obj-$(CONFIG_OVERLAY_DTB) += dt_overlay.o
obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma lzo unlzo unlz4 unzstd earlycpio,$(n).init.o)
diff --git a/xen/common/dt_overlay.c b/xen/common/dt_overlay.c
new file mode 100644
index 00..fcb31de495
--- /dev/null
+++ b/xen/common/dt_overlay.c
@@ -0,0 +1,447 @@
+/*
+ * xen/common/dt_overlay.c
+ *
+ * Device tree overlay support in Xen.
+ *
+ * Copyright (c) 2021 Xilinx Inc.
+ * Written by Vikram Garhwal
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms and conditions of the GNU General Public
+ * License, version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+#include
+#include
+#include
+#include
+#include
+
+static LIST_HEAD(overlay_tracker);
+static DEFINE_SPINLOCK(overlay_lock);
+
+static int dt_overlay_remove_node(struct dt_device_node *device_node)
+{
+struct dt_device_node *np;
+struct dt_device_node *parent_node;
+struct dt_device_node *current_node;
+
+parent_node = device_node->parent;
+
+current_node = parent_node;
+
+if ( parent_node == NULL )
+{
+dt_dprintk("%s's parent node not found\n", device_node->name);
+return -EFAULT;
+}
+
+np = parent_node->child;
+
+if ( np == NULL )
+{
+dt_dprintk("parent node %s's not found\n", parent_node->name);
+return -EFAULT;
+}
+
+/* If node to be removed is only child node or first child. */
+if ( !dt_node_cmp(np->full_name, device_node->full_name) )
+{
+current_node->allnext = np->allnext;
While reviewing the previous patches, I realized that we have nothing to
prevent someone to browse the device-tree while it is modified.
I am not sure this can be solved with just refcounting (like Linux
does). So maybe we need a read-write-lock. I am open to other
suggestions here.
+
+/* If node is first child but not the only child. */
+if ( np->sibling != NULL )
+current_node->child = np->sibling;
+else
+/* If node is only child. */
+current_node->child = NULL;
Those 4 lines can be replaced with one line:
current_node->child = np->sibling;
+return 0;
+}
+
+for ( np = parent_node->child; np->sibling != NULL; np = np->sibling )
+{
+current_node = np;
+if ( !dt_node_cmp(np->sibling->full_name, device_node->full_name) )
+{
+/* Found the node. Now we remove it. */
+current_node->allnext = np->allnext->allnext;
I find this code quite confusing to read. AFAICT, 'np' and
'current_node' are exactly the same here. Why do you use different name
to access it?
+
+if ( np->sibling->sibling )
+current_node->sibling = np->sibling->sibling;
+else
+current_node->sibling = NULL;
Same here. This could be replaced with:
current_node->child = nb->sibling->sibling;
+
+break;
+}
+}
+
+return 0;
+}
+
+/* Basic sanity check for the dtbo tool stack provided to Xen. */
+static int check_overlay_fdt(const void *overlay_fdt, uint32_t
overlay_fdt_size)
+{
+if ( (fdt_totalsize(overlay_fdt) != overlay_fdt_size) ||
+ fdt_check_header(overlay_fdt) )
+{
+
[ovmf test] 170547: regressions - FAIL
flight 170547 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170547/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 708620d29db89d03e822b8d17dc75fbac865c6dc baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 79 days Failing since168258 2022-03-01 01:55:31 Z 78 days 1087 attempts Testing same since 170392 2022-05-13 15:40:22 Z5 days 106 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kun Qin Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Liu, Zhiguang Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min M Xu Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Tom Lendacky Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhiguang Liu Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 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 6662 lines long.)
Re: [PATCH v5 1/2] ns16550: use poll mode if INTERRUPT_LINE is 0xff
On Tue, May 17, 2022 at 05:46:07PM +0200, Jan Beulich wrote: > On 17.05.2022 17:43, Roger Pau Monné wrote: > > On Tue, May 17, 2022 at 05:13:46PM +0200, Jan Beulich wrote: > >> On 17.05.2022 16:48, Roger Pau Monné wrote: > >>> On Tue, May 17, 2022 at 04:41:31PM +0200, Jan Beulich wrote: > On 11.05.2022 16:30, Marek Marczykowski-Górecki wrote: > > --- a/xen/drivers/char/ns16550.c > > +++ b/xen/drivers/char/ns16550.c > > @@ -1238,6 +1238,13 @@ pci_uart_config(struct ns16550 *uart, bool_t > > skip_amt, unsigned int idx) > > pci_conf_read8(PCI_SBDF(0, b, d, f), > > PCI_INTERRUPT_LINE) : 0; > > > > +if ( uart->irq == 0xff ) > > +uart->irq = 0; > > +if ( !uart->irq ) > > +printk(XENLOG_INFO > > + "ns16550: %pp no legacy IRQ, using poll > > mode\n", > > + _SBDF(0, b, d, f)); > > + > > return 0; > > } > > } > > While this code is inside a CONFIG_HAS_PCI conditional, I still > think - as was previously suggested - that the 1st if() should be > inside a CONFIG_X86 conditional, to not leave a trap for other > architectures to fall into. > >>> > >>> The CONFIG_HAS_PCI region is itself inside of a (bigger) CONFIG_X86 > >>> region already. > >> > >> But that's likely to change sooner or later, I expect. I'd rather see > >> the surrounding region be shrunk in scope. Already when that > >> CONFIG_X86 was introduced I had reservations, as I don't think all of > >> the enclosed code really is x86-specific. > > > > My though was that anyone removing the CONFIG_X86 guard will already > > have to deal with setting ->irq properly, as I expect this will differ > > between arches, at which point the check are likely to diverge anyway. > > Hmm, true. What I would really like (and what I should have spelled out) > is that the build would fail if this code was enabled for no-x86, such > that it ends up very obvious that something needs doing there. Hence ... > > > In any case, I don't see an issue with adding an extra guard, albeit a > > comment would also be acceptable IMO. > > ... maybe > > #ifdef CONFIG_X86 > ... > #else > # error > #endif The whole section was wrapped in CONFIG_X86, so I haven't added it once again. But if that's desirable, I can wrap the 0xff IRQ handling in yet another CONFIG_X86 guard (since the spec says this value is x86 specific). I don't think having #error in non-x86 case makes much sense here. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab signature.asc Description: PGP signature
[PATCH v6] Preserve the EFI System Resource Table for dom0
The EFI System Resource Table (ESRT) is necessary for fwupd to identify
firmware updates to install. According to the UEFI specification §23.4,
the ESRT shall be stored in memory of type EfiBootServicesData. However,
memory of type EfiBootServicesData is considered general-purpose memory
by Xen, so the ESRT needs to be moved somewhere where Xen will not
overwrite it. Copy the ESRT to memory of type EfiRuntimeServicesData,
which Xen will not reuse. dom0 can use the ESRT if (and only if) it is
in memory of type EfiRuntimeServicesData.
Earlier versions of this patch reserved the memory in which the ESRT was
located. This created awkward alignment problems, and required either
splitting the E820 table or wasting memory. It also would have required
a new platform op for dom0 to use to indicate if the ESRT is reserved.
By copying the ESRT into EfiRuntimeServicesData memory, the E820 table
does not need to be modified, and dom0 can just check the type of the
memory region containing the ESRT. The copy is only done if the ESRT is
not already in EfiRuntimeServicesData memory, avoiding memory leaks on
repeated kexec.
See https://lore.kernel.org/xen-devel/20200818184018.GN1679@mail-itl/T/
for details.
Signed-off-by: Demi Marie Obenour
---
Changes since v5:
- Use type and field names from EFI specification.
- Remove tags from EFI types that do not have them.
- Use PrintErr() instead of PrintStr() for error conditions.
Changes since v4:
- Do not call blexit() (via PrintErrMsg()) if the new ESRT cannot be
allocated or installed.
- Use the correct comment style.
- Remove leftover change from earlier version.
- Add a blank line between declarations and statements.
- Re-fetch the memory map after allocating the ESRT copy.
Changes since v3:
- Do not modify the memory map.
- Allocate a copy of the ESRT in EfiRuntimeServicesData() memory.
xen/common/efi/boot.c | 108 +-
1 file changed, 106 insertions(+), 2 deletions(-)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index a25e1d29f1..6a829b8278 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -39,6 +39,26 @@
{ 0x605dab50, 0xe046, 0x4300, {0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b,
0x23} }
#define APPLE_PROPERTIES_PROTOCOL_GUID \
{ 0x91bd12fe, 0xf6c3, 0x44fb, { 0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a,
0xe0} }
+#define EFI_SYSTEM_RESOURCE_TABLE_GUID\
+ { 0xb122a263, 0x3661, 0x4f68, {0x99, 0x29, 0x78, 0xf8, 0xb0, 0xd6, 0x21,
0x80} }
+#define EFI_SYSTEM_RESOURCE_TABLE_FIRMWARE_RESOURCE_VERSION 1
+
+typedef struct {
+EFI_GUID FwClass;
+UINT32 FwType;
+UINT32 FwVersion;
+UINT32 LowestSupportedFwVersion;
+UINT32 CapsuleFlags;
+UINT32 LastAttemptVersion;
+UINT32 LastAttemptStatus;
+} EFI_SYSTEM_RESOURCE_ENTRY;
+
+typedef struct {
+UINT32 FwResourceCount;
+UINT32 FwResourceCountMax;
+UINT64 FwResourceVersion;
+EFI_SYSTEM_RESOURCE_ENTRY Entries[];
+} EFI_SYSTEM_RESOURCE_TABLE;
typedef EFI_STATUS
(/* _not_ EFIAPI */ *EFI_SHIM_LOCK_VERIFY) (
@@ -567,6 +587,39 @@ static int __init efi_check_dt_boot(const EFI_LOADED_IMAGE
*loaded_image)
}
#endif
+static UINTN __initdata esrt = EFI_INVALID_TABLE_ADDR;
+
+static size_t __init get_esrt_size(const EFI_MEMORY_DESCRIPTOR *desc)
+{
+size_t available_len, len;
+const UINTN physical_start = desc->PhysicalStart;
+const EFI_SYSTEM_RESOURCE_TABLE *esrt_ptr;
+
+len = desc->NumberOfPages << EFI_PAGE_SHIFT;
+if ( esrt == EFI_INVALID_TABLE_ADDR )
+return 0;
+if ( physical_start > esrt || esrt - physical_start >= len )
+return 0;
+/*
+ * The specification requires EfiBootServicesData, but accept
+ * EfiRuntimeServicesData, which is a more logical choice.
+ */
+if ( (desc->Type != EfiRuntimeServicesData) &&
+ (desc->Type != EfiBootServicesData) )
+return 0;
+available_len = len - (esrt - physical_start);
+if ( available_len <= offsetof(EFI_SYSTEM_RESOURCE_TABLE, Entries) )
+return 0;
+available_len -= offsetof(EFI_SYSTEM_RESOURCE_TABLE, Entries);
+esrt_ptr = (const EFI_SYSTEM_RESOURCE_TABLE *)esrt;
+if ( esrt_ptr->FwResourceVersion !=
EFI_SYSTEM_RESOURCE_TABLE_FIRMWARE_RESOURCE_VERSION ||
+ !esrt_ptr->FwResourceCount )
+return 0;
+if ( esrt_ptr->FwResourceCount > available_len /
sizeof(esrt_ptr->Entries[0]) )
+return 0;
+return esrt_ptr->FwResourceCount * sizeof(esrt_ptr->Entries[0]);
+}
+
/*
* Include architecture specific implementation here, which references the
* static globals defined above.
@@ -845,6 +898,8 @@ static UINTN __init
efi_find_gop_mode(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop,
return gop_mode;
}
+static EFI_GUID __initdata esrt_guid = EFI_SYSTEM_RESOURCE_TABLE_GUID;
+
static void __init efi_tables(void)
{
unsigned int i;
@@ -868,6 +923,8 @@ static void __init efi_tables(void)
efi.smbios = (unsigned
Re: [PATCH 2/3] tools/libxc: change xc_memshr_fork_reset API to match hypervisor
On Wed, May 18, 2022 at 11:48 AM Jan Beulich wrote: > > On 18.05.2022 17:01, Tamas K Lengyel wrote: > > On Thu, May 12, 2022 at 9:46 AM Tamas K Lengyel > > wrote: > >> > >> On Thu, May 5, 2022 at 4:27 AM Roger Pau Monné > >> wrote: > >>> > >>> On Wed, Apr 27, 2022 at 11:34:19AM -0400, Tamas K Lengyel wrote: > Need to separately specify if the reset is for the memory or for the VM > state, > or both. > > Signed-off-by: Tamas K Lengyel > >>> > >>> Reviewed-by: Roger Pau Monné > >> > >> Patch ping. Can this patch be merged please? > > > > Patch ping. > > Your mail (and I guess also your earlier one) was _To_ Roger, which > is odd since he already did provide R-b. What you're missing is a > tool stack maintainer ack aiui, so it may help if you send your > pings _To_ the respective people. True, but all the toolstack maintainers have been CC-d from the start. Is it the case that CC-ing is now officially insufficient? What's the point of ./scripts/add_maintainers.pl then which specifically adds maintainers only as CC? How are you supposed to get their attention? Just know you specifically have to send emails to them and not the mailinglist? I'm getting the distinct impression that the toolstack side has simply become unmaintained/orphaned with no one left who actually is looking at the mailinglist. Tamas
Re: [PATCH V2 5/7] dt-bindings: Add xen,dev-domid property description for xen-grant DMA ops
On Wed, May 18, 2022 at 5:06 PM Oleksandr wrote:
> On 18.05.22 17:32, Arnd Bergmann wrote:
> > On Sat, May 7, 2022 at 7:19 PM Oleksandr Tyshchenko
> > wrote:
>
> > This would mean having a device
> > node for the grant-table mechanism that can be referred to using the
> > 'iommus'
> > phandle property, with the domid as an additional argument.
>
> I assume, you are speaking about something like the following?
>
>
> xen_dummy_iommu {
> compatible = "xen,dummy-iommu";
> #iommu-cells = <1>;
> };
>
> virtio@3000 {
> compatible = "virtio,mmio";
> reg = <0x3000 0x100>;
> interrupts = <41>;
>
> /* The device is located in Xen domain with ID 1 */
> iommus = <_dummy_iommu 1>;
> };
Right, that's that's the idea, except I would not call it a 'dummy'.
>From the perspective of the DT, this behaves just like an IOMMU,
even if the exact mechanism is different from most hardware IOMMU
implementations.
> > It does not quite fit the model that Linux currently uses for iommus,
> > as that has an allocator for dma_addr_t space
>
> yes (# 3/7 adds grant-table based allocator)
>
>
> > , but it would think it's
> > conceptually close enough that it makes sense for the binding.
>
> Interesting idea. I am wondering, do we need an extra actions for this
> to work in Linux guest (dummy IOMMU driver, etc)?
It depends on how closely the guest implementation can be made to
resemble a normal iommu. If you do allocate dma_addr_t addresses,
it may actually be close enough that you can just turn the grant-table
code into a normal iommu driver and change nothing else.
Arnd
[linux-5.4 test] 170532: tolerable FAIL - PUSHED
flight 170532 linux-5.4 real [real] http://logs.test-lab.xenproject.org/osstest/logs/170532/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 170454 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 170454 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 170454 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 170454 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 170454 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 170454 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 170454 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 170454 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 170454 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 170454 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 170454 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 170454 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass version targeted for testing: linux0187300e6aa6246e9cebb22e2afbbc0d395839ee baseline version: linux
[ovmf test] 170544: regressions - FAIL
flight 170544 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170544/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 708620d29db89d03e822b8d17dc75fbac865c6dc baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 79 days Failing since168258 2022-03-01 01:55:31 Z 78 days 1086 attempts Testing same since 170392 2022-05-13 15:40:22 Z5 days 105 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kun Qin Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Liu, Zhiguang Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min M Xu Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Tom Lendacky Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhiguang Liu Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 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 6662 lines long.)
Re: [PATCH V2 5/7] dt-bindings: Add xen,dev-domid property description for xen-grant DMA ops
On 18.05.22 17:32, Arnd Bergmann wrote:
Hello Arnd
On Sat, May 7, 2022 at 7:19 PM Oleksandr Tyshchenko wrote:
diff --git a/Documentation/devicetree/bindings/virtio/mmio.yaml
b/Documentation/devicetree/bindings/virtio/mmio.yaml
index 10c22b5..29a0932 100644
--- a/Documentation/devicetree/bindings/virtio/mmio.yaml
+++ b/Documentation/devicetree/bindings/virtio/mmio.yaml
@@ -13,6 +13,9 @@ description:
See https://www.oasis-open.org/committees/tc_home.php?wg_abbrev=virtio for
more details.
+allOf:
+ - $ref: /schemas/arm/xen,dev-domid.yaml#
+
properties:
compatible:
const: virtio,mmio
@@ -33,6 +36,10 @@ properties:
description: Required for devices making accesses thru an IOMMU.
maxItems: 1
+ xen,dev-domid:
+description: Required when Xen grant mappings need to be enabled for
device.
+$ref: /schemas/types.yaml#/definitions/uint32
+
required:
- compatible
- reg
Sorry for joining the discussion late. Have you considered using the
generic iommu
binding here instead of a custom property?
I have to admit - no, I haven't. I was thinking that Xen specific
feature should be communicated using Xen specific DT property.
This would mean having a device
node for the grant-table mechanism that can be referred to using the 'iommus'
phandle property, with the domid as an additional argument.
I assume, you are speaking about something like the following?
xen_dummy_iommu {
compatible = "xen,dummy-iommu";
#iommu-cells = <1>;
};
virtio@3000 {
compatible = "virtio,mmio";
reg = <0x3000 0x100>;
interrupts = <41>;
/* The device is located in Xen domain with ID 1 */
iommus = <_dummy_iommu 1>;
};
It does not quite fit the model that Linux currently uses for iommus,
as that has an allocator for dma_addr_t space
yes (# 3/7 adds grant-table based allocator)
, but it would think it's
conceptually close enough that it makes sense for the binding.
Interesting idea. I am wondering, do we need an extra actions for this
to work in Linux guest (dummy IOMMU driver, etc)?
Arnd
--
Regards,
Oleksandr Tyshchenko
Re: [PATCH 2/3] tools/libxc: change xc_memshr_fork_reset API to match hypervisor
On 18.05.2022 17:01, Tamas K Lengyel wrote: > On Thu, May 12, 2022 at 9:46 AM Tamas K Lengyel > wrote: >> >> On Thu, May 5, 2022 at 4:27 AM Roger Pau Monné wrote: >>> >>> On Wed, Apr 27, 2022 at 11:34:19AM -0400, Tamas K Lengyel wrote: Need to separately specify if the reset is for the memory or for the VM state, or both. Signed-off-by: Tamas K Lengyel >>> >>> Reviewed-by: Roger Pau Monné >> >> Patch ping. Can this patch be merged please? > > Patch ping. Your mail (and I guess also your earlier one) was _To_ Roger, which is odd since he already did provide R-b. What you're missing is a tool stack maintainer ack aiui, so it may help if you send your pings _To_ the respective people. Jan
[xen-unstable test] 170529: tolerable FAIL
flight 170529 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/170529/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-i386-qemut-rhel6hvm-amd 7 xen-install fail like 170520 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 170520 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 170520 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 170520 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 170520 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 170520 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 170520 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 170520 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 170520 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 170520 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 170520 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 170520 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 170520 test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: xen
Re: [PATCH 3/3] x86/monitor: Add new monitor event to catch all vmexits
On Thu, May 12, 2022 at 9:47 AM Tamas K Lengyel wrote: > > On Wed, May 4, 2022 at 9:12 AM Tamas K Lengyel wrote: > > > > On Wed, Apr 27, 2022 at 11:51 AM Tamas K Lengyel > > wrote: > > > > > > Add monitor event that hooks the vmexit handler allowing for both sync and > > > async monitoring of events. With async monitoring an event is placed on > > > the > > > monitor ring for each exit and the rest of the vmexit handler resumes > > > normally. > > > If there are additional monitor events configured those will also place > > > their > > > respective events on the monitor ring. > > > > > > With the sync version an event is placed on the monitor ring but the > > > handler > > > does not get resumed, thus the sync version is only useful when the VM is > > > not > > > expected to resume normally after the vmexit. Our use-case is primarily > > > with > > > the sync version with VM forks where the fork gets reset after sync vmexit > > > event, thus the rest of the vmexit handler can be safely skipped. This is > > > very useful when we want to avoid Xen crashing the VM under any > > > circumstance, > > > for example during fuzzing. Collecting all vmexit information regardless > > > of > > > the root cause makes it easier to reason about the state of the VM on the > > > monitor side, hence we opt to receive all events, even for external > > > interrupt > > > and NMI exits and let the monitor agent decide how to proceed. > > > > > > Signed-off-by: Tamas K Lengyel > > > --- > > > v5: wrap vmexit fields in arch.vmx structures in the public vm_event ABI > > > > Patch ping. Could a toolstack maintainer please take a look at this? > > The hypervisor side already has a Reviewed-by. > > Patch ping. Patch ping. Tamas
Re: [PATCH 2/3] tools/libxc: change xc_memshr_fork_reset API to match hypervisor
On Thu, May 12, 2022 at 9:46 AM Tamas K Lengyel wrote: > > On Thu, May 5, 2022 at 4:27 AM Roger Pau Monné wrote: > > > > On Wed, Apr 27, 2022 at 11:34:19AM -0400, Tamas K Lengyel wrote: > > > Need to separately specify if the reset is for the memory or for the VM > > > state, > > > or both. > > > > > > Signed-off-by: Tamas K Lengyel > > > > Reviewed-by: Roger Pau Monné > > Patch ping. Can this patch be merged please? Patch ping. Tamas
Re: [PATCH] xen/cpupool: limit number of cpupools
On 18/05/2022 15:44, Juergen Gross wrote: > Today the number of cpupools in a system is unlimited. This can lead to > multiple problems (e.g. duplicate cpupool-id). Not to mention scalability issues. Search by ID is O(n). > > Limit the number of cpupools to twice the number of maximum possible > cpus, allowing to have one cpupool per physical cpu plus some spare > cpupools for special means (there are already existing use cases for > such spare cpupools). Probably one of Suggested-by/Reported-by/Requested-by :) Dealers choice. > Signed-off-by: Juergen Gross Otherwise, LGTM. Acked-by: Andrew Cooper
Re: [PATCH v3 15/21] xen/tpmfront: use xenbus_setup_ring() and xenbus_teardown_ring()
On Wed, May 18, 2022 at 11:41:51AM +0200, Juergen Gross wrote: > On 07.05.22 00:32, Jarkko Sakkinen wrote: > > On Thu, May 05, 2022 at 10:16:34AM +0200, Juergen Gross wrote: > > > Simplify tpmfront's ring creation and removal via xenbus_setup_ring() > > > and xenbus_teardown_ring(). > > > > > > Signed-off-by: Juergen Gross > > > > Please add to the commit message why these provide an equivalent > > functionality. > > Would you be fine with: > > Simplify tpmfront's ring creation and removal via xenbus_setup_ring() > and xenbus_teardown_ring(), which are provided exactly for the use > pattern as seen in this driver. > > > Juergen Looks fine to me! BR, Jarkko
Re: [PATCH v8 00/27] Introduce power-off+restart call chain API
On Tue, May 10, 2022 at 1:33 AM Dmitry Osipenko wrote: > > Problem > --- > > SoC devices require power-off call chaining functionality from kernel. > We have a widely used restart chaining provided by restart notifier API, > but nothing for power-off. > > Solution > > > Introduce new API that provides call chains support for all restart and > power-off modes. The new API is designed with simplicity and extensibility > in mind. > > This is a third attempt to introduce the new API. First was made by > Guenter Roeck back in 2014, second was made by Thierry Reding in 2017. > In fact the work didn't stop and recently arm_pm_restart() was removed > from v5.14 kernel, which was a part of preparatory work started by > Guenter Roeck. > > Adoption plan > - > > This patchset introduces the new API. It also converts multiple drivers > and arch code to the new API to demonstrate how it all looks in practice, > removing the pm_power_off_prepare global variable. > > The plan is: > > 1. Merge the new API and convert arch code to use do_kernel_power_off(). >For now the new API will co-exist with the older API. > > 2. Convert all drivers and platform code to the new API. > > 3. Remove obsoleted pm_power_off and pm_power_off_prepare variables. > > Results > --- > > 1. Devices can be powered off properly. > > 2. Global variables are removed from drivers. > > 3. Global pm_power_off and pm_power_off_prepare callback variables are > removed once all users are converted to the new API. The latter callback > is removed by patch #24 of this series. > > 4. Ambiguous call chain ordering is prohibited for non-default priorities. > > Changelog: > > v8: - Reworked sys-off handler like was suggested by Rafael Wysocki in > the comments to v7. > > - The struct sys-off handler now is private to kernel/reboot.c and > new API is simplified. > > - There is a single sys-off API function for all handler types. > Users shall pass the required sys-off mode type (restart, power-off > and etc). > > - There is single struct sys_off_data callback argument for all > handler modes. > > - User's callback now must return NOTIFY_DONE or NOTIFY_STOP. > > - The default priority level is zero now. > > - Multiple handlers now allowed to be registered at the default > priority level. > > - Power-off call chain is atomic now, like the restart chain. > > - kernel/reboot.c changes are split up into several logical patches. > > - Added r-b from Michał Mirosław to unmodified patches from v7. > > - Added acks that were missing in v7 by accident. The v8 looks much better than the previous versions to me. I actually don't really have any comments on it except for the minor remark regarding patch [1/27] sent separately. Please just send an update of that one patch and I will queue up the series for 5.19. However, I'm going to send a pull request with it in the second half of the merge window, after the majority of the other changes in the subsystems touched by it have been integrated. > v7: - Rebased on a recent linux-next. Dropped the recently removed > NDS32 architecture. Only SH and x86 arches left un-acked. > > - Added acks from Thomas Bogendoerfer and Krzysztof Kozlowski > to the MIPS and memory/emif patches respectively. > > - Made couple minor cosmetic improvements to the new API. > > - A month ago I joined Collabora and continuing to work on this series > on the company's time, so changed my email address to collabora.com > > v6: - Rebased on a recent linux-next. > > - Made minor couple cosmetic changes. > > v5: - Dropped patches which cleaned up notifier/reboot headers, as was > requested by Rafael Wysocki. > > - Dropped WARN_ON() from the code, as was requested by Rafael Wysocki. > Replaced it with pr_err() appropriately. > > - Dropped *_notifier_has_unique_priority() functions and added > *_notifier_chain_register_unique_prio() instead, as was suggested > by Michał Mirosław and Rafael Wysocki. > > - Dropped export of blocking_notifier_call_chain_is_empty() symbol, > as was suggested by Rafael Wysocki. > > - Michał Mirosław suggested that will be better to split up patch > that adds the new API to ease reviewing, but Rafael Wysocki asked > not add more patches, so I kept it as a single patch. > > - Added temporary "weak" stub for pm_power_off() which fixes linkage > failure once symbol is removed from arch/* code. Previously I missed > this problem because was only compile-testing object files. > > v4: - Made a very minor improvement to doc comments, clarifying couple > default values. > > - Corrected list of emails recipient by adding Linus, Sebastian, > Philipp and more NDS people. Removed bouncing emails. > > - Added acks that were given to v3. > > v3: - Renamed power_handler to sys_off_handler as was suggested by >
[PATCH] xen/cpupool: limit number of cpupools
Today the number of cpupools in a system is unlimited. This can lead to
multiple problems (e.g. duplicate cpupool-id).
Limit the number of cpupools to twice the number of maximum possible
cpus, allowing to have one cpupool per physical cpu plus some spare
cpupools for special means (there are already existing use cases for
such spare cpupools).
Signed-off-by: Juergen Gross
---
xen/common/sched/cpupool.c | 12
1 file changed, 12 insertions(+)
diff --git a/xen/common/sched/cpupool.c b/xen/common/sched/cpupool.c
index f6e3d97e52..7cc25ee8b4 100644
--- a/xen/common/sched/cpupool.c
+++ b/xen/common/sched/cpupool.c
@@ -30,6 +30,7 @@ struct cpupool *cpupool0;/* Initial cpupool
with Dom0 */
cpumask_t cpupool_free_cpus; /* cpus not in any cpupool */
static LIST_HEAD(cpupool_list); /* linked list, sorted by poolid */
+static unsigned int n_cpupools;
static int cpupool_moving_cpu = -1;
static struct cpupool *cpupool_cpu_moving = NULL;
@@ -276,6 +277,14 @@ static struct cpupool *cpupool_create(unsigned int poolid,
spin_lock(_lock);
+/* Don't allow too many cpupools. */
+if ( n_cpupools >= 2 * nr_cpu_ids )
+{
+ret = -ENOSPC;
+goto unlock;
+}
+n_cpupools++;
+
if ( poolid != CPUPOOLID_NONE )
{
q = __cpupool_find_by_id(poolid, false);
@@ -332,7 +341,9 @@ static struct cpupool *cpupool_create(unsigned int poolid,
err:
list_del(>list);
+n_cpupools--;
+ unlock:
spin_unlock(_lock);
free_cpupool_struct(c);
@@ -356,6 +367,7 @@ static int cpupool_destroy(struct cpupool *c)
return -EBUSY;
}
+n_cpupools--;
list_del(>list);
spin_unlock(_lock);
--
2.35.3
[ovmf test] 170539: regressions - FAIL
flight 170539 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170539/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 708620d29db89d03e822b8d17dc75fbac865c6dc baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 79 days Failing since168258 2022-03-01 01:55:31 Z 78 days 1085 attempts Testing same since 170392 2022-05-13 15:40:22 Z4 days 104 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kun Qin Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Liu, Zhiguang Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min M Xu Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Tom Lendacky Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhiguang Liu Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 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 6662 lines long.)
Re: [PATCH 2/2] tools/ocaml: Fix stubs the introduction of domain_create.cpupool_id
+ Adding toolstack maintainer > On 18 May 2022, at 12:34, Andrew Cooper wrote: > > On 18/05/2022 11:30, Luca Fancellu wrote: >>> On 18 May 2022, at 11:12, Andrew Cooper wrote: >>> >>> On 18/05/2022 10:51, Edwin Torok wrote: > diff --git a/tools/ocaml/libs/xc/xenctrl.ml > b/tools/ocaml/libs/xc/xenctrl.ml > index 7503031d8f61..8eab6f60eb14 100644 > --- a/tools/ocaml/libs/xc/xenctrl.ml > +++ b/tools/ocaml/libs/xc/xenctrl.ml > @@ -85,6 +85,7 @@ type domctl_create_config = > max_grant_frames: int; > max_maptrack_frames: int; > max_grant_version: int; > + cpupool_id: int32; What are the valid values for a CPU pool id, in particular what value should be passed here to get back the behaviour prior to these changes in Xen? (i.e. would it be cpu pool id 0 or -1 if cpu pools aren't otherwise explicitly configured on the system) >>> cpupools are a non-optional construct in Xen. >>> >>> By default, one cpupool exists, with the id 0, using the default >>> scheduler covering all pCPUs, and dom0 is constructed in this cpupool. >>> >>> Passing 0 here is the backwards compatible option. >>> >>> And on that note, Luca, you ought to patch xl/libxl to apply the pool= >>> setting directly during domain create, rather than depending on cpupool >>> 0 existing and moving the domain later. >> Is it an enhancement or a bug fix? > > This isn't a binary option. > > Your series added an optimisation to DOMCTL_createdomain, then didn't > adjust libxl to use the optimisation (which would have reduced the > number of hypercalls to create the domain, and reduce the number of > dynamic memory allocations in the hypervisor. Marginal, certainly, but > clearly a nice-to-have). > > Therefore, you created technical debt, which is option 3. > > By default, as the contributor, you are expected to address the > technical debt, because it is an important difference between hacking a > feature up for yourself, and integrating the feature nicely for everyone. > > You can of course negotiate with the tools maintainer to see if they > care, and right now that's a bit difficult. It's quite possible that > noone other than me cares, and I'm not libxl maintainer. > > Either you need to pay off the technical debt, or someone else will have > to. Someone else is going to have to start with digging into source > history, which means it's more expensive than you doing it now. > > At an absolute minimum, you need to be aware of where/when you are > creating technical debt. Ok, we've just created a task to handle this work so that we can track it, we will handle it in the future. Cheers, Luca > >> From what I know, please correct me if I’m wrong, cpupool0 >> Is always present, so there won’t be problem on assuming its existence > > From what I can see, your series has reduced the magic involved with > cpupool0, which is good. > > But the fact that it still has magic properties is still technical debt > that someone is going to have to pay off eventually. > > ~Andrew
Re: [PATCH V2 5/7] dt-bindings: Add xen,dev-domid property description for xen-grant DMA ops
On Sat, May 7, 2022 at 7:19 PM Oleksandr Tyshchenko wrote: > > diff --git a/Documentation/devicetree/bindings/virtio/mmio.yaml > b/Documentation/devicetree/bindings/virtio/mmio.yaml > index 10c22b5..29a0932 100644 > --- a/Documentation/devicetree/bindings/virtio/mmio.yaml > +++ b/Documentation/devicetree/bindings/virtio/mmio.yaml > @@ -13,6 +13,9 @@ description: >See https://www.oasis-open.org/committees/tc_home.php?wg_abbrev=virtio for >more details. > > +allOf: > + - $ref: /schemas/arm/xen,dev-domid.yaml# > + > properties: >compatible: > const: virtio,mmio > @@ -33,6 +36,10 @@ properties: > description: Required for devices making accesses thru an IOMMU. > maxItems: 1 > > + xen,dev-domid: > +description: Required when Xen grant mappings need to be enabled for > device. > +$ref: /schemas/types.yaml#/definitions/uint32 > + > required: >- compatible >- reg Sorry for joining the discussion late. Have you considered using the generic iommu binding here instead of a custom property? This would mean having a device node for the grant-table mechanism that can be referred to using the 'iommus' phandle property, with the domid as an additional argument. It does not quite fit the model that Linux currently uses for iommus, as that has an allocator for dma_addr_t space, but it would think it's conceptually close enough that it makes sense for the binding. Arnd
Re: [PATCH v3] tools/libs/light: update xenstore entry when setting max domain memory
Ping?
On 02.05.22 10:47, Juergen Gross wrote:
libxl_domain_setmaxmem() called during "xl mem-max" should update the
domain's memory/static-max Xenstore node, as otherwise "xl mem-set"
won't be able to set the memory size to the new maximum.
Adjust the related comments and documentation accordingly.
Signed-off-by: Juergen Gross
---
V2:
- adjust comments and docs (Anthony Perard)
V3:
- really adjust the docs (Anthony Perard)
---
docs/man/xl.1.pod.in | 11 +--
tools/libs/light/libxl_mem.c | 12 ++--
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/docs/man/xl.1.pod.in b/docs/man/xl.1.pod.in
index e2176bd696..101e14241d 100644
--- a/docs/man/xl.1.pod.in
+++ b/docs/man/xl.1.pod.in
@@ -442,15 +442,14 @@ allocate.
The default unit is kiB. Add 't' for TiB, 'g' for GiB, 'm' for
MiB, 'k' for kiB, and 'b' for bytes (e.g., `2048m` for 2048 MiB).
-NB that users normally shouldn't need this command; B will
-set this as appropriate automatically.
-
I can't be set lower than the current memory target for
I. It is allowed to be higher than the configured maximum
memory size of the domain (B parameter in the domain's
-configuration). Note however that the initial B value is still
-used as an upper limit for B. Also note that calling B will reset this value.
+configuration).
+
+Setting the maximum memory size above the configured maximum memory size
+will require special guest support (memory hotplug) in order to be usable
+by the guest.
The domain will not receive any signal regarding the changed memory
limit.
diff --git a/tools/libs/light/libxl_mem.c b/tools/libs/light/libxl_mem.c
index c739d00f39..92ec09f4cf 100644
--- a/tools/libs/light/libxl_mem.c
+++ b/tools/libs/light/libxl_mem.c
@@ -20,8 +20,7 @@
/*
* Set the maximum memory size of the domain in the hypervisor. There is no
* change of the current memory size involved. The specified memory size can
- * even be above the configured maxmem size of the domain, but the related
- * Xenstore entry memory/static-max isn't modified!
+ * even be above the configured maxmem size of the domain.
*/
int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint64_t max_memkb)
{
@@ -82,6 +81,15 @@ int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid,
uint64_t max_memkb)
goto out;
}
+rc = libxl__xs_printf(gc, XBT_NULL,
+ GCSPRINTF("%s/memory/static-max", dompath),
+ "%"PRIu64, max_memkb);
+if (rc != 0) {
+LOGED(ERROR, domid, "Couldn't set %s/memory/static-max, rc=%d\n",
+ dompath, rc);
+goto out;
+}
+
rc = 0;
out:
libxl_domain_config_dispose(_config);
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH] block: get rid of blk->guest_block_size
On 18/05/2022 14:09, Stefan Hajnoczi wrote:
Commit 1b7fd729559c ("block: rename buffer_alignment to
guest_block_size") noted:
At this point, the field is set by the device emulation, but completely
ignored by the block layer.
The last time the value of buffer_alignment/guest_block_size was
actually used was before commit 339064d50639 ("block: Don't use guest
sector size for qemu_blockalign()").
This value has not been used since 2013. Get rid of it.
Cc: Xie Yongji
Signed-off-by: Stefan Hajnoczi
Xen part...
Reviewed-by: Paul Durrant
Re: [PATCH V2 5/7] dt-bindings: Add xen,dev-domid property description for xen-grant DMA ops
On 17.05.22 03:27, Rob Herring wrote:
Hello Rob, all
On Sat, May 07, 2022 at 09:19:06PM +0300, Oleksandr Tyshchenko wrote:
From: Oleksandr Tyshchenko
Introduce Xen specific binding for the virtualized device (e.g. virtio)
to be used by Xen grant DMA-mapping layer in the subsequent commit.
This binding indicates that Xen grant mappings scheme needs to be
enabled for the device which DT node contains that property and specifies
the ID of Xen domain where the corresponding backend resides. The ID
(domid) is used as an argument to the grant mapping APIs.
This is needed for the option to restrict memory access using Xen grant
mappings to work which primary goal is to enable using virtio devices
in Xen guests.
Signed-off-by: Oleksandr Tyshchenko
---
Changes RFC -> V1:
- update commit subject/description and text in description
- move to devicetree/bindings/arm/
Changes V1 -> V2:
- update text in description
- change the maintainer of the binding
- fix validation issue
- reference xen,dev-domid.yaml schema from virtio/mmio.yaml
---
.../devicetree/bindings/arm/xen,dev-domid.yaml | 37 ++
Documentation/devicetree/bindings/virtio/mmio.yaml | 7
2 files changed, 44 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/xen,dev-domid.yaml
diff --git a/Documentation/devicetree/bindings/arm/xen,dev-domid.yaml
b/Documentation/devicetree/bindings/arm/xen,dev-domid.yaml
new file mode 100644
index ..750e89e
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/xen,dev-domid.yaml
@@ -0,0 +1,37 @@
+# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/xen,dev-domid.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xen specific binding for virtualized devices (e.g. virtio)
+
+maintainers:
+ - Stefano Stabellini
+
+select: true
Omit. No need to apply this on every single node.
ok, will do
+
+description:
+ This binding indicates that Xen grant mappings need to be enabled for
+ the device, and it specifies the ID of the domain where the corresponding
+ device (backend) resides. The property is required to restrict memory
+ access using Xen grant mappings.
+
+properties:
+ xen,dev-domid:
I kind of think 'dev' is redundant. Is there another kind of domid
possible?
In general, yes. It is driver(frontend) domid. But, at least for now, I
don't see why we will need an additional property for that.
Maybe xen,backend-domid or just xen,domid? I don't know Xen
too well, so ultimately up to you all.
xen,domid sounds ambiguous to me.
xen,backend-domid sounds perfectly fine to me, I even think it fits better.
Stefano, Juergen, would you be happy with new xen,backend-domid name?
If yes, Stefano could you please clarify, would you be OK if I retained
your R-b tags (for all patches in current series which touch that
property) after doing such renaming?
+$ref: /schemas/types.yaml#/definitions/uint32
+description:
+ The domid (domain ID) of the domain where the device (backend) is
running.
+
+additionalProperties: true
+
+examples:
+ - |
+virtio@3000 {
+compatible = "virtio,mmio";
+reg = <0x3000 0x100>;
+interrupts = <41>;
+
+/* The device is located in Xen domain with ID 1 */
+xen,dev-domid = <1>;
+};
diff --git a/Documentation/devicetree/bindings/virtio/mmio.yaml
b/Documentation/devicetree/bindings/virtio/mmio.yaml
index 10c22b5..29a0932 100644
--- a/Documentation/devicetree/bindings/virtio/mmio.yaml
+++ b/Documentation/devicetree/bindings/virtio/mmio.yaml
@@ -13,6 +13,9 @@ description:
See https://www.oasis-open.org/committees/tc_home.php?wg_abbrev=virtio for
more details.
+allOf:
+ - $ref: /schemas/arm/xen,dev-domid.yaml#
+
properties:
compatible:
const: virtio,mmio
@@ -33,6 +36,10 @@ properties:
description: Required for devices making accesses thru an IOMMU.
maxItems: 1
+ xen,dev-domid:
+description: Required when Xen grant mappings need to be enabled for
device.
+$ref: /schemas/types.yaml#/definitions/uint32
No need to define the type again nor describe it again.
Instead, just change additionalProperties to unevaluateProperties in
this doc. The diff is the latter takes $ref's into account.
ok, will do. Could you please clarify, shall I use?
unevaluatedProperties: false
or
unevaluatedProperties:
type: object
I am not too familiar with this stuff. Both variants seem to pass
validation.
Thank you.
+
required:
- compatible
- reg
--
2.7.4
--
Regards,
Oleksandr Tyshchenko
Re: [PATCH 2/2] x86/pat: add functions to query specific cache mode availability
On Tue, May 03, 2022 at 03:22:07PM +0200, Juergen Gross wrote: > Some drivers are using pat_enabled() in order to test availability of > special caching modes (WC and UC-). This will lead to false negatives > in case the system was booted e.g. with the "nopat" variant and the > BIOS did setup the PAT MSR supporting the queried mode, or if the > system is running as a Xen PV guest. > > Add test functions for those caching modes instead and use them at the > appropriate places. > > For symmetry reasons export the already existing x86_has_pat_wp() for > modules, too. No, we never export unused functionality.
[xen-unstable-smoke test] 170535: tolerable all pass - PUSHED
flight 170535 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/170535/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass version targeted for testing: xen 25c160a74f4489f031ac79a24078cc12efd5c96b baseline version: xen 69589c374a92d1b4f97db24623e5f760990eaf82 Last test of basis 170514 2022-05-17 18:00:25 Z0 days Testing same since 170535 2022-05-18 10:01:52 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich Julien Grall Marek Marczykowski-Górecki Roger Pau Monné jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 69589c374a..25c160a74f 25c160a74f4489f031ac79a24078cc12efd5c96b -> smoke
[ovmf test] 170538: regressions - FAIL
flight 170538 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170538/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 708620d29db89d03e822b8d17dc75fbac865c6dc baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 79 days Failing since168258 2022-03-01 01:55:31 Z 78 days 1084 attempts Testing same since 170392 2022-05-13 15:40:22 Z4 days 103 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kun Qin Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Liu, Zhiguang Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min M Xu Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Tom Lendacky Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhiguang Liu Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 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 6662 lines long.)
Re: [PATCH v3 9/9] xen/x86: use INFO level for node's without memory log message
On 11.05.2022 03:46, Wei Chen wrote: > In previous code, Xen was using KERN_WARNING for log message > when Xen found a node without memory. Xen will print this > warning message, and said that this may be an BIOS Bug or > mis-configured hardware. But actually, this warning is bogus, > because in an NUMA setting, nodes can only have processors, > and with 0 bytes memory. So it is unreasonable to warn about > BIOS or hardware corruption based on the detection of node > with 0 bytes memory. > > So in this patch, we remove the warning messages, but just > keep an info message to info users that there is one or more > nodes with 0 bytes memory in the system. > > Signed-off-by: Wei Chen Reviewed-by: Jan Beulich preferably with ... > --- a/xen/arch/x86/srat.c > +++ b/xen/arch/x86/srat.c > @@ -549,8 +549,7 @@ int __init acpi_scan_nodes(paddr_t start, paddr_t end) > uint64_t size = nodes[i].end - nodes[i].start; > > if ( size == 0 ) > - printk(KERN_WARNING "SRAT: Node %u has no memory. " > -"BIOS Bug or mis-configured hardware?\n", i); > + printk(KERN_INFO "SRAT: Node %u has no memory.\n", i); ... the full stop also dropped (and maybe the upper-case N converted to lower-case). Jan
Re: [PATCH v3 8/9] xen/x86: add detection of memory interleaves for different nodes
On 11.05.2022 03:46, Wei Chen wrote:
> --- a/xen/arch/x86/srat.c
> +++ b/xen/arch/x86/srat.c
> @@ -42,6 +42,12 @@ static struct node node_memblk_range[NR_NODE_MEMBLKS];
> static nodeid_t memblk_nodeid[NR_NODE_MEMBLKS];
> static __initdata DECLARE_BITMAP(memblk_hotplug, NR_NODE_MEMBLKS);
>
> +enum conflicts {
> + NO_CONFLICT = 0,
No need for the "= 0".
> + ERR_OVERLAP,
While this at least can be an error (the self-overlap case is merely
warned about), ...
> + ERR_INTERLEAVE,
... I don't think this is, and hence I'd recommend to drop "ERR_".
> @@ -119,20 +125,43 @@ int valid_numa_range(paddr_t start, paddr_t end,
> nodeid_t node)
> return 0;
> }
>
> -static __init int conflicting_memblks(paddr_t start, paddr_t end)
> +static enum conflicts __init
> +conflicting_memblks(nodeid_t nid, paddr_t start, paddr_t end,
> + paddr_t nd_start, paddr_t nd_end, int *mblkid)
Why "int"? Can the value passed back be negative?
> {
> int i;
>
> + /*
> + * Scan all recorded nodes' memory blocks to check conflicts:
> + * Overlap or interleave.
> + */
> for (i = 0; i < num_node_memblks; i++) {
> struct node *nd = _memblk_range[i];
> + *mblkid = i;
Style: Please maintain a blank line between declaration(s) and
statement(s).
> @@ -310,42 +342,67 @@ acpi_numa_memory_affinity_init(const struct
> acpi_srat_mem_affinity *ma)
> bad_srat();
> return;
> }
> +
> + /*
> + * For the node that already has some memory blocks, we will
> + * expand the node memory range temporarily to check memory
> + * interleaves with other nodes. We will not use this node
> + * temp memory range to check overlaps, because it will mask
> + * the overlaps in same node.
> + *
> + * Node with 0 bytes memory doesn't need this expandsion.
> + */
> + nd_start = start;
> + nd_end = end;
> + nd = [node];
> + if (nd->start != nd->end) {
> + if (nd_start > nd->start)
> + nd_start = nd->start;
> +
> + if (nd_end < end)
Did you mean nd->end here on the right side of < ? By intentionally
not adding "default:" in the body, you then also allow the compiler
to point out that addition of a new enumerator also needs handling
here.
> + nd_end = nd->end;
> + }
> +
> /* It is fine to add this area to the nodes data it will be used later*/
> - i = conflicting_memblks(start, end);
> - if (i < 0)
> - /* everything fine */;
> - else if (memblk_nodeid[i] == node) {
> - bool mismatch = !(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) !=
> - !test_bit(i, memblk_hotplug);
> -
> - printk("%sSRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr") overlaps with
> itself (%"PRIpaddr"-%"PRIpaddr")\n",
> -mismatch ? KERN_ERR : KERN_WARNING, pxm, start, end,
> -node_memblk_range[i].start, node_memblk_range[i].end);
> - if (mismatch) {
> + status = conflicting_memblks(node, start, end, nd_start, nd_end, );
> + if (status == ERR_OVERLAP) {
Please use switch(status) when checking enumerated values.
> + if (memblk_nodeid[i] == node) {
> + bool mismatch = !(ma->flags &
> + ACPI_SRAT_MEM_HOT_PLUGGABLE) !=
> + !test_bit(i, memblk_hotplug);
Style: The middle line wants indenting by two more characters.
> +
> + printk("%sSRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr")
> overlaps with itself (%"PRIpaddr"-%"PRIpaddr")\n",
> +mismatch ? KERN_ERR : KERN_WARNING, pxm, start,
> +end, node_memblk_range[i].start,
> +node_memblk_range[i].end);
> + if (mismatch) {
> + bad_srat();
> + return;
> + }
> + } else {
> + printk(KERN_ERR
> +"SRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr") overlaps
> with PXM %u (%"PRIpaddr"-%"PRIpaddr")\n",
> +pxm, start, end, node_to_pxm(memblk_nodeid[i]),
> +node_memblk_range[i].start,
> +node_memblk_range[i].end);
> bad_srat();
> return;
> }
> - } else {
> + } else if (status == ERR_INTERLEAVE) {
> printk(KERN_ERR
> -"SRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr") overlaps with
> PXM %u (%"PRIpaddr"-%"PRIpaddr")\n",
> -pxm, start, end, node_to_pxm(memblk_nodeid[i]),
> +"SRAT： Node %u: (%"PRIpaddr"-%"PRIpaddr") interleaves
> with node %u memblk (%"PRIpaddr"-%"PRIpaddr")\n",
> +node, nd_start, nd_end, memblk_nodeid[i],
[PATCH v3] x86/hvm: Widen condition for is_hvm_pv_evtchn_domain() and report fix in CPUID
Have is_hvm_pv_evtchn_domain() return true for vector callbacks for evtchn delivery set up on a per-vCPU basis via HVMOP_set_evtchn_upcall_vector. is_hvm_pv_evtchn_domain() returning true is a condition for setting up physical IRQ to event channel mappings. Therefore, a CPUID bit is added so that guests know whether the check in is_hvm_pv_evtchn_domain() will fail when using HVMOP_set_evtchn_upcall_vector. This matters for guests that route PIRQs over event channels since is_hvm_pv_evtchn_domain() is a condition in physdev_map_pirq(). The naming of the CPUID bit is quite generic about upcall support being available. That's done so that the define name doesn't become overly long like XEN_HVM_CPUID_UPCALL_VECTOR_SUPPORTS_PIRQ or some such. A guest that doesn't care about physical interrupts routed over event channels can just test for the availability of the hypercall directly (HVMOP_set_evtchn_upcall_vector) without checking the CPUID bit. Signed-off-by: Jane Malalane --- CC: Jan Beulich CC: Andrew Cooper CC: "Roger Pau Monné" CC: Wei Liu v3: * Improve commit message and title. v2: * Since the naming of the CPUID bit is quite generic, better explain when it should be checked for, in code comments and commit message. --- xen/arch/x86/include/asm/domain.h | 8 +++- xen/arch/x86/traps.c| 6 ++ xen/include/public/arch-x86/cpuid.h | 5 + 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h index 35898d725f..f044e0a492 100644 --- a/xen/arch/x86/include/asm/domain.h +++ b/xen/arch/x86/include/asm/domain.h @@ -14,8 +14,14 @@ #define has_32bit_shinfo(d)((d)->arch.has_32bit_shinfo) +/* + * Set to true if either the global vector-type callback or per-vCPU + * LAPIC vectors are used. Assume all vCPUs will use + * HVMOP_set_evtchn_upcall_vector as long as the initial vCPU does. + */ #define is_hvm_pv_evtchn_domain(d) (is_hvm_domain(d) && \ -(d)->arch.hvm.irq->callback_via_type == HVMIRQ_callback_vector) +((d)->arch.hvm.irq->callback_via_type == HVMIRQ_callback_vector || \ + (d)->vcpu[0]->arch.hvm.evtchn_upcall_vector)) #define is_hvm_pv_evtchn_vcpu(v) (is_hvm_pv_evtchn_domain(v->domain)) #define is_domain_direct_mapped(d) ((void)(d), 0) diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index 25bffe47d7..1a7f9df067 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -1152,6 +1152,12 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf, res->a |= XEN_HVM_CPUID_DOMID_PRESENT; res->c = d->domain_id; +/* + * Per-vCPU event channel upcalls are implemented and work + * correctly with PIRQs routed over event channels. + */ +res->a |= XEN_HVM_CPUID_UPCALL_VECTOR; + break; case 5: /* PV-specific parameters */ diff --git a/xen/include/public/arch-x86/cpuid.h b/xen/include/public/arch-x86/cpuid.h index f2b2b3632c..c49eefeaf8 100644 --- a/xen/include/public/arch-x86/cpuid.h +++ b/xen/include/public/arch-x86/cpuid.h @@ -109,6 +109,11 @@ * field from 8 to 15 bits, allowing to target APIC IDs up 32768. */ #define XEN_HVM_CPUID_EXT_DEST_ID (1u << 5) +/* + * Per-vCPU event channel upcalls work correctly with physical IRQs + * bound to event channels. + */ +#define XEN_HVM_CPUID_UPCALL_VECTOR(1u << 6) /* * Leaf 6 (0x4x05) -- 2.11.0
Re: [PATCH 19/30] panic: Add the panic hypervisor notifier list
On 18/05/2022 04:33, Petr Mladek wrote: > [...] > Anyway, I would distinguish it the following way. > > + If the notifier is preserving kernel log then it should be ideally > treated as kmsg_dump(). > > + It the notifier is saving another debugging data then it better > fits into the "hypervisor" notifier list. > > Definitely, I agree - it's logical, since we want more info in the logs, and happens some notifiers running in the informational list do that, like ftrace_on_oops for example. > Regarding the reliability. From my POV, any panic notifier enabled > in a generic kernel should be reliable with more than 99,9%. > Otherwise, they should not be in the notifier list at all. > > An exception would be a platform-specific notifier that is > called only on some specific platform and developers maintaining > this platform agree on this. > > The value "99,9%" is arbitrary. I am not sure if it is realistic > even in the other code, for example, console_flush_on_panic() > or emergency_restart(). I just want to point out that the border > should be rather high. Otherwise we would back in the situation > where people would want to disable particular notifiers. > Totally agree, these percentages are just an example, 50% is ridiculous low reliability in my example heheh But some notifiers deep dive in abstraction layers (like regmap or GPIO stuff) and it's hard to determine the probability of a lock issue (take a spinlock already taken inside regmap code and live-lock forever, for example). These are better to run, if possible, later than kdump or even info list. Thanks again for the good analysis Petr! Cheers, Guilherme
Re: [PATCH 19/30] panic: Add the panic hypervisor notifier list
On 18/05/2022 04:58, Petr Mladek wrote: > [...] >> I does similar things like kmsg_dump() so it should be called in >> the same location (after info notifier list and before kdump). >> >> A solution might be to put it at these notifiers at the very >> end of the "info" list or make extra "dump" notifier list. > > I just want to point out that the above idea has problems. > Notifiers storing kernel log need to be treated as kmsg_dump(). > In particular, we would need to know if there are any. > We do not need to call "info" notifier list before kdump > when there is no kernel log dumper registered. > Notifiers respect the priority concept, which is just a number that orders the list addition (and the list is called in order). I've used the last position to panic_print() [in patch 25] - one idea here is to "reserve" the last position (represented by INT_MIN) for notifiers that act like kmsg_dump(). I couldn't find any IIRC, but that doesn't prevent us to save this position and comment about that. Makes sense to you ? Cheers!
Re: [PATCH v3 7/9] xen/x86: use paddr_t for addresses in NUMA node structure
On 11.05.2022 03:46, Wei Chen wrote: > NUMA node structure "struct node" is using u64 as node memory > range. In order to make other architectures can reuse this > NUMA node relative code, we replace the u64 to paddr_t. And > use pfn_to_paddr and paddr_to_pfn to replace explicit shift > operations. The relate PRIx64 in print messages have been > replaced by PRIpaddr at the same time. And some being-phased-out > types like u64 in the lines we have touched also have been > converted to uint64_t or unsigned long. > > Tested-by: Jiamei Xie > Signed-off-by: Wei Chen Acked-by: Jan Beulich
Re: [PATCH 19/30] panic: Add the panic hypervisor notifier list
On 18/05/2022 04:38, Petr Mladek wrote: > [...] > I have answered this in more detail in the other reply, see > https://lore.kernel.org/r/YoShZVYNAdvvjb7z@alley > > I agree that both notifiers in > > drivers/soc/bcm/brcmstb/pm/pm-arm.c > drivers/firmware/google/gsmi.c > > better fit into the hypervisor list after all. > > Best Regards, > Petr Perfect, thanks - will keep both in such list for V2.
[PATCH] block: get rid of blk->guest_block_size
Commit 1b7fd729559c ("block: rename buffer_alignment to
guest_block_size") noted:
At this point, the field is set by the device emulation, but completely
ignored by the block layer.
The last time the value of buffer_alignment/guest_block_size was
actually used was before commit 339064d50639 ("block: Don't use guest
sector size for qemu_blockalign()").
This value has not been used since 2013. Get rid of it.
Cc: Xie Yongji
Signed-off-by: Stefan Hajnoczi
---
include/sysemu/block-backend-io.h| 1 -
block/block-backend.c| 10 --
block/export/vhost-user-blk-server.c | 1 -
hw/block/virtio-blk.c| 1 -
hw/block/xen-block.c | 1 -
hw/ide/core.c| 1 -
hw/scsi/scsi-disk.c | 1 -
hw/scsi/scsi-generic.c | 1 -
8 files changed, 17 deletions(-)
diff --git a/include/sysemu/block-backend-io.h
b/include/sysemu/block-backend-io.h
index 6517c39295..ccef514023 100644
--- a/include/sysemu/block-backend-io.h
+++ b/include/sysemu/block-backend-io.h
@@ -72,7 +72,6 @@ void blk_error_action(BlockBackend *blk, BlockErrorAction
action,
void blk_iostatus_set_err(BlockBackend *blk, int error);
int blk_get_max_iov(BlockBackend *blk);
int blk_get_max_hw_iov(BlockBackend *blk);
-void blk_set_guest_block_size(BlockBackend *blk, int align);
void blk_io_plug(BlockBackend *blk);
void blk_io_unplug(BlockBackend *blk);
diff --git a/block/block-backend.c b/block/block-backend.c
index e0e1aff4b1..d4abdf8faa 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -56,9 +56,6 @@ struct BlockBackend {
const BlockDevOps *dev_ops;
void *dev_opaque;
-/* the block size for which the guest device expects atomicity */
-int guest_block_size;
-
/* If the BDS tree is removed, some of its options are stored here (which
* can be used to restore those options in the new BDS on insert) */
BlockBackendRootState root_state;
@@ -998,7 +995,6 @@ void blk_detach_dev(BlockBackend *blk, DeviceState *dev)
blk->dev = NULL;
blk->dev_ops = NULL;
blk->dev_opaque = NULL;
-blk->guest_block_size = 512;
blk_set_perm(blk, 0, BLK_PERM_ALL, _abort);
blk_unref(blk);
}
@@ -2100,12 +2096,6 @@ int blk_get_max_iov(BlockBackend *blk)
return blk->root->bs->bl.max_iov;
}
-void blk_set_guest_block_size(BlockBackend *blk, int align)
-{
-IO_CODE();
-blk->guest_block_size = align;
-}
-
void *blk_try_blockalign(BlockBackend *blk, size_t size)
{
IO_CODE();
diff --git a/block/export/vhost-user-blk-server.c
b/block/export/vhost-user-blk-server.c
index a129204c44..b2e458ade3 100644
--- a/block/export/vhost-user-blk-server.c
+++ b/block/export/vhost-user-blk-server.c
@@ -495,7 +495,6 @@ static int vu_blk_exp_create(BlockExport *exp,
BlockExportOptions *opts,
return -EINVAL;
}
vexp->blk_size = logical_block_size;
-blk_set_guest_block_size(exp->blk, logical_block_size);
if (vu_opts->has_num_queues) {
num_queues = vu_opts->num_queues;
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index cd804795c6..e9ba752f6b 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1228,7 +1228,6 @@ static void virtio_blk_device_realize(DeviceState *dev,
Error **errp)
s->change = qemu_add_vm_change_state_handler(virtio_blk_dma_restart_cb, s);
blk_set_dev_ops(s->blk, _block_ops, s);
-blk_set_guest_block_size(s->blk, s->conf.conf.logical_block_size);
blk_iostatus_enable(s->blk);
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 674953f1ad..345b284d70 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -243,7 +243,6 @@ static void xen_block_realize(XenDevice *xendev, Error
**errp)
}
blk_set_dev_ops(blk, _block_dev_ops, blockdev);
-blk_set_guest_block_size(blk, conf->logical_block_size);
if (conf->discard_granularity == -1) {
conf->discard_granularity = conf->physical_block_size;
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 3a5afff5d7..f7ec68513f 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2544,7 +2544,6 @@ int ide_init_drive(IDEState *s, BlockBackend *blk,
IDEDriveKind kind,
s->smart_selftest_count = 0;
if (kind == IDE_CD) {
blk_set_dev_ops(blk, _cd_block_ops, s);
-blk_set_guest_block_size(blk, 2048);
} else {
if (!blk_is_inserted(s->blk)) {
error_setg(errp, "Device needs media, but drive is empty");
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 072686ed58..91acb5c0ce 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2419,7 +2419,6 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
} else {
blk_set_dev_ops(s->qdev.conf.blk, _disk_block_ops, s);
}
-blk_set_guest_block_size(s->qdev.conf.blk, s->qdev.blocksize);
blk_iostatus_enable(s->qdev.conf.blk);
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
Re: [PATCH v3 4/9] xen: introduce an arch helper for default dma zone status
On 11.05.2022 03:46, Wei Chen wrote: > In current code, when Xen is running in a multiple nodes > NUMA system, it will set dma_bitsize in end_boot_allocator > to reserve some low address memory as DMA zone. > > There are some x86 implications in the implementation. > Because on x86, memory starts from 0. On a multiple-nodes > NUMA system, if a single node contains the majority or all > of the DMA memory, x86 prefers to give out memory from > non-local allocations rather than exhausting the DMA memory > ranges. Hence x86 uses dma_bitsize to set aside some largely > arbitrary amount of memory for DMA zone. The allocations > from DMA zone would happen only after exhausting all other > nodes' memory. > > But the implications are not shared across all architectures. > For example, Arm cannot guarantee the availability of memory > below a certain boundary for DMA limited-capability devices > either. But currently, Arm doesn't need a reserved DMA zone > in Xen. Because there is no DMA device in Xen. And for guests, > Xen Arm only allows Dom0 to have DMA operations without IOMMU. > Xen will try to allocate memory under 4GB or memory range that > is limited by dma_bitsize for Dom0 in boot time. For DomU, even > Xen can passthrough devices to DomU without IOMMU, but Xen Arm > doesn't guarantee their DMA operations. So, Xen Arm doesn't > need a reserved DMA zone to provide DMA memory for guests. > > In this patch, we introduce an arch_want_default_dmazone helper > for different architectures to determine whether they need to > set dma_bitsize for DMA zone reservation or not. > > At the same time, when x86 Xen is built with CONFIG_PV=n could > probably leverage this new helper to actually not trigger DMA > zone reservation. > > Signed-off-by: Wei Chen > Tested-by: Jiamei Xie Acked-by: Jan Beulich
[ovmf test] 170537: regressions - FAIL
flight 170537 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170537/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 708620d29db89d03e822b8d17dc75fbac865c6dc baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 79 days Failing since168258 2022-03-01 01:55:31 Z 78 days 1083 attempts Testing same since 170392 2022-05-13 15:40:22 Z4 days 102 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kun Qin Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Liu, Zhiguang Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min M Xu Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Tom Lendacky Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhiguang Liu Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 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 6662 lines long.)
Re: [PATCH v3 2/9] xen: reuse x86 EFI stub functions for Arm
On 11.05.2022 03:46, Wei Chen wrote: > x86 is using compiler feature testing to decide EFI build > enable or not. When EFI build is disabled, x86 will use an > efi/stub.c file to replace efi/runtime.c for build objects. > Following this idea, we introduce a stub file for Arm, but > use CONFIG_ARM_EFI to decide EFI build enable or not. > > And the most functions in x86 EFI stub.c can be reused for > other architectures, like Arm. So we move them to common > and keep the x86 specific function in x86/efi/stub.c. > > To avoid the symbol link conflict error when linking common > stub files to x86/efi. We add a regular file check in efi > stub files' link script. Depends on this check we can bypass > the link behaviors for existed stub files in x86/efi. > > As there is no Arm specific EFI stub function for Arm in > current stage, Arm still can use the existed symbol link > method for EFI stub files. Wouldn't it be better to mandate that every arch has its stub.c, and in the Arm one all you'd do (for now) is #include the common one? (But see also below.) > --- a/xen/arch/arm/Makefile > +++ b/xen/arch/arm/Makefile > @@ -1,6 +1,5 @@ > obj-$(CONFIG_ARM_32) += arm32/ > obj-$(CONFIG_ARM_64) += arm64/ > -obj-$(CONFIG_ARM_64) += efi/ > obj-$(CONFIG_ACPI) += acpi/ > obj-$(CONFIG_HAS_PCI) += pci/ > ifneq ($(CONFIG_NO_PLAT),y) > @@ -20,6 +19,7 @@ obj-y += domain.o > obj-y += domain_build.init.o > obj-y += domctl.o > obj-$(CONFIG_EARLY_PRINTK) += early_printk.o > +obj-y += efi/ > obj-y += gic.o > obj-y += gic-v2.o > obj-$(CONFIG_GICV3) += gic-v3.o > diff --git a/xen/arch/arm/efi/Makefile b/xen/arch/arm/efi/Makefile > index 4313c39066..dffe72e589 100644 > --- a/xen/arch/arm/efi/Makefile > +++ b/xen/arch/arm/efi/Makefile > @@ -1,4 +1,12 @@ > include $(srctree)/common/efi/efi-common.mk > > +ifeq ($(CONFIG_ARM_EFI),y) > obj-y += $(EFIOBJ-y) > obj-$(CONFIG_ACPI) += efi-dom0.init.o > +else > +# Add stub.o to EFIOBJ-y to re-use the clean-files in > +# efi-common.mk. Otherwise the link of stub.c in arm/efi > +# will not be cleaned in "make clean". > +EFIOBJ-y += stub.o > +obj-y += stub.o > +endif I realize Stefano indicated he's happy with the Arm side, but I still wonder: What use is the stub on Arm32? Even further - once you have a config option (rather than x86'es build-time check plus x86'es dual- purposing of all object files), why do you need a stub in the first place? You ought to be able to deal with things via inline functions and macros, I would think. > --- a/xen/common/efi/efi-common.mk > +++ b/xen/common/efi/efi-common.mk > @@ -9,7 +9,8 @@ CFLAGS-y += -iquote $(srcdir) > # e.g.: It transforms "dir/foo/bar" into successively > # "dir foo bar", ".. .. ..", "../../.." > $(obj)/%.c: $(srctree)/common/efi/%.c FORCE > - $(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, > ,$(obj/source/common/efi/$( + $(Q)test -f $@ || \ > + ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, > ,$(obj/source/common/efi/$(
Re: [PATCH v4 00/21] IOMMU: superpage support when not sharing pagetables
On 25.04.2022 10:29, Jan Beulich wrote:
> For a long time we've been rather inefficient with IOMMU page table
> management when not sharing page tables, i.e. in particular for PV (and
> further specifically also for PV Dom0) and AMD (where nowadays we never
> share page tables). While up to about 2.5 years ago AMD code had logic
> to un-shatter page mappings, that logic was ripped out for being buggy
> (XSA-275 plus follow-on).
>
> This series enables use of large pages in AMD and Intel (VT-d) code;
> Arm is presently not in need of any enabling as pagetables are always
> shared there. It also augments PV Dom0 creation with suitable explicit
> IOMMU mapping calls to facilitate use of large pages there. Depending
> on the amount of memory handed to Dom0 this improves booting time
> (latency until Dom0 actually starts) quite a bit; subsequent shattering
> of some of the large pages may of course consume some of the saved time.
>
> Known fallout has been spelled out here:
> https://lists.xen.org/archives/html/xen-devel/2021-08/msg00781.html
>
> There's a dependency on 'PCI: replace "secondary" flavors of
> PCI_{DEVFN,BDF,SBDF}()', in particular by patch 8. Its prereq patch
> still lacks an Arm ack, so it couldn't go in yet.
>
> I'm inclined to say "of course" there are also a few seemingly unrelated
> changes included here, which I just came to consider necessary or at
> least desirable (in part for having been in need of adjustment for a
> long time) along the way. Some of these changes are likely independent
> of the bulk of the work here, and hence may be fine to go in ahead of
> earlier patches.
>
> See individual patches for details on the v4 changes.
>
> 01: AMD/IOMMU: correct potentially-UB shifts
> 02: IOMMU: simplify unmap-on-error in iommu_map()
> 03: IOMMU: add order parameter to ->{,un}map_page() hooks
> 04: IOMMU: have iommu_{,un}map() split requests into largest possible chunks
These first 4 patches are in principle ready to go in. If only there
wasn't (sadly once again) the unclear state with comments on the
first 2 that you had given on Apr 27. I did reply verbally, and hence
I'm intending to commit these 4 by the end of the week - on the
assumption that no response to my replies means I've sufficiently
addressed the concerns - unless I hear back otherwise.
Thanks for you understanding, Jan
[ovmf test] 170536: regressions - FAIL
flight 170536 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170536/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 708620d29db89d03e822b8d17dc75fbac865c6dc baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 79 days Failing since168258 2022-03-01 01:55:31 Z 78 days 1082 attempts Testing same since 170392 2022-05-13 15:40:22 Z4 days 101 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kun Qin Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Liu, Zhiguang Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min M Xu Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Tom Lendacky Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhiguang Liu Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 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 6662 lines long.)
Re: [PATCH v4 19/21] IOMMU/x86: add perf counters for page table splitting / coalescing
On 11.05.2022 15:48, Roger Pau Monné wrote: > On Mon, Apr 25, 2022 at 10:44:11AM +0200, Jan Beulich wrote: >> Signed-off-by: Jan Beulich >> Reviewed-by: Kevin tian > > Reviewed-by: Roger Pau Monné Thanks. > Would be helpful to also have those per-guest I think. Perhaps, but we don't have per-guest counter infrastructure, do we? Jan
Re: [PATCH 2/2] tools/ocaml: Fix stubs the introduction of domain_create.cpupool_id
On 18/05/2022 11:30, Luca Fancellu wrote: >> On 18 May 2022, at 11:12, Andrew Cooper wrote: >> >> On 18/05/2022 10:51, Edwin Torok wrote: diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml index 7503031d8f61..8eab6f60eb14 100644 --- a/tools/ocaml/libs/xc/xenctrl.ml +++ b/tools/ocaml/libs/xc/xenctrl.ml @@ -85,6 +85,7 @@ type domctl_create_config = max_grant_frames: int; max_maptrack_frames: int; max_grant_version: int; + cpupool_id: int32; >>> What are the valid values for a CPU pool id, in particular what value >>> should be passed here to get back the behaviour prior to these changes in >>> Xen? >>> (i.e. would it be cpu pool id 0 or -1 if cpu pools aren't otherwise >>> explicitly configured on the system) >> cpupools are a non-optional construct in Xen. >> >> By default, one cpupool exists, with the id 0, using the default >> scheduler covering all pCPUs, and dom0 is constructed in this cpupool. >> >> Passing 0 here is the backwards compatible option. >> >> And on that note, Luca, you ought to patch xl/libxl to apply the pool= >> setting directly during domain create, rather than depending on cpupool >> 0 existing and moving the domain later. > Is it an enhancement or a bug fix? This isn't a binary option. Your series added an optimisation to DOMCTL_createdomain, then didn't adjust libxl to use the optimisation (which would have reduced the number of hypercalls to create the domain, and reduce the number of dynamic memory allocations in the hypervisor. Marginal, certainly, but clearly a nice-to-have). Therefore, you created technical debt, which is option 3. By default, as the contributor, you are expected to address the technical debt, because it is an important difference between hacking a feature up for yourself, and integrating the feature nicely for everyone. You can of course negotiate with the tools maintainer to see if they care, and right now that's a bit difficult. It's quite possible that noone other than me cares, and I'm not libxl maintainer. Either you need to pay off the technical debt, or someone else will have to. Someone else is going to have to start with digging into source history, which means it's more expensive than you doing it now. At an absolute minimum, you need to be aware of where/when you are creating technical debt. > From what I know, please correct me if I’m wrong, cpupool0 > Is always present, so there won’t be problem on assuming its existence From what I can see, your series has reduced the magic involved with cpupool0, which is good. But the fact that it still has magic properties is still technical debt that someone is going to have to pay off eventually. ~Andrew
Re: [PATCH 1/2] xen/cpupool: Reject attempts to add a domain to CPUPOOLID_NONE
On 18/05/2022 11:27, Luca Fancellu wrote:
> Hi Andrew,
>
>> On 17 May 2022, at 20:41, Andrew Cooper wrote:
>>
>> c/s cfc52148444f ("xen/domain: Reduce the quantity of initialisation for
>> system domains") removed the path in domain_create() which called
>> sched_init_domain() with CPUPOOLID_NONE for system domains.
>>
>> Arguably, that changeset should have cleaned up this path too.
>>
>> However, c/s 92ea9c54fc81 ("arm/dom0less: assign dom0less guests to
>> cpupools")
>> changed domain_create() from using a hardcoded poolid of 0, to using a value
>> passed by the toolstack.
>>
>> While CPUPOOLID_NONE is an internal constant, userspace can pass -1 for the
>> cpupool_id parameter and attempt to construct a real domain using default
>> ops,
>> which at a minimum will fail the assertion in dom_scheduler().
>>
>> Fixes: 92ea9c54fc81 ("arm/dom0less: assign dom0less guests to cpupools")
>> Signed-off-by: Andrew Cooper
> Thanks for this fix, with the introduction of 92ea9c54fc81 ("arm/dom0less:
> assign dom0less guests to cpupools”)
> we’ve checked all the path passing struct xen_domctl_createdomain, and at
> that time it seems to be that
> the new cpupool_id member would have been always zero when created from the
> tool stack, am I wrong?
Hypercalls are an entirely public API/ABI.
Looking through xen.git gets you the common users, but it most
definitely doesn't get you all users of the interface.
This hypercall specifically gets fuzzed (there's a KFX PoC somewhere),
but it's a bug for any hypercall to be able to hit an assertion/crash/etc.
> I’m asking so that I will keep in mind for the future.
>
> However with your second patch of this serie, the tool stack is able to write
> it, so I guess this fix now is mandatory.
>
> I’ve tested your patch, enabling boot time cpupools, on an arm machine and
> booting Xen+Dom0 and another DomU
> by dom0less feature, and all works.
>
> Reviewed-by: Luca Fancellu
> Tested-by: Luca Fancellu
Thanks.
~Andrew
Re: [PATCH V8 2/2] libxl: Introduce basic virtio-mmio support on Arm
On Tue, May 03, 2022 at 08:26:03PM +0300, Oleksandr Tyshchenko wrote:
> From: Julien Grall
>
> This patch introduces helpers to allocate Virtio MMIO params
> (IRQ and memory region) and create specific device node in
> the Guest device-tree with allocated params. In order to deal
> with multiple Virtio devices, reserve corresponding ranges.
> For now, we reserve 1MB for memory regions and 10 SPIs.
>
> As these helpers should be used for every Virtio device attached
> to the Guest, call them for Virtio disk(s).
>
> Please note, with statically allocated Virtio IRQs there is
> a risk of a clash with a physical IRQs of passthrough devices.
> For the first version, it's fine, but we should consider allocating
> the Virtio IRQs automatically. Thankfully, we know in advance which
> IRQs will be used for passthrough to be able to choose non-clashed
> ones.
>
> Signed-off-by: Julien Grall
> Signed-off-by: Oleksandr Tyshchenko
> ---
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index eef1de0..37403a2 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -8,6 +8,46 @@
> #include
> #include
>
> +/*
> + * There is no clear requirements for the total size of Virtio MMIO region.
> + * The size of control registers is 0x100 and device-specific configuration
> + * registers starts at the offset 0x100, however it's size depends on the
> device
> + * and the driver. Pick the biggest known size at the moment to cover most
> + * of the devices (also consider allowing the user to configure the size via
> + * config file for the one not conforming with the proposed value).
> + */
> +#define VIRTIO_MMIO_DEV_SIZE xen_mk_ullong(0x200)
> +
> +static uint64_t alloc_virtio_mmio_base(libxl__gc *gc, uint64_t
> *virtio_mmio_base)
> +{
> +uint64_t base = *virtio_mmio_base;
> +
> +/* Make sure we have enough reserved resources */
> +if ((base + VIRTIO_MMIO_DEV_SIZE >
> +GUEST_VIRTIO_MMIO_BASE + GUEST_VIRTIO_MMIO_SIZE)) {
Could you remove the second set of parentheses? I'd like the compiler to
warn us if there's an assignment.
> @@ -26,8 +66,10 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
> {
> uint32_t nr_spis = 0;
> unsigned int i;
> -uint32_t vuart_irq;
> -bool vuart_enabled = false;
> +uint32_t vuart_irq, virtio_irq = 0;
> +bool vuart_enabled = false, virtio_enabled = false;
> +uint64_t virtio_mmio_base = GUEST_VIRTIO_MMIO_BASE;
> +uint32_t virtio_mmio_irq = GUEST_VIRTIO_MMIO_SPI_FIRST;
>
> /*
> * If pl011 vuart is enabled then increment the nr_spis to allow
> allocation
> @@ -39,6 +81,30 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
> vuart_enabled = true;
> }
>
> +for (i = 0; i < d_config->num_disks; i++) {
> +libxl_device_disk *disk = _config->disks[i];
> +
> +if (disk->specification == LIBXL_DISK_SPECIFICATION_VIRTIO) {
> +disk->base = alloc_virtio_mmio_base(gc, _mmio_base);
> +if (!disk->base)
> +return ERROR_FAIL;
> +
> +disk->irq = alloc_virtio_mmio_irq(gc, _mmio_irq);
> +if (!disk->irq)
> +return ERROR_FAIL;
> +
> +if (virtio_irq < disk->irq)
> +virtio_irq = disk->irq;
> +virtio_enabled = true;
> +
> +LOG(DEBUG, "Allocate Virtio MMIO params for Vdev %s: IRQ %u BASE
> 0x%"PRIx64,
> +disk->vdev, disk->irq, disk->base);
> +}
> +}
> +
> +if (virtio_enabled)
> +nr_spis += (virtio_irq - 32) + 1;
Is it possible to update "nr_spis" inside the loop? The added value
seems to be "number of virtio device + 1", so updating "nr_spis" and
adding +1 after the loop could work, right?
Also, what is "32"? Is it "GUEST_VIRTIO_MMIO_SPI_FIRST - 1" ?
> +
> for (i = 0; i < d_config->b_info.num_irqs; i++) {
> uint32_t irq = d_config->b_info.irqs[i];
> uint32_t spi;
> @@ -787,6 +860,39 @@ static int make_vpci_node(libxl__gc *gc, void *fdt,
> return 0;
> }
>
> +
> +static int make_virtio_mmio_node(libxl__gc *gc, void *fdt,
> + uint64_t base, uint32_t irq)
> +{
> +int res;
> +gic_interrupt intr;
> +/* Placeholder for virtio@ + a 64-bit number + \0 */
> +char buf[24];
> +
> +snprintf(buf, sizeof(buf), "virtio@%"PRIx64, base);
Could you use GCSPRINTF() here instead of using a buffer of a static
size calculated by hand which is potentially wrong? Also, the return
value of snprintf isn't checked so the string could be truncated without
warning. So I think GCSPRINTF is better than a static buffer.
The rest of the patch looks fine.
Thanks,
--
Anthony PERARD
[ovmf test] 170534: regressions - FAIL
flight 170534 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170534/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 708620d29db89d03e822b8d17dc75fbac865c6dc baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 79 days Failing since168258 2022-03-01 01:55:31 Z 78 days 1081 attempts Testing same since 170392 2022-05-13 15:40:22 Z4 days 100 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kun Qin Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Liu, Zhiguang Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min M Xu Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Tom Lendacky Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhiguang Liu Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 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 6662 lines long.)
Re: [PATCH V8 1/2] libxl: Add support for Virtio disk configuration
On Tue, May 03, 2022 at 08:26:02PM +0300, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko
>
> This patch adds basic support for configuring and assisting virtio-mmio
> based virtio-disk backend (emulator) which is intended to run out of
> Qemu and could be run in any domain.
> Although the Virtio block device is quite different from traditional
> Xen PV block device (vbd) from the toolstack's point of view:
> - as the frontend is virtio-blk which is not a Xenbus driver, nothing
>written to Xenstore are fetched by the frontend currently ("vdev"
>is not passed to the frontend). But this might need to be revised
>in future, so frontend data might be written to Xenstore in order to
>support hotplugging virtio devices or passing the backend domain id
>on arch where the device-tree is not available.
> - the ring-ref/event-channel are not used for the backend<->frontend
>communication, the proposed IPC for Virtio is IOREQ/DM
> it is still a "block device" and ought to be integrated in existing
> "disk" handling. So, re-use (and adapt) "disk" parsing/configuration
> logic to deal with Virtio devices as well.
>
> For the immediate purpose and an ability to extend that support for
> other use-cases in future (Qemu, virtio-pci, etc) perform the following
> actions:
> - Add new disk backend type (LIBXL_DISK_BACKEND_OTHER) and reflect
> that in the configuration
> - Introduce new disk "specification" and "transport" fields to struct
> libxl_device_disk. Both are written to the Xenstore. The transport
> field is only used for the specification "virtio" and it assumes
> only "mmio" value for now.
> - Introduce new "specification" option with "xen" communication
> protocol being default value.
> - Add new device kind (LIBXL__DEVICE_KIND_VIRTIO_DISK) as current
> one (LIBXL__DEVICE_KIND_VBD) doesn't fit into Virtio disk model
Is this still an issue? Since v5, the "disk/vbd" kind is used. Also see
my comment about libxl_device_disk_get_path() regarding this.
> An example of domain configuration for Virtio disk:
> disk = [ 'phy:/dev/mmcblk0p3, xvda1, backendtype=other, specification=virtio']
>
> Nothing has changed for default Xen disk configuration.
>
> Please note, this patch is not enough for virtio-disk to work
> on Xen (Arm), as for every Virtio device (including disk) we need
> to allocate Virtio MMIO params (IRQ and memory region) and pass
> them to the backend, also update Guest device-tree. The subsequent
> patch will add these missing bits. For the current patch,
> the default "irq" and "base" are just written to the Xenstore.
> This is not an ideal splitting, but this way we avoid breaking
> the bisectability.
>
> Signed-off-by: Oleksandr Tyshchenko
> ---
> diff --git a/tools/libs/light/libxl_disk.c b/tools/libs/light/libxl_disk.c
> index a5ca778..7fd98ce 100644
> --- a/tools/libs/light/libxl_disk.c
> +++ b/tools/libs/light/libxl_disk.c
> @@ -163,6 +163,19 @@ static int libxl__device_disk_setdefault(libxl__gc *gc,
> uint32_t domid,
> rc = libxl__resolve_domid(gc, disk->backend_domname,
> >backend_domid);
> if (rc < 0) return rc;
>
> +if (disk->specification == LIBXL_DISK_SPECIFICATION_UNKNOWN)
> +disk->specification = LIBXL_DISK_SPECIFICATION_XEN;
> +
> +/*
> + * The transport field is only used for the specification "virtio" and
> + * it assumes only "mmio" value for now. When there will be a need to add
> + * "pci" support, we will need to remove the enforcement here and
> + * respective assert(s) down the code and let the toolstack to decide
> + * the transport to use.
> + */
> +if (disk->specification == LIBXL_DISK_SPECIFICATION_VIRTIO)
> +disk->transport = LIBXL_DISK_TRANSPORT_MMIO;
Could you check that `disk->transport` is unset when
`specification==xen` ? And probably return ERROR_INVAL in this case.
Also, I don't think you should overwrite the value set by an application
in _setdefault(). If `specification==virtio`, check first that
`transport` as a supported value (unknown or mmio) then you can then you
can set the `transport` value expected by virtio if it wasn't set by the
application. ( An example of this is done the function already when
enforcing qdisk for cdroms. )
> +
> /* Force Qdisk backend for CDROM devices of guests with a device model.
> */
> if (disk->is_cdrom != 0 &&
> libxl__domain_type(gc, domid) == LIBXL_DOMAIN_TYPE_HVM) {
> @@ -317,6 +334,11 @@ static void device_disk_add(libxl__egc *egc, uint32_t
> domid,
> goto out;
> }
>
> +assert((disk->specification == LIBXL_DISK_SPECIFICATION_VIRTIO &&
> +disk->backend == LIBXL_DISK_BACKEND_OTHER) ||
> + (disk->specification != LIBXL_DISK_SPECIFICATION_VIRTIO &&
> +disk->backend != LIBXL_DISK_BACKEND_OTHER));
I'm not sure whether this assert() is useful. The value should already
be correct as we call _setdefault(). It seems
Re: [PATCH] x86/hvm: Widen condition for is_hvm_pv_evtchn_vcpu()
On 18/05/2022 10:07, Jan Beulich wrote: > [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments > unless you have verified the sender and know the content is safe. > > On 11.05.2022 17:14, Jane Malalane wrote: >> Have is_hvm_pv_evtchn_vcpu() return true for vector callbacks for >> evtchn delivery set up on a per-vCPU basis via >> HVMOP_set_evtchn_upcall_vector. > > I'm confused: You say "per-vCPU" here, but ... > >> --- a/xen/arch/x86/include/asm/domain.h >> +++ b/xen/arch/x86/include/asm/domain.h >> @@ -14,8 +14,14 @@ >> >> #define has_32bit_shinfo(d)((d)->arch.has_32bit_shinfo) >> >> +/* >> + * Set to true if either the global vector-type callback or per-vCPU >> + * LAPIC vectors are used. Assume all vCPUs will use >> + * HVMOP_set_evtchn_upcall_vector as long as the initial vCPU does. >> + */ >> #define is_hvm_pv_evtchn_domain(d) (is_hvm_domain(d) && \ >> -(d)->arch.hvm.irq->callback_via_type == HVMIRQ_callback_vector) >> +((d)->arch.hvm.irq->callback_via_type == HVMIRQ_callback_vector || \ >> + (d)->vcpu[0]->arch.hvm.evtchn_upcall_vector)) > > ... you use (d)->vcpu[0] here (and, yes, you say so in the comment) > and ... > >> #define is_hvm_pv_evtchn_vcpu(v) (is_hvm_pv_evtchn_domain(v->domain)) > > ... you don't alter this at all. > > Also (re-ordering context) this ... > >> is_hvm_pv_evtchn_vcpu() returning true is a condition for setting up >> physical IRQ to event channel mappings. > > ... isn't really true - it's is_hvm_pv_evtchn_domain() which controls > this (which in turn is why above you need to make the assumption I've > put under question). With that assumption I think is_hvm_pv_evtchn_vcpu() > would better go away. Here only is_hvm_pv_evtchn_domain() should have been mentioned. The "per-VCPU" was in regard to the vector callback for evthcn delivery setup not being global but now done on a per-vCPU basis, in any case, I will amend the description and title. Thanks for the feedback. Jane.
Re: [PATCH] x86/hvm: Widen condition for is_hvm_pv_evtchn_vcpu()
On 18.05.2022 12:38, Jane Malalane wrote: > On 18/05/2022 10:09, Jan Beulich wrote: >> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments >> unless you have verified the sender and know the content is safe. >> >> On 13.05.2022 17:39, Roger Pau Monné wrote: >>> On Wed, May 11, 2022 at 04:14:23PM +0100, Jane Malalane wrote: Have is_hvm_pv_evtchn_vcpu() return true for vector callbacks for evtchn delivery set up on a per-vCPU basis via HVMOP_set_evtchn_upcall_vector. is_hvm_pv_evtchn_vcpu() returning true is a condition for setting up physical IRQ to event channel mappings. >>> >>> I would add something like: >>> >>> The naming of the CPUID bit is a bit generic about upcall support >>> being available. That's done so that the define name doesn't get >>> overly long like XEN_HVM_CPUID_UPCALL_VECTOR_SUPPORTS_PIRQ or some >>> such. >> >> On top of this at least half a sentence wants saying on why a new >> CPUID bit is introduced in the first place. This doesn't derive in >> any way from title or description. It would be only then when it >> is additionally explained why the name was chosen like this.Indeed it is >> incomplete, thanks for pointing that out. > > I could add: > "A CPUID bit is added so that guests know whether the check > in is_hvm_pv_evtchn_domain() will fail when using > HVMOP_set_evtchn_upcall_vector. This matters for guests that route > PIRQs over event channels since is_hvm_pv_evtchn_domain() is a > condition in physdev_map_pirq()." > > Would this be enough clarification? Yes, thanks. Jan
Re: [PATCH v4 18/21] VT-d: replace all-contiguous page tables by superpage mappings
On 11.05.2022 13:08, Roger Pau Monné wrote: > On Mon, Apr 25, 2022 at 10:43:45AM +0200, Jan Beulich wrote: >> When a page table ends up with all contiguous entries (including all >> identical attributes), it can be replaced by a superpage entry at the >> next higher level. The page table itself can then be scheduled for >> freeing. >> >> The adjustment to LEVEL_MASK is merely to avoid leaving a latent trap >> for whenever we (and obviously hardware) start supporting 512G mappings. >> >> Signed-off-by: Jan Beulich >> Reviewed-by: Kevin Tian > > Like on the AMD side, I wonder whether you can get away with only FTAOD I take it you mean "like on the all-empty side", as on AMD we don't need to do any cache flushing? > doing a cache flush for the last (highest level) PTE, as the lower > ones won't be reachable anyway, as the page-table is freed. But that freeing will happen only later, with a TLB flush in between. Until then we would better make sure the IOMMU sees what was written, even if it reading a stale value _should_ be benign. Jan > Then the flush could be done outside of the locked region. > > The rest LGTM. > > Thanks, Roger. >
Re: [PATCH v4 17/21] AMD/IOMMU: replace all-contiguous page tables by superpage mappings
On 10.05.2022 17:31, Roger Pau Monné wrote:
> On Mon, Apr 25, 2022 at 10:43:16AM +0200, Jan Beulich wrote:
>> @@ -94,11 +95,15 @@ static union amd_iommu_pte set_iommu_pte
>> old.iw != iw || old.ir != ir )
>> {
>> set_iommu_pde_present(pde, next_mfn, 0, iw, ir);
>> -pt_update_contig_markers(>raw, pfn_to_pde_idx(dfn, level),
>> - level, PTE_kind_leaf);
>> +*contig = pt_update_contig_markers(>raw,
>> + pfn_to_pde_idx(dfn, level),
>> + level, PTE_kind_leaf);
>> }
>> else
>> +{
>> old.pr = false; /* signal "no change" to the caller */
>> +*contig = false;
>
> So we assume that any caller getting contig == true must have acted
> and coalesced the page table?
Yes, except that I wouldn't use "must", but "would". It's not a
requirement after all, functionality-wise all will be fine without
re-coalescing.
> Might be worth a comment, to note that the function assumes that a
> previous return of contig == true will have coalesced the page table
> and hence a "no change" PTE write is not expected to happen on a
> contig page table.
I'm not convinced, as there's effectively only one caller,
amd_iommu_map_page(). I also don't see why "no change" would be a
problem. "No change" can't result in a fully contiguous page table
if the page table wasn't fully contiguous already before (at which
point it would have been replaced by a superpage).
Jan
Re: [PATCH] x86/hvm: Widen condition for is_hvm_pv_evtchn_vcpu()
On 18/05/2022 10:09, Jan Beulich wrote: > [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments > unless you have verified the sender and know the content is safe. > > On 13.05.2022 17:39, Roger Pau Monné wrote: >> On Wed, May 11, 2022 at 04:14:23PM +0100, Jane Malalane wrote: >>> Have is_hvm_pv_evtchn_vcpu() return true for vector callbacks for >>> evtchn delivery set up on a per-vCPU basis via >>> HVMOP_set_evtchn_upcall_vector. >>> >>> is_hvm_pv_evtchn_vcpu() returning true is a condition for setting up >>> physical IRQ to event channel mappings. >> >> I would add something like: >> >> The naming of the CPUID bit is a bit generic about upcall support >> being available. That's done so that the define name doesn't get >> overly long like XEN_HVM_CPUID_UPCALL_VECTOR_SUPPORTS_PIRQ or some >> such. > > On top of this at least half a sentence wants saying on why a new > CPUID bit is introduced in the first place. This doesn't derive in > any way from title or description. It would be only then when it > is additionally explained why the name was chosen like this.Indeed it is > incomplete, thanks for pointing that out. I could add: "A CPUID bit is added so that guests know whether the check in is_hvm_pv_evtchn_domain() will fail when using HVMOP_set_evtchn_upcall_vector. This matters for guests that route PIRQs over event channels since is_hvm_pv_evtchn_domain() is a condition in physdev_map_pirq()." Would this be enough clarification? Thank you, Jane.
Re: [PATCH 2/2] tools/ocaml: Fix stubs the introduction of domain_create.cpupool_id
> On 18 May 2022, at 11:12, Andrew Cooper wrote: > > On 18/05/2022 10:51, Edwin Torok wrote: >>> diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml >>> index 7503031d8f61..8eab6f60eb14 100644 >>> --- a/tools/ocaml/libs/xc/xenctrl.ml >>> +++ b/tools/ocaml/libs/xc/xenctrl.ml >>> @@ -85,6 +85,7 @@ type domctl_create_config = >>> max_grant_frames: int; >>> max_maptrack_frames: int; >>> max_grant_version: int; >>> + cpupool_id: int32; >> What are the valid values for a CPU pool id, in particular what value should >> be passed here to get back the behaviour prior to these changes in Xen? >> (i.e. would it be cpu pool id 0 or -1 if cpu pools aren't otherwise >> explicitly configured on the system) > > cpupools are a non-optional construct in Xen. > > By default, one cpupool exists, with the id 0, using the default > scheduler covering all pCPUs, and dom0 is constructed in this cpupool. > > Passing 0 here is the backwards compatible option. > > And on that note, Luca, you ought to patch xl/libxl to apply the pool= > setting directly during domain create, rather than depending on cpupool > 0 existing and moving the domain later. Is it an enhancement or a bug fix? From what I know, please correct me if I’m wrong, cpupool0 Is always present, so there won’t be problem on assuming its existence > > ~Andrew
Re: [PATCH 1/2] xen/cpupool: Reject attempts to add a domain to CPUPOOLID_NONE
Hi Andrew,
> On 17 May 2022, at 20:41, Andrew Cooper wrote:
>
> c/s cfc52148444f ("xen/domain: Reduce the quantity of initialisation for
> system domains") removed the path in domain_create() which called
> sched_init_domain() with CPUPOOLID_NONE for system domains.
>
> Arguably, that changeset should have cleaned up this path too.
>
> However, c/s 92ea9c54fc81 ("arm/dom0less: assign dom0less guests to cpupools")
> changed domain_create() from using a hardcoded poolid of 0, to using a value
> passed by the toolstack.
>
> While CPUPOOLID_NONE is an internal constant, userspace can pass -1 for the
> cpupool_id parameter and attempt to construct a real domain using default ops,
> which at a minimum will fail the assertion in dom_scheduler().
>
> Fixes: 92ea9c54fc81 ("arm/dom0less: assign dom0less guests to cpupools")
> Signed-off-by: Andrew Cooper
Thanks for this fix, with the introduction of 92ea9c54fc81 ("arm/dom0less:
assign dom0less guests to cpupools”)
we’ve checked all the path passing struct xen_domctl_createdomain, and at that
time it seems to be that
the new cpupool_id member would have been always zero when created from the
tool stack, am I wrong?
I’m asking so that I will keep in mind for the future.
However with your second patch of this serie, the tool stack is able to write
it, so I guess this fix now is mandatory.
I’ve tested your patch, enabling boot time cpupools, on an arm machine and
booting Xen+Dom0 and another DomU
by dom0less feature, and all works.
Reviewed-by: Luca Fancellu
Tested-by: Luca Fancellu
Cheers,
Luca
Re: [PATCH v4 16/21] VT-d: free all-empty page tables
On 10.05.2022 16:30, Roger Pau Monné wrote:
> On Mon, Apr 25, 2022 at 10:42:50AM +0200, Jan Beulich wrote:
>> When a page table ends up with no present entries left, it can be
>> replaced by a non-present entry at the next higher level. The page table
>> itself can then be scheduled for freeing.
>>
>> Note that while its output isn't used there yet,
>> pt_update_contig_markers() right away needs to be called in all places
>> where entries get updated, not just the one where entries get cleared.
>>
>> Note further that while pt_update_contig_markers() updates perhaps
>> several PTEs within the table, since these are changes to "avail" bits
>> only I do not think that cache flushing would be needed afterwards. Such
>> cache flushing (of entire pages, unless adding yet more logic to me more
>> selective) would be quite noticable performance-wise (very prominent
>> during Dom0 boot).
>>
>> Signed-off-by: Jan Beulich
>> ---
>> v4: Re-base over changes earlier in the series.
>> v3: Properly bound loop. Re-base over changes earlier in the series.
>> v2: New.
>> ---
>> The hang during boot on my Latitude E6410 (see the respective code
>> comment) was pretty close after iommu_enable_translation(). No errors,
>> no watchdog would kick in, just sometimes the first few pixel lines of
>> the next log message's (XEN) prefix would have made it out to the screen
>> (and there's no serial there). It's been a lot of experimenting until I
>> figured the workaround (which I consider ugly, but halfway acceptable).
>> I've been trying hard to make sure the workaround wouldn't be masking a
>> real issue, yet I'm still wary of it possibly doing so ... My best guess
>> at this point is that on these old IOMMUs the ignored bits 52...61
>> aren't really ignored for present entries, but also aren't "reserved"
>> enough to trigger faults. This guess is from having tried to set other
>> bits in this range (unconditionally, and with the workaround here in
>> place), which yielded the same behavior.
>
> Should we take Kevin's Reviewed-by as a heads up that bits 52..61 on
> some? IOMMUs are not usable?
>
> Would be good if we could get a more formal response I think.
A more formal response would be nice, but given the age of the affected
hardware I don't expect anything more will be done there by Intel.
>> @@ -405,6 +408,9 @@ static uint64_t addr_to_dma_page_maddr(s
>>
>> write_atomic(>val, new_pte.val);
>> iommu_sync_cache(pte, sizeof(struct dma_pte));
>> +pt_update_contig_markers(>val,
>> + address_level_offset(addr, level),
>
> I think (unless previous patches in the series have changed this)
> there already is an 'offset' local variable that you could use.
The variable is clobbered by "IOMMU/x86: prefill newly allocate page
tables".
>> @@ -837,9 +843,31 @@ static int dma_pte_clear_one(struct doma
>>
>> old = *pte;
>> dma_clear_pte(*pte);
>> +iommu_sync_cache(pte, sizeof(*pte));
>> +
>> +while ( pt_update_contig_markers(>val,
>> + address_level_offset(addr, level),
>> + level, PTE_kind_null) &&
>> +++level < min_pt_levels )
>> +{
>> +struct page_info *pg = maddr_to_page(pg_maddr);
>> +
>> +unmap_vtd_domain_page(page);
>> +
>> +pg_maddr = addr_to_dma_page_maddr(domain, addr, level, flush_flags,
>> + false);
>> +BUG_ON(pg_maddr < PAGE_SIZE);
>> +
>> +page = map_vtd_domain_page(pg_maddr);
>> +pte = [address_level_offset(addr, level)];
>> +dma_clear_pte(*pte);
>> +iommu_sync_cache(pte, sizeof(*pte));
>> +
>> +*flush_flags |= IOMMU_FLUSHF_all;
>> +iommu_queue_free_pgtable(hd, pg);
>> +}
>
> I think I'm setting myself for trouble, but do we need to sync cache
> the lower lever entries if higher level ones are to be changed.
>
> IOW, would it be fine to just flush the highest level modified PTE?
> As the lower lever ones won't be reachable anyway.
I definitely want to err on the safe side here. If later we can
prove that some cache flush is unneeded, I'd be happy to see it
go away.
>> @@ -2182,8 +2210,21 @@ static int __must_check cf_check intel_i
>> }
>>
>> *pte = new;
>> -
>> iommu_sync_cache(pte, sizeof(struct dma_pte));
>> +
>> +/*
>> + * While the (ab)use of PTE_kind_table here allows to save some work in
>> + * the function, the main motivation for it is that it avoids a so far
>> + * unexplained hang during boot (while preparing Dom0) on a Westmere
>> + * based laptop.
>> + */
>> +pt_update_contig_markers(>val,
>> + address_level_offset(dfn_to_daddr(dfn), level),
>> + level,
>> + (hd->platform_ops->page_sizes &
>> + (1UL << level_to_offset_bits(level + 1))
>> +
Re: [PATCH 1/2] xen/cpupool: Reject attempts to add a domain to CPUPOOLID_NONE
On 17.05.22 21:41, Andrew Cooper wrote:
c/s cfc52148444f ("xen/domain: Reduce the quantity of initialisation for
system domains") removed the path in domain_create() which called
sched_init_domain() with CPUPOOLID_NONE for system domains.
Arguably, that changeset should have cleaned up this path too.
However, c/s 92ea9c54fc81 ("arm/dom0less: assign dom0less guests to cpupools")
changed domain_create() from using a hardcoded poolid of 0, to using a value
passed by the toolstack.
While CPUPOOLID_NONE is an internal constant, userspace can pass -1 for the
cpupool_id parameter and attempt to construct a real domain using default ops,
which at a minimum will fail the assertion in dom_scheduler().
Fixes: 92ea9c54fc81 ("arm/dom0less: assign dom0less guests to cpupools")
Signed-off-by: Andrew Cooper
Reviewed-by: Juergen Gross
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
RE: [PATCH v6 0/3] amd/msr: implement MSR_VIRT_SPEC_CTRL for HVM guests
Hi Jan, > -Original Message- > From: Jan Beulich > > On 18.05.2022 11:51, Henry Wang wrote: > >> -Original Message- > >> From: Jan Beulich > >> > >> On 17.05.2022 17:31, Roger Pau Monne wrote: > >>> Roger Pau Monne (3): > >>> amd/msr: implement VIRT_SPEC_CTRL for HVM guests on top of > >> SPEC_CTRL > >>> amd/msr: allow passthrough of VIRT_SPEC_CTRL for HVM guests > >>> amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy > SSBD > >> > >> FTAOD, while besides a missing ack for ... > >> > >>> CHANGELOG.md| 3 + > >> > >> ... this addition the series would now look to be ready to go in, > >> I'd like to have some form of confirmation by you, Andrew, that > >> you now view this as meeting the comments you gave on an earlier > >> version. > > > > Not completely sure if I am proper to do that but for the CHANGELOG.md > > change: > > Well, no-one except you actually can ack changes to this file, as per > ./MAINTAINERS. Thanks for confirming and sending the reminder to help me to understand that I should ack the changes in CHANGELOG.md for this series, I will keep the information in mind and I guess I am gradually acquiring experiences :) Kind regards, Henry > > > Acked-by: Henry Wang > > Thanks. > > Jan
Re: [PATCH v4 15/21] AMD/IOMMU: free all-empty page tables
On 10.05.2022 15:30, Roger Pau Monné wrote:
> On Mon, Apr 25, 2022 at 10:42:19AM +0200, Jan Beulich wrote:
>> When a page table ends up with no present entries left, it can be
>> replaced by a non-present entry at the next higher level. The page table
>> itself can then be scheduled for freeing.
>>
>> Note that while its output isn't used there yet,
>> pt_update_contig_markers() right away needs to be called in all places
>> where entries get updated, not just the one where entries get cleared.
>>
>> Signed-off-by: Jan Beulich
>
> Reviewed-by: Roger Pau Monné
Thanks.
>> @@ -85,7 +92,11 @@ static union amd_iommu_pte set_iommu_pte
>> if ( !old.pr || old.next_level ||
>> old.mfn != next_mfn ||
>> old.iw != iw || old.ir != ir )
>> +{
>> set_iommu_pde_present(pde, next_mfn, 0, iw, ir);
>> +pt_update_contig_markers(>raw, pfn_to_pde_idx(dfn, level),
>> + level, PTE_kind_leaf);
>
> It would be better to call pt_update_contig_markers inside of
> set_iommu_pde_present, but that would imply changing the parameters
> passed to the function. It's cumbersome (and error prone) to have to
> pair calls to set_iommu_pde_present() with pt_update_contig_markers().
Right, but then already the sheer number of parameters would become
excessive (imo).
>> @@ -474,8 +491,24 @@ int cf_check amd_iommu_unmap_page(
>>
>> if ( pt_mfn )
>> {
>> +bool free;
>> +
>> /* Mark PTE as 'page not present'. */
>> -old = clear_iommu_pte_present(pt_mfn, dfn_x(dfn), level);
>> +old = clear_iommu_pte_present(pt_mfn, dfn_x(dfn), level, );
>> +
>> +while ( unlikely(free) && ++level < hd->arch.amd.paging_mode )
>> +{
>> +struct page_info *pg = mfn_to_page(_mfn(pt_mfn));
>> +
>> +if ( iommu_pde_from_dfn(d, dfn_x(dfn), level, _mfn,
>> +flush_flags, false) )
>> +BUG();
>> +BUG_ON(!pt_mfn);
>> +
>> +clear_iommu_pte_present(pt_mfn, dfn_x(dfn), level, );
>
> Not sure it's worth initializing free to false (at definition and
> before each call to clear_iommu_pte_present), just in case we manage
> to return early from clear_iommu_pte_present without having updated
> 'free'.
There's no such path now, so I'd view it as dead code to do so. If
necessary a patch introducing such an early exit would need to deal
with this. But even then I'd rather see this being dealt with right
in clear_iommu_pte_present().
Jan
Re: [PATCH 2/2] tools/ocaml: Fix stubs the introduction of domain_create.cpupool_id
On 18/05/2022 10:51, Edwin Torok wrote: >> diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml >> index 7503031d8f61..8eab6f60eb14 100644 >> --- a/tools/ocaml/libs/xc/xenctrl.ml >> +++ b/tools/ocaml/libs/xc/xenctrl.ml >> @@ -85,6 +85,7 @@ type domctl_create_config = >> max_grant_frames: int; >> max_maptrack_frames: int; >> max_grant_version: int; >> +cpupool_id: int32; > What are the valid values for a CPU pool id, in particular what value should > be passed here to get back the behaviour prior to these changes in Xen? > (i.e. would it be cpu pool id 0 or -1 if cpu pools aren't otherwise > explicitly configured on the system) cpupools are a non-optional construct in Xen. By default, one cpupool exists, with the id 0, using the default scheduler covering all pCPUs, and dom0 is constructed in this cpupool. Passing 0 here is the backwards compatible option. And on that note, Luca, you ought to patch xl/libxl to apply the pool= setting directly during domain create, rather than depending on cpupool 0 existing and moving the domain later. ~Andrew
Re: [PATCH v6 0/3] amd/msr: implement MSR_VIRT_SPEC_CTRL for HVM guests
On 18.05.2022 11:51, Henry Wang wrote: >> -Original Message- >> From: Jan Beulich >> >> On 17.05.2022 17:31, Roger Pau Monne wrote: >>> Roger Pau Monne (3): >>> amd/msr: implement VIRT_SPEC_CTRL for HVM guests on top of >> SPEC_CTRL >>> amd/msr: allow passthrough of VIRT_SPEC_CTRL for HVM guests >>> amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD >> >> FTAOD, while besides a missing ack for ... >> >>> CHANGELOG.md| 3 + >> >> ... this addition the series would now look to be ready to go in, >> I'd like to have some form of confirmation by you, Andrew, that >> you now view this as meeting the comments you gave on an earlier >> version. > > Not completely sure if I am proper to do that but for the CHANGELOG.md > change: Well, no-one except you actually can ack changes to this file, as per ./MAINTAINERS. > Acked-by: Henry Wang Thanks. Jan
Re: [PATCH v4 14/21] x86: introduce helper for recording degree of contiguity in page tables
On 06.05.2022 15:25, Roger Pau Monné wrote:
> On Mon, Apr 25, 2022 at 10:41:23AM +0200, Jan Beulich wrote:
>> --- /dev/null
>> +++ b/xen/arch/x86/include/asm/pt-contig-markers.h
>> @@ -0,0 +1,105 @@
>> +#ifndef __ASM_X86_PT_CONTIG_MARKERS_H
>> +#define __ASM_X86_PT_CONTIG_MARKERS_H
>> +
>> +/*
>> + * Short of having function templates in C, the function defined below is
>> + * intended to be used by multiple parties interested in recording the
>> + * degree of contiguity in mappings by a single page table.
>> + *
>> + * Scheme: Every entry records the order of contiguous successive entries,
>> + * up to the maximum order covered by that entry (which is the number of
>> + * clear low bits in its index, with entry 0 being the exception using
>> + * the base-2 logarithm of the number of entries in a single page table).
>> + * While a few entries need touching upon update, knowing whether the
>> + * table is fully contiguous (and can hence be replaced by a higher level
>> + * leaf entry) is then possible by simply looking at entry 0's marker.
>> + *
>> + * Prereqs:
>> + * - CONTIG_MASK needs to be #define-d, to a value having at least 4
>> + * contiguous bits (ignored by hardware), before including this file,
>> + * - page tables to be passed here need to be initialized with correct
>> + * markers.
>
> Not sure it's very relevant, but might we worth adding that:
>
> - Null entries must have the PTE zeroed except for the CONTIG_MASK
> region in order to be considered as inactive.
NP, I've added an item along these lines.
>> +static bool pt_update_contig_markers(uint64_t *pt, unsigned int idx,
>> + unsigned int level, enum PTE_kind kind)
>> +{
>> +unsigned int b, i = idx;
>> +unsigned int shift = (level - 1) * CONTIG_LEVEL_SHIFT + PAGE_SHIFT;
>> +
>> +ASSERT(idx < CONTIG_NR);
>> +ASSERT(!(pt[idx] & CONTIG_MASK));
>> +
>> +/* Step 1: Reduce markers in lower numbered entries. */
>> +while ( i )
>> +{
>> +b = find_first_set_bit(i);
>> +i &= ~(1U << b);
>> +if ( GET_MARKER(pt[i]) > b )
>> +SET_MARKER(pt[i], b);
>
> Can't you exit early when you find an entry that already has the
> to-be-set contiguous marker <= b, as lower numbered entries will then
> also be <= b'?
>
> Ie:
>
> if ( GET_MARKER(pt[i]) <= b )
> break;
> else
> SET_MARKER(pt[i], b);
Almost - I think it would need to be
if ( GET_MARKER(pt[i]) < b )
break;
if ( GET_MARKER(pt[i]) > b )
SET_MARKER(pt[i], b);
or, accepting redundant updates,
if ( GET_MARKER(pt[i]) < b )
break;
SET_MARKER(pt[i], b);
. Neither the redundant updates nor the extra (easily mis-predicted)
conditional looked very appealing to me, but I guess I could change
this if you are convinced that's better than continuing a loop with
at most 9 (typically less) iterations.
Jan
Re: [PATCH 2/2] tools/ocaml: Fix stubs the introduction of domain_create.cpupool_id
> On 17 May 2022, at 20:41, Andrew Cooper wrote:
>
> Sadly, cpupool IDs are chosen by the caller, not assigned sequentially, so
> this does need to have a full 32 bits of range.
>
> Also leave a BUILD_BUG_ON() to catch more obvious ABI changes in the future.
>
> Fixes: 92ea9c54fc81 ("arm/dom0less: assign dom0less guests to cpupools")
> Signed-off-by: Andrew Cooper
Thanks for the fix.
> ---
> CC: Christian Lindig
> CC: Edwin Török
> CC: Luca Fancellu
> ---
> tools/ocaml/libs/xc/xenctrl.ml | 1 +
> tools/ocaml/libs/xc/xenctrl.mli | 1 +
> tools/ocaml/libs/xc/xenctrl_stubs.c | 8 +++-
> 3 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
> index 7503031d8f61..8eab6f60eb14 100644
> --- a/tools/ocaml/libs/xc/xenctrl.ml
> +++ b/tools/ocaml/libs/xc/xenctrl.ml
> @@ -85,6 +85,7 @@ type domctl_create_config =
> max_grant_frames: int;
> max_maptrack_frames: int;
> max_grant_version: int;
> + cpupool_id: int32;
What are the valid values for a CPU pool id, in particular what value should be
passed here to get back the behaviour prior to these changes in Xen?
(i.e. would it be cpu pool id 0 or -1 if cpu pools aren't otherwise explicitly
configured on the system)
Thanks,
--Edwin
> arch: arch_domainconfig;
> }
>
> diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
> index d1d9c9247afc..d3014a2708d8 100644
> --- a/tools/ocaml/libs/xc/xenctrl.mli
> +++ b/tools/ocaml/libs/xc/xenctrl.mli
> @@ -77,6 +77,7 @@ type domctl_create_config = {
> max_grant_frames: int;
> max_maptrack_frames: int;
> max_grant_version: int;
> + cpupool_id: int32;
> arch: arch_domainconfig;
> }
>
> diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c
> b/tools/ocaml/libs/xc/xenctrl_stubs.c
> index 5b4fe72c8dec..513ee142d2a0 100644
> --- a/tools/ocaml/libs/xc/xenctrl_stubs.c
> +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
> @@ -189,7 +189,8 @@ CAMLprim value stub_xc_domain_create(value xch, value
> wanted_domid, value config
> #define VAL_MAX_GRANT_FRAMESField(config, 6)
> #define VAL_MAX_MAPTRACK_FRAMES Field(config, 7)
> #define VAL_MAX_GRANT_VERSION Field(config, 8)
> -#define VAL_ARCHField(config, 9)
> +#define VAL_CPUPOOL_ID Field(config, 9)
> +#define VAL_ARCHField(config, 10)
>
> uint32_t domid = Int_val(wanted_domid);
> int result;
> @@ -201,6 +202,7 @@ CAMLprim value stub_xc_domain_create(value xch, value
> wanted_domid, value config
> .max_maptrack_frames = Int_val(VAL_MAX_MAPTRACK_FRAMES),
> .grant_opts =
> XEN_DOMCTL_GRANT_version(Int_val(VAL_MAX_GRANT_VERSION)),
> + .cpupool_id = Int32_val(VAL_CPUPOOL_ID),
> };
>
> domain_handle_of_uuid_string(cfg.handle, String_val(VAL_HANDLE));
> @@ -225,6 +227,9 @@ CAMLprim value stub_xc_domain_create(value xch, value
> wanted_domid, value config
> case 1: /* X86 - emulation flags in the block */
> #if defined(__i386__) || defined(__x86_64__)
>
> + /* Quick & dirty check for ABI changes. */
> + BUILD_BUG_ON(sizeof(cfg) != 64);
> +
> /* Mnemonics for the named fields inside xen_x86_arch_domainconfig */
> #define VAL_EMUL_FLAGS Field(arch_domconfig, 0)
>
> @@ -254,6 +259,7 @@ CAMLprim value stub_xc_domain_create(value xch, value
> wanted_domid, value config
> }
>
> #undef VAL_ARCH
> +#undef VAL_CPUPOOL_ID
> #undef VAL_MAX_GRANT_VERSION
> #undef VAL_MAX_MAPTRACK_FRAMES
> #undef VAL_MAX_GRANT_FRAMES
> --
> 2.11.0
>
RE: [PATCH v6 0/3] amd/msr: implement MSR_VIRT_SPEC_CTRL for HVM guests
Hi Jan, Roger and Andrew,
> -Original Message-
> From: Jan Beulich
>
> On 17.05.2022 17:31, Roger Pau Monne wrote:
> > Hello,
> >
> > The following series implements support for MSR_VIRT_SPEC_CTRL
> > (VIRT_SSBD) on different AMD CPU families.
> >
> > Note that the support is added backwards, starting with the newer CPUs
> > that support MSR_SPEC_CTRL and moving to the older ones either using
> > MSR_VIRT_SPEC_CTRL or the SSBD bit in LS_CFG.
> >
> > Xen is still free to use it's own SSBD setting, as the selection is
> > context switched on vm{entry,exit}.
> >
> > On Zen2 and later, SPEC_CTRL.SSBD should exist and should be used in
> > preference to VIRT_SPEC_CTRL.SSBD. However, for migration
> > compatibility, Xen offers VIRT_SSBD to guests (in the max cpuid policy,
> > not default) implemented in terms of SPEC_CTRL.SSBD.
> >
> > On Fam15h thru Zen1, Xen exposes VIRT_SSBD to guests by default to
> > abstract away the model and/or hypervisor specific differences in
> > MSR_LS_CFG/MSR_VIRT_SPEC_CTRL.
> >
> > So the implementation of VIRT_SSBD exposed to HVM guests will use one
> of
> > the following underlying mechanisms, in the preference order listed
> > below:
> >
> > * SPEC_CTRL.SSBD: patch 1
> > * VIRT_SPEC_CTRL.SSBD: patch 2.
> > * Non-architectural way using LS_CFG: patch 3.
> >
> > NB: patch 3 introduces some logic in GIF=0 context, such logic has been
> > kept to a minimum due to the special context it's running on.
> >
> > Roger Pau Monne (3):
> > amd/msr: implement VIRT_SPEC_CTRL for HVM guests on top of
> SPEC_CTRL
> > amd/msr: allow passthrough of VIRT_SPEC_CTRL for HVM guests
> > amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD
>
> FTAOD, while besides a missing ack for ...
>
> > CHANGELOG.md| 3 +
>
> ... this addition the series would now look to be ready to go in,
> I'd like to have some form of confirmation by you, Andrew, that
> you now view this as meeting the comments you gave on an earlier
> version.
Not completely sure if I am proper to do that but for the CHANGELOG.md
change:
Acked-by: Henry Wang
Kind regards,
Henry
>
> Jan
Re: [PATCH v6 0/9] xen: drop hypercall function tables
On 18.05.2022 11:45, Juergen Gross wrote: > BTW, the question regarding patches 1, 2 and 4 to go in as being cleanups > still stands. I would long have committed these (without waiting for Andrew), but patch 1 continues to lack an x86 side ack (which, as indicated before, I'm not happy to provide, while I also don't mean to nak the change). From an earlier attempt I also seem to recall that patches 2 and 4 can't go in ahead of patch 1. Jan
Re: [PATCH v6 0/9] xen: drop hypercall function tables
On 04.05.22 09:53, Juergen Gross wrote: On 19.04.22 10:01, Juergen Gross wrote: On 24.03.22 15:01, Juergen Gross wrote: In order to avoid indirect function calls on the hypercall path as much as possible this series is removing the hypercall function tables and is replacing the hypercall handler calls via the function array by automatically generated call macros. Another by-product of generating the call macros is the automatic generating of the hypercall handler prototypes from the same data base which is used to generate the macros. This has the additional advantage of using type safe calls of the handlers and to ensure related handler (e.g. PV and HVM ones) share the same prototypes. A very brief performance test (parallel build of the Xen hypervisor in a 6 vcpu guest) showed a very slim improvement (less than 1%) of the performance with the patches applied. The test was performed using a PV and a PVH guest. A gentle ping regarding this series. I think patch 1 still lacks an Ack from x86 side. Other than that patches 1, 2 and 4 should be fine to go in, as they are cleanups which are fine on their own IMHO. Andrew, you wanted to get some performance numbers of the series using the Citrix test environment. Any news on the progress here? And another ping. Andrew, could you please give some feedback regarding performance testing progress? This is becoming ridiculous. Andrew, I know you are busy, but not reacting at all to explicit questions is kind of annoying. BTW, the question regarding patches 1, 2 and 4 to go in as being cleanups still stands. Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH v6 0/3] amd/msr: implement MSR_VIRT_SPEC_CTRL for HVM guests
On 17.05.2022 17:31, Roger Pau Monne wrote:
> Hello,
>
> The following series implements support for MSR_VIRT_SPEC_CTRL
> (VIRT_SSBD) on different AMD CPU families.
>
> Note that the support is added backwards, starting with the newer CPUs
> that support MSR_SPEC_CTRL and moving to the older ones either using
> MSR_VIRT_SPEC_CTRL or the SSBD bit in LS_CFG.
>
> Xen is still free to use it's own SSBD setting, as the selection is
> context switched on vm{entry,exit}.
>
> On Zen2 and later, SPEC_CTRL.SSBD should exist and should be used in
> preference to VIRT_SPEC_CTRL.SSBD. However, for migration
> compatibility, Xen offers VIRT_SSBD to guests (in the max cpuid policy,
> not default) implemented in terms of SPEC_CTRL.SSBD.
>
> On Fam15h thru Zen1, Xen exposes VIRT_SSBD to guests by default to
> abstract away the model and/or hypervisor specific differences in
> MSR_LS_CFG/MSR_VIRT_SPEC_CTRL.
>
> So the implementation of VIRT_SSBD exposed to HVM guests will use one of
> the following underlying mechanisms, in the preference order listed
> below:
>
> * SPEC_CTRL.SSBD: patch 1
> * VIRT_SPEC_CTRL.SSBD: patch 2.
> * Non-architectural way using LS_CFG: patch 3.
>
> NB: patch 3 introduces some logic in GIF=0 context, such logic has been
> kept to a minimum due to the special context it's running on.
>
> Roger Pau Monne (3):
> amd/msr: implement VIRT_SPEC_CTRL for HVM guests on top of SPEC_CTRL
> amd/msr: allow passthrough of VIRT_SPEC_CTRL for HVM guests
> amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD
FTAOD, while besides a missing ack for ...
> CHANGELOG.md| 3 +
... this addition the series would now look to be ready to go in,
I'd like to have some form of confirmation by you, Andrew, that
you now view this as meeting the comments you gave on an earlier
version.
Jan
Re: [PATCH v3 15/21] xen/tpmfront: use xenbus_setup_ring() and xenbus_teardown_ring()
On 07.05.22 00:32, Jarkko Sakkinen wrote: On Thu, May 05, 2022 at 10:16:34AM +0200, Juergen Gross wrote: Simplify tpmfront's ring creation and removal via xenbus_setup_ring() and xenbus_teardown_ring(). Signed-off-by: Juergen Gross Please add to the commit message why these provide an equivalent functionality. Would you be fine with: Simplify tpmfront's ring creation and removal via xenbus_setup_ring() and xenbus_teardown_ring(), which are provided exactly for the use pattern as seen in this driver. Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH v5] Preserve the EFI System Resource Table for dom0
On 17.05.2022 19:07, Demi Marie Obenour wrote:
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -39,6 +39,25 @@
>{ 0x605dab50, 0xe046, 0x4300, {0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b,
> 0x23} }
> #define APPLE_PROPERTIES_PROTOCOL_GUID \
>{ 0x91bd12fe, 0xf6c3, 0x44fb, { 0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a,
> 0xe0} }
> +#define ESRT_GUID\
> + { 0xb122a263, 0x3661, 0x4f68, {0x99, 0x29, 0x78, 0xf8, 0xb0, 0xd6, 0x21,
> 0x80} }
I'm sorry, but it looks like my earlier comments still weren't clear
enough: The spec calls this EFI_SYSTEM_RESOURCE_TABLE_GUID, ...
> +typedef struct _ESRT_ENTRY {
... has no tag here, ...
> +EFI_GUID FwClass;
> +UINT32 FwType;
> +UINT32 FwVersion;
> +UINT32 FwLowestSupportedVersion;
> +UINT32 FwCapsuleFlags;
> +UINT32 FwLastAttemptVersion;
> +UINT32 FwLastAttemptStatus;
> +} ESRT_ENTRY;
... calls this EFI_SYSTEM_RESOURCE_ENTRY, ...
> +typedef struct _ESRT {
... again has no tag here, and ...
> +UINT32 Count;
> +UINT32 Max;
> +UINT64 Version;
> +ESRT_ENTRY Entries[];
> +} ESRT;
... calls this EFI_SYSTEM_RESOURCE_TABLE. Also some of the field
names still aren't matching the spec.
> @@ -1067,6 +1120,46 @@ static void __init efi_exit_boot(EFI_HANDLE
> ImageHandle, EFI_SYSTEM_TABLE *Syste
> if ( !efi_memmap )
> blexit(L"Unable to allocate memory for EFI memory map");
>
> +efi_memmap_size = info_size;
> +status = SystemTable->BootServices->GetMemoryMap(_memmap_size,
> + efi_memmap, _key,
> + _mdesc_size,
> + _ver);
> +if ( EFI_ERROR(status) )
> +PrintErrMesg(L"Cannot obtain memory map", status);
> +
> +/* Try to obtain the ESRT. Errors are not fatal. */
> +for ( i = 0; i < efi_memmap_size; i += efi_mdesc_size )
> +{
> +/*
> + * ESRT needs to be moved to memory of type EfiRuntimeServicesData
> + * so that the memory it is in will not be used for other purposes.
> + */
> +void *new_esrt = NULL;
> +size_t esrt_size = get_esrt_size(efi_memmap + i);
> +
> +if ( !esrt_size )
> +continue;
> +if ( ((EFI_MEMORY_DESCRIPTOR *)(efi_memmap + i))->Type ==
> + EfiRuntimeServicesData )
> +break; /* ESRT already safe from reuse */
> +status = efi_bs->AllocatePool(EfiRuntimeServicesData, esrt_size,
> + _esrt);
> +if ( status == EFI_SUCCESS && new_esrt )
> +{
> +memcpy(new_esrt, (void *)esrt, esrt_size);
> +status = efi_bs->InstallConfigurationTable(_guid, new_esrt);
> +if ( status != EFI_SUCCESS )
> +{
> +PrintStr(L"Cannot install new ESRT\r\n");
Perhaps better PrintErr() here and ...
> +efi_bs->FreePool(new_esrt);
> +}
> +}
> +else
> +PrintStr(L"Cannot allocate memory for ESRT\r\n");
... here?
Jan
[ovmf test] 170533: regressions - FAIL
flight 170533 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170533/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 708620d29db89d03e822b8d17dc75fbac865c6dc baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 78 days Failing since168258 2022-03-01 01:55:31 Z 78 days 1080 attempts Testing same since 170392 2022-05-13 15:40:22 Z4 days 99 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kun Qin Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Liu, Zhiguang Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min M Xu Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Tom Lendacky Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhiguang Liu Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 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 6662 lines long.)
Re: [PATCH] x86/hvm: Widen condition for is_hvm_pv_evtchn_vcpu()
On 13.05.2022 17:39, Roger Pau Monné wrote: > On Wed, May 11, 2022 at 04:14:23PM +0100, Jane Malalane wrote: >> Have is_hvm_pv_evtchn_vcpu() return true for vector callbacks for >> evtchn delivery set up on a per-vCPU basis via >> HVMOP_set_evtchn_upcall_vector. >> >> is_hvm_pv_evtchn_vcpu() returning true is a condition for setting up >> physical IRQ to event channel mappings. > > I would add something like: > > The naming of the CPUID bit is a bit generic about upcall support > being available. That's done so that the define name doesn't get > overly long like XEN_HVM_CPUID_UPCALL_VECTOR_SUPPORTS_PIRQ or some > such. On top of this at least half a sentence wants saying on why a new CPUID bit is introduced in the first place. This doesn't derive in any way from title or description. It would be only then when it is additionally explained why the name was chosen like this. Jan
Re: [PATCH] x86/hvm: Widen condition for is_hvm_pv_evtchn_vcpu()
On 11.05.2022 17:14, Jane Malalane wrote: > Have is_hvm_pv_evtchn_vcpu() return true for vector callbacks for > evtchn delivery set up on a per-vCPU basis via > HVMOP_set_evtchn_upcall_vector. I'm confused: You say "per-vCPU" here, but ... > --- a/xen/arch/x86/include/asm/domain.h > +++ b/xen/arch/x86/include/asm/domain.h > @@ -14,8 +14,14 @@ > > #define has_32bit_shinfo(d)((d)->arch.has_32bit_shinfo) > > +/* > + * Set to true if either the global vector-type callback or per-vCPU > + * LAPIC vectors are used. Assume all vCPUs will use > + * HVMOP_set_evtchn_upcall_vector as long as the initial vCPU does. > + */ > #define is_hvm_pv_evtchn_domain(d) (is_hvm_domain(d) && \ > -(d)->arch.hvm.irq->callback_via_type == HVMIRQ_callback_vector) > +((d)->arch.hvm.irq->callback_via_type == HVMIRQ_callback_vector || \ > + (d)->vcpu[0]->arch.hvm.evtchn_upcall_vector)) ... you use (d)->vcpu[0] here (and, yes, you say so in the comment) and ... > #define is_hvm_pv_evtchn_vcpu(v) (is_hvm_pv_evtchn_domain(v->domain)) ... you don't alter this at all. Also (re-ordering context) this ... > is_hvm_pv_evtchn_vcpu() returning true is a condition for setting up > physical IRQ to event channel mappings. ... isn't really true - it's is_hvm_pv_evtchn_domain() which controls this (which in turn is why above you need to make the assumption I've put under question). With that assumption I think is_hvm_pv_evtchn_vcpu() would better go away. Jan
Re: [PATCH] x86/flushtlb: remove flush_area check on system state
On 16.05.2022 16:31, Roger Pau Monne wrote:
> --- a/xen/arch/x86/include/asm/flushtlb.h
> +++ b/xen/arch/x86/include/asm/flushtlb.h
> @@ -146,7 +146,8 @@ void flush_area_mask(const cpumask_t *, const void *va,
> unsigned int flags);
> #define flush_mask(mask, flags) flush_area_mask(mask, NULL, flags)
>
> /* Flush all CPUs' TLBs/caches */
> -#define flush_area_all(va, flags) flush_area_mask(_online_map, va, flags)
> +#define flush_area(va, flags) \
> +flush_area_mask(_online_map, (const void *)(va), flags)
I have to admit that I would prefer if we kept the "_all" name suffix,
to continue to clearly express the scope of the flush. I'm also not
really happy to see the cast being added globally now.
> --- a/xen/arch/x86/smp.c
> +++ b/xen/arch/x86/smp.c
> @@ -262,7 +262,8 @@ void flush_area_mask(const cpumask_t *mask, const void
> *va, unsigned int flags)
> {
> unsigned int cpu = smp_processor_id();
>
> -ASSERT(local_irq_is_enabled());
> +/* Local flushes can be performed with interrupts disabled. */
> +ASSERT(local_irq_is_enabled() || cpumask_equal(mask, cpumask_of(cpu)));
Further down we use cpumask_subset(mask, cpumask_of(cpu)),
apparently to also cover the case where mask is empty. I think
you want to do so here as well.
> if ( (flags & ~(FLUSH_VCPU_STATE | FLUSH_ORDER_MASK)) &&
> cpumask_test_cpu(cpu, mask) )
I suppose we want a further precaution here: Despite the
!cpumask_subset(mask, cpumask_of(cpu)) below I think we want to
extend what c64bf2d2a625 ("x86: make CPU state flush requests
explicit") and later changes (isolating uses of FLUSH_VCPU_STATE
from other FLUSH_*) did and exclude the use of FLUSH_VCPU_STATE
for the local CPU altogether. That's because if such somehow made
it into the conditional below here, it would still involve an IPI.
E.g.
ASSERT(local_irq_is_enabled() ||
(cpumask_subset(mask, cpumask_of(cpu)) &&
!(flags & FLUSH_VCPU_STATE)));
Jan
[linux-linus test] 170523: tolerable FAIL - PUSHED
flight 170523 linux-linus real [real] flight 170530 linux-linus real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/170523/ http://logs.test-lab.xenproject.org/osstest/logs/170530/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-arm64-arm64-libvirt-raw 13 guest-start fail pass in 170530-retest Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-raw 14 migrate-support-check fail in 170530 never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-check fail in 170530 never pass test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 170472 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 170472 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 170472 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 170472 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 170472 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 170472 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 170472 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 170472 test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass version targeted for testing: linux210e04ff768142b96452030c4c2627512b30ad95 baseline version: linux42226c989789d8da4af1de0c31070c96726d990c Last test of basis 170472 2022-05-16 05:17:50 Z2 days Testing same since 170523 2022-05-18 00:11:33 Z0 days1 attempts People who touched revisions under test: Bjorn Andersson Bjorn Helgaas Linus Torvalds Pali Rohár Rafael J. Wysocki Srinivas Pandruvada Stanimir Varbanov Steev
Re: [PATCH 2/2] tools/ocaml: Fix stubs the introduction of domain_create.cpupool_id
Acked-by: Christian Lindig
mailto:christian.lin...@citrix.com>>
On 17 May 2022, at 20:41, Andrew Cooper
mailto:andrew.coop...@citrix.com>> wrote:
Sadly, cpupool IDs are chosen by the caller, not assigned sequentially, so
this does need to have a full 32 bits of range.
Also leave a BUILD_BUG_ON() to catch more obvious ABI changes in the future.
Fixes: 92ea9c54fc81 ("arm/dom0less: assign dom0less guests to cpupools")
Signed-off-by: Andrew Cooper
mailto:andrew.coop...@citrix.com>>
---
CC: Christian Lindig
mailto:christian.lin...@citrix.com>>
CC: Edwin Török mailto:edvin.to...@citrix.com>>
CC: Luca Fancellu mailto:luca.fance...@arm.com>>
---
tools/ocaml/libs/xc/xenctrl.ml | 1 +
tools/ocaml/libs/xc/xenctrl.mli | 1 +
tools/ocaml/libs/xc/xenctrl_stubs.c | 8 +++-
3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index 7503031d8f61..8eab6f60eb14 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -85,6 +85,7 @@ type domctl_create_config =
max_grant_frames: int;
max_maptrack_frames: int;
max_grant_version: int;
+ cpupool_id: int32;
arch: arch_domainconfig;
}
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index d1d9c9247afc..d3014a2708d8 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -77,6 +77,7 @@ type domctl_create_config = {
max_grant_frames: int;
max_maptrack_frames: int;
max_grant_version: int;
+ cpupool_id: int32;
arch: arch_domainconfig;
}
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c
b/tools/ocaml/libs/xc/xenctrl_stubs.c
index 5b4fe72c8dec..513ee142d2a0 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -189,7 +189,8 @@ CAMLprim value stub_xc_domain_create(value xch, value
wanted_domid, value config
#define VAL_MAX_GRANT_FRAMESField(config, 6)
#define VAL_MAX_MAPTRACK_FRAMES Field(config, 7)
#define VAL_MAX_GRANT_VERSION Field(config, 8)
-#define VAL_ARCHField(config, 9)
+#define VAL_CPUPOOL_ID Field(config, 9)
+#define VAL_ARCHField(config, 10)
uint32_t domid = Int_val(wanted_domid);
int result;
@@ -201,6 +202,7 @@ CAMLprim value stub_xc_domain_create(value xch, value
wanted_domid, value config
.max_maptrack_frames = Int_val(VAL_MAX_MAPTRACK_FRAMES),
.grant_opts =
XEN_DOMCTL_GRANT_version(Int_val(VAL_MAX_GRANT_VERSION)),
+ .cpupool_id = Int32_val(VAL_CPUPOOL_ID),
};
domain_handle_of_uuid_string(cfg.handle, String_val(VAL_HANDLE));
@@ -225,6 +227,9 @@ CAMLprim value stub_xc_domain_create(value xch, value
wanted_domid, value config
case 1: /* X86 - emulation flags in the block */
#if defined(__i386__) || defined(__x86_64__)
+ /* Quick & dirty check for ABI changes. */
+ BUILD_BUG_ON(sizeof(cfg) != 64);
+
/* Mnemonics for the named fields inside xen_x86_arch_domainconfig */
#define VAL_EMUL_FLAGS Field(arch_domconfig, 0)
@@ -254,6 +259,7 @@ CAMLprim value stub_xc_domain_create(value xch, value
wanted_domid, value config
}
#undef VAL_ARCH
+#undef VAL_CPUPOOL_ID
#undef VAL_MAX_GRANT_VERSION
#undef VAL_MAX_MAPTRACK_FRAMES
#undef VAL_MAX_GRANT_FRAMES
--
2.11.0
Re: [PATCH 19/30] panic: Add the panic hypervisor notifier list
On Tue 2022-05-17 15:57:34, Petr Mladek wrote: > On Mon 2022-05-16 12:06:17, Guilherme G. Piccoli wrote: > > >> --- a/drivers/soc/bcm/brcmstb/pm/pm-arm.c > > >> +++ b/drivers/soc/bcm/brcmstb/pm/pm-arm.c > > >> @@ -814,7 +814,7 @@ static int brcmstb_pm_probe(struct platform_device > > >> *pdev) > > >> goto out; > > >> } > > >> > > >> -atomic_notifier_chain_register(_notifier_list, > > >> +atomic_notifier_chain_register(_hypervisor_list, > > >> _pm_panic_nb); > > > > > > I am not sure about this one. It instruct some HW to preserve DRAM. > > > IMHO, it better fits into pre_reboot category but I do not have > > > strong opinion. > > > > Disagree here, I'm CCing Florian for information. > > > > This notifier preserves RAM so it's *very interesting* if we have > > kmsg_dump() for example, but maybe might be also relevant in case kdump > > kernel is configured to store something in a persistent RAM (then, > > without this notifier, after kdump reboots the system data would be lost). > > I see. It is actually similar problem as with > drivers/firmware/google/gsmi.c. As discussed in the other other reply, it seems that both affected notifiers do not store kernel logs and should stay in the "hypervisor". > I does similar things like kmsg_dump() so it should be called in > the same location (after info notifier list and before kdump). > > A solution might be to put it at these notifiers at the very > end of the "info" list or make extra "dump" notifier list. I just want to point out that the above idea has problems. Notifiers storing kernel log need to be treated as kmsg_dump(). In particular, we would need to know if there are any. We do not need to call "info" notifier list before kdump when there is no kernel log dumper registered. Best Regards, Petr
[ovmf test] 170531: regressions - FAIL
flight 170531 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170531/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 708620d29db89d03e822b8d17dc75fbac865c6dc baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 78 days Failing since168258 2022-03-01 01:55:31 Z 78 days 1079 attempts Testing same since 170392 2022-05-13 15:40:22 Z4 days 98 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kun Qin Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Liu, Zhiguang Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min M Xu Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Tom Lendacky Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhiguang Liu Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 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 6662 lines long.)
Re: [PATCH 19/30] panic: Add the panic hypervisor notifier list
On Tue 2022-05-17 13:42:06, Guilherme G. Piccoli wrote: > On 17/05/2022 10:57, Petr Mladek wrote: > >> Disagree here, I'm CCing Florian for information. > >> > >> This notifier preserves RAM so it's *very interesting* if we have > >> kmsg_dump() for example, but maybe might be also relevant in case kdump > >> kernel is configured to store something in a persistent RAM (then, > >> without this notifier, after kdump reboots the system data would be lost). > > > > I see. It is actually similar problem as with > > drivers/firmware/google/gsmi.c. > > > > I does similar things like kmsg_dump() so it should be called in > > the same location (after info notifier list and before kdump). > > > > A solution might be to put it at these notifiers at the very > > end of the "info" list or make extra "dump" notifier list. > > Here I still disagree. I've commented in the other response thread > (about Google gsmi) about the semantics of the hypervisor list, but > again: this list should contain callbacks that > > (a) Should run early, _by default_ before a kdump; > (b) Communicate with the firmware/hypervisor in a "low-risk" way; > > Imagine a scenario where users configure kdump kernel to save something > in a persistent form in DRAM - it'd be like a late pstore, in the next > kernel. This callback enables that, it's meant to inform FW "hey, panic > happened, please from now on don't clear the RAM in the next FW-reboot". > I don't see a reason to postpone that - let's see if the maintainers > have an opinion. I have answered this in more detail in the other reply, see https://lore.kernel.org/r/YoShZVYNAdvvjb7z@alley I agree that both notifiers in drivers/soc/bcm/brcmstb/pm/pm-arm.c drivers/firmware/google/gsmi.c better fit into the hypervisor list after all. Best Regards, Petr
Re: Process for cherry-picking patches from other projects
Hi Stefano, On 18/05/2022 04:12, Stefano Stabellini wrote: On Tue, 17 May 2022, Jan Beulich wrote: Hmm. The present rules written down in docs/process/sending-patches.pandoc are a result of me having been accused of unduly stripping S-o-b (and maybe a few other) tags. If that was for a real reason (and not just because of someone's taste), how could it ever be okay to remove S-o-b? (Personally I agree with what you propose, it just doesn't line up with that discussion we had not all that long ago.) This is the meaning of the DCO: https://developercertificate.org The relevant case is: (b) The contribution is based upon previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the same open source license (unless I am permitted to submit under a different license), as indicated in the file; or IANAL but I read this as to mean that only the submitter Signed-off-by is required. Also consider that the code could come from a place where Signed-off-by is not used. As long as the copyright checks out, then we are OK. I don't think I can write better than what Ian wrote back then: " Please can we keep the Linux S-o-b. Note that S-o-b is not a chain of *approval* (whose relevance to us is debateable) but but a chain of *custody and transmission* for copyright/licence/gdpr purposes. That latter chain is hightly relevant to us. All such S-o-b should be retained. Of course if you got the patch somewhere other than the Linux commit, then the chain of custody doesn't pass through the Linux commit. But in that case I expect you to be able to say where you got it. " Cheers, -- Julien Grall
Re: [PATCH 19/30] panic: Add the panic hypervisor notifier list
On Tue 2022-05-17 13:37:58, Guilherme G. Piccoli wrote: > On 17/05/2022 10:28, Petr Mladek wrote: > > [...] > >>> Disagree here. I'm looping Google maintainers, so they can comment. > >>> (CCed Evan, David, Julius) > >>> > >>> This notifier is clearly a hypervisor notification mechanism. I've fixed > >>> a locking stuff there (in previous patch), I feel it's low-risk but even > >>> if it's mid-risk, the class of such callback remains a perfect fit with > >>> the hypervisor list IMHO. > >> > >> This logs a panic to our "eventlog", a tiny logging area in SPI flash > >> for critical and power-related events. In some cases this ends up > >> being the only clue we get in a Chromebook feedback report that a > >> panic occurred, so from my perspective moving it to the front of the > >> line seems like a good idea. > > > > IMHO, this would really better fit into the pre-reboot notifier list: > > > >+ the callback stores the log so it is similar to kmsg_dump() > > or console_flush_on_panic() > > > >+ the callback should be proceed after "info" notifiers > > that might add some other useful information. > > > > Honestly, I am not sure what exactly hypervisor callbacks do. But I > > think that they do not try to extract the kernel log because they > > would need to handle the internal format. > > > > I guess the main point in your response is : "I am not sure what exactly > hypervisor callbacks do". We need to be sure about the semantics of such > list, and agree on that. > > So, my opinion about this first list, that we call "hypervisor list", > is: it contains callbacks that > > (1) should run early, preferably before kdump (or even if kdump isn't > set, should run ASAP); > > (2) these callbacks perform some communication with an abstraction that > runs "below" the kernel, like a firmware or hypervisor. Classic example: > pvpanic, that communicates with VMM (usually qemu) and allow such VMM to > snapshot the full guest memory, for example. > > (3) Should be low-risk. What defines risk is the level of reliability of > subsequent operations - if the callback have 50% of chance of "bricking" > the system totally and prevent kdump / kmsg_dump() / reboot , this is > high risk one for example. > > Some good fits IMO: pvpanic, sstate_panic_event() [sparc], fadump in > powerpc, etc. > > So, this is a good case for the Google notifier as well - it's not > collecting data like the dmesg (hence your second bullet seems to not > apply here, info notifiers won't add info to be collected by gsmi). It > is a firmware/hypervisor/whatever-gsmi-is notification mechanism, that > tells such "lower" abstraction a panic occurred. It seems low risk and > we want it to run ASAP, if possible. " > >> This logs a panic to our "eventlog", a tiny logging area in SPI flash > >> for critical and power-related events. In some cases this ends up I see. I somehow assumed that it was about the kernel log because Evans wrote: "This logs a panic to our "eventlog", a tiny logging area in SPI flash for critical and power-related events. In some cases this ends up" Anyway, I would distinguish it the following way. + If the notifier is preserving kernel log then it should be ideally treated as kmsg_dump(). + It the notifier is saving another debugging data then it better fits into the "hypervisor" notifier list. Regarding the reliability. From my POV, any panic notifier enabled in a generic kernel should be reliable with more than 99,9%. Otherwise, they should not be in the notifier list at all. An exception would be a platform-specific notifier that is called only on some specific platform and developers maintaining this platform agree on this. The value "99,9%" is arbitrary. I am not sure if it is realistic even in the other code, for example, console_flush_on_panic() or emergency_restart(). I just want to point out that the border should be rather high. Otherwise we would back in the situation where people would want to disable particular notifiers. Best Regards, Petr
[libvirt test] 170527: regressions - FAIL
flight 170527 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/170527/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-armhf-libvirt 6 libvirt-buildfail REGR. vs. 151777 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 151777 build-i386-libvirt6 libvirt-buildfail REGR. vs. 151777 build-arm64-libvirt 6 libvirt-buildfail REGR. vs. 151777 Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-raw 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) blocked n/a version targeted for testing: libvirt 9cd2c5257afc9ca985b9b713922c5c68f524f44a baseline version: libvirt 2c846fa6bcc11929c9fb857a22430fb9945654ad Last test of basis 151777 2020-07-10 04:19:19 Z 677 days Failing since151818 2020-07-11 04:18:52 Z 676 days 658 attempts Testing same since 170527 2022-05-18 04:18:55 Z0 days1 attempts People who touched revisions under test: Adolfo Jayme Barrientos Aleksandr Alekseev Aleksei Zakharov Amneesh Singh Andika Triwidada Andrea Bolognani Andrew Melnychenko Ani Sinha Balázs Meskó Barrett Schonefeld Bastian Germann Bastien Orivel BiaoXiang Ye Bihong Yu Binfeng Wu Bjoern Walk Boris Fiuczynski Brad Laue Brian Turek Bruno Haible Chris Mayo Christian Borntraeger Christian Ehrhardt Christian Kirbach Christian Schoenebeck Christophe Fergeau Claudio Fontana Cole Robinson Collin Walling Cornelia Huck Cédric Bosdonnat Côme Borsoi Daniel Henrique Barboza Daniel Letai Daniel P. Berrange Daniel P. Berrangé Didik Supriadi dinglimin Divya Garg Dmitrii Shcherbakov Dmytro Linkin Eiichi Tsukata Emilio Herrera Eric Farman Erik Skultety Fabian Affolter Fabian Freyer Fabiano Fidêncio Fangge Jin Farhan Ali Fedora Weblate Translation Franck Ridel Gavi Teitz gongwei Guoyi Tu Göran Uddeborg Halil Pasic Han Han Hao Wang Haonan Wang Hela Basa Helmut Grohne Hiroki Narukawa Hyman Huang(黄勇) Ian Wienand Ioanna Alifieraki Ivan Teterevkov Jakob Meng Jamie Strandboge Jamie Strandboge Jan Kuparinen jason lee Jean-Baptiste Holcroft Jia Zhou Jianan Gao Jim Fehlig Jin Yan Jing Qi Jinsheng Zhang Jiri Denemark Joachim Falk John Ferlan John Levon John Levon Jonathan Watt Jonathon Jongsma Julio Faracco Justin Gatzen Ján Tomko Kashyap Chamarthy Kevin Locke Kim InSoo Koichi Murase Kristina Hanicova Laine Stump Laszlo Ersek Lee Yarwood Lei Yang Lena Voytek Liang Yan Liang Yan Liao Pingfang Lin Ma Lin Ma Lin Ma Liu Yiding Lubomir Rintel Luke Yue Luyao Zhong luzhipeng Marc Hartmayer Marc-André Lureau Marek Marczykowski-Górecki Markus Schade Martin Kletzander Martin Pitt Masayoshi Mizuma Matej Cepl Matt Coleman Matt Coleman Mauro Matteo Cascella Maxim Nestratov Meina Li Michal Privoznik Michał Smyk Milo Casagrande Moshe Levi Moteen Shah Moteen Shah Muha Aliss Nathan Neal Gompa Nick Chevsky Nick Shyrokovskiy Nickys Music Group Nico Pache Nicolas Lécureuil Nicolas Lécureuil Nikolay Shirokovskiy Nikolay Shirokovskiy Nikolay Shirokovskiy Niteesh Dubey Olaf Hering Olesya Gerasimenko Or Ozeri Orion Poplawski Pany Paolo Bonzini Patrick Magauran Paulo de Rezende Pinatti Pavel Hrdina Peng Liang Peter Krempa Pino Toscano Pino
Re: [PATCH] x86: xen: remove STACK_FRAME_NON_STANDARD from xen_cpuid
On 17.05.22 18:42, Josh Poimboeuf wrote:
On Tue, May 17, 2022 at 04:24:25PM +, Maximilian Heyne wrote:
Since commit 4d65adfcd119 ("x86: xen: insn: Decode Xen and KVM
emulate-prefix signature"), objtool is able to correctly parse the
prefixed instruction in xen_cpuid and emit correct orc unwind
information. Hence, marking the function as STACKFRAME_NON_STANDARD is
no longer needed.
This commit is basically a revert of commit 983bb6d254c7 ("x86/xen: Mark
xen_cpuid() stack frame as non-standard").
Signed-off-by: Maximilian Heyne
CC: Josh Poimboeuf
cr: https://code.amazon.com/reviews/CR-69645080
^
This looks like an internal amazon link and should be removed.
Otherwise, looks good to me.
Can be done while committing.
Reviewed-by: Juergen Gross
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
