[linux-linus test] 180917: regressions - FAIL

2023-05-23 Thread osstest service owner
flight 180917 linux-linus real [real]
flight 180923 linux-linus real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/180917/
http://logs.test-lab.xenproject.org/osstest/logs/180923/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-credit1   8 xen-boot fail REGR. vs. 180278

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt  8 xen-boot fail  like 180278
 test-armhf-armhf-examine  8 reboot   fail  like 180278
 test-armhf-armhf-xl-arndale   8 xen-boot fail  like 180278
 test-armhf-armhf-libvirt-qcow2  8 xen-bootfail like 180278
 test-armhf-armhf-libvirt-raw  8 xen-boot fail  like 180278
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180278
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180278
 test-armhf-armhf-xl-vhd   8 xen-boot fail  like 180278
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180278
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180278
 test-armhf-armhf-xl-multivcpu  8 xen-boot fail like 180278
 test-armhf-armhf-xl-credit2   8 xen-boot fail  like 180278
 test-armhf-armhf-xl-rtds  8 xen-boot fail  like 180278
 test-armhf-armhf-xl   8 xen-boot fail  like 180278
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180278
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 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-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-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  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-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-amd64-amd64-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-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass

version targeted for testing:
 linuxae8373a5add4ea39f032563cf12a02946d1e3546
baseline version:
 linux6c538e1adbfc696ac4747fb10d63e704344f763d

Last test of basis   180278  2023-04-16 19:41:46 Z   37 days
Failing since180281  2023-04-17 06:24:36 Z   36 days   68 attempts
Testing same since   180907  2023-05-23 01:11:53 Z1 days2 attempts


2480 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl  pass
 

Re: PVH Dom0 related UART failure

2023-05-23 Thread Stefano Stabellini
On Tue, 23 May 2023, Jan Beulich wrote:
> On 23.05.2023 00:20, Stefano Stabellini wrote:
> > On Sat, 20 May 2023, Roger Pau Monné wrote:
> >> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> >> index ec2e978a4e6b..0ff8e940fa8d 100644
> >> --- a/xen/drivers/vpci/header.c
> >> +++ b/xen/drivers/vpci/header.c
> >> @@ -289,6 +289,13 @@ static int modify_bars(const struct pci_dev *pdev, 
> >> uint16_t cmd, bool rom_only)
> >>   */
> >>  for_each_pdev ( pdev->domain, tmp )
> >>  {
> >> +if ( !tmp->vpci )
> >> +{
> >> +printk(XENLOG_G_WARNING "%pp: not handled by vPCI for %pd\n",
> >> +   >sbdf, pdev->domain);
> >> +continue;
> >> +}
> >> +
> >>  if ( tmp == pdev )
> >>  {
> >>  /*
> >> diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
> >> index 652807a4a454..0baef3a8d3a1 100644
> >> --- a/xen/drivers/vpci/vpci.c
> >> +++ b/xen/drivers/vpci/vpci.c
> >> @@ -72,7 +72,12 @@ int vpci_add_handlers(struct pci_dev *pdev)
> >>  unsigned int i;
> >>  int rc = 0;
> >>  
> >> -if ( !has_vpci(pdev->domain) )
> >> +if ( !has_vpci(pdev->domain) ||
> >> + /*
> >> +  * Ignore RO and hidden devices, those are in use by Xen and vPCI
> >> +  * won't work on them.
> >> +  */
> >> + pci_get_pdev(dom_xen, pdev->sbdf) )
> >>  return 0;
> >>  
> >>  /* We should not get here twice for the same device. */
> > 
> > 
> > Now this patch works! Thank you!! :-)
> > 
> > You can check the full logs here
> > https://gitlab.com/xen-project/people/sstabellini/xen/-/jobs/4329259080
> > 
> > Is the patch ready to be upstreamed aside from the commit message?
> 
> I don't think so. vPCI ought to work on "r/o" devices. Out of curiosity,
> have you also tried my (hackish and hence RFC) patch [1]?
> 
> [1] https://lists.xen.org/archives/html/xen-devel/2021-08/msg01489.html

I don't know the code well enough to discuss what is the best solution.
I'll let you and Roger figure it out. I would only kindly request to
solve this in few days so that we can enable the real hardware PVH test
in gitlab-ci as soon as possible. I think it is critical as it will
allow us to catch many real issues going forward.

For sure I can test your patch. BTW it is also really easy for you to do
it your simply by pushing a branch to a repo on gitlab-ci and watch for
the results. If you are interested let me know I can give you a
tutorial, you just need to create a repo, and register the gitlab runner
and voila'.

This is the outcome:

https://gitlab.com/xen-project/people/sstabellini/xen/-/pipelines/876808194


(XEN) PCI add device :00:00.0
(XEN) PCI add device :00:00.2
(XEN) PCI add device :00:01.0
(XEN) PCI add device :00:02.0
(XEN) Assertion 'd == dom_xen && system_state < SYS_STATE_active' failed at 
drivers/vpci/header.c:313
(XEN) [ Xen-4.18-unstable  x86_64  debug=y  Tainted:   C]
(XEN) CPU:14
(XEN) RIP:e008:[] 
drivers/vpci/header.c#modify_bars+0x3ba/0x4a3
(XEN) RFLAGS: 00010202   CONTEXT: hypervisor (d0v0)
(XEN) rax: 830390164000   rbx: 83038bd8a7f0   rcx: 0010
(XEN) rdx: 83038e3b7fff   rsi: 83038e3c83a0   rdi: 83038e3c8398
(XEN) rbp: 83038e3b7c08   rsp: 83038e3b7b98   r8:  0001
(XEN) r9:  83038dcfa000   r10: 000e   r11: 
(XEN) r12: 0007   r13: 000dcc03   r14: 83039016ad10
(XEN) r15: 000dcc00   cr0: 8005003b   cr4: 00f506e0
(XEN) cr3: 00038ddfe000   cr2: 88814e3ff000
(XEN) fsb:    gsb: 888149a0   gss: 
(XEN) ds:    es:    fs:    gs:    ss:    cs: e008
(XEN) Xen code around  
(drivers/vpci/header.c#modify_bars+0x3ba/0x4a3):
(XEN)  3d c4 d1 37 00 02 76 c0 <0f> 0b 4c 8b 75 b0 0f ae e8 48 83 7d 98 00 74 2b
(XEN) Xen stack trace from rsp=83038e3b7b98:
(XEN)0002 830390150230 830390164000 830390164158
(XEN)830390150230 00ff8303 83038dcf8b00 0003
(XEN)0206 83038bd8c010  0002
(XEN)0002 0004 83038e3b7c18 82d040268909
(XEN)83038e3b7ca0 82d040267998 00118e3b7ca0 00117803
(XEN)0004 830390150230  
(XEN)00040002   
(XEN)0002  0004 
(XEN)82d04041df40 83038e3b7cd0 82d0402cb649 001140332c9e
(XEN)83038e3b7d88  83038dd094a0 83038e3b7d30
(XEN)82d0402caa72 83038e3b7ce0 0cfc0002 
(XEN)0002  83038dd094a0 83038e3b7d88
(XEN)0001   

[xen-4.16-testing test] 180918: tolerable trouble: fail/pass/starved - PUSHED

2023-05-23 Thread osstest service owner
flight 180918 xen-4.16-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/180918/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 180478
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180478
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 180478
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180478
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180478
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 180478
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 180478
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180478
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180478
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 180478
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 180478
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180478
 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-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 15 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-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  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-arndale  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-raw  14 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-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 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
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2   3 hosts-allocate   starved  n/a
 test-arm64-arm64-xl   3 hosts-allocate   starved  n/a
 test-arm64-arm64-xl-credit1   3 hosts-allocate   starved  n/a
 test-arm64-arm64-xl-thunderx  3 hosts-allocate   starved  n/a
 test-arm64-arm64-xl-vhd   3 hosts-allocate   starved  n/a
 test-arm64-arm64-xl-xsm   3 hosts-allocate   starved  n/a
 test-arm64-arm64-libvirt-raw  3 hosts-allocate   starved  n/a
 test-arm64-arm64-libvirt-xsm  3 hosts-allocate   starved  n/a

version targeted for testing:
 xen  b0806d84d48d983d40a29534e663652887287a78
baseline version:
 xen  7251cea957cbe4a0772651a4ab110ed76f689f96

Last test of basis   180478  2023-04-29 03:21:22 Z   24 days
Testing same since   180918  2023-05-23 13:08:46 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Dario Faggioli 
  Jan Beulich 
  Jason Andryuk 
  Juergen Gross 
  Marek Marczykowski-Górecki 
  Olaf Hering 
  Roger Pau Monné 
  Stewart Hildebrand 
  Yann Dirson 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64-xtf  pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386   

[xen-unstable test] 180913: tolerable FAIL

2023-05-23 Thread osstest service owner
flight 180913 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/180913/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-qemuu-win7-amd64 12 windows-install fail in 180900 pass in 
180913
 test-amd64-i386-xl-qemuu-debianhvm-amd64  7 xen-installfail pass in 180900
 test-amd64-i386-libvirt-raw   7 xen-installfail pass in 180900
 test-amd64-amd64-libvirt-vhd 19 guest-start/debian.repeat  fail pass in 180900
 test-amd64-i386-xl-vhd   22 guest-start.2  fail pass in 180900

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stop  fail blocked in 180900
 test-amd64-i386-libvirt-raw 14 migrate-support-check fail in 180900 never pass
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180900
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 180900
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180900
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 180900
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 180900
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 180900
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 180900
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 180900
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180900
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180900
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180900
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-i386-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-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 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-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 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-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-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 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-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  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-amd64-amd64-libvirt-xsm 15 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-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-libvirt 15 migrate-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


Re: [PATCH] [v2] x86: xen: add missing prototypes

2023-05-23 Thread Boris Ostrovsky




On 5/23/23 4:37 PM, Arnd Bergmann wrote:

On Sat, May 20, 2023, at 00:24, Boris Ostrovsky wrote:

On 5/19/23 5:28 AM, Arnd Bergmann wrote:


diff --git a/arch/x86/xen/smp.h b/arch/x86/xen/smp.h
index 22fb982ff971..81a7821dd07f 100644
--- a/arch/x86/xen/smp.h
+++ b/arch/x86/xen/smp.h
@@ -1,7 +1,11 @@
   /* SPDX-License-Identifier: GPL-2.0 */
   #ifndef _XEN_SMP_H
   
+void asm_cpu_bringup_and_idle(void);

+asmlinkage void cpu_bringup_and_idle(void);


These can go under CONFIG_SMP


Not sure if there is much point for the second one, since
it's only called from assembler, so the #else path is
never seen, but I can do that for consistency if you
like.

I generally prefer to avoid the extra #if checks
when there is no strict need for an empty stub.


Do we need the empty stubs? Neither of these should be referenced if !SMP (or 
more precisely if !CONFIG_XEN_PV_SMP)



I guess you also want me to add the empty stubs for the
other functions that only have a bit in #ifdef but not
#else then?


+void xen_force_evtchn_callback(void);


These ...


+pteval_t xen_pte_val(pte_t pte);
+pgdval_t xen_pgd_val(pgd_t pgd);
+pte_t xen_make_pte(pteval_t pte);
+pgd_t xen_make_pgd(pgdval_t pgd);
+pmdval_t xen_pmd_val(pmd_t pmd);
+pmd_t xen_make_pmd(pmdval_t pmd);
+pudval_t xen_pud_val(pud_t pud);
+pud_t xen_make_pud(pudval_t pud);
+p4dval_t xen_p4d_val(p4d_t p4d);
+p4d_t xen_make_p4d(p4dval_t p4d);
+pte_t xen_make_pte_init(pteval_t pte);
+void xen_start_kernel(struct start_info *si);



... should go under '#ifdef CONFIG_XEN_PV'


What should the stubs do here? I guess just return the
unmodified value?



Same here -- these should only be referenced if CONFIG_XEN_PV is defined which 
is why I suggested moving them under ifdef.


-boris



[PATCH] [v3] x86: xen: add missing prototypes

2023-05-23 Thread Arnd Bergmann
From: Arnd Bergmann 

These function are all called from assembler files, or from inline assembler,
so there is no immediate need for a prototype in a header, but if 
-Wmissing-prototypes
is enabled, the compiler warns about them:

arch/x86/xen/efi.c:130:13: error: no previous prototype for 'xen_efi_init' 
[-Werror=missing-prototypes]
arch/x86/platform/pvh/enlighten.c:120:13: error: no previous prototype for 
'xen_prepare_pvh' [-Werror=missing-prototypes]
arch/x86/xen/mmu_pv.c:358:20: error: no previous prototype for 'xen_pte_val' 
[-Werror=missing-prototypes]
arch/x86/xen/mmu_pv.c:366:20: error: no previous prototype for 'xen_pgd_val' 
[-Werror=missing-prototypes]
arch/x86/xen/mmu_pv.c:372:17: error: no previous prototype for 'xen_make_pte' 
[-Werror=missing-prototypes]
arch/x86/xen/mmu_pv.c:380:17: error: no previous prototype for 'xen_make_pgd' 
[-Werror=missing-prototypes]
arch/x86/xen/mmu_pv.c:387:20: error: no previous prototype for 'xen_pmd_val' 
[-Werror=missing-prototypes]
arch/x86/xen/mmu_pv.c:425:17: error: no previous prototype for 'xen_make_pmd' 
[-Werror=missing-prototypes]
arch/x86/xen/mmu_pv.c:432:20: error: no previous prototype for 'xen_pud_val' 
[-Werror=missing-prototypes]
arch/x86/xen/mmu_pv.c:438:17: error: no previous prototype for 'xen_make_pud' 
[-Werror=missing-prototypes]
arch/x86/xen/mmu_pv.c:522:20: error: no previous prototype for 'xen_p4d_val' 
[-Werror=missing-prototypes]
arch/x86/xen/mmu_pv.c:528:17: error: no previous prototype for 'xen_make_p4d' 
[-Werror=missing-prototypes]
arch/x86/xen/mmu_pv.c:1442:17: error: no previous prototype for 
'xen_make_pte_init' [-Werror=missing-prototypes]
arch/x86/xen/enlighten_pv.c:1233:34: error: no previous prototype for 
'xen_start_kernel' [-Werror=missing-prototypes]
arch/x86/xen/irq.c:22:14: error: no previous prototype for 
'xen_force_evtchn_callback' [-Werror=missing-prototypes]
arch/x86/entry/common.c:302:24: error: no previous prototype for 
'xen_pv_evtchn_do_upcall' [-Werror=missing-prototypes]

Declare all of them in an appropriate header file to avoid the warnings.
For consistency, also move the asm_cpu_bringup_and_idle() declaration out of
smp_pv.c.

Signed-off-by: Arnd Bergmann 
---
v3: move declations of conditional function in an #ifdef with a stub
v2: fix up changelog
---
 arch/x86/xen/efi.c |  2 ++
 arch/x86/xen/smp.h | 11 +++
 arch/x86/xen/smp_pv.c  |  1 -
 arch/x86/xen/xen-ops.h | 26 ++
 include/xen/xen.h  |  3 +++
 5 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index 7d7ffb9c826a..863d0d6b3edc 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -16,6 +16,8 @@
 #include 
 #include 
 
+#include "xen-ops.h"
+
 static efi_char16_t vendor[100] __initdata;
 
 static efi_system_table_t efi_systab_xen __initdata = {
diff --git a/arch/x86/xen/smp.h b/arch/x86/xen/smp.h
index 22fb982ff971..9367502281dc 100644
--- a/arch/x86/xen/smp.h
+++ b/arch/x86/xen/smp.h
@@ -2,6 +2,10 @@
 #ifndef _XEN_SMP_H
 
 #ifdef CONFIG_SMP
+
+void asm_cpu_bringup_and_idle(void);
+asmlinkage void cpu_bringup_and_idle(void);
+
 extern void xen_send_IPI_mask(const struct cpumask *mask,
  int vector);
 extern void xen_send_IPI_mask_allbutself(const struct cpumask *mask,
@@ -29,6 +33,13 @@ struct xen_common_irq {
 };
 #else /* CONFIG_SMP */
 
+static inline void asm_cpu_bringup_and_idle(void)
+{
+}
+static inline void cpu_bringup_and_idle(void)
+{
+}
+
 static inline int xen_smp_intr_init(unsigned int cpu)
 {
return 0;
diff --git a/arch/x86/xen/smp_pv.c b/arch/x86/xen/smp_pv.c
index a92e8002b5cf..d5ae5de2daa2 100644
--- a/arch/x86/xen/smp_pv.c
+++ b/arch/x86/xen/smp_pv.c
@@ -55,7 +55,6 @@ static DEFINE_PER_CPU(struct xen_common_irq, xen_irq_work) = 
{ .irq = -1 };
 static DEFINE_PER_CPU(struct xen_common_irq, xen_pmu_irq) = { .irq = -1 };
 
 static irqreturn_t xen_irq_work_interrupt(int irq, void *dev_id);
-void asm_cpu_bringup_and_idle(void);
 
 static void cpu_bringup(void)
 {
diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index 6d7f6318fc07..eb4cb30570c7 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -146,12 +146,38 @@ int xen_cpuhp_setup(int (*cpu_up_prepare_cb)(unsigned 
int),
 void xen_pin_vcpu(int cpu);
 
 void xen_emergency_restart(void);
+void xen_force_evtchn_callback(void);
+
 #ifdef CONFIG_XEN_PV
 void xen_pv_pre_suspend(void);
 void xen_pv_post_suspend(int suspend_cancelled);
+pteval_t xen_pte_val(pte_t pte);
+pgdval_t xen_pgd_val(pgd_t pgd);
+pmdval_t xen_pmd_val(pmd_t pmd);
+pudval_t xen_pud_val(pud_t pud);
+p4dval_t xen_p4d_val(p4d_t p4d);
+pte_t xen_make_pte(pteval_t pte);
+pgd_t xen_make_pgd(pgdval_t pgd);
+pmd_t xen_make_pmd(pmdval_t pmd);
+pud_t xen_make_pud(pudval_t pud);
+p4d_t xen_make_p4d(p4dval_t p4d);
+pte_t xen_make_pte_init(pteval_t pte);
+void xen_start_kernel(struct start_info *si);
 #else
 static inline void xen_pv_pre_suspend(void) {}
 static inline 

Re: [PATCH] [v2] x86: xen: add missing prototypes

2023-05-23 Thread Arnd Bergmann
On Sat, May 20, 2023, at 00:24, Boris Ostrovsky wrote:
> On 5/19/23 5:28 AM, Arnd Bergmann wrote:
>
>> diff --git a/arch/x86/xen/smp.h b/arch/x86/xen/smp.h
>> index 22fb982ff971..81a7821dd07f 100644
>> --- a/arch/x86/xen/smp.h
>> +++ b/arch/x86/xen/smp.h
>> @@ -1,7 +1,11 @@
>>   /* SPDX-License-Identifier: GPL-2.0 */
>>   #ifndef _XEN_SMP_H
>>   
>> +void asm_cpu_bringup_and_idle(void);
>> +asmlinkage void cpu_bringup_and_idle(void);
>
> These can go under CONFIG_SMP

Not sure if there is much point for the second one, since
it's only called from assembler, so the #else path is
never seen, but I can do that for consistency if you
like.

I generally prefer to avoid the extra #if checks
when there is no strict need for an empty stub.

I guess you also want me to add the empty stubs for the
other functions that only have a bit in #ifdef but not
#else then?

>> +void xen_force_evtchn_callback(void);
>
> These ...
>
>> +pteval_t xen_pte_val(pte_t pte);
>> +pgdval_t xen_pgd_val(pgd_t pgd);
>> +pte_t xen_make_pte(pteval_t pte);
>> +pgd_t xen_make_pgd(pgdval_t pgd);
>> +pmdval_t xen_pmd_val(pmd_t pmd);
>> +pmd_t xen_make_pmd(pmdval_t pmd);
>> +pudval_t xen_pud_val(pud_t pud);
>> +pud_t xen_make_pud(pudval_t pud);
>> +p4dval_t xen_p4d_val(p4d_t p4d);
>> +p4d_t xen_make_p4d(p4dval_t p4d);
>> +pte_t xen_make_pte_init(pteval_t pte);
>> +void xen_start_kernel(struct start_info *si);
>
>
> ... should go under '#ifdef CONFIG_XEN_PV'

What should the stubs do here? I guess just return the
unmodified value?

Arnd



Re: [XEN PATCH 13/15] build: fix compile.h compiler version command line

2023-05-23 Thread Andrew Cooper
On 23/05/2023 5:38 pm, Anthony PERARD wrote:
> CFLAGS is just from Config.mk, instead use the flags used to build
> Xen.
>
> Signed-off-by: Anthony PERARD 
> ---
>
> Notes:
> I don't know if CFLAGS is even useful there, just --version without the
> flags might produce the same result.

I can't think of any legitimate reason for CFLAGS to be here.

Any compiler which does differ its output based on CLFAGS is probably
one we don't want to be using...

~Andrew



[qemu-mainline test] 180916: regressions - FAIL

2023-05-23 Thread osstest service owner
flight 180916 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/180916/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   6 xen-buildfail REGR. vs. 180691
 build-amd64-xsm   6 xen-buildfail REGR. vs. 180691
 build-i386-xsm6 xen-buildfail REGR. vs. 180691
 build-i3866 xen-buildfail REGR. vs. 180691
 build-arm64   6 xen-buildfail REGR. vs. 180691
 build-arm64-xsm   6 xen-buildfail REGR. vs. 180691
 build-armhf   6 xen-buildfail REGR. vs. 180691

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-ws16-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-shadow 1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-vhd1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-raw  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit1   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-thunderx  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-vhd   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-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
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-raw   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-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-coresched-i386-xl  1 build-check(1)   blocked  n/a
 test-amd64-coresched-amd64-xl  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-xsm   1 build-check(1)   blocked  n/a
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-shadow1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-rtds  1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ws16-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 build-armhf-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm  1 build-check(1) blocked n/a
 test-amd64-amd64-dom0pvh-xl-amd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1) blocked n/a
 test-amd64-amd64-dom0pvh-xl-intel  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked  n/a
 

[PATCH v2 5/6] block/linux-aio: convert to blk_io_plug_call() API

2023-05-23 Thread Stefan Hajnoczi
Stop using the .bdrv_co_io_plug() API because it is not multi-queue
block layer friendly. Use the new blk_io_plug_call() API to batch I/O
submission instead.

Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Eric Blake 
---
 include/block/raw-aio.h |  7 ---
 block/file-posix.c  | 28 
 block/linux-aio.c   | 41 +++--
 3 files changed, 11 insertions(+), 65 deletions(-)

diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h
index da60ca13ef..0f63c2800c 100644
--- a/include/block/raw-aio.h
+++ b/include/block/raw-aio.h
@@ -62,13 +62,6 @@ int coroutine_fn laio_co_submit(int fd, uint64_t offset, 
QEMUIOVector *qiov,
 
 void laio_detach_aio_context(LinuxAioState *s, AioContext *old_context);
 void laio_attach_aio_context(LinuxAioState *s, AioContext *new_context);
-
-/*
- * laio_io_plug/unplug work in the thread's current AioContext, therefore the
- * caller must ensure that they are paired in the same IOThread.
- */
-void laio_io_plug(void);
-void laio_io_unplug(uint64_t dev_max_batch);
 #endif
 /* io_uring.c - Linux io_uring implementation */
 #ifdef CONFIG_LINUX_IO_URING
diff --git a/block/file-posix.c b/block/file-posix.c
index 7baa8491dd..ac1ed54811 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2550,26 +2550,6 @@ static int coroutine_fn raw_co_pwritev(BlockDriverState 
*bs, int64_t offset,
 return raw_co_prw(bs, offset, bytes, qiov, QEMU_AIO_WRITE);
 }
 
-static void coroutine_fn raw_co_io_plug(BlockDriverState *bs)
-{
-BDRVRawState __attribute__((unused)) *s = bs->opaque;
-#ifdef CONFIG_LINUX_AIO
-if (s->use_linux_aio) {
-laio_io_plug();
-}
-#endif
-}
-
-static void coroutine_fn raw_co_io_unplug(BlockDriverState *bs)
-{
-BDRVRawState __attribute__((unused)) *s = bs->opaque;
-#ifdef CONFIG_LINUX_AIO
-if (s->use_linux_aio) {
-laio_io_unplug(s->aio_max_batch);
-}
-#endif
-}
-
 static int coroutine_fn raw_co_flush_to_disk(BlockDriverState *bs)
 {
 BDRVRawState *s = bs->opaque;
@@ -3914,8 +3894,6 @@ BlockDriver bdrv_file = {
 .bdrv_co_copy_range_from = raw_co_copy_range_from,
 .bdrv_co_copy_range_to  = raw_co_copy_range_to,
 .bdrv_refresh_limits = raw_refresh_limits,
-.bdrv_co_io_plug= raw_co_io_plug,
-.bdrv_co_io_unplug  = raw_co_io_unplug,
 .bdrv_attach_aio_context = raw_aio_attach_aio_context,
 
 .bdrv_co_truncate   = raw_co_truncate,
@@ -4286,8 +4264,6 @@ static BlockDriver bdrv_host_device = {
 .bdrv_co_copy_range_from = raw_co_copy_range_from,
 .bdrv_co_copy_range_to  = raw_co_copy_range_to,
 .bdrv_refresh_limits = raw_refresh_limits,
-.bdrv_co_io_plug= raw_co_io_plug,
-.bdrv_co_io_unplug  = raw_co_io_unplug,
 .bdrv_attach_aio_context = raw_aio_attach_aio_context,
 
 .bdrv_co_truncate   = raw_co_truncate,
@@ -4424,8 +4400,6 @@ static BlockDriver bdrv_host_cdrom = {
 .bdrv_co_pwritev= raw_co_pwritev,
 .bdrv_co_flush_to_disk  = raw_co_flush_to_disk,
 .bdrv_refresh_limits= cdrom_refresh_limits,
-.bdrv_co_io_plug= raw_co_io_plug,
-.bdrv_co_io_unplug  = raw_co_io_unplug,
 .bdrv_attach_aio_context = raw_aio_attach_aio_context,
 
 .bdrv_co_truncate   = raw_co_truncate,
@@ -4552,8 +4526,6 @@ static BlockDriver bdrv_host_cdrom = {
 .bdrv_co_pwritev= raw_co_pwritev,
 .bdrv_co_flush_to_disk  = raw_co_flush_to_disk,
 .bdrv_refresh_limits= cdrom_refresh_limits,
-.bdrv_co_io_plug= raw_co_io_plug,
-.bdrv_co_io_unplug  = raw_co_io_unplug,
 .bdrv_attach_aio_context = raw_aio_attach_aio_context,
 
 .bdrv_co_truncate   = raw_co_truncate,
diff --git a/block/linux-aio.c b/block/linux-aio.c
index 442c86209b..5021aed68f 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -15,6 +15,7 @@
 #include "qemu/event_notifier.h"
 #include "qemu/coroutine.h"
 #include "qapi/error.h"
+#include "sysemu/block-backend.h"
 
 /* Only used for assertions.  */
 #include "qemu/coroutine_int.h"
@@ -46,7 +47,6 @@ struct qemu_laiocb {
 };
 
 typedef struct {
-int plugged;
 unsigned int in_queue;
 unsigned int in_flight;
 bool blocked;
@@ -236,7 +236,7 @@ static void 
qemu_laio_process_completions_and_submit(LinuxAioState *s)
 {
 qemu_laio_process_completions(s);
 
-if (!s->io_q.plugged && !QSIMPLEQ_EMPTY(>io_q.pending)) {
+if (!QSIMPLEQ_EMPTY(>io_q.pending)) {
 ioq_submit(s);
 }
 }
@@ -277,7 +277,6 @@ static void qemu_laio_poll_ready(EventNotifier *opaque)
 static void ioq_init(LaioQueue *io_q)
 {
 QSIMPLEQ_INIT(_q->pending);
-io_q->plugged = 0;
 io_q->in_queue = 0;
 io_q->in_flight = 0;
 io_q->blocked = false;
@@ -354,31 +353,11 @@ static uint64_t laio_max_batch(LinuxAioState *s, uint64_t 
dev_max_batch)
 return max_batch;
 }
 
-void laio_io_plug(void)
+static void laio_unplug_fn(void *opaque)
 {

[PATCH v2 6/6] block: remove bdrv_co_io_plug() API

2023-05-23 Thread Stefan Hajnoczi
No block driver implements .bdrv_co_io_plug() anymore. Get rid of the
function pointers.

Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Eric Blake 
---
 include/block/block-io.h |  3 ---
 include/block/block_int-common.h | 11 --
 block/io.c   | 37 
 3 files changed, 51 deletions(-)

diff --git a/include/block/block-io.h b/include/block/block-io.h
index a27e471a87..43af816d75 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -259,9 +259,6 @@ void coroutine_fn bdrv_co_leave(BlockDriverState *bs, 
AioContext *old_ctx);
 
 AioContext *child_of_bds_get_parent_aio_context(BdrvChild *c);
 
-void coroutine_fn GRAPH_RDLOCK bdrv_co_io_plug(BlockDriverState *bs);
-void coroutine_fn GRAPH_RDLOCK bdrv_co_io_unplug(BlockDriverState *bs);
-
 bool coroutine_fn GRAPH_RDLOCK
 bdrv_co_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name,
uint32_t granularity, Error **errp);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 6492a1e538..958962aa3a 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -753,11 +753,6 @@ struct BlockDriver {
 void coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_debug_event)(
 BlockDriverState *bs, BlkdebugEvent event);
 
-/* io queue for linux-aio */
-void coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_io_plug)(BlockDriverState 
*bs);
-void coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_io_unplug)(
-BlockDriverState *bs);
-
 /**
  * bdrv_drain_begin is called if implemented in the beginning of a
  * drain operation to drain and stop any internal sources of requests in
@@ -1227,12 +1222,6 @@ struct BlockDriverState {
 unsigned int in_flight;
 unsigned int serialising_in_flight;
 
-/*
- * counter for nested bdrv_io_plug.
- * Accessed with atomic ops.
- */
-unsigned io_plugged;
-
 /* do we need to tell the quest if we have a volatile write cache? */
 int enable_write_cache;
 
diff --git a/block/io.c b/block/io.c
index 4d54fda593..56b0c1ce6c 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3219,43 +3219,6 @@ void *qemu_try_blockalign0(BlockDriverState *bs, size_t 
size)
 return mem;
 }
 
-void coroutine_fn bdrv_co_io_plug(BlockDriverState *bs)
-{
-BdrvChild *child;
-IO_CODE();
-assert_bdrv_graph_readable();
-
-QLIST_FOREACH(child, >children, next) {
-bdrv_co_io_plug(child->bs);
-}
-
-if (qatomic_fetch_inc(>io_plugged) == 0) {
-BlockDriver *drv = bs->drv;
-if (drv && drv->bdrv_co_io_plug) {
-drv->bdrv_co_io_plug(bs);
-}
-}
-}
-
-void coroutine_fn bdrv_co_io_unplug(BlockDriverState *bs)
-{
-BdrvChild *child;
-IO_CODE();
-assert_bdrv_graph_readable();
-
-assert(bs->io_plugged);
-if (qatomic_fetch_dec(>io_plugged) == 1) {
-BlockDriver *drv = bs->drv;
-if (drv && drv->bdrv_co_io_unplug) {
-drv->bdrv_co_io_unplug(bs);
-}
-}
-
-QLIST_FOREACH(child, >children, next) {
-bdrv_co_io_unplug(child->bs);
-}
-}
-
 /* Helper that undoes bdrv_register_buf() when it fails partway through */
 static void GRAPH_RDLOCK
 bdrv_register_buf_rollback(BlockDriverState *bs, void *host, size_t size,
-- 
2.40.1




[PATCH v2 3/6] block/blkio: convert to blk_io_plug_call() API

2023-05-23 Thread Stefan Hajnoczi
Stop using the .bdrv_co_io_plug() API because it is not multi-queue
block layer friendly. Use the new blk_io_plug_call() API to batch I/O
submission instead.

Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Eric Blake 
---
v2
- Add missing #include and fix blkio_unplug_fn() prototype [Stefano]
---
 block/blkio.c | 43 ---
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/block/blkio.c b/block/blkio.c
index 0cdc99a729..93c6d20d39 100644
--- a/block/blkio.c
+++ b/block/blkio.c
@@ -17,6 +17,7 @@
 #include "qemu/error-report.h"
 #include "qapi/qmp/qdict.h"
 #include "qemu/module.h"
+#include "sysemu/block-backend.h"
 #include "exec/memory.h" /* for ram_block_discard_disable() */
 
 #include "block/block-io.h"
@@ -325,16 +326,30 @@ static void blkio_detach_aio_context(BlockDriverState *bs)
false, NULL, NULL, NULL, NULL, NULL);
 }
 
-/* Call with s->blkio_lock held to submit I/O after enqueuing a new request */
-static void blkio_submit_io(BlockDriverState *bs)
+/*
+ * Called by blk_io_unplug() or immediately if not plugged. Called without
+ * blkio_lock.
+ */
+static void blkio_unplug_fn(void *opaque)
 {
-if (qatomic_read(>io_plugged) == 0) {
-BDRVBlkioState *s = bs->opaque;
+BDRVBlkioState *s = opaque;
 
+WITH_QEMU_LOCK_GUARD(>blkio_lock) {
 blkioq_do_io(s->blkioq, NULL, 0, 0, NULL);
 }
 }
 
+/*
+ * Schedule I/O submission after enqueuing a new request. Called without
+ * blkio_lock.
+ */
+static void blkio_submit_io(BlockDriverState *bs)
+{
+BDRVBlkioState *s = bs->opaque;
+
+blk_io_plug_call(blkio_unplug_fn, s);
+}
+
 static int coroutine_fn
 blkio_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes)
 {
@@ -345,9 +360,9 @@ blkio_co_pdiscard(BlockDriverState *bs, int64_t offset, 
int64_t bytes)
 
 WITH_QEMU_LOCK_GUARD(>blkio_lock) {
 blkioq_discard(s->blkioq, offset, bytes, , 0);
-blkio_submit_io(bs);
 }
 
+blkio_submit_io(bs);
 qemu_coroutine_yield();
 return cod.ret;
 }
@@ -378,9 +393,9 @@ blkio_co_preadv(BlockDriverState *bs, int64_t offset, 
int64_t bytes,
 
 WITH_QEMU_LOCK_GUARD(>blkio_lock) {
 blkioq_readv(s->blkioq, offset, iov, iovcnt, , 0);
-blkio_submit_io(bs);
 }
 
+blkio_submit_io(bs);
 qemu_coroutine_yield();
 
 if (use_bounce_buffer) {
@@ -423,9 +438,9 @@ static int coroutine_fn blkio_co_pwritev(BlockDriverState 
*bs, int64_t offset,
 
 WITH_QEMU_LOCK_GUARD(>blkio_lock) {
 blkioq_writev(s->blkioq, offset, iov, iovcnt, , blkio_flags);
-blkio_submit_io(bs);
 }
 
+blkio_submit_io(bs);
 qemu_coroutine_yield();
 
 if (use_bounce_buffer) {
@@ -444,9 +459,9 @@ static int coroutine_fn blkio_co_flush(BlockDriverState *bs)
 
 WITH_QEMU_LOCK_GUARD(>blkio_lock) {
 blkioq_flush(s->blkioq, , 0);
-blkio_submit_io(bs);
 }
 
+blkio_submit_io(bs);
 qemu_coroutine_yield();
 return cod.ret;
 }
@@ -472,22 +487,13 @@ static int coroutine_fn 
blkio_co_pwrite_zeroes(BlockDriverState *bs,
 
 WITH_QEMU_LOCK_GUARD(>blkio_lock) {
 blkioq_write_zeroes(s->blkioq, offset, bytes, , blkio_flags);
-blkio_submit_io(bs);
 }
 
+blkio_submit_io(bs);
 qemu_coroutine_yield();
 return cod.ret;
 }
 
-static void coroutine_fn blkio_co_io_unplug(BlockDriverState *bs)
-{
-BDRVBlkioState *s = bs->opaque;
-
-WITH_QEMU_LOCK_GUARD(>blkio_lock) {
-blkio_submit_io(bs);
-}
-}
-
 typedef enum {
 BMRR_OK,
 BMRR_SKIP,
@@ -1009,7 +1015,6 @@ static void blkio_refresh_limits(BlockDriverState *bs, 
Error **errp)
 .bdrv_co_pwritev = blkio_co_pwritev, \
 .bdrv_co_flush_to_disk   = blkio_co_flush, \
 .bdrv_co_pwrite_zeroes   = blkio_co_pwrite_zeroes, \
-.bdrv_co_io_unplug   = blkio_co_io_unplug, \
 .bdrv_refresh_limits = blkio_refresh_limits, \
 .bdrv_register_buf   = blkio_register_buf, \
 .bdrv_unregister_buf = blkio_unregister_buf, \
-- 
2.40.1




[PATCH v2 1/6] block: add blk_io_plug_call() API

2023-05-23 Thread Stefan Hajnoczi
Introduce a new API for thread-local blk_io_plug() that does not
traverse the block graph. The goal is to make blk_io_plug() multi-queue
friendly.

Instead of having block drivers track whether or not we're in a plugged
section, provide an API that allows them to defer a function call until
we're unplugged: blk_io_plug_call(fn, opaque). If blk_io_plug_call() is
called multiple times with the same fn/opaque pair, then fn() is only
called once at the end of the function - resulting in batching.

This patch introduces the API and changes blk_io_plug()/blk_io_unplug().
blk_io_plug()/blk_io_unplug() no longer require a BlockBackend argument
because the plug state is now thread-local.

Later patches convert block drivers to blk_io_plug_call() and then we
can finally remove .bdrv_co_io_plug() once all block drivers have been
converted.

Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Eric Blake 
---
v2
- "is not be freed" -> "is not freed" [Eric]
---
 MAINTAINERS   |   1 +
 include/sysemu/block-backend-io.h |  13 +--
 block/block-backend.c |  22 -
 block/plug.c  | 159 ++
 hw/block/dataplane/xen-block.c|   8 +-
 hw/block/virtio-blk.c |   4 +-
 hw/scsi/virtio-scsi.c |   6 +-
 block/meson.build |   1 +
 8 files changed, 173 insertions(+), 41 deletions(-)
 create mode 100644 block/plug.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 1b6466496d..2be6f0c26b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2646,6 +2646,7 @@ F: util/aio-*.c
 F: util/aio-*.h
 F: util/fdmon-*.c
 F: block/io.c
+F: block/plug.c
 F: migration/block*
 F: include/block/aio.h
 F: include/block/aio-wait.h
diff --git a/include/sysemu/block-backend-io.h 
b/include/sysemu/block-backend-io.h
index d62a7ee773..be4dcef59d 100644
--- a/include/sysemu/block-backend-io.h
+++ b/include/sysemu/block-backend-io.h
@@ -100,16 +100,9 @@ void blk_iostatus_set_err(BlockBackend *blk, int error);
 int blk_get_max_iov(BlockBackend *blk);
 int blk_get_max_hw_iov(BlockBackend *blk);
 
-/*
- * blk_io_plug/unplug are thread-local operations. This means that multiple
- * IOThreads can simultaneously call plug/unplug, but the caller must ensure
- * that each unplug() is called in the same IOThread of the matching plug().
- */
-void coroutine_fn blk_co_io_plug(BlockBackend *blk);
-void co_wrapper blk_io_plug(BlockBackend *blk);
-
-void coroutine_fn blk_co_io_unplug(BlockBackend *blk);
-void co_wrapper blk_io_unplug(BlockBackend *blk);
+void blk_io_plug(void);
+void blk_io_unplug(void);
+void blk_io_plug_call(void (*fn)(void *), void *opaque);
 
 AioContext *blk_get_aio_context(BlockBackend *blk);
 BlockAcctStats *blk_get_stats(BlockBackend *blk);
diff --git a/block/block-backend.c b/block/block-backend.c
index ca537cd0ad..1f1d226ba6 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2568,28 +2568,6 @@ void blk_add_insert_bs_notifier(BlockBackend *blk, 
Notifier *notify)
 notifier_list_add(>insert_bs_notifiers, notify);
 }
 
-void coroutine_fn blk_co_io_plug(BlockBackend *blk)
-{
-BlockDriverState *bs = blk_bs(blk);
-IO_CODE();
-GRAPH_RDLOCK_GUARD();
-
-if (bs) {
-bdrv_co_io_plug(bs);
-}
-}
-
-void coroutine_fn blk_co_io_unplug(BlockBackend *blk)
-{
-BlockDriverState *bs = blk_bs(blk);
-IO_CODE();
-GRAPH_RDLOCK_GUARD();
-
-if (bs) {
-bdrv_co_io_unplug(bs);
-}
-}
-
 BlockAcctStats *blk_get_stats(BlockBackend *blk)
 {
 IO_CODE();
diff --git a/block/plug.c b/block/plug.c
new file mode 100644
index 00..98a155d2f4
--- /dev/null
+++ b/block/plug.c
@@ -0,0 +1,159 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Block I/O plugging
+ *
+ * Copyright Red Hat.
+ *
+ * This API defers a function call within a blk_io_plug()/blk_io_unplug()
+ * section, allowing multiple calls to batch up. This is a performance
+ * optimization that is used in the block layer to submit several I/O requests
+ * at once instead of individually:
+ *
+ *   blk_io_plug(); <-- start of plugged region
+ *   ...
+ *   blk_io_plug_call(my_func, my_obj); <-- deferred my_func(my_obj) call
+ *   blk_io_plug_call(my_func, my_obj); <-- another
+ *   blk_io_plug_call(my_func, my_obj); <-- another
+ *   ...
+ *   blk_io_unplug(); <-- end of plugged region, my_func(my_obj) is called once
+ *
+ * This code is actually generic and not tied to the block layer. If another
+ * subsystem needs this functionality, it could be renamed.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/coroutine-tls.h"
+#include "qemu/notify.h"
+#include "qemu/thread.h"
+#include "sysemu/block-backend.h"
+
+/* A function call that has been deferred until unplug() */
+typedef struct {
+void (*fn)(void *);
+void *opaque;
+} UnplugFn;
+
+/* Per-thread state */
+typedef struct {
+unsigned count;   /* how many times has plug() been called? */
+GArray *unplug_fns;   /* functions to call at unplug time */
+} Plug;
+

[PATCH v2 2/6] block/nvme: convert to blk_io_plug_call() API

2023-05-23 Thread Stefan Hajnoczi
Stop using the .bdrv_co_io_plug() API because it is not multi-queue
block layer friendly. Use the new blk_io_plug_call() API to batch I/O
submission instead.

Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Eric Blake 
---
v2
- Remove unused nvme_process_completion_queue_plugged trace event
  [Stefano]
---
 block/nvme.c   | 44 
 block/trace-events |  1 -
 2 files changed, 12 insertions(+), 33 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c
index 5b744c2bda..100b38b592 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -25,6 +25,7 @@
 #include "qemu/vfio-helpers.h"
 #include "block/block-io.h"
 #include "block/block_int.h"
+#include "sysemu/block-backend.h"
 #include "sysemu/replay.h"
 #include "trace.h"
 
@@ -119,7 +120,6 @@ struct BDRVNVMeState {
 int blkshift;
 
 uint64_t max_transfer;
-bool plugged;
 
 bool supports_write_zeroes;
 bool supports_discard;
@@ -282,7 +282,7 @@ static void nvme_kick(NVMeQueuePair *q)
 {
 BDRVNVMeState *s = q->s;
 
-if (s->plugged || !q->need_kick) {
+if (!q->need_kick) {
 return;
 }
 trace_nvme_kick(s, q->index);
@@ -387,10 +387,6 @@ static bool nvme_process_completion(NVMeQueuePair *q)
 NvmeCqe *c;
 
 trace_nvme_process_completion(s, q->index, q->inflight);
-if (s->plugged) {
-trace_nvme_process_completion_queue_plugged(s, q->index);
-return false;
-}
 
 /*
  * Support re-entrancy when a request cb() function invokes aio_poll().
@@ -480,6 +476,15 @@ static void nvme_trace_command(const NvmeCmd *cmd)
 }
 }
 
+static void nvme_unplug_fn(void *opaque)
+{
+NVMeQueuePair *q = opaque;
+
+QEMU_LOCK_GUARD(>lock);
+nvme_kick(q);
+nvme_process_completion(q);
+}
+
 static void nvme_submit_command(NVMeQueuePair *q, NVMeRequest *req,
 NvmeCmd *cmd, BlockCompletionFunc cb,
 void *opaque)
@@ -496,8 +501,7 @@ static void nvme_submit_command(NVMeQueuePair *q, 
NVMeRequest *req,
q->sq.tail * NVME_SQ_ENTRY_BYTES, cmd, sizeof(*cmd));
 q->sq.tail = (q->sq.tail + 1) % NVME_QUEUE_SIZE;
 q->need_kick++;
-nvme_kick(q);
-nvme_process_completion(q);
+blk_io_plug_call(nvme_unplug_fn, q);
 qemu_mutex_unlock(>lock);
 }
 
@@ -1567,27 +1571,6 @@ static void nvme_attach_aio_context(BlockDriverState *bs,
 }
 }
 
-static void coroutine_fn nvme_co_io_plug(BlockDriverState *bs)
-{
-BDRVNVMeState *s = bs->opaque;
-assert(!s->plugged);
-s->plugged = true;
-}
-
-static void coroutine_fn nvme_co_io_unplug(BlockDriverState *bs)
-{
-BDRVNVMeState *s = bs->opaque;
-assert(s->plugged);
-s->plugged = false;
-for (unsigned i = INDEX_IO(0); i < s->queue_count; i++) {
-NVMeQueuePair *q = s->queues[i];
-qemu_mutex_lock(>lock);
-nvme_kick(q);
-nvme_process_completion(q);
-qemu_mutex_unlock(>lock);
-}
-}
-
 static bool nvme_register_buf(BlockDriverState *bs, void *host, size_t size,
   Error **errp)
 {
@@ -1664,9 +1647,6 @@ static BlockDriver bdrv_nvme = {
 .bdrv_detach_aio_context  = nvme_detach_aio_context,
 .bdrv_attach_aio_context  = nvme_attach_aio_context,
 
-.bdrv_co_io_plug  = nvme_co_io_plug,
-.bdrv_co_io_unplug= nvme_co_io_unplug,
-
 .bdrv_register_buf= nvme_register_buf,
 .bdrv_unregister_buf  = nvme_unregister_buf,
 };
diff --git a/block/trace-events b/block/trace-events
index 32665158d6..048ad27519 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -141,7 +141,6 @@ nvme_kick(void *s, unsigned q_index) "s %p q #%u"
 nvme_dma_flush_queue_wait(void *s) "s %p"
 nvme_error(int cmd_specific, int sq_head, int sqid, int cid, int status) 
"cmd_specific %d sq_head %d sqid %d cid %d status 0x%x"
 nvme_process_completion(void *s, unsigned q_index, int inflight) "s %p q #%u 
inflight %d"
-nvme_process_completion_queue_plugged(void *s, unsigned q_index) "s %p q #%u"
 nvme_complete_command(void *s, unsigned q_index, int cid) "s %p q #%u cid %d"
 nvme_submit_command(void *s, unsigned q_index, int cid) "s %p q #%u cid %d"
 nvme_submit_command_raw(int c0, int c1, int c2, int c3, int c4, int c5, int 
c6, int c7) "%02x %02x %02x %02x %02x %02x %02x %02x"
-- 
2.40.1




[PATCH v2 4/6] block/io_uring: convert to blk_io_plug_call() API

2023-05-23 Thread Stefan Hajnoczi
Stop using the .bdrv_co_io_plug() API because it is not multi-queue
block layer friendly. Use the new blk_io_plug_call() API to batch I/O
submission instead.

Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Eric Blake 
---
v2
- Removed whitespace hunk [Eric]
---
 include/block/raw-aio.h |  7 ---
 block/file-posix.c  | 10 --
 block/io_uring.c| 44 -
 block/trace-events  |  5 ++---
 4 files changed, 19 insertions(+), 47 deletions(-)

diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h
index 0fe85ade77..da60ca13ef 100644
--- a/include/block/raw-aio.h
+++ b/include/block/raw-aio.h
@@ -81,13 +81,6 @@ int coroutine_fn luring_co_submit(BlockDriverState *bs, int 
fd, uint64_t offset,
   QEMUIOVector *qiov, int type);
 void luring_detach_aio_context(LuringState *s, AioContext *old_context);
 void luring_attach_aio_context(LuringState *s, AioContext *new_context);
-
-/*
- * luring_io_plug/unplug work in the thread's current AioContext, therefore the
- * caller must ensure that they are paired in the same IOThread.
- */
-void luring_io_plug(void);
-void luring_io_unplug(void);
 #endif
 
 #ifdef _WIN32
diff --git a/block/file-posix.c b/block/file-posix.c
index 0ab158efba..7baa8491dd 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2558,11 +2558,6 @@ static void coroutine_fn raw_co_io_plug(BlockDriverState 
*bs)
 laio_io_plug();
 }
 #endif
-#ifdef CONFIG_LINUX_IO_URING
-if (s->use_linux_io_uring) {
-luring_io_plug();
-}
-#endif
 }
 
 static void coroutine_fn raw_co_io_unplug(BlockDriverState *bs)
@@ -2573,11 +2568,6 @@ static void coroutine_fn 
raw_co_io_unplug(BlockDriverState *bs)
 laio_io_unplug(s->aio_max_batch);
 }
 #endif
-#ifdef CONFIG_LINUX_IO_URING
-if (s->use_linux_io_uring) {
-luring_io_unplug();
-}
-#endif
 }
 
 static int coroutine_fn raw_co_flush_to_disk(BlockDriverState *bs)
diff --git a/block/io_uring.c b/block/io_uring.c
index 82cab6a5bd..4e25b56c9c 100644
--- a/block/io_uring.c
+++ b/block/io_uring.c
@@ -16,6 +16,7 @@
 #include "block/raw-aio.h"
 #include "qemu/coroutine.h"
 #include "qapi/error.h"
+#include "sysemu/block-backend.h"
 #include "trace.h"
 
 /* Only used for assertions.  */
@@ -41,7 +42,6 @@ typedef struct LuringAIOCB {
 } LuringAIOCB;
 
 typedef struct LuringQueue {
-int plugged;
 unsigned int in_queue;
 unsigned int in_flight;
 bool blocked;
@@ -267,7 +267,7 @@ static void 
luring_process_completions_and_submit(LuringState *s)
 {
 luring_process_completions(s);
 
-if (!s->io_q.plugged && s->io_q.in_queue > 0) {
+if (s->io_q.in_queue > 0) {
 ioq_submit(s);
 }
 }
@@ -301,29 +301,17 @@ static void qemu_luring_poll_ready(void *opaque)
 static void ioq_init(LuringQueue *io_q)
 {
 QSIMPLEQ_INIT(_q->submit_queue);
-io_q->plugged = 0;
 io_q->in_queue = 0;
 io_q->in_flight = 0;
 io_q->blocked = false;
 }
 
-void luring_io_plug(void)
+static void luring_unplug_fn(void *opaque)
 {
-AioContext *ctx = qemu_get_current_aio_context();
-LuringState *s = aio_get_linux_io_uring(ctx);
-trace_luring_io_plug(s);
-s->io_q.plugged++;
-}
-
-void luring_io_unplug(void)
-{
-AioContext *ctx = qemu_get_current_aio_context();
-LuringState *s = aio_get_linux_io_uring(ctx);
-assert(s->io_q.plugged);
-trace_luring_io_unplug(s, s->io_q.blocked, s->io_q.plugged,
-   s->io_q.in_queue, s->io_q.in_flight);
-if (--s->io_q.plugged == 0 &&
-!s->io_q.blocked && s->io_q.in_queue > 0) {
+LuringState *s = opaque;
+trace_luring_unplug_fn(s, s->io_q.blocked, s->io_q.in_queue,
+   s->io_q.in_flight);
+if (!s->io_q.blocked && s->io_q.in_queue > 0) {
 ioq_submit(s);
 }
 }
@@ -370,14 +358,16 @@ static int luring_do_submit(int fd, LuringAIOCB 
*luringcb, LuringState *s,
 
 QSIMPLEQ_INSERT_TAIL(>io_q.submit_queue, luringcb, next);
 s->io_q.in_queue++;
-trace_luring_do_submit(s, s->io_q.blocked, s->io_q.plugged,
-   s->io_q.in_queue, s->io_q.in_flight);
-if (!s->io_q.blocked &&
-(!s->io_q.plugged ||
- s->io_q.in_flight + s->io_q.in_queue >= MAX_ENTRIES)) {
-ret = ioq_submit(s);
-trace_luring_do_submit_done(s, ret);
-return ret;
+trace_luring_do_submit(s, s->io_q.blocked, s->io_q.in_queue,
+   s->io_q.in_flight);
+if (!s->io_q.blocked) {
+if (s->io_q.in_flight + s->io_q.in_queue >= MAX_ENTRIES) {
+ret = ioq_submit(s);
+trace_luring_do_submit_done(s, ret);
+return ret;
+}
+
+blk_io_plug_call(luring_unplug_fn, s);
 }
 return 0;
 }
diff --git a/block/trace-events b/block/trace-events
index 048ad27519..6f121b7636 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -64,9 +64,8 @@ file_paio_submit(void *acb, void 

[PATCH v2 0/6] block: add blk_io_plug_call() API

2023-05-23 Thread Stefan Hajnoczi
v2
- Patch 1: "is not be freed" -> "is not freed" [Eric]
- Patch 2: Remove unused nvme_process_completion_queue_plugged trace event
  [Stefano]
- Patch 3: Add missing #include and fix blkio_unplug_fn() prototype [Stefano]
- Patch 4: Removed whitespace hunk [Eric]

The existing blk_io_plug() API is not block layer multi-queue friendly because
the plug state is per-BlockDriverState.

Change blk_io_plug()'s implementation so it is thread-local. This is done by
introducing the blk_io_plug_call() function that block drivers use to batch
calls while plugged. It is relatively easy to convert block drivers from
.bdrv_co_io_plug() to blk_io_plug_call().

Random read 4KB performance with virtio-blk on a host NVMe block device:

iodepth   iops   change vs today
145612   -4%
287967   +2%
4   129872   +0%
8   171096   -3%
16  194508   -4%
32  208947   -1%
64  217647   +0%
128 229629   +0%

The results are within the noise for these benchmarks. This is to be expected
because the plugging behavior for a single thread hasn't changed in this patch
series, only that the state is thread-local now.

The following graph compares several approaches:
https://vmsplice.net/~stefan/blk_io_plug-thread-local.png
- v7.2.0: before most of the multi-queue block layer changes landed.
- with-blk_io_plug: today's post-8.0.0 QEMU.
- blk_io_plug-thread-local: this patch series.
- no-blk_io_plug: what happens when we simply remove plugging?
- call-after-dispatch: what if we integrate plugging into the event loop? I
  decided against this approach in the end because it's more likely to
  introduce performance regressions since I/O submission is deferred until the
  end of the event loop iteration.

Aside from the no-blk_io_plug case, which bottlenecks much earlier than the
others, we see that all plugging approaches are more or less equivalent in this
benchmark. It is also clear that QEMU 8.0.0 has lower performance than 7.2.0.

The Ansible playbook, fio results, and a Jupyter notebook are available here:
https://github.com/stefanha/qemu-perf/tree/remove-blk_io_plug

Stefan Hajnoczi (6):
  block: add blk_io_plug_call() API
  block/nvme: convert to blk_io_plug_call() API
  block/blkio: convert to blk_io_plug_call() API
  block/io_uring: convert to blk_io_plug_call() API
  block/linux-aio: convert to blk_io_plug_call() API
  block: remove bdrv_co_io_plug() API

 MAINTAINERS   |   1 +
 include/block/block-io.h  |   3 -
 include/block/block_int-common.h  |  11 ---
 include/block/raw-aio.h   |  14 ---
 include/sysemu/block-backend-io.h |  13 +--
 block/blkio.c |  43 
 block/block-backend.c |  22 -
 block/file-posix.c|  38 ---
 block/io.c|  37 ---
 block/io_uring.c  |  44 -
 block/linux-aio.c |  41 +++-
 block/nvme.c  |  44 +++--
 block/plug.c  | 159 ++
 hw/block/dataplane/xen-block.c|   8 +-
 hw/block/virtio-blk.c |   4 +-
 hw/scsi/virtio-scsi.c |   6 +-
 block/meson.build |   1 +
 block/trace-events|   6 +-
 18 files changed, 239 insertions(+), 256 deletions(-)
 create mode 100644 block/plug.c

-- 
2.40.1




Re: [PATCH v7 10/12] xen/tools: add sve parameter in XL configuration

2023-05-23 Thread Anthony PERARD
On Tue, May 23, 2023 at 08:43:24AM +0100, Luca Fancellu wrote:
> Add sve parameter in XL configuration to allow guests to use
> SVE feature.
> 
> Signed-off-by: Luca Fancellu 

Reviewed-by: Anthony PERARD 

Thanks,

-- 
Anthony PERARD



[XEN PATCH 10/15] build: rename $(AFLAGS) to $(XEN_AFLAGS)

2023-05-23 Thread Anthony PERARD
We don't want the AFLAGS from the environment, they are usually meant
to build user space application and not for the hypervisor.

Config.mk doesn't provied any $(AFLAGS) so we can start a fresh
$(XEN_AFLAGS).

Signed-off-by: Anthony PERARD 
---
 xen/Makefile | 10 ++
 xen/arch/x86/arch.mk |  2 +-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/xen/Makefile b/xen/Makefile
index 127c0e40b5..c4a83fca76 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -258,6 +258,8 @@ export KBUILD_DEFCONFIG := $(ARCH)_defconfig
 # reparsing Config.mk by e.g. arch/x86/boot/.
 export XEN_TREEWIDE_CFLAGS := $(CFLAGS)
 
+XEN_AFLAGS =
+
 # CLANG_FLAGS needs to be calculated before calling Kconfig
 ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),)
 CLANG_FLAGS :=
@@ -412,9 +414,9 @@ ifneq ($(CONFIG_CC_IS_CLANG),y)
 CFLAGS += -Wa,--strip-local-absolute
 endif
 
-AFLAGS += -D__ASSEMBLY__
+XEN_AFLAGS += -D__ASSEMBLY__
 
-$(call cc-option-add,AFLAGS,CC,-Wa$(comma)--noexecstack)
+$(call cc-option-add,XEN_AFLAGS,CC,-Wa$(comma)--noexecstack)
 
 LDFLAGS-$(call ld-option,--warn-rwx-segments) += --no-warn-rwx-segments
 
@@ -425,7 +427,7 @@ CFLAGS += $(EXTRA_CFLAGS_XEN_CORE)
 # Most CFLAGS are safe for assembly files:
 #  -std=gnu{89,99} gets confused by #-prefixed end-of-line comments
 #  -flto makes no sense and annoys clang
-AFLAGS += $(filter-out -std=gnu% -flto,$(CFLAGS)) $(AFLAGS-y)
+XEN_AFLAGS += $(filter-out -std=gnu% -flto,$(CFLAGS)) $(AFLAGS-y)
 
 # LDFLAGS are only passed directly to $(LD)
 LDFLAGS += $(LDFLAGS_DIRECT) $(LDFLAGS-y)
@@ -462,7 +464,7 @@ include $(srctree)/arch/$(TARGET_ARCH)/arch.mk
 
 # define new variables to avoid the ones defined in Config.mk
 export XEN_CFLAGS := $(CFLAGS)
-export XEN_AFLAGS := $(AFLAGS)
+export XEN_AFLAGS := $(XEN_AFLAGS)
 export XEN_LDFLAGS := $(LDFLAGS)
 export CFLAGS_UBSAN
 
diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
index 7b5be9fe46..13ec88a628 100644
--- a/xen/arch/x86/arch.mk
+++ b/xen/arch/x86/arch.mk
@@ -80,7 +80,7 @@ ifeq ($(CONFIG_LD_IS_GNU),y)
 AFLAGS-$(call ld-option,--print-output-format) += 
-DHAVE_LD_SORT_BY_INIT_PRIORITY
 else
 # Assume all versions of LLD support this.
-AFLAGS += -DHAVE_LD_SORT_BY_INIT_PRIORITY
+XEN_AFLAGS += -DHAVE_LD_SORT_BY_INIT_PRIORITY
 endif
 
 ifneq ($(CONFIG_PV_SHIM_EXCLUSIVE),y)
-- 
Anthony PERARD




[XEN PATCH 12/15] build: avoid Config.mk's CFLAGS

2023-05-23 Thread Anthony PERARD
The variable $(CFLAGS) is too often set in the environment,
especially when building a package for a distribution. Often, those
CFLAGS are intended to be use to build user spaces binaries, not a
kernel. This mean packager needs to takes extra steps to build Xen by
overriding the CFLAGS provided by the package build environment.

With this patch, we avoid using the variable $(CFLAGS). Also, the
hypervisor's build system have complete control over which CFLAGS are
used.

No change intended to XEN_CFLAGS used, beside some flags which may be
in a different order on the command line.

Signed-off-by: Anthony PERARD 
---

Notes:
There's still $(EXTRA_CFLAGS_XEN_CORE) which allows to add more CFLAGS
if someone building Xen needs to add more CFLAGS to the hypervisor
build.

 xen/Makefile | 11 ++-
 xen/arch/arm/arch.mk |  4 
 xen/arch/x86/arch.mk |  2 ++
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/xen/Makefile b/xen/Makefile
index b3bffe8c6f..4dc3acf2a6 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -259,7 +259,16 @@ export KBUILD_DEFCONFIG := $(ARCH)_defconfig
 export XEN_TREEWIDE_CFLAGS := $(CFLAGS)
 
 XEN_AFLAGS =
-XEN_CFLAGS = $(CFLAGS)
+XEN_CFLAGS =
+ifeq ($(XEN_OS),SunOS)
+XEN_CFLAGS +=  -Wa,--divide -D_POSIX_C_SOURCE=200112L -D__EXTENSIONS__
+endif
+XEN_CFLAGS += -fno-strict-aliasing
+XEN_CFLAGS += -std=gnu99
+XEN_CFLAGS += -Wall -Wstrict-prototypes
+$(call cc-option-add,XEN_CFLAGS,CC,-Wdeclaration-after-statement)
+$(call cc-option-add,XEN_CFLAGS,CC,-Wno-unused-but-set-variable)
+$(call cc-option-add,XEN_CFLAGS,CC,-Wno-unused-local-typedefs)
 
 # CLANG_FLAGS needs to be calculated before calling Kconfig
 ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),)
diff --git a/xen/arch/arm/arch.mk b/xen/arch/arm/arch.mk
index 300b8bf7ae..0478fadde2 100644
--- a/xen/arch/arm/arch.mk
+++ b/xen/arch/arm/arch.mk
@@ -1,6 +1,10 @@
 
 # arm-specific definitions
 
+ifeq ($(XEN_TARGET_ARCH),arm32)
+XEN_CFLAGS += -marm
+endif
+
 $(call cc-options-add,XEN_CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
 $(call cc-option-add,XEN_CFLAGS,CC,-Wnested-externs)
 
diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
index 5df3cf6bc3..fc3b1dc922 100644
--- a/xen/arch/x86/arch.mk
+++ b/xen/arch/x86/arch.mk
@@ -1,6 +1,8 @@
 
 # x86-specific definitions
 
+XEN_CFLAGS += -m64
+
 export XEN_IMG_OFFSET := 0x20
 
 XEN_CFLAGS += -I$(srctree)/arch/x86/include/asm/mach-generic
-- 
Anthony PERARD




[XEN PATCH 15/15] build: remove Config.mk include from Rules.mk

2023-05-23 Thread Anthony PERARD
Everything needed to build the hypervisor should already be configured
by "xen/Makefile", thus Config.mk shouldn't be needed.

Then, Config.mk keeps on testing support of some CFLAGS with CC, the
result of this testing is not used at this stage so the build is
slowed unnecessarily.

Likewise, GCC is checked to be at the minimum at 4.2 when entering
every sub-directory, so the check have run countless time at this
stage.

We only need to export a few more configuration variables. And add
some variables in Kbuild.include, and macro fallbacks for Make older
than 3.81. (Adding `or` just in case. it's only used in xen/Makefile,
which includes Config.mk and so has already the fallback.)

Signed-off-by: Anthony PERARD 
---

Notes:
Removing Config.mk benefit:
Rebuild on my computer is aboud 1 second faster overall.
Save maybe 3 seconds of user time
system less loaded

 xen/Makefile   | 4 
 xen/Rules.mk   | 1 -
 xen/scripts/Kbuild.include | 7 +++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/xen/Makefile b/xen/Makefile
index 4dc3acf2a6..9af7223c66 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -246,10 +246,14 @@ export TARGET_ARCH := $(shell echo $(XEN_TARGET_ARCH) 
| \
 sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g' \
 -e s'/riscv.*/riscv/g')
 
+export XEN_COMPILE_ARCH XEN_TARGET_ARCH
 export CONFIG_SHELL := $(SHELL)
 export CC CXX LD NM OBJCOPY OBJDUMP ADDR2LINE
+export CPP AR
 export YACC = $(if $(BISON),$(BISON),bison)
 export LEX = $(if $(FLEX),$(FLEX),flex)
+export HOSTCC HOSTCXX HOSTCFLAGS
+export EMBEDDED_EXTRA_CFLAGS LDFLAGS_DIRECT
 
 # Default file for 'make defconfig'.
 export KBUILD_DEFCONFIG := $(ARCH)_defconfig
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 8177d405c3..8291e0a573 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -17,7 +17,6 @@ __build:
 
 -include $(objtree)/include/config/auto.conf
 
-include $(XEN_ROOT)/Config.mk
 include $(srctree)/scripts/Kbuild.include
 include $(XEN_ROOT)/config/compiler-testing.mk
 
diff --git a/xen/scripts/Kbuild.include b/xen/scripts/Kbuild.include
index d820595e2f..dfa66f2c8a 100644
--- a/xen/scripts/Kbuild.include
+++ b/xen/scripts/Kbuild.include
@@ -8,6 +8,13 @@ empty   :=
 space   := $(empty) $(empty)
 space_escape := _-_SPACE_-_
 pound   := \#
+comma   := ,
+open:= (
+close   := )
+
+# fallbacks for GNU Make older than 3.81
+realpath = $(wildcard $(foreach file,$(1),$(shell cd -P $(dir $(file)) && echo 
"$$PWD/$(notdir $(file))")))
+or   = $(if $(strip $(1)),$(1),$(if $(strip $(2)),$(2),$(if $(strip 
$(3)),$(3),$(if $(strip $(4)),$(4)
 
 ###
 # Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o
-- 
Anthony PERARD




Re: [XEN PATCH] tools/xendomains: Don't auto save/restore/migrate on Arm*

2023-05-23 Thread Anthony PERARD
On Mon, May 22, 2023 at 08:34:52PM +0100, Julien Grall wrote:
> On 19/05/2023 17:24, Anthony PERARD wrote:
> > diff --git a/tools/configure.ac b/tools/configure.ac
> > index 3cccf41960..0f0983f6b7 100644
> > --- a/tools/configure.ac
> > +++ b/tools/configure.ac
> > @@ -517,4 +517,17 @@ AS_IF([test "x$pvshim" = "xy"], [
> >   AX_FIND_HEADER([INCLUDE_ENDIAN_H], [endian.h sys/endian.h])
> > +dnl Disable autosave of domain in xendomains on shutdown
> > +dnl due to missing support. This should be in sync with
> > +dnl LIBXL_HAVE_NO_SUSPEND_RESUME in libxl.h
> 
> Quite likely, a developer adding a new arch will first look at the
> definition of LIXBL_HAVE_NO_SUSPEND_RESUME. So it would be good if we have a
> similar message there to remind them to update this case. That said...

Probably true, I'll look at adding a comment there.

> > +case "$host_cpu" in
> > +arm*|aarch64)
> > +XENDOMAINS_SAVE_DIR=
> > +;;
> > +*)
> > +XENDOMAINS_SAVE_DIR="$XEN_LIB_DIR/save"
> > +;;
> > +esac
> 
> ... I am wondering if the switch should be the other way around. IOW, the
> default should be no support for suspend/resume. This will make easier to
> add support for RISC-V (I suspect support for suspend/resume will not be in
> the first version) or any new other arch.

Sounds good, I'll look at that.

Thanks,

-- 
Anthony PERARD



[XEN PATCH 14/15] Config.mk: move $(cc-option, ) to config/compiler-testing.mk

2023-05-23 Thread Anthony PERARD
In xen/, it isn't necessary to include Config.mk in every Makefile in
subdirectories as nearly all necessary variables should be calculated
in xen/Makefile. But some Makefile make use of the macro $(cc-option,)
that is only available in Config.mk.

Extract $(cc-option,) from Config.mk so we can use it without
including Config.mk and thus without having to recalculate some CFLAGS
which would be ignored.

Signed-off-by: Anthony PERARD 
---
 Config.mk  | 27 +--
 config/compiler-testing.mk | 25 +
 xen/Rules.mk   |  1 +
 3 files changed, 27 insertions(+), 26 deletions(-)
 create mode 100644 config/compiler-testing.mk

diff --git a/Config.mk b/Config.mk
index 27f48f654a..ebc8d0dfdd 100644
--- a/Config.mk
+++ b/Config.mk
@@ -18,6 +18,7 @@ realpath = $(wildcard $(foreach file,$(1),$(shell cd -P $(dir 
$(file)) && echo "
 or   = $(if $(strip $(1)),$(1),$(if $(strip $(2)),$(2),$(if $(strip 
$(3)),$(3),$(if $(strip $(4)),$(4)
 
 -include $(XEN_ROOT)/.config
+include $(XEN_ROOT)/config/compiler-testing.mk
 
 XEN_COMPILE_ARCH?= $(shell uname -m | sed -e s/i.86/x86_32/ \
  -e s/i86pc/x86_32/ -e s/amd64/x86_64/ \
@@ -79,32 +80,6 @@ PYTHON_PREFIX_ARG ?= --prefix="$(prefix)"
 # to permit the user to set PYTHON_PREFIX_ARG to '' to workaround this bug:
 #  https://bugs.launchpad.net/ubuntu/+bug/362570
 
-# cc-option: Check if compiler supports first option, else fall back to second.
-#
-# This is complicated by the fact that unrecognised -Wno-* options:
-#   (a) are ignored unless the compilation emits a warning; and
-#   (b) even then produce a warning rather than an error
-# To handle this we do a test compile, passing the option-under-test, on a code
-# fragment that will always produce a warning (integer assigned to pointer).
-# We then grep for the option-under-test in the compiler's output, the presence
-# of which would indicate an "unrecognized command-line option" warning/error.
-#
-# Usage: cflags-y += $(call cc-option,$(CC),-march=winchip-c6,-march=i586)
-cc-option = $(shell if test -z "`echo 'void*p=1;' | \
-  $(1) $(2) -c -o /dev/null -x c - 2>&1 | grep -- 
$(2:-Wa$(comma)%=%) -`"; \
-  then echo "$(2)"; else echo "$(3)"; fi ;)
-
-# cc-option-add: Add an option to compilation flags, but only if supported.
-# Usage: $(call cc-option-add CFLAGS,CC,-march=winchip-c6)
-cc-option-add = $(eval $(call cc-option-add-closure,$(1),$(2),$(3)))
-define cc-option-add-closure
-ifneq ($$(call cc-option,$$($(2)),$(3),n),n)
-$(1) += $(3)
-endif
-endef
-
-cc-options-add = $(foreach o,$(3),$(call cc-option-add,$(1),$(2),$(o)))
-
 # cc-ver: Check compiler against the version requirement. Return boolean 
'y'/'n'.
 # Usage: ifeq ($(call cc-ver,$(CC),ge,0x030400),y)
 cc-ver = $(shell if [ $$((`$(1) -dumpversion | awk -F. \
diff --git a/config/compiler-testing.mk b/config/compiler-testing.mk
new file mode 100644
index 00..9677f91abe
--- /dev/null
+++ b/config/compiler-testing.mk
@@ -0,0 +1,25 @@
+# cc-option: Check if compiler supports first option, else fall back to second.
+#
+# This is complicated by the fact that unrecognised -Wno-* options:
+#   (a) are ignored unless the compilation emits a warning; and
+#   (b) even then produce a warning rather than an error
+# To handle this we do a test compile, passing the option-under-test, on a code
+# fragment that will always produce a warning (integer assigned to pointer).
+# We then grep for the option-under-test in the compiler's output, the presence
+# of which would indicate an "unrecognized command-line option" warning/error.
+#
+# Usage: cflags-y += $(call cc-option,$(CC),-march=winchip-c6,-march=i586)
+cc-option = $(shell if test -z "`echo 'void*p=1;' | \
+  $(1) $(2) -c -o /dev/null -x c - 2>&1 | grep -- 
$(2:-Wa$(comma)%=%) -`"; \
+  then echo "$(2)"; else echo "$(3)"; fi ;)
+
+# cc-option-add: Add an option to compilation flags, but only if supported.
+# Usage: $(call cc-option-add CFLAGS,CC,-march=winchip-c6)
+cc-option-add = $(eval $(call cc-option-add-closure,$(1),$(2),$(3)))
+define cc-option-add-closure
+ifneq ($$(call cc-option,$$($(2)),$(3),n),n)
+$(1) += $(3)
+endif
+endef
+
+cc-options-add = $(foreach o,$(3),$(call cc-option-add,$(1),$(2),$(o)))
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 68b10ca5ef..8177d405c3 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -19,6 +19,7 @@ __build:
 
 include $(XEN_ROOT)/Config.mk
 include $(srctree)/scripts/Kbuild.include
+include $(XEN_ROOT)/config/compiler-testing.mk
 
 # Initialise some variables
 obj-y :=
-- 
Anthony PERARD




[XEN PATCH 11/15] build: rename CFLAGS to XEN_CFLAGS in xen/Makefile

2023-05-23 Thread Anthony PERARD
This is a preparatory patch. A future patch will not even use
$(CFLAGS) to seed $(XEN_CFLAGS).

Signed-off-by: Anthony PERARD 
---
 xen/Makefile   | 41 ++---
 xen/arch/arm/arch.mk   |  4 +--
 xen/arch/riscv/arch.mk |  4 +--
 xen/arch/x86/arch.mk   | 58 +-
 4 files changed, 54 insertions(+), 53 deletions(-)

diff --git a/xen/Makefile b/xen/Makefile
index c4a83fca76..b3bffe8c6f 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -259,6 +259,7 @@ export KBUILD_DEFCONFIG := $(ARCH)_defconfig
 export XEN_TREEWIDE_CFLAGS := $(CFLAGS)
 
 XEN_AFLAGS =
+XEN_CFLAGS = $(CFLAGS)
 
 # CLANG_FLAGS needs to be calculated before calling Kconfig
 ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),)
@@ -284,7 +285,7 @@ CLANG_FLAGS += $(call or,$(t1),$(t2),$(t3))
 endif
 
 CLANG_FLAGS += -Werror=unknown-warning-option
-CFLAGS += $(CLANG_FLAGS)
+XEN_CFLAGS += $(CLANG_FLAGS)
 export CLANG_FLAGS
 endif
 
@@ -293,7 +294,7 @@ ifeq ($(call ld-ver-build-id,$(LD)),n)
 XEN_LDFLAGS_BUILD_ID :=
 XEN_HAS_BUILD_ID := n
 else
-CFLAGS += -DBUILD_ID
+XEN_CFLAGS += -DBUILD_ID
 XEN_TREEWIDE_CFLAGS += -DBUILD_ID
 XEN_HAS_BUILD_ID := y
 XEN_LDFLAGS_BUILD_ID := --build-id=sha1
@@ -388,30 +389,30 @@ include/config/%.conf include/config/%.conf.cmd: 
$(KCONFIG_CONFIG)
$(Q)$(MAKE) $(build)=tools/kconfig syncconfig
 
 ifeq ($(CONFIG_DEBUG),y)
-CFLAGS += -O1
+XEN_CFLAGS += -O1
 else
-CFLAGS += -O2
+XEN_CFLAGS += -O2
 endif
 
 ifeq ($(CONFIG_FRAME_POINTER),y)
-CFLAGS += -fno-omit-frame-pointer
+XEN_CFLAGS += -fno-omit-frame-pointer
 else
-CFLAGS += -fomit-frame-pointer
+XEN_CFLAGS += -fomit-frame-pointer
 endif
 
 CFLAGS-$(CONFIG_CC_SPLIT_SECTIONS) += -ffunction-sections -fdata-sections
 
-CFLAGS += -nostdinc -fno-builtin -fno-common
-CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
-$(call cc-option-add,CFLAGS,CC,-Wvla)
-CFLAGS += -pipe -D__XEN__ -include $(srctree)/include/xen/config.h
+XEN_CFLAGS += -nostdinc -fno-builtin -fno-common
+XEN_CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
+$(call cc-option-add,XEN_CFLAGS,CC,-Wvla)
+XEN_CFLAGS += -pipe -D__XEN__ -include $(srctree)/include/xen/config.h
 CFLAGS-$(CONFIG_DEBUG_INFO) += -g
 
 ifneq ($(CONFIG_CC_IS_CLANG),y)
 # Clang doesn't understand this command line argument, and doesn't appear to
 # have a suitable alternative.  The resulting compiled binary does function,
 # but has an excessively large symbol table.
-CFLAGS += -Wa,--strip-local-absolute
+XEN_CFLAGS += -Wa,--strip-local-absolute
 endif
 
 XEN_AFLAGS += -D__ASSEMBLY__
@@ -420,14 +421,14 @@ $(call 
cc-option-add,XEN_AFLAGS,CC,-Wa$(comma)--noexecstack)
 
 LDFLAGS-$(call ld-option,--warn-rwx-segments) += --no-warn-rwx-segments
 
-CFLAGS += $(CFLAGS-y)
+XEN_CFLAGS += $(CFLAGS-y)
 # allow extra CFLAGS externally via EXTRA_CFLAGS_XEN_CORE
-CFLAGS += $(EXTRA_CFLAGS_XEN_CORE)
+XEN_CFLAGS += $(EXTRA_CFLAGS_XEN_CORE)
 
 # Most CFLAGS are safe for assembly files:
 #  -std=gnu{89,99} gets confused by #-prefixed end-of-line comments
 #  -flto makes no sense and annoys clang
-XEN_AFLAGS += $(filter-out -std=gnu% -flto,$(CFLAGS)) $(AFLAGS-y)
+XEN_AFLAGS += $(filter-out -std=gnu% -flto,$(XEN_CFLAGS)) $(AFLAGS-y)
 
 # LDFLAGS are only passed directly to $(LD)
 LDFLAGS += $(LDFLAGS_DIRECT) $(LDFLAGS-y)
@@ -439,16 +440,16 @@ CFLAGS_UBSAN :=
 endif
 
 ifeq ($(CONFIG_LTO),y)
-CFLAGS += -flto
+XEN_CFLAGS += -flto
 LDFLAGS-$(CONFIG_CC_IS_CLANG) += -plugin LLVMgold.so
 endif
 
 ifdef building_out_of_srctree
-CFLAGS += -I$(objtree)/include
-CFLAGS += -I$(objtree)/arch/$(TARGET_ARCH)/include
+XEN_CFLAGS += -I$(objtree)/include
+XEN_CFLAGS += -I$(objtree)/arch/$(TARGET_ARCH)/include
 endif
-CFLAGS += -I$(srctree)/include
-CFLAGS += -I$(srctree)/arch/$(TARGET_ARCH)/include
+XEN_CFLAGS += -I$(srctree)/include
+XEN_CFLAGS += -I$(srctree)/arch/$(TARGET_ARCH)/include
 
 # Note that link order matters!
 ALL_OBJS-y:= common/built_in.o
@@ -463,7 +464,7 @@ ALL_LIBS-y:= lib/lib.a
 include $(srctree)/arch/$(TARGET_ARCH)/arch.mk
 
 # define new variables to avoid the ones defined in Config.mk
-export XEN_CFLAGS := $(CFLAGS)
+export XEN_CFLAGS := $(XEN_CFLAGS)
 export XEN_AFLAGS := $(XEN_AFLAGS)
 export XEN_LDFLAGS := $(LDFLAGS)
 export CFLAGS_UBSAN
diff --git a/xen/arch/arm/arch.mk b/xen/arch/arm/arch.mk
index 58db76c4e1..300b8bf7ae 100644
--- a/xen/arch/arm/arch.mk
+++ b/xen/arch/arm/arch.mk
@@ -1,8 +1,8 @@
 
 # arm-specific definitions
 
-$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
-$(call cc-option-add,CFLAGS,CC,-Wnested-externs)
+$(call cc-options-add,XEN_CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
+$(call cc-option-add,XEN_CFLAGS,CC,-Wnested-externs)
 
 # Prevent floating-point variables from creeping into Xen.
 CFLAGS-$(CONFIG_ARM_32) += -msoft-float
diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk
index 7448f759b4..aadf373ce8 100644
--- a/xen/arch/riscv/arch.mk

[XEN PATCH 05/15] build: introduce a generic command for gzip

2023-05-23 Thread Anthony PERARD
Make the gzip command generic and use -9 which wasn't use for
config.gz. (xen.gz does use -9)

Signed-off-by: Anthony PERARD 
---
 xen/Rules.mk| 5 +
 xen/common/Makefile | 8 
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 59072ae8df..68b10ca5ef 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -63,6 +63,11 @@ cmd_objcopy = $(OBJCOPY) $(OBJCOPYFLAGS) $< $@
 quiet_cmd_binfile = BINFILE $@
 cmd_binfile = $(SHELL) $(srctree)/tools/binfile $(BINFILE_FLAGS) $@ $(2)
 
+# gzip
+quiet_cmd_gzip = GZIP$@
+cmd_gzip = \
+cat $(real-prereqs) | gzip -n -f -9 > $@
+
 # Figure out what we need to build from the various variables
 # ===
 
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 46049eac35..f45f19c391 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -78,13 +78,13 @@ obj-$(CONFIG_NEEDS_LIBELF) += libelf/
 obj-$(CONFIG_HAS_DEVICE_TREE) += libfdt/
 
 CONF_FILE := $(if $(patsubst 
/%,,$(KCONFIG_CONFIG)),$(objtree)/)$(KCONFIG_CONFIG)
-$(obj)/config.gz: $(CONF_FILE)
-   gzip -n -c $< >$@
+$(obj)/config.gz: $(CONF_FILE) FORCE
+   $(call if_changed,gzip)
+
+targets += config.gz
 
 $(obj)/config_data.o: $(obj)/config.gz
 
 $(obj)/config_data.S: $(srctree)/tools/binfile FORCE
$(call if_changed,binfile,$(obj)/config.gz xen_config_data)
 targets += config_data.S
-
-clean-files := config.gz
-- 
Anthony PERARD




[XEN PATCH 13/15] build: fix compile.h compiler version command line

2023-05-23 Thread Anthony PERARD
CFLAGS is just from Config.mk, instead use the flags used to build
Xen.

Signed-off-by: Anthony PERARD 
---

Notes:
I don't know if CFLAGS is even useful there, just --version without the
flags might produce the same result.

 xen/build.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/build.mk b/xen/build.mk
index e2a78aa806..d468bb6e26 100644
--- a/xen/build.mk
+++ b/xen/build.mk
@@ -23,7 +23,7 @@ define cmd_compile.h
-e 's/@@whoami@@/$(XEN_WHOAMI)/g' \
-e 's/@@domain@@/$(XEN_DOMAIN)/g' \
-e 's/@@hostname@@/$(XEN_BUILD_HOST)/g' \
-   -e 's!@@compiler@@!$(shell $(CC) $(CFLAGS) --version 2>&1 | head 
-1)!g' \
+   -e 's!@@compiler@@!$(shell $(CC) $(XEN_CFLAGS) --version 2>&1 | 
head -1)!g' \
-e 's/@@version@@/$(XEN_VERSION)/g' \
-e 's/@@subversion@@/$(XEN_SUBVERSION)/g' \
-e 's/@@extraversion@@/$(XEN_EXTRAVERSION)/g' \
-- 
Anthony PERARD




[XEN PATCH 09/15] build: hide commands run for kconfig

2023-05-23 Thread Anthony PERARD
but still show a log entry for syncconfig. We have to use kecho
instead of $(cmd,) to avoid issue with prompt from kconfig.

linux commits for reference:
23cd88c91343 ("kbuild: hide commands to run Kconfig, and show short log for 
syncconfig")
d952cfaf0cff ("kbuild: do not suppress Kconfig prompts for silent build")

Signed-off-by: Anthony PERARD 
---
 xen/Makefile   |  1 +
 xen/tools/kconfig/Makefile | 14 +++---
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/xen/Makefile b/xen/Makefile
index 4dc960df2c..127c0e40b5 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -382,6 +382,7 @@ $(KCONFIG_CONFIG): tools_fixdep
 # This exploits the 'multi-target pattern rule' trick.
 # The syncconfig should be executed only once to make all the targets.
 include/config/%.conf include/config/%.conf.cmd: $(KCONFIG_CONFIG)
+   $(Q)$(kecho) "  SYNC$@"
$(Q)$(MAKE) $(build)=tools/kconfig syncconfig
 
 ifeq ($(CONFIG_DEBUG),y)
diff --git a/xen/tools/kconfig/Makefile b/xen/tools/kconfig/Makefile
index b7b9a419ad..18c0f5d4f1 100644
--- a/xen/tools/kconfig/Makefile
+++ b/xen/tools/kconfig/Makefile
@@ -24,19 +24,19 @@ endif
 unexport CONFIG_
 
 xconfig: $(obj)/qconf
-   $< $(silent) $(Kconfig)
+   $(Q)$< $(silent) $(Kconfig)
 
 gconfig: $(obj)/gconf
-   $< $(silent) $(Kconfig)
+   $(Q)$< $(silent) $(Kconfig)
 
 menuconfig: $(obj)/mconf
-   $< $(silent) $(Kconfig)
+   $(Q)$< $(silent) $(Kconfig)
 
 config: $(obj)/conf
-   $< $(silent) --oldaskconfig $(Kconfig)
+   $(Q)$< $(silent) --oldaskconfig $(Kconfig)
 
 nconfig: $(obj)/nconf
-   $< $(silent) $(Kconfig)
+   $(Q)$< $(silent) $(Kconfig)
 
 build_menuconfig: $(obj)/mconf
 
@@ -70,12 +70,12 @@ simple-targets := oldconfig allnoconfig allyesconfig 
allmodconfig \
 PHONY += $(simple-targets)
 
 $(simple-targets): $(obj)/conf
-   $< $(silent) --$@ $(Kconfig)
+   $(Q)$< $(silent) --$@ $(Kconfig)
 
 PHONY += savedefconfig defconfig
 
 savedefconfig: $(obj)/conf
-   $< $(silent) --$@=defconfig $(Kconfig)
+   $(Q)$< $(silent) --$@=defconfig $(Kconfig)
 
 defconfig: $(obj)/conf
 ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/configs/$(KBUILD_DEFCONFIG)),)
-- 
Anthony PERARD




[XEN PATCH 08/15] build: use $(filechk, ) for all compat/.xlat/%.lst

2023-05-23 Thread Anthony PERARD
Make use of filechk mean that we don't have to use
$(move-if-changed,). It also mean that will have sometime "UPD .." in
the build output when the target changed, rather than having "GEN ..."
all the time when "xlat.lst" happen to have a more recent modification
timestamp.

While there, replace `grep -v` by `sed '//d'` to avoid an extra
fork and pipe when building.

Signed-off-by: Anthony PERARD 
---
 xen/include/Makefile | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/xen/include/Makefile b/xen/include/Makefile
index 96d5f6f3c8..2e61b50139 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -93,15 +93,13 @@ targets += $(patsubst compat/%, compat/.xlat/%, 
$(headers-y))
 $(obj)/compat/.xlat/%.h: $(obj)/compat/%.h $(obj)/compat/.xlat/%.lst 
$(srctree)/tools/compat-xlat-header.py FORCE
$(call if_changed,xlat_headers)
 
-quiet_cmd_xlat_lst = GEN $@
-cmd_xlat_lst = \
-   grep -v '^[[:blank:]]*$(pound)' $< | sed -ne 
's,@arch@,$(compat-arch-y),g' -re 's,[[:blank:]]+$*\.h[[:blank:]]*$$,,p' 
>$@.new; \
-   $(call move-if-changed,$@.new,$@)
+filechk_xlat_lst = \
+   sed -ne '/^[[:blank:]]*$(pound)/d' -e 's,@arch@,$(compat-arch-y),g' -re 
's,[[:blank:]]+$*\.h[[:blank:]]*$$,,p' $<
 
 .PRECIOUS: $(obj)/compat/.xlat/%.lst
 targets += $(patsubst compat/%.h, compat/.xlat/%.lst, $(headers-y))
 $(obj)/compat/.xlat/%.lst: $(srcdir)/xlat.lst FORCE
-   $(call if_changed,xlat_lst)
+   $(call filechk,xlat_lst)
 
 xlat-y := $(shell sed -ne 's,@arch@,$(compat-arch-y),g' -re 
's,^[?!][[:blank:]]+[^[:blank:]]+[[:blank:]]+,,p' $(srcdir)/xlat.lst | uniq)
 xlat-y := $(filter $(patsubst compat/%,%,$(headers-y)),$(xlat-y))
-- 
Anthony PERARD




[XEN PATCH 06/15] build: quiet for .allconfig.tmp target

2023-05-23 Thread Anthony PERARD
Signed-off-by: Anthony PERARD 
---
 xen/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/Makefile b/xen/Makefile
index e89fc461fc..27f70d2200 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -339,7 +339,7 @@ filechk_kconfig_allconfig = \
 :
 
 .allconfig.tmp: FORCE
-   set -e; { $(call filechk_kconfig_allconfig); } > $@
+   $(Q)set -e; { $(call filechk_kconfig_allconfig); } > $@
 
 config: tools_fixdep outputmakefile FORCE
$(Q)$(MAKE) $(build)=tools/kconfig $@
-- 
Anthony PERARD




[XEN PATCH 03/15] build, x86: clean build log for boot/ targets

2023-05-23 Thread Anthony PERARD
We are adding %.lnk to .PRECIOUS or make treat them as intermediate
targets and remove them.

Signed-off-by: Anthony PERARD 
---
 xen/arch/x86/boot/Makefile | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
index 03d8ce3a9e..2693b938bd 100644
--- a/xen/arch/x86/boot/Makefile
+++ b/xen/arch/x86/boot/Makefile
@@ -5,6 +5,8 @@ head-bin-objs := cmdline.o reloc.o
 nocov-y   += $(head-bin-objs)
 noubsan-y += $(head-bin-objs)
 targets   += $(head-bin-objs)
+targets   += $(head-bin-objs:.o=.bin)
+targets   += $(head-bin-objs:.o=.lnk)
 
 head-bin-objs := $(addprefix $(obj)/,$(head-bin-objs))
 
@@ -26,10 +28,16 @@ $(head-bin-objs): XEN_CFLAGS := $(CFLAGS_x86_32) -fpic
 LDFLAGS_DIRECT-$(call ld-option,--warn-rwx-segments) := --no-warn-rwx-segments
 LDFLAGS_DIRECT += $(LDFLAGS_DIRECT-y)
 
-%.bin: %.lnk
-   $(OBJCOPY) -j .text -O binary $< $@
+%.bin: OBJCOPYFLAGS := -j .text -O binary
+%.bin: %.lnk FORCE
+   $(call if_changed,objcopy)
 
-%.lnk: %.o $(src)/build32.lds
-   $(LD) $(subst x86_64,i386,$(LDFLAGS_DIRECT)) -N -T $(filter %.lds,$^) 
-o $@ $<
+quiet_cmd_ld_lnk_o = LD  $@
+cmd_ld_lnk_o = $(LD) $(subst x86_64,i386,$(LDFLAGS_DIRECT)) -N -T $(filter 
%.lds,$^) -o $@ $<
+
+%.lnk: %.o $(src)/build32.lds FORCE
+   $(call if_changed,ld_lnk_o)
 
 clean-files := *.lnk *.bin
+
+.PRECIOUS: %.lnk
-- 
Anthony PERARD




[XEN PATCH 07/15] build: move XEN_HAS_BUILD_ID out of Config.mk

2023-05-23 Thread Anthony PERARD
Whether or not the linker can do build id is only used by the
hypervisor build, so move that there.

Rename $(build_id_linker) to $(XEN_LDFLAGS_BUILD_ID) as this is a
better name to be exported as to use the "XEN_*" namespace.

Also update XEN_TREEWIDE_CFLAGS so flags can be used for
arch/x86/boot/ CFLAGS_x86_32

Beside a reordering of the command line where CFLAGS is used, there
shouldn't be any other changes.

Signed-off-by: Anthony PERARD 
---
 Config.mk   | 12 
 xen/Makefile| 12 
 xen/arch/arm/Makefile   |  2 +-
 xen/arch/riscv/Makefile |  2 +-
 xen/arch/x86/Makefile   | 12 ++--
 xen/scripts/Kbuild.include  |  3 +++
 xen/test/livepatch/Makefile |  4 ++--
 7 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/Config.mk b/Config.mk
index d12d4c2b8f..27f48f654a 100644
--- a/Config.mk
+++ b/Config.mk
@@ -125,18 +125,6 @@ endef
 check-$(gcc) = $(call cc-ver-check,CC,0x040100,"Xen requires at least gcc-4.1")
 $(eval $(check-y))
 
-ld-ver-build-id = $(shell $(1) --build-id 2>&1 | \
-   grep -q build-id && echo n || echo y)
-
-export XEN_HAS_BUILD_ID ?= n
-ifeq ($(call ld-ver-build-id,$(LD)),n)
-build_id_linker :=
-else
-CFLAGS += -DBUILD_ID
-export XEN_HAS_BUILD_ID=y
-build_id_linker := --build-id=sha1
-endif
-
 define buildmakevars2shellvars
 export PREFIX="$(prefix)";\
 export XEN_SCRIPT_DIR="$(XEN_SCRIPT_DIR)";\
diff --git a/xen/Makefile b/xen/Makefile
index 27f70d2200..4dc960df2c 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -286,6 +286,18 @@ CFLAGS += $(CLANG_FLAGS)
 export CLANG_FLAGS
 endif
 
+# XEN_HAS_BUILD_ID needed by Kconfig
+ifeq ($(call ld-ver-build-id,$(LD)),n)
+XEN_LDFLAGS_BUILD_ID :=
+XEN_HAS_BUILD_ID := n
+else
+CFLAGS += -DBUILD_ID
+XEN_TREEWIDE_CFLAGS += -DBUILD_ID
+XEN_HAS_BUILD_ID := y
+XEN_LDFLAGS_BUILD_ID := --build-id=sha1
+endif
+export XEN_HAS_BUILD_ID XEN_LDFLAGS_BUILD_ID
+
 export XEN_HAS_CHECKPOLICY := $(call success,$(CHECKPOLICY) -h 2>&1 | grep -q 
xen)
 
 # ===
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 4d076b278b..1cc57d2cf0 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -102,7 +102,7 @@ $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
$(NM) -pa --format=sysv $(@D)/.$(@F).1 \
| $(objtree)/tools/symbols $(all_symbols) --sysv --sort 
>$(@D)/.$(@F).1.S
$(MAKE) $(build)=$(@D) $(@D)/.$(@F).1.o
-   $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \
+   $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(XEN_LDFLAGS_BUILD_ID) \
$(@D)/.$(@F).1.o -o $@
$(NM) -pa --format=sysv $(@D)/$(@F) \
| $(objtree)/tools/symbols --all-symbols --xensyms --sysv 
--sort \
diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index 443f6bf15f..8a0e483c66 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -9,7 +9,7 @@ $(TARGET): $(TARGET)-syms
$(OBJCOPY) -O binary -S $< $@
 
 $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
-   $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) -o $@
+   $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(XEN_LDFLAGS_BUILD_ID) -o 
$@
$(NM) -pa --format=sysv $(@D)/$(@F) \
| $(objtree)/tools/symbols --all-symbols --xensyms --sysv 
--sort \
>$(@D)/$(@F).map
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 2672d7f4ee..c7ec315fe6 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -100,7 +100,7 @@ efi-y := $(shell if [ ! -r $(objtree)/include/xen/compile.h 
-o \
  $(space)
 efi-$(CONFIG_PV_SHIM_EXCLUSIVE) :=
 
-ifneq ($(build_id_linker),)
+ifneq ($(XEN_LDFLAGS_BUILD_ID),)
 notes_phdrs = --notes
 else
 ifeq ($(CONFIG_PVH_GUEST),y)
@@ -136,19 +136,19 @@ $(TARGET): $(TARGET)-syms $(efi-y) $(obj)/boot/mkelf32
 CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
 
 $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
-   $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \
+   $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(XEN_LDFLAGS_BUILD_ID) \
$(objtree)/common/symbols-dummy.o -o $(@D)/.$(@F).0
$(NM) -pa --format=sysv $(@D)/.$(@F).0 \
| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
>$(@D)/.$(@F).0.S
$(MAKE) $(build)=$(@D) $(@D)/.$(@F).0.o
-   $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \
+   $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(XEN_LDFLAGS_BUILD_ID) \
$(@D)/.$(@F).0.o -o $(@D)/.$(@F).1
$(NM) -pa --format=sysv $(@D)/.$(@F).1 \
| $(objtree)/tools/symbols $(all_symbols) --sysv --sort 
$(syms-warn-dup-y) \
>$(@D)/.$(@F).1.S
$(MAKE) $(build)=$(@D) $(@D)/.$(@F).1.o
-   

[XEN PATCH 04/15] build: hide policy.bin commands

2023-05-23 Thread Anthony PERARD
Instead, show only when "policy.bin" is been updated.

We still have the full command from the flask/policy Makefile, but we
can't change that Makefile.

Signed-off-by: Anthony PERARD 
---
 xen/xsm/flask/Makefile | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
index 3fdcf7727e..fc44ad684f 100644
--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -48,10 +48,15 @@ targets += flask-policy.S
 FLASK_BUILD_DIR := $(abs_objtree)/$(obj)
 POLICY_SRC := $(FLASK_BUILD_DIR)/xenpolicy-$(XEN_FULLVERSION)
 
+policy_chk = \
+$(Q)if ! cmp -s $(POLICY_SRC) $@; then \
+$(kecho) '  UPD $@'; \
+cp $(POLICY_SRC) $@; \
+fi
 $(obj)/policy.bin: FORCE
-   $(MAKE) -f $(XEN_ROOT)/tools/flask/policy/Makefile.common \
+   $(Q)$(MAKE) -f $(XEN_ROOT)/tools/flask/policy/Makefile.common \
-C $(XEN_ROOT)/tools/flask/policy \
FLASK_BUILD_DIR=$(FLASK_BUILD_DIR) POLICY_FILENAME=$(POLICY_SRC)
-   cmp -s $(POLICY_SRC) $@ || cp $(POLICY_SRC) $@
+   $(call policy_chk)
 
 clean-files := policy.* $(POLICY_SRC)
-- 
Anthony PERARD




[XEN PATCH 01/15] build: hide that we are updating xen/lib/x86

2023-05-23 Thread Anthony PERARD
Change of directory to xen/lib/x86 isn't needed to be shown. If
something gets updated, make will print the command line.

Signed-off-by: Anthony PERARD 
---
 xen/include/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/include/Makefile b/xen/include/Makefile
index edd5322e88..96d5f6f3c8 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -229,7 +229,7 @@ ifeq ($(XEN_TARGET_ARCH),x86_64)
 .PHONY: lib-x86-all
 lib-x86-all:
@mkdir -p $(obj)/xen/lib/x86
-   $(MAKE) -C $(obj)/xen/lib/x86 -f 
$(abs_srctree)/$(src)/xen/lib/x86/Makefile all
+   $(Q)$(MAKE) -C $(obj)/xen/lib/x86 -f 
$(abs_srctree)/$(src)/xen/lib/x86/Makefile all
 
 all: lib-x86-all
 endif
-- 
Anthony PERARD




[XEN PATCH 00/15] build: cleanup build log, avoid user's CFLAGS, avoid too many include of Config.mk

2023-05-23 Thread Anthony PERARD
Patch series available in this git branch:
https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git 
br.build-system-xen-removing-config.mk-v1

Hi,

This series of patch cleanup the remaining rules still displaying their command
line.

Then, some change are made in Config.mk to remove build-id stuff only used by
Xen build.

Then, the variable AFLAGS and CFLAGS are been renamed XEN_AFLAGS and XEN_CFLAGS
from the beginning to about inclusion of users CFLAGS as those are usually made
for user space program, not kernel, especially in build environment for distro
packages.

Last patch removes the inclusion of Config.mk from xen/Rules.mk, as this slow
down the build unnecessarily. xen/Makefile should do everything necessary to
setup the build of the hypervisor, and is its only entry point.

Thanks,

Anthony PERARD (15):
  build: hide that we are updating xen/lib/x86
  build: rework asm-offsets.* build step to use kbuild
  build, x86: clean build log for boot/ targets
  build: hide policy.bin commands
  build: introduce a generic command for gzip
  build: quiet for .allconfig.tmp target
  build: move XEN_HAS_BUILD_ID out of Config.mk
  build: use $(filechk, ) for all compat/.xlat/%.lst
  build: hide commands run for kconfig
  build: rename $(AFLAGS) to $(XEN_AFLAGS)
  build: rename CFLAGS to XEN_CFLAGS in xen/Makefile
  build: avoid Config.mk's CFLAGS
  build: fix compile.h compiler version command line
  Config.mk: move $(cc-option, ) to config/compiler-testing.mk
  build: remove Config.mk include from Rules.mk

 Config.mk   | 39 +--
 config/compiler-testing.mk  | 25 +
 xen/Makefile| 75 +
 xen/Rules.mk|  7 +++-
 xen/arch/arm/Makefile   |  2 +-
 xen/arch/arm/arch.mk|  8 +++-
 xen/arch/riscv/Makefile |  2 +-
 xen/arch/riscv/arch.mk  |  4 +-
 xen/arch/x86/Makefile   | 12 +++---
 xen/arch/x86/arch.mk| 62 +++---
 xen/arch/x86/boot/Makefile  | 16 ++--
 xen/build.mk| 24 +++-
 xen/common/Makefile |  8 ++--
 xen/include/Makefile| 10 ++---
 xen/scripts/Kbuild.include  | 10 +
 xen/test/livepatch/Makefile |  4 +-
 xen/tools/kconfig/Makefile  | 14 +++
 xen/xsm/flask/Makefile  |  9 -
 18 files changed, 193 insertions(+), 138 deletions(-)
 create mode 100644 config/compiler-testing.mk

-- 
Anthony PERARD




[XEN PATCH 02/15] build: rework asm-offsets.* build step to use kbuild

2023-05-23 Thread Anthony PERARD
Use $(if_changed_dep, ) macro to generate "asm-offsets.s" and remove
the use of $(move-if-changes,). That mean that "asm-offset.s" will be
changed even when the output doesn't change.

But "asm-offsets.s" is only used to generated "asm-offsets.h". So
instead of regenerating "asm-offsets.h" every time "asm-offsets.s"
change, we will use "$(filechk, )" to only update the ".h" when the
output change. Also, with "$(filechk, )", the file does get
regenerated when the rule change in the makefile.

This changes also result in a cleaner build log.

Signed-off-by: Anthony PERARD 
---

Instead of having a special $(cmd_asm-offsets.s) command, we could
probably reuse $(cmd_cc_s_c) from Rules.mk, but that would mean that
an hypothetical additional flags "-flto" in CFLAGS would not be
removed anymore, not sure if that matter here.

But then we could write this:

targets += arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s
arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s: CFLAGS-y += -g0
arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: 
arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s FORCE

instead of having to write a rule for asm-offsets.s
---
 xen/build.mk | 22 ++
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/xen/build.mk b/xen/build.mk
index 758590c68e..e2a78aa806 100644
--- a/xen/build.mk
+++ b/xen/build.mk
@@ -40,13 +40,15 @@ include/xen/compile.h: include/xen/compile.h.in .banner 
FORCE
 
 targets += include/xen/compile.h
 
--include $(wildcard .asm-offsets.s.d)
-asm-offsets.s: arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.c
-   $(CC) $(call cpp_flags,$(c_flags)) -S -g0 -o $@.new -MQ $@ $<
-   $(call move-if-changed,$@.new,$@)
+quiet_cmd_asm-offsets.s = CC  $@
+cmd_asm-offsets.s = $(CC) $(call cpp_flags,$(c_flags)) -S -g0 $< -o $@
 
-arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: asm-offsets.s
-   @(set -e; \
+asm-offsets.s: arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.c FORCE
+   $(call if_changed_dep,asm-offsets.s)
+
+targets += asm-offsets.s
+
+define filechk_asm-offsets.h
  echo "/*"; \
  echo " * DO NOT MODIFY."; \
  echo " *"; \
@@ -57,9 +59,13 @@ arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: asm-offsets.s
  echo "#ifndef __ASM_OFFSETS_H__"; \
  echo "#define __ASM_OFFSETS_H__"; \
  echo ""; \
- sed -rne "/^[^#].*==>/{s:.*==>(.*)<==.*:\1:; s: [\$$#]: :; p;}"; \
+ sed -rne "/^[^#].*==>/{s:.*==>(.*)<==.*:\1:; s: [\$$#]: :; p;}" $<; \
  echo ""; \
- echo "#endif") <$< >$@
+ echo "#endif"
+endef
+
+arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: asm-offsets.s FORCE
+   $(call filechk,asm-offsets.h)
 
 build-dirs := $(patsubst %/built_in.o,%,$(filter %/built_in.o,$(ALL_OBJS) 
$(ALL_LIBS)))
 
-- 
Anthony PERARD




[libvirt test] 180910: tolerable all pass - PUSHED

2023-05-23 Thread osstest service owner
flight 180910 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/180910/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 180714
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 180714
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 180714
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-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-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt 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-arm64-arm64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-arm64-arm64-libvirt-qcow2 15 saverestore-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-amd64-amd64-libvirt-vhd 14 migrate-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-libvirt 15 migrate-support-checkfail   never pass

version targeted for testing:
 libvirt  3b6d69237f0a07bf8d9807cd68a387f8d42b076f
baseline version:
 libvirt  90404c53682f464b4a26efd618887dc336d9da80

Last test of basis   180714  2023-05-19 04:18:50 Z4 days
Testing same since   180910  2023-05-23 04:18:51 Z0 days1 attempts


People who touched revisions under test:
  Andrea Bolognani 
  Michal Privoznik 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-libvirt-xsm pass
 test-arm64-arm64-libvirt-xsm pass
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-libvirt pass
 test-arm64-arm64-libvirt pass
 test-armhf-armhf-libvirt pass
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-i386-libvirt-pair pass
 test-arm64-arm64-libvirt-qcow2   pass
 test-armhf-armhf-libvirt-qcow2   pass
 test-arm64-arm64-libvirt-raw pass
 test-armhf-armhf-libvirt-raw pass
 test-amd64-i386-libvirt-raw  pass
 test-amd64-amd64-libvirt-vhd pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of 

Re: [PATCH 3/6] block/blkio: convert to blk_io_plug_call() API

2023-05-23 Thread Stefan Hajnoczi
On Fri, May 19, 2023 at 10:47:00AM +0200, Stefano Garzarella wrote:
> On Wed, May 17, 2023 at 06:10:19PM -0400, Stefan Hajnoczi wrote:
> > Stop using the .bdrv_co_io_plug() API because it is not multi-queue
> > block layer friendly. Use the new blk_io_plug_call() API to batch I/O
> > submission instead.
> > 
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> > block/blkio.c | 40 +---
> > 1 file changed, 21 insertions(+), 19 deletions(-)
> 
> With this patch, the build fails in several places, maybe it's an old
> version:
> 
> ../block/blkio.c:347:5: error: implicit declaration of function
> ‘blk_io_plug_call’ [-Werror=implicit-function-declaration]
>   347 | blk_io_plug_call(blkio_unplug_fn, bs);

Will fix, thanks!

Stefan


signature.asc
Description: PGP signature


Re: [PATCH 4/6] block/io_uring: convert to blk_io_plug_call() API

2023-05-23 Thread Stefan Hajnoczi
On Thu, May 18, 2023 at 07:18:42PM -0500, Eric Blake wrote:
> On Wed, May 17, 2023 at 06:10:20PM -0400, Stefan Hajnoczi wrote:
> > Stop using the .bdrv_co_io_plug() API because it is not multi-queue
> > block layer friendly. Use the new blk_io_plug_call() API to batch I/O
> > submission instead.
> > 
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> >  include/block/raw-aio.h |  7 ---
> >  block/file-posix.c  | 10 -
> >  block/io_uring.c| 45 -
> >  block/trace-events  |  5 ++---
> >  4 files changed, 19 insertions(+), 48 deletions(-)
> > 
> 
> > @@ -337,7 +325,6 @@ void luring_io_unplug(void)
> >   * @type: type of request
> >   *
> >   * Fetches sqes from ring, adds to pending queue and preps them
> > - *
> >   */
> >  static int luring_do_submit(int fd, LuringAIOCB *luringcb, LuringState *s,
> >  uint64_t offset, int type)
> > @@ -370,14 +357,16 @@ static int luring_do_submit(int fd, LuringAIOCB 
> > *luringcb, LuringState *s,
> 
> Looks a bit like a stray hunk, but you are touching the function, so
> it's okay.

I'm respinning, so I'll drop this.

Stefan


signature.asc
Description: PGP signature


Re: [PATCH 1/6] block: add blk_io_plug_call() API

2023-05-23 Thread Stefan Hajnoczi
On Thu, May 18, 2023 at 07:04:52PM -0500, Eric Blake wrote:
> On Wed, May 17, 2023 at 06:10:17PM -0400, Stefan Hajnoczi wrote:
> > Introduce a new API for thread-local blk_io_plug() that does not
> > traverse the block graph. The goal is to make blk_io_plug() multi-queue
> > friendly.
> > 
> > Instead of having block drivers track whether or not we're in a plugged
> > section, provide an API that allows them to defer a function call until
> > we're unplugged: blk_io_plug_call(fn, opaque). If blk_io_plug_call() is
> > called multiple times with the same fn/opaque pair, then fn() is only
> > called once at the end of the function - resulting in batching.
> > 
> > This patch introduces the API and changes blk_io_plug()/blk_io_unplug().
> > blk_io_plug()/blk_io_unplug() no longer require a BlockBackend argument
> > because the plug state is now thread-local.
> > 
> > Later patches convert block drivers to blk_io_plug_call() and then we
> > can finally remove .bdrv_co_io_plug() once all block drivers have been
> > converted.
> > 
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> 
> > +++ b/block/plug.c
> > +
> > +/**
> > + * blk_io_plug_call:
> > + * @fn: a function pointer to be invoked
> > + * @opaque: a user-defined argument to @fn()
> > + *
> > + * Call @fn(@opaque) immediately if not within a 
> > blk_io_plug()/blk_io_unplug()
> > + * section.
> > + *
> > + * Otherwise defer the call until the end of the outermost
> > + * blk_io_plug()/blk_io_unplug() section in this thread. If the same
> > + * @fn/@opaque pair has already been deferred, it will only be called once 
> > upon
> > + * blk_io_unplug() so that accumulated calls are batched into a single 
> > call.
> > + *
> > + * The caller must ensure that @opaque is not be freed before @fn() is 
> > invoked.
> 
> s/be //

Will fix, thanks!

Stefan


signature.asc
Description: PGP signature


Re: [PATCH 2/6] block/nvme: convert to blk_io_plug_call() API

2023-05-23 Thread Stefan Hajnoczi
On Fri, May 19, 2023 at 10:46:25AM +0200, Stefano Garzarella wrote:
> On Wed, May 17, 2023 at 06:10:18PM -0400, Stefan Hajnoczi wrote:
> > Stop using the .bdrv_co_io_plug() API because it is not multi-queue
> > block layer friendly. Use the new blk_io_plug_call() API to batch I/O
> > submission instead.
> > 
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> > block/nvme.c | 44 
> > 1 file changed, 12 insertions(+), 32 deletions(-)
> > 
> > diff --git a/block/nvme.c b/block/nvme.c
> > index 5b744c2bda..100b38b592 100644
> > --- a/block/nvme.c
> > +++ b/block/nvme.c
> > @@ -25,6 +25,7 @@
> > #include "qemu/vfio-helpers.h"
> > #include "block/block-io.h"
> > #include "block/block_int.h"
> > +#include "sysemu/block-backend.h"
> > #include "sysemu/replay.h"
> > #include "trace.h"
> > 
> > @@ -119,7 +120,6 @@ struct BDRVNVMeState {
> > int blkshift;
> > 
> > uint64_t max_transfer;
> > -bool plugged;
> > 
> > bool supports_write_zeroes;
> > bool supports_discard;
> > @@ -282,7 +282,7 @@ static void nvme_kick(NVMeQueuePair *q)
> > {
> > BDRVNVMeState *s = q->s;
> > 
> > -if (s->plugged || !q->need_kick) {
> > +if (!q->need_kick) {
> > return;
> > }
> > trace_nvme_kick(s, q->index);
> > @@ -387,10 +387,6 @@ static bool nvme_process_completion(NVMeQueuePair *q)
> > NvmeCqe *c;
> > 
> > trace_nvme_process_completion(s, q->index, q->inflight);
> > -if (s->plugged) {
> > -trace_nvme_process_completion_queue_plugged(s, q->index);
> 
> Should we remove "nvme_process_completion_queue_plugged(void *s,
> unsigned q_index) "s %p q #%u" from block/trace-events?

Will fix, thanks!

Stefan


signature.asc
Description: PGP signature


Re: [PATCH 1/6] block: add blk_io_plug_call() API

2023-05-23 Thread Stefan Hajnoczi
On Fri, May 19, 2023 at 10:45:57AM +0200, Stefano Garzarella wrote:
> On Wed, May 17, 2023 at 06:10:17PM -0400, Stefan Hajnoczi wrote:
> > Introduce a new API for thread-local blk_io_plug() that does not
> > traverse the block graph. The goal is to make blk_io_plug() multi-queue
> > friendly.
> > 
> > Instead of having block drivers track whether or not we're in a plugged
> > section, provide an API that allows them to defer a function call until
> > we're unplugged: blk_io_plug_call(fn, opaque). If blk_io_plug_call() is
> > called multiple times with the same fn/opaque pair, then fn() is only
> > called once at the end of the function - resulting in batching.
> > 
> > This patch introduces the API and changes blk_io_plug()/blk_io_unplug().
> > blk_io_plug()/blk_io_unplug() no longer require a BlockBackend argument
> > because the plug state is now thread-local.
> > 
> > Later patches convert block drivers to blk_io_plug_call() and then we
> > can finally remove .bdrv_co_io_plug() once all block drivers have been
> > converted.
> > 
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> > MAINTAINERS   |   1 +
> > include/sysemu/block-backend-io.h |  13 +--
> > block/block-backend.c |  22 -
> > block/plug.c  | 159 ++
> > hw/block/dataplane/xen-block.c|   8 +-
> > hw/block/virtio-blk.c |   4 +-
> > hw/scsi/virtio-scsi.c |   6 +-
> > block/meson.build |   1 +
> > 8 files changed, 173 insertions(+), 41 deletions(-)
> > create mode 100644 block/plug.c
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 50585117a0..574202295c 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -2644,6 +2644,7 @@ F: util/aio-*.c
> > F: util/aio-*.h
> > F: util/fdmon-*.c
> > F: block/io.c
> > +F: block/plug.c
> > F: migration/block*
> > F: include/block/aio.h
> > F: include/block/aio-wait.h
> > diff --git a/include/sysemu/block-backend-io.h 
> > b/include/sysemu/block-backend-io.h
> > index d62a7ee773..be4dcef59d 100644
> > --- a/include/sysemu/block-backend-io.h
> > +++ b/include/sysemu/block-backend-io.h
> > @@ -100,16 +100,9 @@ void blk_iostatus_set_err(BlockBackend *blk, int 
> > error);
> > int blk_get_max_iov(BlockBackend *blk);
> > int blk_get_max_hw_iov(BlockBackend *blk);
> > 
> > -/*
> > - * blk_io_plug/unplug are thread-local operations. This means that multiple
> > - * IOThreads can simultaneously call plug/unplug, but the caller must 
> > ensure
> > - * that each unplug() is called in the same IOThread of the matching 
> > plug().
> > - */
> > -void coroutine_fn blk_co_io_plug(BlockBackend *blk);
> > -void co_wrapper blk_io_plug(BlockBackend *blk);
> > -
> > -void coroutine_fn blk_co_io_unplug(BlockBackend *blk);
> > -void co_wrapper blk_io_unplug(BlockBackend *blk);
> > +void blk_io_plug(void);
> > +void blk_io_unplug(void);
> > +void blk_io_plug_call(void (*fn)(void *), void *opaque);
> > 
> > AioContext *blk_get_aio_context(BlockBackend *blk);
> > BlockAcctStats *blk_get_stats(BlockBackend *blk);
> > diff --git a/block/block-backend.c b/block/block-backend.c
> > index ca537cd0ad..1f1d226ba6 100644
> > --- a/block/block-backend.c
> > +++ b/block/block-backend.c
> > @@ -2568,28 +2568,6 @@ void blk_add_insert_bs_notifier(BlockBackend *blk, 
> > Notifier *notify)
> > notifier_list_add(>insert_bs_notifiers, notify);
> > }
> > 
> > -void coroutine_fn blk_co_io_plug(BlockBackend *blk)
> > -{
> > -BlockDriverState *bs = blk_bs(blk);
> > -IO_CODE();
> > -GRAPH_RDLOCK_GUARD();
> > -
> > -if (bs) {
> > -bdrv_co_io_plug(bs);
> > -}
> > -}
> > -
> > -void coroutine_fn blk_co_io_unplug(BlockBackend *blk)
> > -{
> > -BlockDriverState *bs = blk_bs(blk);
> > -IO_CODE();
> > -GRAPH_RDLOCK_GUARD();
> > -
> > -if (bs) {
> > -bdrv_co_io_unplug(bs);
> > -}
> > -}
> > -
> > BlockAcctStats *blk_get_stats(BlockBackend *blk)
> > {
> > IO_CODE();
> > diff --git a/block/plug.c b/block/plug.c
> > new file mode 100644
> > index 00..6738a568ba
> > --- /dev/null
> > +++ b/block/plug.c
> > @@ -0,0 +1,159 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Block I/O plugging
> > + *
> > + * Copyright Red Hat.
> > + *
> > + * This API defers a function call within a blk_io_plug()/blk_io_unplug()
> > + * section, allowing multiple calls to batch up. This is a performance
> > + * optimization that is used in the block layer to submit several I/O 
> > requests
> > + * at once instead of individually:
> > + *
> > + *   blk_io_plug(); <-- start of plugged region
> > + *   ...
> > + *   blk_io_plug_call(my_func, my_obj); <-- deferred my_func(my_obj) call
> > + *   blk_io_plug_call(my_func, my_obj); <-- another
> > + *   blk_io_plug_call(my_func, my_obj); <-- another
> > + *   ...
> > + *   blk_io_unplug(); <-- end of plugged region, my_func(my_obj) is called 
> > once
> > + *
> > + * This code is actually generic and not tied to the 

[OSSTEST PATCH] ts-xen-build-prep: Install python3-venv for QEMU

2023-05-23 Thread Anthony PERARD
Since QEMU commit 81e2b198a8cb ("configure: create a python venv
unconditionally"), a python venv is always created and this new package
is needed on Debian.

Signed-off-by: Anthony PERARD 
---
 ts-xen-build-prep | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ts-xen-build-prep b/ts-xen-build-prep
index 3ae8f215..547bbc16 100755
--- a/ts-xen-build-prep
+++ b/ts-xen-build-prep
@@ -209,6 +209,7 @@ sub prep () {
   libdevmapper-dev libxml-xpath-perl libelf-dev
   ccache nasm checkpolicy ebtables
  python3-docutils python3-dev
+ python3-venv
   libtirpc-dev
   libgnutls28-dev);
 
-- 
Anthony PERARD




Re: [PATCH] xen/netback: Pass (void *) to virt_to_page()

2023-05-23 Thread Wei Liu
On Tue, May 23, 2023 at 04:03:42PM +0200, Linus Walleij wrote:
> virt_to_page() takes a virtual address as argument but
> the driver passes an unsigned long, which works because
> the target platform(s) uses polymorphic macros to calculate
> the page.
> 
> Since many architectures implement virt_to_pfn() as
> a macro, this function becomes polymorphic and accepts both a
> (unsigned long) and a (void *).
> 
> Fix this up by an explicit (void *) cast.
> 
> Cc: Wei Liu 
> Cc: Paul Durrant 
> Cc: xen-devel@lists.xenproject.org
> Cc: net...@vger.kernel.org
> Signed-off-by: Linus Walleij 

Acked-by: Wei Liu 



Re: [RFC] Xen crashes on ASSERT on suspend/resume, suggested fix

2023-05-23 Thread Roger Pau Monné
On Tue, May 23, 2023 at 03:54:36PM +0200, Roger Pau Monné wrote:
> On Thu, May 18, 2023 at 04:44:53PM -0700, Stefano Stabellini wrote:
> > Hi all,
> > 
> > After many PVH Dom0 suspend/resume cycles we are seeing the following
> > Xen crash (it is random and doesn't reproduce reliably):
> > 
> > (XEN) [555.042981][] R 
> > arch/x86/irq.c#_clear_irq_vector+0x214/0x2bd
> > (XEN) [555.042986][] F destroy_irq+0xe2/0x1b8
> > (XEN) [555.042991][] F msi_free_irq+0x5e/0x1a7
> > (XEN) [555.042995][] F unmap_domain_pirq+0x441/0x4b4
> > (XEN) [555.043001][] F 
> > arch/x86/hvm/vmsi.c#vpci_msi_disable+0xc0/0x155
> > (XEN) [555.043006][] F vpci_msi_arch_disable+0x1e/0x2b
> > (XEN) [555.043013][] F 
> > drivers/vpci/msi.c#control_write+0x109/0x10e
> > (XEN) [555.043018][] F vpci_write+0x11f/0x268
> > (XEN) [555.043024][] F 
> > arch/x86/hvm/io.c#vpci_portio_write+0xa0/0xa7
> > (XEN) [555.043029][] F 
> > hvm_process_io_intercept+0x203/0x26f
> > (XEN) [555.043034][] F hvm_io_intercept+0x2a/0x4c
> > (XEN) [555.043039][] F 
> > arch/x86/hvm/emulate.c#hvmemul_do_io+0x29b/0x5f6
> > (XEN) [555.043043][] F 
> > arch/x86/hvm/emulate.c#hvmemul_do_io_buffer+0x2f/0x6a
> > (XEN) [555.043048][] F hvmemul_do_pio_buffer+0x33/0x35
> > (XEN) [555.043053][] F handle_pio+0x6d/0x1b4
> > (XEN) [555.043059][] F svm_vmexit_handler+0x10bf/0x18b0
> > (XEN) [555.043064][] F svm_stgi_label+0x8/0x18
> > (XEN) [555.043067]
> > (XEN) [555.469861]
> > (XEN) [555.471855] 
> > (XEN) [555.477315] Panic on CPU 9:
> > (XEN) [555.480608] Assertion 'per_cpu(vector_irq, cpu)[old_vector] == irq' 
> > failed at arch/x86/irq.c:233
> > (XEN) [555.489882] 
> > 
> > Looking at the code in question, the ASSERT looks wrong to me.
> > 
> > Specifically, if you see send_cleanup_vector and
> > irq_move_cleanup_interrupt, it is entirely possible to have old_vector
> > still valid and also move_in_progress still set, but only some of the
> > per_cpu(vector_irq, me)[vector] cleared. It seems to me that this could
> > happen especially when an MSI has a large old_cpu_mask.
> 
> i guess the only way to get into such situation would be if you happen
> to execute _clear_irq_vector() with a cpu_online_map smaller than the
> one in old_cpu_mask, at which point you will leave old_vector fields
> not updated.
> 
> Maybe somehow you get into this situation when doing suspend/resume?
> 
> Could you try to add a:
> 
> ASSERT(cpumask_equal(tmp_mask, desc->arch.old_cpu_mask));
> 
> Before the `for_each_cpu(cpu, tmp_mask)` loop?

I see that the old_cpu_mask is cleared in release_old_vec(), so that
suggestion is not very useful.

Does the crash happen at specific points, for example just after
resume or before suspend?

Roger.



[PATCH] xen/netback: Pass (void *) to virt_to_page()

2023-05-23 Thread Linus Walleij
virt_to_page() takes a virtual address as argument but
the driver passes an unsigned long, which works because
the target platform(s) uses polymorphic macros to calculate
the page.

Since many architectures implement virt_to_pfn() as
a macro, this function becomes polymorphic and accepts both a
(unsigned long) and a (void *).

Fix this up by an explicit (void *) cast.

Cc: Wei Liu 
Cc: Paul Durrant 
Cc: xen-devel@lists.xenproject.org
Cc: net...@vger.kernel.org
Signed-off-by: Linus Walleij 
---
 drivers/net/xen-netback/netback.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/xen-netback/netback.c 
b/drivers/net/xen-netback/netback.c
index c1501f41e2d8..caf0c815436c 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -689,7 +689,7 @@ static void xenvif_fill_frags(struct xenvif_queue *queue, 
struct sk_buff *skb)
prev_pending_idx = pending_idx;
 
txp = >pending_tx_info[pending_idx].req;
-   page = virt_to_page(idx_to_kaddr(queue, pending_idx));
+   page = virt_to_page((void *)idx_to_kaddr(queue, pending_idx));
__skb_fill_page_desc(skb, i, page, txp->offset, txp->size);
skb->len += txp->size;
skb->data_len += txp->size;
-- 
2.34.1




Re: [RFC] Xen crashes on ASSERT on suspend/resume, suggested fix

2023-05-23 Thread Roger Pau Monné
On Thu, May 18, 2023 at 04:44:53PM -0700, Stefano Stabellini wrote:
> Hi all,
> 
> After many PVH Dom0 suspend/resume cycles we are seeing the following
> Xen crash (it is random and doesn't reproduce reliably):
> 
> (XEN) [555.042981][] R 
> arch/x86/irq.c#_clear_irq_vector+0x214/0x2bd
> (XEN) [555.042986][] F destroy_irq+0xe2/0x1b8
> (XEN) [555.042991][] F msi_free_irq+0x5e/0x1a7
> (XEN) [555.042995][] F unmap_domain_pirq+0x441/0x4b4
> (XEN) [555.043001][] F 
> arch/x86/hvm/vmsi.c#vpci_msi_disable+0xc0/0x155
> (XEN) [555.043006][] F vpci_msi_arch_disable+0x1e/0x2b
> (XEN) [555.043013][] F 
> drivers/vpci/msi.c#control_write+0x109/0x10e
> (XEN) [555.043018][] F vpci_write+0x11f/0x268
> (XEN) [555.043024][] F 
> arch/x86/hvm/io.c#vpci_portio_write+0xa0/0xa7
> (XEN) [555.043029][] F hvm_process_io_intercept+0x203/0x26f
> (XEN) [555.043034][] F hvm_io_intercept+0x2a/0x4c
> (XEN) [555.043039][] F 
> arch/x86/hvm/emulate.c#hvmemul_do_io+0x29b/0x5f6
> (XEN) [555.043043][] F 
> arch/x86/hvm/emulate.c#hvmemul_do_io_buffer+0x2f/0x6a
> (XEN) [555.043048][] F hvmemul_do_pio_buffer+0x33/0x35
> (XEN) [555.043053][] F handle_pio+0x6d/0x1b4
> (XEN) [555.043059][] F svm_vmexit_handler+0x10bf/0x18b0
> (XEN) [555.043064][] F svm_stgi_label+0x8/0x18
> (XEN) [555.043067]
> (XEN) [555.469861]
> (XEN) [555.471855] 
> (XEN) [555.477315] Panic on CPU 9:
> (XEN) [555.480608] Assertion 'per_cpu(vector_irq, cpu)[old_vector] == irq' 
> failed at arch/x86/irq.c:233
> (XEN) [555.489882] 
> 
> Looking at the code in question, the ASSERT looks wrong to me.
> 
> Specifically, if you see send_cleanup_vector and
> irq_move_cleanup_interrupt, it is entirely possible to have old_vector
> still valid and also move_in_progress still set, but only some of the
> per_cpu(vector_irq, me)[vector] cleared. It seems to me that this could
> happen especially when an MSI has a large old_cpu_mask.

i guess the only way to get into such situation would be if you happen
to execute _clear_irq_vector() with a cpu_online_map smaller than the
one in old_cpu_mask, at which point you will leave old_vector fields
not updated.

Maybe somehow you get into this situation when doing suspend/resume?

Could you try to add a:

ASSERT(cpumask_equal(tmp_mask, desc->arch.old_cpu_mask));

Before the `for_each_cpu(cpu, tmp_mask)` loop?

Thanks, Roger.



Re: [PATCH v2] maintainers: add regex matching for xsm

2023-05-23 Thread Julien Grall

Hi Daniel,

On 22/05/2023 20:14, Daniel P. Smith wrote:

XSM is a subsystem where it is equally important of how and where its hooks are
called as is the implementation of the hooks. The people best suited for
evaluating the how and where are the XSM maintainers and reviewers. This
creates a challenge as the hooks are used throughout the hypervisor for which
the XSM maintainers and reviewers are not, and should not be, a reviewer for
each of these subsystems in the MAINTAINERS file. Though the MAINTAINERS file
does support the use of regex matches, 'K' identifier, that are applied to both
the commit message and the commit delta. Adding the 'K' identifier will declare
that any patch relating to XSM require the input from the XSM maintainers and
reviewers. For those that use the get_maintianers script, the 'K' identifier
will automatically add the XSM maintainers and reviewers. Any one not using
get_maintainers, it will be their responsibility to ensure that if their work
touches and XSM hook, to ensure the XSM maintainers and reviewers are copied.

This patch adds a pair of regex expressions to the XSM section. The first is
`xsm_.*` which seeks to match XSM hooks in the commit's delta. The second is
`\b(xsm|XSM)\b` which seeks to match strictly the words xsm or XSM and should
not capture words with a substring of "xsm".

Signed-off-by: Daniel P. Smith 


Acked-by: Julien Grall 

Cheers,


---
  MAINTAINERS | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index f2f1881b32..b0f0823d21 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -674,6 +674,8 @@ F:  tools/flask/
  F:xen/include/xsm/
  F:xen/xsm/
  F:docs/misc/xsm-flask.txt
+K: xsm_.*
+K: \b(xsm|XSM)\b
  
  THE REST

  M:Andrew Cooper 


--
Julien Grall



Re: [PATCH v2] x86: do away with HAVE_AS_NEGATIVE_TRUE

2023-05-23 Thread Andrew Cooper
On 23/05/2023 1:37 pm, Jan Beulich wrote:
> There's no real need for the associated probing - we can easily convert
> to a uniform value without knowing the specific behavior (note also that
> the respective comments weren't fully correct and have gone stale).
>
> No difference in generated code.
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 

Thanks.  I do think this form is easier to follow.

~Andrew



Re: PVH Dom0 related UART failure

2023-05-23 Thread Jan Beulich
On 23.05.2023 12:59, Roger Pau Monné wrote:
> On Tue, May 23, 2023 at 08:44:48AM +0200, Jan Beulich wrote:
>> On 23.05.2023 00:20, Stefano Stabellini wrote:
>>> On Sat, 20 May 2023, Roger Pau Monné wrote:
 diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
 index ec2e978a4e6b..0ff8e940fa8d 100644
 --- a/xen/drivers/vpci/header.c
 +++ b/xen/drivers/vpci/header.c
 @@ -289,6 +289,13 @@ static int modify_bars(const struct pci_dev *pdev, 
 uint16_t cmd, bool rom_only)
   */
  for_each_pdev ( pdev->domain, tmp )
  {
 +if ( !tmp->vpci )
 +{
 +printk(XENLOG_G_WARNING "%pp: not handled by vPCI for %pd\n",
 +   >sbdf, pdev->domain);
 +continue;
 +}
 +
  if ( tmp == pdev )
  {
  /*
 diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
 index 652807a4a454..0baef3a8d3a1 100644
 --- a/xen/drivers/vpci/vpci.c
 +++ b/xen/drivers/vpci/vpci.c
 @@ -72,7 +72,12 @@ int vpci_add_handlers(struct pci_dev *pdev)
  unsigned int i;
  int rc = 0;
  
 -if ( !has_vpci(pdev->domain) )
 +if ( !has_vpci(pdev->domain) ||
 + /*
 +  * Ignore RO and hidden devices, those are in use by Xen and vPCI
 +  * won't work on them.
 +  */
 + pci_get_pdev(dom_xen, pdev->sbdf) )
  return 0;
  
  /* We should not get here twice for the same device. */
>>>
>>>
>>> Now this patch works! Thank you!! :-)
>>>
>>> You can check the full logs here
>>> https://gitlab.com/xen-project/people/sstabellini/xen/-/jobs/4329259080
>>>
>>> Is the patch ready to be upstreamed aside from the commit message?
>>
>> I don't think so. vPCI ought to work on "r/o" devices. Out of curiosity,
>> have you also tried my (hackish and hence RFC) patch [1]?
> 
> For r/o devices there should be no need of vPCI handlers, reading the
> config space of such devices can be done directly.
> 
> There's some work to be done for hidden devices, as for those dom0 has
> write access to the config space and thus needs vPCI to be setup
> properly.

But then isn't it going to complicate things when dealing with r/o and
hidden devices differently?

> The change to modify_bars() in order to handle devices without vpci
> populated is a bugfix, as it's already possible to have devices
> assigned to a domain that don't have vpci setup, if the call to
> vpci_add_handlers() in setup_one_hwdom_device() fails.  That one could
> go in separately of the rest of the work in order to enable support
> for hidden devices.

You saying "assigned to a domain" makes this sound more problematic
than it probably is: If it really was any domain other than Dom0, I
think there would be a security concern. Yet even for Dom0 I wonder
what good can come out of there not being proper vPCI setup for a
device.

Jan



Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature

2023-05-23 Thread Jan Beulich
On 23.05.2023 13:57, Luca Fancellu wrote:
>> On 23 May 2023, at 12:53, Jan Beulich  wrote:
>> On 23.05.2023 13:50, Luca Fancellu wrote:
 On 23 May 2023, at 11:31, Jan Beulich  wrote:
 On 23.05.2023 12:21, Luca Fancellu wrote:
>> On 23 May 2023, at 11:02, Jan Beulich  wrote:
>> On 23.05.2023 09:43, Luca Fancellu wrote:
>>> @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 
>>> systems.
>>>
>>>   If using this option is necessary to fix an issue, please report a 
>>> bug.
>>>
>>> +Enables features on dom0 on Arm systems.
>>> +
>>> +*   The `sve` integer parameter enables Arm SVE usage for Dom0 domain 
>>> and sets
>>> +the maximum SVE vector length, the option is applicable only to 
>>> AArch64
>>> +guests.
>>
>> Why "guests"? Does the option affect more than Dom0?
>
> I used “guests” because in my mind I was referring to all the aarch64 OS 
> that can be used
> as control domain, I can change it if it sounds bad.

 If you means OSes then better also say OSes. But maybe this doesn't need
 specifically expressing, by saying e.g. "..., the option is applicable
 only on AArch64"? Or can a Dom0 be 32-bit on Arm64 Xen?
>>>
>>> I think there is no limitation so Dom0 can be 32 bit or 64. Maybe I can say
>>> “... AArch64 kernel guests.”?
>>
>> I'd recommend to avoid the term "guest" when you talk about Dom0 alone.
>> Commonly "guest" means ordinary domains only, i.e. in particular excluding
>> Dom0. What's wrong with "AArch64 Dom0 kernels"?
> 
> Ok works for me, I will use “AArch64 Dom0 kernels", I thought “guests” were a 
> generic category
> and then we have “privileged  guests”, for example Dom0 or driver domain, and 
> “unprivileged guests”
> like DomUs.

Well, yes - "commonly" doesn't mean "always".

>>> +A value equal to 0 disables the feature, this is the default value.
>>> +Values below 0 means the feature uses the maximum SVE vector length
>>> +supported by hardware, if SVE is supported.
>>> +Values above 0 explicitly set the maximum SVE vector length for 
>>> Dom0,
>>> +allowed values are from 128 to maximum 2048, being multiple of 128.
>>> +Please note that when the user explicitly specifies the value, if 
>>> that value
>>> +is above the hardware supported maximum SVE vector length, the 
>>> domain
>>> +creation will fail and the system will stop, the same will occur 
>>> if the
>>> +option is provided with a non zero value, but the platform doesn't 
>>> support
>>> +SVE.
>>
>> Assuming this also covers the -1 case, I wonder if that isn't a little 
>> too
>> strict. "Maximum supported" imo can very well be 0.
>
> Maximum supported, when platforms uses SVE, can be at minimum 128 by arm 
> specs.

 When there is SVE - sure. But when there's no SVE, 0 is kind of the implied
 length. And I'd view a command line option value of -1 quite okay in that
 case: They've asked for the maximum supported, so they'll get 0. No reason
 to crash the system during boot.
>>>
>>> Ok I see what you mean, for example when Kconfig SVE is enabled, but the 
>>> platform doesn’t
>>> have SVE feature, requesting sve=-1 will keep the value to 0, and no system 
>>> will be stopped.
>>>
>>> Maybe I can say: 
>>>
>>> “... the same will occur if the option is provided with a positive non zero 
>>> value,
>>> but the platform doesn't support SVE."
>>
>> Right, provided that matches the implementation.
> 
> Ok I will do the changes, can I retain your R-by? I suppose it covers also 
> documentation right?

I guess whether doc is covered is fuzzy. Since the doc part is Arm-
specific, I'd probably consider it not covered with the "!arm" that
I appended. But whichever way you look at it, you can keep the tag
in place.

Jan



[PATCH v2] x86: do away with HAVE_AS_NEGATIVE_TRUE

2023-05-23 Thread Jan Beulich
There's no real need for the associated probing - we can easily convert
to a uniform value without knowing the specific behavior (note also that
the respective comments weren't fully correct and have gone stale).

No difference in generated code.

Signed-off-by: Jan Beulich 
---
v2: Use "& 1".

--- a/xen/arch/x86/arch.mk
+++ b/xen/arch/x86/arch.mk
@@ -26,10 +26,6 @@ $(call as-option-add,CFLAGS,CC,"invpcid
 $(call as-option-add,CFLAGS,CC,"movdiri %rax$(comma)(%rax)",-DHAVE_AS_MOVDIR)
 $(call as-option-add,CFLAGS,CC,"enqcmd (%rax)$(comma)%rax",-DHAVE_AS_ENQCMD)
 
-# GAS's idea of true is -1.  Clang's idea is 1
-$(call as-option-add,CFLAGS,CC,\
-".if ((1 > 0) < 0); .error \"\";.endif",,-DHAVE_AS_NEGATIVE_TRUE)
-
 # Check to see whether the assmbler supports the .nop directive.
 $(call as-option-add,CFLAGS,CC,\
 ".L1: .L2: .nops (.L2 - .L1)$(comma)9",-DHAVE_AS_NOPS_DIRECTIVE)
--- a/xen/arch/x86/include/asm/alternative.h
+++ b/xen/arch/x86/include/asm/alternative.h
@@ -35,19 +35,19 @@ extern void alternative_branches(void);
 #define alt_repl_e(num)".LXEN%=_repl_e"#num
 #define alt_repl_len(num)  "(" alt_repl_e(num) " - " alt_repl_s(num) ")"
 
-/* GAS's idea of true is -1, while Clang's idea is 1. */
-#ifdef HAVE_AS_NEGATIVE_TRUE
-# define AS_TRUE "-"
-#else
-# define AS_TRUE ""
-#endif
+/*
+ * GAS's idea of true is sometimes 1 and sometimes -1, while Clang's idea
+ * was consistently 1 up to 6.x (it matches GAS's now).  Transform it to
+ * uniformly 1.
+ */
+#define AS_TRUE(x) "((" x ") & 1)"
 
-#define as_max(a, b) "(("a") ^ ((("a") ^ ("b")) & -("AS_TRUE"(("a") < 
("b")"
+#define as_max(a, b) "(("a") ^ ((("a") ^ ("b")) & -("AS_TRUE("("a") < 
("b")")")))"
 
 #define OLDINSTR(oldinstr, padding)  \
 ".LXEN%=_orig_s:\n\t" oldinstr "\n .LXEN%=_orig_e:\n\t"  \
 ".LXEN%=_diff = " padding "\n\t" \
-"mknops ("AS_TRUE"(.LXEN%=_diff > 0) * .LXEN%=_diff)\n\t"\
+"mknops ("AS_TRUE(".LXEN%=_diff > 0")" * .LXEN%=_diff)\n\t"  \
 ".LXEN%=_orig_p:\n\t"
 
 #define OLDINSTR_1(oldinstr, n1) \
--- a/xen/arch/x86/include/asm/alternative-asm.h
+++ b/xen/arch/x86/include/asm/alternative-asm.h
@@ -29,12 +29,12 @@
 #endif
 .endm
 
-/* GAS's idea of true is -1, while Clang's idea is 1. */
-#ifdef HAVE_AS_NEGATIVE_TRUE
-# define as_true(x) (-(x))
-#else
-# define as_true(x) (x)
-#endif
+/*
+ * GAS's idea of true is sometimes 1 and sometimes -1, while Clang's idea
+ * was consistently 1 up to 6.x (it matches GAS's now).  Transform it to
+ * uniformly 1.
+ */
+#define as_true(x) ((x) & 1)
 
 #define decl_orig(insn, padding)  \
  .L\@_orig_s: insn; .L\@_orig_e:  \



Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature

2023-05-23 Thread Luca Fancellu


> On 23 May 2023, at 12:53, Jan Beulich  wrote:
> 
> On 23.05.2023 13:50, Luca Fancellu wrote:
>>> On 23 May 2023, at 11:31, Jan Beulich  wrote:
>>> On 23.05.2023 12:21, Luca Fancellu wrote:
> On 23 May 2023, at 11:02, Jan Beulich  wrote:
> On 23.05.2023 09:43, Luca Fancellu wrote:
>> @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems.
>> 
>>   If using this option is necessary to fix an issue, please report a bug.
>> 
>> +Enables features on dom0 on Arm systems.
>> +
>> +*   The `sve` integer parameter enables Arm SVE usage for Dom0 domain 
>> and sets
>> +the maximum SVE vector length, the option is applicable only to 
>> AArch64
>> +guests.
> 
> Why "guests"? Does the option affect more than Dom0?
 
 I used “guests” because in my mind I was referring to all the aarch64 OS 
 that can be used
 as control domain, I can change it if it sounds bad.
>>> 
>>> If you means OSes then better also say OSes. But maybe this doesn't need
>>> specifically expressing, by saying e.g. "..., the option is applicable
>>> only on AArch64"? Or can a Dom0 be 32-bit on Arm64 Xen?
>> 
>> I think there is no limitation so Dom0 can be 32 bit or 64. Maybe I can say
>> “... AArch64 kernel guests.”?
> 
> I'd recommend to avoid the term "guest" when you talk about Dom0 alone.
> Commonly "guest" means ordinary domains only, i.e. in particular excluding
> Dom0. What's wrong with "AArch64 Dom0 kernels"?

Ok works for me, I will use “AArch64 Dom0 kernels", I thought “guests” were a 
generic category
and then we have “privilegedguests”, for example Dom0 or driver domain, and 
“unprivileged guests”
like DomUs.

> 
>> +A value equal to 0 disables the feature, this is the default value.
>> +Values below 0 means the feature uses the maximum SVE vector length
>> +supported by hardware, if SVE is supported.
>> +Values above 0 explicitly set the maximum SVE vector length for 
>> Dom0,
>> +allowed values are from 128 to maximum 2048, being multiple of 128.
>> +Please note that when the user explicitly specifies the value, if 
>> that value
>> +is above the hardware supported maximum SVE vector length, the 
>> domain
>> +creation will fail and the system will stop, the same will occur if 
>> the
>> +option is provided with a non zero value, but the platform doesn't 
>> support
>> +SVE.
> 
> Assuming this also covers the -1 case, I wonder if that isn't a little too
> strict. "Maximum supported" imo can very well be 0.
 
 Maximum supported, when platforms uses SVE, can be at minimum 128 by arm 
 specs.
>>> 
>>> When there is SVE - sure. But when there's no SVE, 0 is kind of the implied
>>> length. And I'd view a command line option value of -1 quite okay in that
>>> case: They've asked for the maximum supported, so they'll get 0. No reason
>>> to crash the system during boot.
>> 
>> Ok I see what you mean, for example when Kconfig SVE is enabled, but the 
>> platform doesn’t
>> have SVE feature, requesting sve=-1 will keep the value to 0, and no system 
>> will be stopped.
>> 
>> Maybe I can say: 
>> 
>> “... the same will occur if the option is provided with a positive non zero 
>> value,
>> but the platform doesn't support SVE."
> 
> Right, provided that matches the implementation.

Ok I will do the changes, can I retain your R-by? I suppose it covers also 
documentation right?

> 
> Jan




Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature

2023-05-23 Thread Jan Beulich
On 23.05.2023 13:50, Luca Fancellu wrote:
>> On 23 May 2023, at 11:31, Jan Beulich  wrote:
>> On 23.05.2023 12:21, Luca Fancellu wrote:
 On 23 May 2023, at 11:02, Jan Beulich  wrote:
 On 23.05.2023 09:43, Luca Fancellu wrote:
> @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems.
>
>If using this option is necessary to fix an issue, please report a bug.
>
> +Enables features on dom0 on Arm systems.
> +
> +*   The `sve` integer parameter enables Arm SVE usage for Dom0 domain 
> and sets
> +the maximum SVE vector length, the option is applicable only to 
> AArch64
> +guests.

 Why "guests"? Does the option affect more than Dom0?
>>>
>>> I used “guests” because in my mind I was referring to all the aarch64 OS 
>>> that can be used
>>> as control domain, I can change it if it sounds bad.
>>
>> If you means OSes then better also say OSes. But maybe this doesn't need
>> specifically expressing, by saying e.g. "..., the option is applicable
>> only on AArch64"? Or can a Dom0 be 32-bit on Arm64 Xen?
> 
> I think there is no limitation so Dom0 can be 32 bit or 64. Maybe I can say
> “... AArch64 kernel guests.”?

I'd recommend to avoid the term "guest" when you talk about Dom0 alone.
Commonly "guest" means ordinary domains only, i.e. in particular excluding
Dom0. What's wrong with "AArch64 Dom0 kernels"?

> +A value equal to 0 disables the feature, this is the default value.
> +Values below 0 means the feature uses the maximum SVE vector length
> +supported by hardware, if SVE is supported.
> +Values above 0 explicitly set the maximum SVE vector length for Dom0,
> +allowed values are from 128 to maximum 2048, being multiple of 128.
> +Please note that when the user explicitly specifies the value, if 
> that value
> +is above the hardware supported maximum SVE vector length, the domain
> +creation will fail and the system will stop, the same will occur if 
> the
> +option is provided with a non zero value, but the platform doesn't 
> support
> +SVE.

 Assuming this also covers the -1 case, I wonder if that isn't a little too
 strict. "Maximum supported" imo can very well be 0.
>>>
>>> Maximum supported, when platforms uses SVE, can be at minimum 128 by arm 
>>> specs.
>>
>> When there is SVE - sure. But when there's no SVE, 0 is kind of the implied
>> length. And I'd view a command line option value of -1 quite okay in that
>> case: They've asked for the maximum supported, so they'll get 0. No reason
>> to crash the system during boot.
> 
> Ok I see what you mean, for example when Kconfig SVE is enabled, but the 
> platform doesn’t
> have SVE feature, requesting sve=-1 will keep the value to 0, and no system 
> will be stopped.
> 
> Maybe I can say: 
> 
> “... the same will occur if the option is provided with a positive non zero 
> value,
> but the platform doesn't support SVE."

Right, provided that matches the implementation.

Jan



Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature

2023-05-23 Thread Luca Fancellu


> On 23 May 2023, at 11:31, Jan Beulich  wrote:
> 
> On 23.05.2023 12:21, Luca Fancellu wrote:
>>> On 23 May 2023, at 11:02, Jan Beulich  wrote:
>>> On 23.05.2023 09:43, Luca Fancellu wrote:
 @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems.
 
If using this option is necessary to fix an issue, please report a bug.
 
 +Enables features on dom0 on Arm systems.
 +
 +*   The `sve` integer parameter enables Arm SVE usage for Dom0 domain and 
 sets
 +the maximum SVE vector length, the option is applicable only to 
 AArch64
 +guests.
>>> 
>>> Why "guests"? Does the option affect more than Dom0?
>> 
>> I used “guests” because in my mind I was referring to all the aarch64 OS 
>> that can be used
>> as control domain, I can change it if it sounds bad.
> 
> If you means OSes then better also say OSes. But maybe this doesn't need
> specifically expressing, by saying e.g. "..., the option is applicable
> only on AArch64"? Or can a Dom0 be 32-bit on Arm64 Xen?

I think there is no limitation so Dom0 can be 32 bit or 64. Maybe I can say
“... AArch64 kernel guests.”?

> 
 +A value equal to 0 disables the feature, this is the default value.
 +Values below 0 means the feature uses the maximum SVE vector length
 +supported by hardware, if SVE is supported.
 +Values above 0 explicitly set the maximum SVE vector length for Dom0,
 +allowed values are from 128 to maximum 2048, being multiple of 128.
 +Please note that when the user explicitly specifies the value, if 
 that value
 +is above the hardware supported maximum SVE vector length, the domain
 +creation will fail and the system will stop, the same will occur if 
 the
 +option is provided with a non zero value, but the platform doesn't 
 support
 +SVE.
>>> 
>>> Assuming this also covers the -1 case, I wonder if that isn't a little too
>>> strict. "Maximum supported" imo can very well be 0.
>> 
>> Maximum supported, when platforms uses SVE, can be at minimum 128 by arm 
>> specs.
> 
> When there is SVE - sure. But when there's no SVE, 0 is kind of the implied
> length. And I'd view a command line option value of -1 quite okay in that
> case: They've asked for the maximum supported, so they'll get 0. No reason
> to crash the system during boot.

Ok I see what you mean, for example when Kconfig SVE is enabled, but the 
platform doesn’t
have SVE feature, requesting sve=-1 will keep the value to 0, and no system 
will be stopped.

Maybe I can say: 

“... the same will occur if the option is provided with a positive non zero 
value,
but the platform doesn't support SVE."



> 
> Jan




[PATCH v2 2/2] x86: also mark assembler globals hidden

2023-05-23 Thread Jan Beulich
Let's have assembler symbols be consistent with C ones. In principle
there are (a few) cases where gas can produce smaller code this way,
just that for now there's a gas bug causing smaller code to be emitted
even when that shouldn't be the case.

Signed-off-by: Jan Beulich 
---
v2: New.

--- a/xen/arch/x86/include/asm/asm_defns.h
+++ b/xen/arch/x86/include/asm/asm_defns.h
@@ -83,7 +83,7 @@ register unsigned long current_stack_poi
 
 #define SYM_ALIGN(algn...) .balign algn
 
-#define SYM_L_GLOBAL(name) .globl name
+#define SYM_L_GLOBAL(name) .globl name; .hidden name
 #define SYM_L_WEAK(name)   .weak name
 #define SYM_L_LOCAL(name)  /* nothing */
 
--- a/xen/arch/x86/include/asm/config.h
+++ b/xen/arch/x86/include/asm/config.h
@@ -45,11 +45,11 @@
 #ifdef __ASSEMBLY__
 #define ALIGN .align 16,0x90
 #define ENTRY(name) \
-  .globl name;  \
   ALIGN;\
-  name:
+  GLOBAL(name)
 #define GLOBAL(name)\
   .globl name;  \
+  .hidden name; \
   name:
 #endif
 




[PATCH v2 1/2] x86: annotate entry points with type and size

2023-05-23 Thread Jan Beulich
Recent gas versions generate minimalistic Dwarf debug info for items
annotated as functions and having their sizes specified [1]. "Borrow"
Arm's END() and (remotely) derive other annotation infrastructure from
Linux'es.

For switch_to_kernel() and restore_all_guest() so far implicit alignment
(from being first in their respective sections) is being made explicit
(as in: using FUNC() without 2nd argument). Whereas for
{,compat}create_bounce_frame() and autogen_entrypoints[] alignment is
newly arranged for.

Except for the added alignment padding (including their knock-on
effects) no change in generated code/data.

Signed-off-by: Jan Beulich 

[1] 
https://sourceware.org/git?p=binutils-gdb.git;a=commitdiff;h=591cc9fbbfd6d51131c0f1d4a92e7893edcc7a28
---
v2: Full rework.
---
Only two of the assembly files are being converted for now. More could
be done right here or as follow-on in separate patches.

In principle the framework should be possible to use by other
architectures as well. If we want this, the main questions are going to
be:
- What header file name? (I don't really like Linux'es linkage.h, so I'd
  prefer e.g. asm-defns.h or asm_defns.h as we already have in x86.)
- How much per-arch customization do we want to permit up front (i.e.
  without knowing how much of it is going to be needed)? Initially I'd
  expect only the default function alignment (and padding) to require
  per-arch definitions.

Note that the FB-label in autogen_stubs() cannot be converted just yet:
Such labels cannot be used with .type. We could further diverge from
Linux'es model and avoid setting STT_NOTYPE explicitly (that's the type
labels get by default anyway).

Note that we can't use ALIGN() (in place of SYM_ALIGN()) as long as we
still have ALIGN.

--- a/xen/arch/x86/include/asm/asm_defns.h
+++ b/xen/arch/x86/include/asm/asm_defns.h
@@ -81,6 +81,45 @@ register unsigned long current_stack_poi
 
 #ifdef __ASSEMBLY__
 
+#define SYM_ALIGN(algn...) .balign algn
+
+#define SYM_L_GLOBAL(name) .globl name
+#define SYM_L_WEAK(name)   .weak name
+#define SYM_L_LOCAL(name)  /* nothing */
+
+#define SYM_T_FUNC STT_FUNC
+#define SYM_T_DATA STT_OBJECT
+#define SYM_T_NONE STT_NOTYPE
+
+#define SYM(name, typ, linkage, algn...)  \
+.type name, SYM_T_ ## typ;\
+SYM_L_ ## linkage(name);  \
+SYM_ALIGN(algn);  \
+name:
+
+#define END(name) .size name, . - name
+
+#define ARG1_(x, y...) (x)
+#define ARG2_(x, y...) ARG1_(y)
+
+#define LAST__(nr) ARG ## nr ## _
+#define LAST_(nr)  LAST__(nr)
+#define LAST(x, y...) LAST_(count_args(x, ## y))(x, ## y)
+
+#define FUNC(name, algn...) \
+SYM(name, FUNC, GLOBAL, LAST(16, ## algn), 0x90)
+#define LABEL(name, algn...) \
+SYM(name, NONE, GLOBAL, LAST(16, ## algn), 0x90)
+#define DATA(name, algn...) \
+SYM(name, DATA, GLOBAL, LAST(0, ## algn), 0xff)
+
+#define FUNC_LOCAL(name, algn...) \
+SYM(name, FUNC, LOCAL, LAST(16, ## algn), 0x90)
+#define LABEL_LOCAL(name, algn...) \
+SYM(name, NONE, LOCAL, LAST(16, ## algn), 0x90)
+#define DATA_LOCAL(name, algn...) \
+SYM(name, DATA, LOCAL, LAST(0, ## algn), 0xff)
+
 #ifdef HAVE_AS_QUOTED_SYM
 #define SUBSECTION_LBL(tag)\
 .ifndef .L.tag;\
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -8,10 +8,11 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
-ENTRY(entry_int82)
+FUNC(entry_int82)
 ENDBR64
 ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP
 pushq $0
@@ -27,9 +28,10 @@ ENTRY(entry_int82)
 
 mov   %rsp, %rdi
 call  do_entry_int82
+END(entry_int82)
 
 /* %rbx: struct vcpu */
-ENTRY(compat_test_all_events)
+FUNC(compat_test_all_events)
 ASSERT_NOT_IN_ATOMIC
 cli # tests must not race interrupts
 /*compat_test_softirqs:*/
@@ -66,24 +68,21 @@ compat_test_guest_events:
 call  compat_create_bounce_frame
 jmp   compat_test_all_events
 
-ALIGN
 /* %rbx: struct vcpu */
-compat_process_softirqs:
+LABEL_LOCAL(compat_process_softirqs)
 sti
 call  do_softirq
 jmp   compat_test_all_events
 
-ALIGN
 /* %rbx: struct vcpu, %rdx: struct trap_bounce */
-.Lcompat_process_trapbounce:
+LABEL_LOCAL(.Lcompat_process_trapbounce)
 sti
 .Lcompat_bounce_exception:
 call  compat_create_bounce_frame
 jmp   compat_test_all_events
 
-   ALIGN
 /* %rbx: struct vcpu */
-compat_process_mce:
+LABEL_LOCAL(compat_process_mce)
 testb $1 << VCPU_TRAP_MCE,VCPU_async_exception_mask(%rbx)
 jnz   .Lcompat_test_guest_nmi
 sti
@@ -97,9 +96,8 @@ compat_process_mce:
 movb %dl,VCPU_async_exception_mask(%rbx)
 jmp   compat_process_trap
 
-   ALIGN
 /* %rbx: struct vcpu */
-compat_process_nmi:
+LABEL_LOCAL(compat_process_nmi)
  

[PATCH v2 0/2] x86: annotate entry points with type and size

2023-05-23 Thread Jan Beulich
The model introduced in patch 1 is in principle arch-agnostic, hence
why I'm including Arm and RISC-V reviewers here as well.

1: annotate entry points with type and size
2: also mark assembler globals hidden

Jan



Re: PVH Dom0 related UART failure

2023-05-23 Thread Roger Pau Monné
On Tue, May 23, 2023 at 08:44:48AM +0200, Jan Beulich wrote:
> On 23.05.2023 00:20, Stefano Stabellini wrote:
> > On Sat, 20 May 2023, Roger Pau Monné wrote:
> >> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> >> index ec2e978a4e6b..0ff8e940fa8d 100644
> >> --- a/xen/drivers/vpci/header.c
> >> +++ b/xen/drivers/vpci/header.c
> >> @@ -289,6 +289,13 @@ static int modify_bars(const struct pci_dev *pdev, 
> >> uint16_t cmd, bool rom_only)
> >>   */
> >>  for_each_pdev ( pdev->domain, tmp )
> >>  {
> >> +if ( !tmp->vpci )
> >> +{
> >> +printk(XENLOG_G_WARNING "%pp: not handled by vPCI for %pd\n",
> >> +   >sbdf, pdev->domain);
> >> +continue;
> >> +}
> >> +
> >>  if ( tmp == pdev )
> >>  {
> >>  /*
> >> diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
> >> index 652807a4a454..0baef3a8d3a1 100644
> >> --- a/xen/drivers/vpci/vpci.c
> >> +++ b/xen/drivers/vpci/vpci.c
> >> @@ -72,7 +72,12 @@ int vpci_add_handlers(struct pci_dev *pdev)
> >>  unsigned int i;
> >>  int rc = 0;
> >>  
> >> -if ( !has_vpci(pdev->domain) )
> >> +if ( !has_vpci(pdev->domain) ||
> >> + /*
> >> +  * Ignore RO and hidden devices, those are in use by Xen and vPCI
> >> +  * won't work on them.
> >> +  */
> >> + pci_get_pdev(dom_xen, pdev->sbdf) )
> >>  return 0;
> >>  
> >>  /* We should not get here twice for the same device. */
> > 
> > 
> > Now this patch works! Thank you!! :-)
> > 
> > You can check the full logs here
> > https://gitlab.com/xen-project/people/sstabellini/xen/-/jobs/4329259080
> > 
> > Is the patch ready to be upstreamed aside from the commit message?
> 
> I don't think so. vPCI ought to work on "r/o" devices. Out of curiosity,
> have you also tried my (hackish and hence RFC) patch [1]?

For r/o devices there should be no need of vPCI handlers, reading the
config space of such devices can be done directly.

There's some work to be done for hidden devices, as for those dom0 has
write access to the config space and thus needs vPCI to be setup
properly.

The change to modify_bars() in order to handle devices without vpci
populated is a bugfix, as it's already possible to have devices
assigned to a domain that don't have vpci setup, if the call to
vpci_add_handlers() in setup_one_hwdom_device() fails.  That one could
go in separately of the rest of the work in order to enable support
for hidden devices.

Roger.



[linux-linus test] 180907: regressions - FAIL

2023-05-23 Thread osstest service owner
flight 180907 linux-linus real [real]
flight 180915 linux-linus real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/180907/
http://logs.test-lab.xenproject.org/osstest/logs/180915/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-credit1   8 xen-boot fail REGR. vs. 180278

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt  8 xen-boot fail  like 180278
 test-armhf-armhf-xl-arndale   8 xen-boot fail  like 180278
 test-armhf-armhf-libvirt-qcow2  8 xen-bootfail like 180278
 test-armhf-armhf-libvirt-raw  8 xen-boot fail  like 180278
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180278
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180278
 test-armhf-armhf-xl-vhd   8 xen-boot fail  like 180278
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180278
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180278
 test-armhf-armhf-examine  8 reboot   fail  like 180278
 test-armhf-armhf-xl-multivcpu  8 xen-boot fail like 180278
 test-armhf-armhf-xl-credit2   8 xen-boot fail  like 180278
 test-armhf-armhf-xl-rtds  8 xen-boot fail  like 180278
 test-armhf-armhf-xl   8 xen-boot fail  like 180278
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180278
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 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-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-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  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-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-amd64-amd64-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-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass

version targeted for testing:
 linuxae8373a5add4ea39f032563cf12a02946d1e3546
baseline version:
 linux6c538e1adbfc696ac4747fb10d63e704344f763d

Last test of basis   180278  2023-04-16 19:41:46 Z   36 days
Failing since180281  2023-04-17 06:24:36 Z   36 days   67 attempts
Testing same since   180907  2023-05-23 01:11:53 Z0 days1 attempts


2480 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl  pass
 

RE: [PATCH v4 09/17] xen/arm: introduce a helper to parse device tree NUMA distance map

2023-05-23 Thread Henry Wang
Hi Jan,

> -Original Message-
> On 23.05.2023 09:32, Henry Wang wrote:
> > By looking into the existing implementation of NUMA for DT,
> > in Linux, from drivers/base/arch_numa.c: numa_set_distance(), there is a
> > "if ((u8)distance != distance)" then return. So I think at least in Linux 
> > the
> > distance value is consistent with the ACPI spec.
> 
> Can we simply gain a similar check (suitably commented), eliminating the
> need for saturation?

Yes, I will use the similar check and add the related comments in v5. Thanks
for your confirmation!

Kind regards,
Henry

> 
> Jan


Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature

2023-05-23 Thread Jan Beulich
On 23.05.2023 12:21, Luca Fancellu wrote:
>> On 23 May 2023, at 11:02, Jan Beulich  wrote:
>> On 23.05.2023 09:43, Luca Fancellu wrote:
>>> @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems.
>>>
>>> If using this option is necessary to fix an issue, please report a bug.
>>>
>>> +Enables features on dom0 on Arm systems.
>>> +
>>> +*   The `sve` integer parameter enables Arm SVE usage for Dom0 domain and 
>>> sets
>>> +the maximum SVE vector length, the option is applicable only to AArch64
>>> +guests.
>>
>> Why "guests"? Does the option affect more than Dom0?
> 
> I used “guests” because in my mind I was referring to all the aarch64 OS that 
> can be used
> as control domain, I can change it if it sounds bad.

If you means OSes then better also say OSes. But maybe this doesn't need
specifically expressing, by saying e.g. "..., the option is applicable
only on AArch64"? Or can a Dom0 be 32-bit on Arm64 Xen?

>>> +A value equal to 0 disables the feature, this is the default value.
>>> +Values below 0 means the feature uses the maximum SVE vector length
>>> +supported by hardware, if SVE is supported.
>>> +Values above 0 explicitly set the maximum SVE vector length for Dom0,
>>> +allowed values are from 128 to maximum 2048, being multiple of 128.
>>> +Please note that when the user explicitly specifies the value, if that 
>>> value
>>> +is above the hardware supported maximum SVE vector length, the domain
>>> +creation will fail and the system will stop, the same will occur if the
>>> +option is provided with a non zero value, but the platform doesn't 
>>> support
>>> +SVE.
>>
>> Assuming this also covers the -1 case, I wonder if that isn't a little too
>> strict. "Maximum supported" imo can very well be 0.
> 
> Maximum supported, when platforms uses SVE, can be at minimum 128 by arm 
> specs.

When there is SVE - sure. But when there's no SVE, 0 is kind of the implied
length. And I'd view a command line option value of -1 quite okay in that
case: They've asked for the maximum supported, so they'll get 0. No reason
to crash the system during boot.

Jan



Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature

2023-05-23 Thread Luca Fancellu


> On 23 May 2023, at 11:02, Jan Beulich  wrote:
> 
> On 23.05.2023 09:43, Luca Fancellu wrote:
>> Add a command line parameter to allow Dom0 the use of SVE resources,
>> the command line parameter sve=, sub argument of dom0=,
>> controls the feature on this domain and sets the maximum SVE vector
>> length for Dom0.
>> 
>> Add a new function, parse_signed_integer(), to parse an integer
>> command line argument.
>> 
>> Signed-off-by: Luca Fancellu 
> 
> Reviewed-by: Jan Beulich  # !arm
> 
>> --- a/docs/misc/xen-command-line.pandoc
>> +++ b/docs/misc/xen-command-line.pandoc
>> @@ -777,9 +777,9 @@ Specify the bit width of the DMA heap.
>> 
>> ### dom0
>> = List of [ pv | pvh, shadow=, verbose=,
>> -cpuid-faulting=, msr-relaxed= ]
>> +cpuid-faulting=, msr-relaxed= ] (x86)
>> 
>> -Applicability: x86
>> += List of [ sve= ] (Arm)
> 
> While in the text below you mention this is Arm64 only, I think the tag
> here would better express this as well.

Ok I can use Arm64 instead if there is no opposition from others

> 
>> @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems.
>> 
>> If using this option is necessary to fix an issue, please report a bug.
>> 
>> +Enables features on dom0 on Arm systems.
>> +
>> +*   The `sve` integer parameter enables Arm SVE usage for Dom0 domain and 
>> sets
>> +the maximum SVE vector length, the option is applicable only to AArch64
>> +guests.
> 
> Why "guests"? Does the option affect more than Dom0?

I used “guests” because in my mind I was referring to all the aarch64 OS that 
can be used
as control domain, I can change it if it sounds bad.

> 
>> +A value equal to 0 disables the feature, this is the default value.
>> +Values below 0 means the feature uses the maximum SVE vector length
>> +supported by hardware, if SVE is supported.
>> +Values above 0 explicitly set the maximum SVE vector length for Dom0,
>> +allowed values are from 128 to maximum 2048, being multiple of 128.
>> +Please note that when the user explicitly specifies the value, if that 
>> value
>> +is above the hardware supported maximum SVE vector length, the domain
>> +creation will fail and the system will stop, the same will occur if the
>> +option is provided with a non zero value, but the platform doesn't 
>> support
>> +SVE.
> 
> Assuming this also covers the -1 case, I wonder if that isn't a little too
> strict. "Maximum supported" imo can very well be 0.

Maximum supported, when platforms uses SVE, can be at minimum 128 by arm specs.



> 
> Jan



Re: [patch V4 33/37] cpu/hotplug: Allow "parallel" bringup up to CPUHP_BP_KICK_AP_STATE

2023-05-23 Thread Mark Brown
On Tue, May 23, 2023 at 01:12:26AM +0200, Thomas Gleixner wrote:

> Let me find a brown paperbag and go to sleep before I even try to
> compile the obvious fix.

That fixes the problem on TX2 - thanks!

Tested-by: Mark Brown 


signature.asc
Description: PGP signature


[qemu-mainline test] 180912: regressions - FAIL

2023-05-23 Thread osstest service owner
flight 180912 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/180912/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   6 xen-buildfail REGR. vs. 180691
 build-amd64-xsm   6 xen-buildfail REGR. vs. 180691
 build-i386-xsm6 xen-buildfail REGR. vs. 180691
 build-i3866 xen-buildfail REGR. vs. 180691
 build-arm64   6 xen-buildfail REGR. vs. 180691
 build-arm64-xsm   6 xen-buildfail REGR. vs. 180691
 build-armhf   6 xen-buildfail REGR. vs. 180691

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-ws16-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-shadow 1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-vhd1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-raw  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit1   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-thunderx  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-vhd   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-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
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-raw   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-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-coresched-i386-xl  1 build-check(1)   blocked  n/a
 test-amd64-coresched-amd64-xl  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-xsm   1 build-check(1)   blocked  n/a
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-shadow1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-rtds  1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ws16-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 build-armhf-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm  1 build-check(1) blocked n/a
 test-amd64-amd64-dom0pvh-xl-amd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1) blocked n/a
 test-amd64-amd64-dom0pvh-xl-intel  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked  n/a
 

Re: [PATCH v4 09/17] xen/arm: introduce a helper to parse device tree NUMA distance map

2023-05-23 Thread Jan Beulich
On 23.05.2023 09:32, Henry Wang wrote:
>> -Original Message-
>> Subject: Re: [PATCH v4 09/17] xen/arm: introduce a helper to parse device
>> tree NUMA distance map
>>
>> The [2] link you provided discusses NUMA_LOCAL_DISTANCE.
>
> I inferred the discussion as "we should try to keep consistent between the
 value
> used in device tree and ACPI tables". Maybe my inference is wrong.
>
>> Looking at
>> Linux'es Documentation/devicetree/numa.txt, there's no mention of an
>> upper bound on the distance values. It only says that on the diagonal
>> entries should be 10 (i.e. matching ACPI, without really saying so).
>
> I agree that the NUMA device tree binding is a little bit vague. So I 
> cannot
> say the case that you provided is not valid. I would like to ask Arm
 maintainers
> (putting them into To:) opinion on this as I think I am not the one to
>> decide
 the
> expected behavior on Arm.
>
> Bertrand/Julien/Stefano: Would you please kindly share your opinion on
 which
> value should be used as the default value of the node distance map? Do
 you
> think reusing the "unreachable" distance, i.e. 0xFF, as the default node
 distance
> is acceptable here? Thanks!

 My suggestion would be to, rather than rejecting values >= 0xff, to 
 saturate
 at 0xfe, while keeping 0xff for NUMA_NO_DISTANCE (and overall keeping
 things
 consistent with ACPI).
>>>
>>> Since it has been a while and there were no feedback from Arm
>> maintainers,
>>> I would like to follow your suggestions for v5. However while I was writing
>> the
>>> code for the "saturation", i.e., adding below snippet in
>> numa_set_distance():
>>> ```
>>> if ( distance > NUMA_NO_DISTANCE )
>>> distance = NUMA_MAX_DISTANCE;
>>> ```
>>> I noticed an issue which needs your clarification:
>>> Currently, the default value of the distance map is NUMA_NO_DISTANCE,
>>> which indicates the nodes are not reachable. IMHO, if in device tree, we do
>>> saturations like above for ACPI invalid distances (distances >= 0xff), by
>> saturating
>>> the distance to 0xfe, we are making the unreachable nodes to reachable. I
>> am
>>> not sure if this will lead to problems. Do you have any more thoughts?
>> Thanks!
>>
>> I can only answer this with a question back: How is "unreachable"
>> represented
>> in DT? 
> 
> Exactly, that is also what we I am trying to find but failed. My understanding
> is that, the spec of NUMA is defined in the ACPI, and the DT NUMA binding
> only specifies how users can use DT to represent the same set of ACPI data,
> instead of define another standard.
> 
> By looking into the existing implementation of NUMA for DT,
> in Linux, from drivers/base/arch_numa.c: numa_set_distance(), there is a
> "if ((u8)distance != distance)" then return. So I think at least in Linux the
> distance value is consistent with the ACPI spec. 

Can we simply gain a similar check (suitably commented), eliminating the
need for saturation?

Jan

>> Or is "unreachable" simply expressed by the absence of any data?
> 
> Maybe I am wrong but I don't think so, as in the Linux implementation,
> drivers/of/of_numa.c: of_numa_parse_distance_map_v1(), the for loop
> "for (i = 0; i + 2 < entry_count; i += 3)" basically implies no fields should
> be omitted in the distance map entry.
> 
> Kind regards,
> Henry




Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature

2023-05-23 Thread Jan Beulich
On 23.05.2023 09:43, Luca Fancellu wrote:
> Add a command line parameter to allow Dom0 the use of SVE resources,
> the command line parameter sve=, sub argument of dom0=,
> controls the feature on this domain and sets the maximum SVE vector
> length for Dom0.
> 
> Add a new function, parse_signed_integer(), to parse an integer
> command line argument.
> 
> Signed-off-by: Luca Fancellu 

Reviewed-by: Jan Beulich  # !arm

> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -777,9 +777,9 @@ Specify the bit width of the DMA heap.
>  
>  ### dom0
>  = List of [ pv | pvh, shadow=, verbose=,
> -cpuid-faulting=, msr-relaxed= ]
> +cpuid-faulting=, msr-relaxed= ] (x86)
>  
> -Applicability: x86
> += List of [ sve= ] (Arm)

While in the text below you mention this is Arm64 only, I think the tag
here would better express this as well.

> @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems.
>  
>  If using this option is necessary to fix an issue, please report a bug.
>  
> +Enables features on dom0 on Arm systems.
> +
> +*   The `sve` integer parameter enables Arm SVE usage for Dom0 domain and 
> sets
> +the maximum SVE vector length, the option is applicable only to AArch64
> +guests.

Why "guests"? Does the option affect more than Dom0?

> +A value equal to 0 disables the feature, this is the default value.
> +Values below 0 means the feature uses the maximum SVE vector length
> +supported by hardware, if SVE is supported.
> +Values above 0 explicitly set the maximum SVE vector length for Dom0,
> +allowed values are from 128 to maximum 2048, being multiple of 128.
> +Please note that when the user explicitly specifies the value, if that 
> value
> +is above the hardware supported maximum SVE vector length, the domain
> +creation will fail and the system will stop, the same will occur if the
> +option is provided with a non zero value, but the platform doesn't 
> support
> +SVE.

Assuming this also covers the -1 case, I wonder if that isn't a little too
strict. "Maximum supported" imo can very well be 0.

Jan



[PATCH v7 12/12] xen/changelog: Add SVE and "dom0" options to the changelog for Arm

2023-05-23 Thread Luca Fancellu
Arm now can use the "dom0=" Xen command line option and the support
for guests running SVE instructions is added, put entries in the
changelog.

Mention the "Tech Preview" status and add an entry in SUPPORT.md

Signed-off-by: Luca Fancellu 
Acked-by: Henry Wang  # CHANGELOG
---
Changes from v6:
 - Add Henry's A-by to CHANGELOG
Changes from v5:
 - Add Tech Preview status and add entry in SUPPORT.md (Bertrand)
Changes from v4:
 - No changes
Change from v3:
 - new patch
---
 CHANGELOG.md | 3 +++
 SUPPORT.md   | 6 ++
 2 files changed, 9 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5bfd3aa5c0d5..512b7bdc0fcb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,8 @@ The format is based on [Keep a 
Changelog](https://keepachangelog.com/en/1.0.0/)
cap toolstack provided values.
  - Ignore VCPUOP_set_singleshot_timer's VCPU_SSHOTTMR_future flag. The only
known user doesn't use it properly, leading to in-guest breakage.
+ - The "dom0" option is now supported on Arm and "sve=" sub-option can be used
+   to enable dom0 guest to use SVE/SVE2 instructions.
 
 ### Added
  - On x86, support for features new in Intel Sapphire Rapids CPUs:
@@ -20,6 +22,7 @@ The format is based on [Keep a 
Changelog](https://keepachangelog.com/en/1.0.0/)
- Bus-lock detection, used by Xen to mitigate (by rate-limiting) the system
  wide impact of a guest misusing atomic instructions.
  - xl/libxl can customize SMBIOS strings for HVM guests.
+ - On Arm, Xen supports guests running SVE/SVE2 instructions. (Tech Preview)
 
 ## 
[4.17.0](https://xenbits.xen.org/gitweb/?p=xen.git;a=shortlog;h=RELEASE-4.17.0) 
- 2022-12-12
 
diff --git a/SUPPORT.md b/SUPPORT.md
index 6dbed9d5d029..e0fa2246807b 100644
--- a/SUPPORT.md
+++ b/SUPPORT.md
@@ -99,6 +99,12 @@ Extension to the GICv3 interrupt controller to support MSI.
 
 Status: Experimental
 
+### ARM Scalable Vector Extension (SVE/SVE2)
+
+AArch64 guest can use Scalable Vector Extension (SVE/SVE2).
+
+Status: Tech Preview
+
 ## Guest Type
 
 ### x86/PV
-- 
2.34.1




[PATCH v7 11/12] xen/arm: add sve property for dom0less domUs

2023-05-23 Thread Luca Fancellu
Add a device tree property in the dom0less domU configuration
to enable the guest to use SVE.

Update documentation.

Signed-off-by: Luca Fancellu 
---
Changes from v6:
 - Use ifdef in create_domUs and fail if 'sve' is used on systems
   with CONFIG_ARM64_SVE not selected (Bertrand, Julien, Jan)
Changes from v5:
 - Stop the domain creation if SVE not supported or SVE VL
   errors (Julien, Bertrand)
 - now sve_sanitize_vl_param is renamed to sve_domctl_vl_param
   and returns a boolean, change the affected code.
 - Reworded documentation.
Changes from v4:
 - Now it is possible to specify the property "sve" for dom0less
   device tree node without any value, that means the platform
   supported VL will be used.
Changes from v3:
 - Now domainconfig_encode_vl is named sve_encode_vl
Changes from v2:
 - xen_domctl_createdomain field name has changed into sve_vl
   and its value is the VL/128, use domainconfig_encode_vl
   to encode a plain VL in bits.
Changes from v1:
 - No changes
Changes from RFC:
 - Changed documentation
---
 docs/misc/arm/device-tree/booting.txt | 16 +++
 xen/arch/arm/domain_build.c   | 28 +++
 2 files changed, 44 insertions(+)

diff --git a/docs/misc/arm/device-tree/booting.txt 
b/docs/misc/arm/device-tree/booting.txt
index 3879340b5e0a..32a0e508c471 100644
--- a/docs/misc/arm/device-tree/booting.txt
+++ b/docs/misc/arm/device-tree/booting.txt
@@ -193,6 +193,22 @@ with the following properties:
 Optional. Handle to a xen,cpupool device tree node that identifies the
 cpupool where the guest will be started at boot.
 
+- sve
+
+Optional. The `sve` property enables Arm SVE usage for the domain and sets
+the maximum SVE vector length, the option is applicable only to AArch64
+guests.
+A value equal to 0 disables the feature, this is the default value.
+Specifying this property with no value, means that the SVE vector length
+will be set equal to the maximum vector length supported by the platform.
+Values above 0 explicitly set the maximum SVE vector length for the domain,
+allowed values are from 128 to maximum 2048, being multiple of 128.
+Please note that when the user explicitly specifies the value, if that 
value
+is above the hardware supported maximum SVE vector length, the domain
+creation will fail and the system will stop, the same will occur if the
+option is provided with a non zero value, but the platform doesn't support
+SVE.
+
 - xen,enhanced
 
 A string property. Possible property values are:
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 9202a96d9c28..ba4fe9e165ee 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -4008,6 +4008,34 @@ void __init create_domUs(void)
 d_cfg.max_maptrack_frames = val;
 }
 
+if ( dt_get_property(node, "sve", ) )
+{
+#ifdef CONFIG_ARM64_SVE
+unsigned int sve_vl_bits;
+bool ret = false;
+
+if ( !val )
+{
+/* Property found with no value, means max HW VL supported */
+ret = sve_domctl_vl_param(-1, _vl_bits);
+}
+else
+{
+if ( dt_property_read_u32(node, "sve", ) )
+ret = sve_domctl_vl_param(val, _vl_bits);
+else
+panic("Error reading 'sve' property");
+}
+
+if ( ret )
+d_cfg.arch.sve_vl = sve_encode_vl(sve_vl_bits);
+else
+panic("SVE vector length error\n");
+#else
+panic("'sve' property found, but CONFIG_ARM64_SVE not selected");
+#endif
+}
+
 /*
  * The variable max_init_domid is initialized with zero, so here it's
  * very important to use the pre-increment operator to call
-- 
2.34.1




[PATCH v7 09/12] tools: add physinfo arch_capabilities handling for Arm

2023-05-23 Thread Luca Fancellu
On Arm, the SVE vector length is encoded in arch_capabilities field
of struct xen_sysctl_physinfo, make use of this field in the tools
when building for arm.

Create header arm-arch-capabilities.h to handle the arch_capabilities
field of physinfo for Arm.

Removed include for xen-tools/common-macros.h in
python/xen/lowlevel/xc/xc.c because it is already included by the
arm-arch-capabilities.h header.

Signed-off-by: Luca Fancellu 
Acked-by: George Dunlap 
Acked-by: Christian Lindig 
Reviewed-by: Anthony PERARD 
---
Changes from v6:
 - Fix licence header in arm-atch-capabilities.h, add R-by (Anthony)
Changes from v5:
 - no changes
Changes from v4:
 - Move arm-arch-capabilities.h into xen-tools/, add LIBXL_HAVE_,
   fixed python return type to I instead of i. (Anthony)
Changes from v3:
 - add Ack-by for the Golang bits (George)
 - add Ack-by for the OCaml tools (Christian)
 - now xen-tools/libs.h is named xen-tools/common-macros.h
 - changed commit message to explain why the header modification
   in python/xen/lowlevel/xc/xc.c
Changes from v2:
 - rename arm_arch_capabilities.h in arm-arch-capabilities.h, use
   MASK_EXTR.
 - Now arm-arch-capabilities.h needs MASK_EXTR macro, but it is
   defined in libxl_internal.h, it doesn't feel right to include
   that header so move MASK_EXTR into xen-tools/libs.h that is also
   included in libxl_internal.h
Changes from v1:
 - now SVE VL is encoded in arch_capabilities on Arm
Changes from RFC:
 - new patch
---
 tools/golang/xenlight/helpers.gen.go  |  2 ++
 tools/golang/xenlight/types.gen.go|  1 +
 tools/include/libxl.h |  6 
 .../include/xen-tools/arm-arch-capabilities.h | 28 +++
 tools/include/xen-tools/common-macros.h   |  2 ++
 tools/libs/light/libxl.c  |  1 +
 tools/libs/light/libxl_internal.h |  1 -
 tools/libs/light/libxl_types.idl  |  1 +
 tools/ocaml/libs/xc/xenctrl.ml|  4 +--
 tools/ocaml/libs/xc/xenctrl.mli   |  4 +--
 tools/ocaml/libs/xc/xenctrl_stubs.c   |  8 --
 tools/python/xen/lowlevel/xc/xc.c |  8 --
 tools/xl/xl_info.c|  8 ++
 13 files changed, 62 insertions(+), 12 deletions(-)
 create mode 100644 tools/include/xen-tools/arm-arch-capabilities.h

diff --git a/tools/golang/xenlight/helpers.gen.go 
b/tools/golang/xenlight/helpers.gen.go
index 0a203d22321f..35397be2f9e2 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -3506,6 +3506,7 @@ x.CapVmtrace = bool(xc.cap_vmtrace)
 x.CapVpmu = bool(xc.cap_vpmu)
 x.CapGnttabV1 = bool(xc.cap_gnttab_v1)
 x.CapGnttabV2 = bool(xc.cap_gnttab_v2)
+x.ArchCapabilities = uint32(xc.arch_capabilities)
 
  return nil}
 
@@ -3540,6 +3541,7 @@ xc.cap_vmtrace = C.bool(x.CapVmtrace)
 xc.cap_vpmu = C.bool(x.CapVpmu)
 xc.cap_gnttab_v1 = C.bool(x.CapGnttabV1)
 xc.cap_gnttab_v2 = C.bool(x.CapGnttabV2)
+xc.arch_capabilities = C.uint32_t(x.ArchCapabilities)
 
  return nil
  }
diff --git a/tools/golang/xenlight/types.gen.go 
b/tools/golang/xenlight/types.gen.go
index a7c17699f80e..3d968a496744 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -1079,6 +1079,7 @@ CapVmtrace bool
 CapVpmu bool
 CapGnttabV1 bool
 CapGnttabV2 bool
+ArchCapabilities uint32
 }
 
 type Connectorinfo struct {
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index cfa1a191318c..4fa09ff7635a 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -525,6 +525,12 @@
  */
 #define LIBXL_HAVE_PHYSINFO_CAP_GNTTAB 1
 
+/*
+ * LIBXL_HAVE_PHYSINFO_ARCH_CAPABILITIES indicates that libxl_physinfo has a
+ * arch_capabilities field.
+ */
+#define LIBXL_HAVE_PHYSINFO_ARCH_CAPABILITIES 1
+
 /*
  * LIBXL_HAVE_MAX_GRANT_VERSION indicates libxl_domain_build_info has a
  * max_grant_version field for setting the max grant table version per
diff --git a/tools/include/xen-tools/arm-arch-capabilities.h 
b/tools/include/xen-tools/arm-arch-capabilities.h
new file mode 100644
index ..3849e897925d
--- /dev/null
+++ b/tools/include/xen-tools/arm-arch-capabilities.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: LGPL-2.1-only */
+/*
+ * Copyright (C) 2023 ARM Ltd.
+ */
+
+#ifndef ARM_ARCH_CAPABILITIES_H
+#define ARM_ARCH_CAPABILITIES_H
+
+#include 
+#include 
+
+#include 
+
+static inline
+unsigned int arch_capabilities_arm_sve(unsigned int arch_capabilities)
+{
+#if defined(__aarch64__)
+unsigned int sve_vl = MASK_EXTR(arch_capabilities,
+XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK);
+
+/* Vector length is divided by 128 before storing it in arch_capabilities 
*/
+return sve_vl * 128U;
+#else
+return 0;
+#endif
+}
+
+#endif /* ARM_ARCH_CAPABILITIES_H */
diff --git a/tools/include/xen-tools/common-macros.h 
b/tools/include/xen-tools/common-macros.h
index 76b55bf62085..d53b88182560 100644
--- a/tools/include/xen-tools/common-macros.h
+++ 

[PATCH v7 10/12] xen/tools: add sve parameter in XL configuration

2023-05-23 Thread Luca Fancellu
Add sve parameter in XL configuration to allow guests to use
SVE feature.

Signed-off-by: Luca Fancellu 
---
Changes from v6:
 - Add check for sve_vl be multiple of 128 (Anthony)
Changes from v5:
 - Update documentation
 - re-generated golang files
Changes from v4:
 - Rename sve field to sve_vl (Anthony), changed type to
   libxl_sve_type
 - Sanity check of sve field in libxl instead of xl, update docs
   (Anthony)
 - drop Ack-by from George because of the changes in the Golang bits
Changes from v3:
 - no changes
Changes from v2:
 - domain configuration field name has changed to sve_vl,
   also its value now is VL/128.
 - Add Ack-by George for the Golang bits
Changes from v1:
 - updated to use arch_capabilities field for vector length
Changes from RFC:
 - changed libxl_types.idl sve field to uint16
 - now toolstack uses info from physinfo to check against the
   sve XL value
 - Changed documentation
---
 docs/man/xl.cfg.5.pod.in | 16 ++
 tools/golang/xenlight/helpers.gen.go |  2 ++
 tools/golang/xenlight/types.gen.go   | 23 +++
 tools/include/libxl.h|  5 +
 tools/libs/light/libxl_arm.c | 33 
 tools/libs/light/libxl_types.idl | 22 +++
 tools/xl/xl_parse.c  |  8 +++
 7 files changed, 109 insertions(+)

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 24ac92718288..1b4e13ab647b 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -2955,6 +2955,22 @@ Currently, only the "sbsa_uart" model is supported for 
ARM.
 
 =back
 
+=item B
+
+The `sve` parameter enables Arm Scalable Vector Extension (SVE) usage for the
+guest and sets the maximum SVE vector length, the option is applicable only to
+AArch64 guests.
+A value equal to "disabled" disables the feature, this is the default value.
+Allowed values are "disabled", "128", "256", "384", "512", "640", "768", "896",
+"1024", "1152", "1280", "1408", "1536", "1664", "1792", "1920", "2048", "hw".
+Specifying "hw" means that the maximum vector length supported by the platform
+will be used.
+Please be aware that if a specific vector length is passed and its value is
+above the maximum vector length supported by the platform, an error will be
+raised.
+
+=back
+
 =head3 x86
 
 =over 4
diff --git a/tools/golang/xenlight/helpers.gen.go 
b/tools/golang/xenlight/helpers.gen.go
index 35397be2f9e2..cd1a16e32eac 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -1149,6 +1149,7 @@ default:
 return fmt.Errorf("invalid union key '%v'", x.Type)}
 x.ArchArm.GicVersion = GicVersion(xc.arch_arm.gic_version)
 x.ArchArm.Vuart = VuartType(xc.arch_arm.vuart)
+x.ArchArm.SveVl = SveType(xc.arch_arm.sve_vl)
 if err := x.ArchX86.MsrRelaxed.fromC(_x86.msr_relaxed);err != nil {
 return fmt.Errorf("converting field ArchX86.MsrRelaxed: %v", err)
 }
@@ -1653,6 +1654,7 @@ default:
 return fmt.Errorf("invalid union key '%v'", x.Type)}
 xc.arch_arm.gic_version = C.libxl_gic_version(x.ArchArm.GicVersion)
 xc.arch_arm.vuart = C.libxl_vuart_type(x.ArchArm.Vuart)
+xc.arch_arm.sve_vl = C.libxl_sve_type(x.ArchArm.SveVl)
 if err := x.ArchX86.MsrRelaxed.toC(_x86.msr_relaxed); err != nil {
 return fmt.Errorf("converting field ArchX86.MsrRelaxed: %v", err)
 }
diff --git a/tools/golang/xenlight/types.gen.go 
b/tools/golang/xenlight/types.gen.go
index 3d968a496744..b131a7eedc9d 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -490,6 +490,28 @@ TeeTypeNone TeeType = 0
 TeeTypeOptee TeeType = 1
 )
 
+type SveType int
+const(
+SveTypeHw SveType = -1
+SveTypeDisabled SveType = 0
+SveType128 SveType = 128
+SveType256 SveType = 256
+SveType384 SveType = 384
+SveType512 SveType = 512
+SveType640 SveType = 640
+SveType768 SveType = 768
+SveType896 SveType = 896
+SveType1024 SveType = 1024
+SveType1152 SveType = 1152
+SveType1280 SveType = 1280
+SveType1408 SveType = 1408
+SveType1536 SveType = 1536
+SveType1664 SveType = 1664
+SveType1792 SveType = 1792
+SveType1920 SveType = 1920
+SveType2048 SveType = 2048
+)
+
 type RdmReserve struct {
 Strategy RdmReserveStrategy
 Policy RdmReservePolicy
@@ -564,6 +586,7 @@ TypeUnion DomainBuildInfoTypeUnion
 ArchArm struct {
 GicVersion GicVersion
 Vuart VuartType
+SveVl SveType
 }
 ArchX86 struct {
 MsrRelaxed Defbool
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index 4fa09ff7635a..cac641a7eba2 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -283,6 +283,11 @@
  */
 #define LIBXL_HAVE_BUILDINFO_ARCH_ARM_TEE 1
 
+/*
+ * libxl_domain_build_info has the arch_arm.sve_vl field.
+ */
+#define LIBXL_HAVE_BUILDINFO_ARCH_ARM_SVE_VL 1
+
 /*
  * LIBXL_HAVE_SOFT_RESET indicates that libxl supports performing
  * 'soft reset' for domains and there is 'soft_reset' shutdown reason
diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 97c80d7ed0fa..35f76dfc21e4 100644
--- 

[PATCH v7 06/12] xen/common: add dom0 xen command line argument for Arm

2023-05-23 Thread Luca Fancellu
Currently x86 defines a Xen command line argument dom0= where
there can be specified dom0 controlling sub-options, to use it also
on Arm, move the code that loops through the list of arguments from
x86 to the common code and from there, call architecture specific
functions to handle the comma separated sub-options.

No functional changes are intended.

Signed-off-by: Luca Fancellu 
Reviewed-by: Jan Beulich 
Reviewed-by: Bertrand Marquis 
---
Changes from v6:
 - no changes
Changes from v5:
 - Add Bertrand R-by
Changes from v4:
 - return EINVAL in Arm implementation of parse_arch_dom0_param,
   shorten variable name in the funtion from str_begin, str_end to
   s, e. Removed variable rc from x86 parse_arch_dom0_param
   implementation. (Jan)
 - Add R-By Jan
Changes from v3:
 - new patch
---
 xen/arch/arm/domain_build.c |  5 
 xen/arch/x86/dom0_build.c   | 48 ++---
 xen/common/domain.c | 23 ++
 xen/include/xen/domain.h|  1 +
 4 files changed, 47 insertions(+), 30 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 9dd1ed5bce44..f373a5024783 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -60,6 +60,11 @@ static int __init parse_dom0_mem(const char *s)
 }
 custom_param("dom0_mem", parse_dom0_mem);
 
+int __init parse_arch_dom0_param(const char *s, const char *e)
+{
+return -EINVAL;
+}
+
 /* Override macros from asm/page.h to make them work with mfn_t */
 #undef virt_to_mfn
 #define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 79234f18ff01..9f5300a3efbb 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -266,42 +266,30 @@ bool __initdata opt_dom0_pvh = !IS_ENABLED(CONFIG_PV);
 bool __initdata opt_dom0_verbose = IS_ENABLED(CONFIG_VERBOSE_DEBUG);
 bool __initdata opt_dom0_msr_relaxed;
 
-static int __init cf_check parse_dom0_param(const char *s)
+int __init parse_arch_dom0_param(const char *s, const char *e)
 {
-const char *ss;
-int rc = 0;
+int val;
 
-do {
-int val;
-
-ss = strchr(s, ',');
-if ( !ss )
-ss = strchr(s, '\0');
-
-if ( IS_ENABLED(CONFIG_PV) && !cmdline_strcmp(s, "pv") )
-opt_dom0_pvh = false;
-else if ( IS_ENABLED(CONFIG_HVM) && !cmdline_strcmp(s, "pvh") )
-opt_dom0_pvh = true;
+if ( IS_ENABLED(CONFIG_PV) && !cmdline_strcmp(s, "pv") )
+opt_dom0_pvh = false;
+else if ( IS_ENABLED(CONFIG_HVM) && !cmdline_strcmp(s, "pvh") )
+opt_dom0_pvh = true;
 #ifdef CONFIG_SHADOW_PAGING
-else if ( (val = parse_boolean("shadow", s, ss)) >= 0 )
-opt_dom0_shadow = val;
+else if ( (val = parse_boolean("shadow", s, e)) >= 0 )
+opt_dom0_shadow = val;
 #endif
-else if ( (val = parse_boolean("verbose", s, ss)) >= 0 )
-opt_dom0_verbose = val;
-else if ( IS_ENABLED(CONFIG_PV) &&
-  (val = parse_boolean("cpuid-faulting", s, ss)) >= 0 )
-opt_dom0_cpuid_faulting = val;
-else if ( (val = parse_boolean("msr-relaxed", s, ss)) >= 0 )
-opt_dom0_msr_relaxed = val;
-else
-rc = -EINVAL;
-
-s = ss + 1;
-} while ( *ss );
+else if ( (val = parse_boolean("verbose", s, e)) >= 0 )
+opt_dom0_verbose = val;
+else if ( IS_ENABLED(CONFIG_PV) &&
+  (val = parse_boolean("cpuid-faulting", s, e)) >= 0 )
+opt_dom0_cpuid_faulting = val;
+else if ( (val = parse_boolean("msr-relaxed", s, e)) >= 0 )
+opt_dom0_msr_relaxed = val;
+else
+return -EINVAL;
 
-return rc;
+return 0;
 }
-custom_param("dom0", parse_dom0_param);
 
 static char __initdata opt_dom0_ioports_disable[200] = "";
 string_param("dom0_ioports_disable", opt_dom0_ioports_disable);
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 6a440590fe2a..caaa40263792 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -364,6 +364,29 @@ static int __init cf_check parse_extra_guest_irqs(const 
char *s)
 }
 custom_param("extra_guest_irqs", parse_extra_guest_irqs);
 
+static int __init cf_check parse_dom0_param(const char *s)
+{
+const char *ss;
+int rc = 0;
+
+do {
+int ret;
+
+ss = strchr(s, ',');
+if ( !ss )
+ss = strchr(s, '\0');
+
+ret = parse_arch_dom0_param(s, ss);
+if ( ret && !rc )
+rc = ret;
+
+s = ss + 1;
+} while ( *ss );
+
+return rc;
+}
+custom_param("dom0", parse_dom0_param);
+
 /*
  * Release resources held by a domain.  There may or may not be live
  * references to the domain, and it may or may not be fully constructed.
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 26f9c4f6dd5b..1df8f933d076 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -16,6 +16,7 @@ typedef union {
 struct vcpu 

[PATCH v7 04/12] xen/arm: add SVE exception class handling

2023-05-23 Thread Luca Fancellu
SVE has a new exception class with code 0x19, introduce the new code
and handle the exception.

Signed-off-by: Luca Fancellu 
Reviewed-by: Bertrand Marquis 
Reviewed-by: Julien Grall 
---
Changes from v6:
 - Add R-by Julien
Changes from v5:
 - modified error messages (Julien)
 - add R-by Bertrand
Changes from v4:
 - No changes
Changes from v3:
 - No changes
Changes from v2:
 - No changes
Changes from v1:
 - No changes
Changes from RFC:
 - No changes
---
 xen/arch/arm/include/asm/processor.h | 1 +
 xen/arch/arm/traps.c | 9 +
 2 files changed, 10 insertions(+)

diff --git a/xen/arch/arm/include/asm/processor.h 
b/xen/arch/arm/include/asm/processor.h
index bc683334125c..7e42ff8811fc 100644
--- a/xen/arch/arm/include/asm/processor.h
+++ b/xen/arch/arm/include/asm/processor.h
@@ -426,6 +426,7 @@
 #define HSR_EC_HVC640x16
 #define HSR_EC_SMC640x17
 #define HSR_EC_SYSREG   0x18
+#define HSR_EC_SVE  0x19
 #endif
 #define HSR_EC_INSTR_ABORT_LOWER_EL 0x20
 #define HSR_EC_INSTR_ABORT_CURR_EL  0x21
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 3393e10b52e6..f6437f6aa9c9 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2172,6 +2172,11 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
 perfc_incr(trap_sysreg);
 do_sysreg(regs, hsr);
 break;
+case HSR_EC_SVE:
+GUEST_BUG_ON(regs_mode_is_32bit(regs));
+gprintk(XENLOG_WARNING, "Domain tried to use SVE while not allowed\n");
+inject_undef_exception(regs, hsr);
+break;
 #endif
 
 case HSR_EC_INSTR_ABORT_LOWER_EL:
@@ -2201,6 +2206,10 @@ void do_trap_hyp_sync(struct cpu_user_regs *regs)
 case HSR_EC_BRK:
 do_trap_brk(regs, hsr);
 break;
+case HSR_EC_SVE:
+/* An SVE exception is a bug somewhere in hypervisor code */
+do_unexpected_trap("SVE trap at EL2", regs);
+break;
 #endif
 case HSR_EC_DATA_ABORT_CURR_EL:
 case HSR_EC_INSTR_ABORT_CURR_EL:
-- 
2.34.1




[PATCH v7 05/12] arm/sve: save/restore SVE context switch

2023-05-23 Thread Luca Fancellu
Save/restore context switch for SVE, allocate memory to contain
the Z0-31 registers whose length is maximum 2048 bits each and
FFR who can be maximum 256 bits, the allocated memory depends on
how many bits is the vector length for the domain and how many bits
are supported by the platform.

Save P0-15 whose length is maximum 256 bits each, in this case the
memory used is from the fpregs field in struct vfp_state,
because V0-31 are part of Z0-31 and this space would have been
unused for SVE domain otherwise.

Create zcr_el{1,2} fields in arch_vcpu, initialise zcr_el2 on vcpu
creation given the requested vector length and restore it on
context switch, save/restore ZCR_EL1 value as well.

List import macros from Linux in README.LinuxPrimitives.

Signed-off-by: Luca Fancellu 
---
Changes from v6:
 - Add comment for explain why sve_save/sve_load are different from
   Linux, add macros in xen/arch/arm/README.LinuxPrimitives (Julien)
 - Add comments in sve_context_init and sve_context_free, handle the
   case where sve_zreg_ctx_end is NULL, move setting of v->arch.zcr_el2
   in sve_context_init (Julien)
 - remove stubs for sve_context_* and sve_save_* and rely on compiler
   DCE (Jan)
 - Add comments for sve_save_ctx/sve_load_ctx (Julien)
Changes from v5:
 - use XFREE instead of xfree, keep the headers (Julien)
 - Avoid math computation for every save/restore, store the computation
   in struct vfp_state once (Bertrand)
 - protect access to v->domain->arch.sve_vl inside arch_vcpu_create now
   that sve_vl is available only on arm64
Changes from v4:
 - No changes
Changes from v3:
 - don't use fixed len types when not needed (Jan)
 - now VL is an encoded value, decode it before using.
Changes from v2:
 - No changes
Changes from v1:
 - No changes
Changes from RFC:
 - Moved zcr_el2 field introduction in this patch, restore its
   content inside sve_restore_state function. (Julien)

fix patch 5

Signed-off-by: Luca Fancellu 
Change-Id: Ief65b2ff14fd579afa4fd110ce08a19980e64fa9
---
 xen/arch/arm/README.LinuxPrimitives  |   4 +-
 xen/arch/arm/arm64/sve-asm.S | 147 +++
 xen/arch/arm/arm64/sve.c |  91 ++
 xen/arch/arm/arm64/vfp.c |  79 ++--
 xen/arch/arm/domain.c|   6 +
 xen/arch/arm/include/asm/arm64/sve.h |   4 +
 xen/arch/arm/include/asm/arm64/sysregs.h |   3 +
 xen/arch/arm/include/asm/arm64/vfp.h |  12 ++
 xen/arch/arm/include/asm/domain.h|   2 +
 9 files changed, 313 insertions(+), 35 deletions(-)

diff --git a/xen/arch/arm/README.LinuxPrimitives 
b/xen/arch/arm/README.LinuxPrimitives
index 76c8df29e416..301c0271bbe4 100644
--- a/xen/arch/arm/README.LinuxPrimitives
+++ b/xen/arch/arm/README.LinuxPrimitives
@@ -69,7 +69,9 @@ SVE assembly macro: last sync @ v6.3.0 (last commit: 
457391b03803)
 linux/arch/arm64/include/asm/fpsimdmacros.h   
xen/arch/arm/include/asm/arm64/sve-asm.S
 
 The following macros were taken from Linux:
-_check_general_reg, _check_num, _sve_rdvl
+_check_general_reg, _check_num, _sve_rdvl, __for, _for, _sve_check_zreg,
+_sve_check_preg, _sve_str_v, _sve_ldr_v, _sve_str_p, _sve_ldr_p, 
_sve_rdffr,
+_sve_wrffr
 
 =
 arm32
diff --git a/xen/arch/arm/arm64/sve-asm.S b/xen/arch/arm/arm64/sve-asm.S
index 4d1549344733..59dbefbbb252 100644
--- a/xen/arch/arm/arm64/sve-asm.S
+++ b/xen/arch/arm/arm64/sve-asm.S
@@ -17,6 +17,18 @@
 .endif
 .endm
 
+.macro _sve_check_zreg znr
+.if (\znr) < 0 || (\znr) > 31
+.error "Bad Scalable Vector Extension vector register number \znr."
+.endif
+.endm
+
+.macro _sve_check_preg pnr
+.if (\pnr) < 0 || (\pnr) > 15
+.error "Bad Scalable Vector Extension predicate register number \pnr."
+.endif
+.endm
+
 .macro _check_num n, min, max
 .if (\n) < (\min) || (\n) > (\max)
 .error "Number \n out of range [\min,\max]"
@@ -26,6 +38,54 @@
 /* SVE instruction encodings for non-SVE-capable assemblers */
 /* (pre binutils 2.28, all kernel capable clang versions support SVE) */
 
+/* STR (vector): STR Z\nz, [X\nxbase, #\offset, MUL VL] */
+.macro _sve_str_v nz, nxbase, offset=0
+_sve_check_zreg \nz
+_check_general_reg \nxbase
+_check_num (\offset), -0x100, 0xff
+.inst 0xe5804000\
+| (\nz) \
+| ((\nxbase) << 5)  \
+| (((\offset) & 7) << 10)   \
+| (((\offset) & 0x1f8) << 13)
+.endm
+
+/* LDR (vector): LDR Z\nz, [X\nxbase, #\offset, MUL VL] */
+.macro _sve_ldr_v nz, nxbase, offset=0
+_sve_check_zreg \nz
+_check_general_reg \nxbase
+_check_num (\offset), -0x100, 0xff
+.inst 0x85804000\
+| (\nz) \
+| ((\nxbase) << 5)  \
+| (((\offset) & 7) << 10)   \
+| (((\offset) & 0x1f8) << 13)
+.endm
+
+/* STR (predicate): STR P\np, [X\nxbase, #\offset, MUL VL] */
+.macro 

[PATCH v7 08/12] xen/physinfo: encode Arm SVE vector length in arch_capabilities

2023-05-23 Thread Luca Fancellu
When the arm platform supports SVE, advertise the feature in the
field arch_capabilities in struct xen_sysctl_physinfo by encoding
the SVE vector length in it.

Signed-off-by: Luca Fancellu 
Reviewed-by: Bertrand Marquis 
---
Changes from v6:
 - no changes
Changes from v5:
 - Add R-by from Bertrand
Changes from v4:
 - Write arch_capabilities from arch_do_physinfo instead of using
   stub functions (Jan)
Changes from v3:
 - domainconfig_encode_vl is now named sve_encode_vl
Changes from v2:
 - Remove XEN_SYSCTL_PHYSCAP_ARM_SVE_SHFT, use MASK_INSR and
   protect with ifdef XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK (Jan)
 - Use the helper function sve_arch_cap_physinfo to encode
   the VL into physinfo arch_capabilities field.
Changes from v1:
 - Use only arch_capabilities and some defines to encode SVE VL
   (Bertrand, Stefano, Jan)
Changes from RFC:
 - new patch
---
 xen/arch/arm/sysctl.c   | 4 
 xen/include/public/sysctl.h | 4 
 2 files changed, 8 insertions(+)

diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
index b0a78a8b10d0..e9a0661146e4 100644
--- a/xen/arch/arm/sysctl.c
+++ b/xen/arch/arm/sysctl.c
@@ -11,11 +11,15 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
 {
 pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm | XEN_SYSCTL_PHYSCAP_hap;
+
+pi->arch_capabilities |= MASK_INSR(sve_encode_vl(get_sys_vl_len()),
+   XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK);
 }
 
 long arch_do_sysctl(struct xen_sysctl *sysctl,
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 2b24d6bfd00e..9d06e92d0f6a 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -94,6 +94,10 @@ struct xen_sysctl_tbuf_op {
 /* Max XEN_SYSCTL_PHYSCAP_* constant.  Used for ABI checking. */
 #define XEN_SYSCTL_PHYSCAP_MAX XEN_SYSCTL_PHYSCAP_gnttab_v2
 
+#if defined(__arm__) || defined(__aarch64__)
+#define XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK  (0x1FU)
+#endif
+
 struct xen_sysctl_physinfo {
 uint32_t threads_per_core;
 uint32_t cores_per_socket;
-- 
2.34.1




[PATCH v7 07/12] xen: enable Dom0 to use SVE feature

2023-05-23 Thread Luca Fancellu
Add a command line parameter to allow Dom0 the use of SVE resources,
the command line parameter sve=, sub argument of dom0=,
controls the feature on this domain and sets the maximum SVE vector
length for Dom0.

Add a new function, parse_signed_integer(), to parse an integer
command line argument.

Signed-off-by: Luca Fancellu 
---
Changes from v6:
 - Fixed case for e==NULL in parse_signed_integer, drop parenthesis
   from if conditions, delete inline sve_domctl_vl_param and rely on
   DCE from the compiler (Jan)
 - Drop parenthesis from opt_dom0_sve (Julien)
 - Do not continue if 'sve' is in command line args but
   CONFIG_ARM64_SVE is not selected:
   https://lore.kernel.org/all/7614ae25-f59d-430a-9c3e-30b1ce0e1...@arm.com/
Changes from v5:
 - stop the domain if VL error occurs (Julien, Bertrand)
 - update the documentation
 - Rename sve_sanitize_vl_param to sve_domctl_vl_param to
   mark the fact that we are sanitizing a parameter coming from
   the user before encoding it into sve_vl in domctl structure.
   (suggestion from Bertrand in a separate discussion)
 - update comment in parse_signed_integer, return boolean in
   sve_domctl_vl_param (Jan).
Changes from v4:
 - Negative values as user param means max supported HW VL (Jan)
 - update documentation, make use of no_config_param(), rename
   parse_integer into parse_signed_integer and take long long *,
   also put a comment on the -2 return condition, update
   declaration comment to reflect the modifications (Jan)
Changes from v3:
 - Don't use fixed len types when not needed (Jan)
 - renamed domainconfig_encode_vl to sve_encode_vl
 - Use a sub argument of dom0= to enable the feature (Jan)
 - Add parse_integer() function
Changes from v2:
 - xen_domctl_createdomain field has changed into sve_vl and its
   value now is the VL / 128, create an helper function for that.
Changes from v1:
 - No changes
Changes from RFC:
 - Changed docs to explain that the domain won't be created if the
   requested vector length is above the supported one from the
   platform.
---
 docs/misc/xen-command-line.pandoc| 20 ++--
 xen/arch/arm/arm64/sve.c | 20 
 xen/arch/arm/domain_build.c  | 26 ++
 xen/arch/arm/include/asm/arm64/sve.h | 10 ++
 xen/common/kernel.c  | 28 
 xen/include/xen/lib.h| 10 ++
 6 files changed, 112 insertions(+), 2 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc 
b/docs/misc/xen-command-line.pandoc
index e0b89b7d3319..47e5b4eb6199 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -777,9 +777,9 @@ Specify the bit width of the DMA heap.
 
 ### dom0
 = List of [ pv | pvh, shadow=, verbose=,
-cpuid-faulting=, msr-relaxed= ]
+cpuid-faulting=, msr-relaxed= ] (x86)
 
-Applicability: x86
+= List of [ sve= ] (Arm)
 
 Controls for how dom0 is constructed on x86 systems.
 
@@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems.
 
 If using this option is necessary to fix an issue, please report a bug.
 
+Enables features on dom0 on Arm systems.
+
+*   The `sve` integer parameter enables Arm SVE usage for Dom0 domain and sets
+the maximum SVE vector length, the option is applicable only to AArch64
+guests.
+A value equal to 0 disables the feature, this is the default value.
+Values below 0 means the feature uses the maximum SVE vector length
+supported by hardware, if SVE is supported.
+Values above 0 explicitly set the maximum SVE vector length for Dom0,
+allowed values are from 128 to maximum 2048, being multiple of 128.
+Please note that when the user explicitly specifies the value, if that 
value
+is above the hardware supported maximum SVE vector length, the domain
+creation will fail and the system will stop, the same will occur if the
+option is provided with a non zero value, but the platform doesn't support
+SVE.
+
 ### dom0-cpuid
 = List of comma separated booleans
 
diff --git a/xen/arch/arm/arm64/sve.c b/xen/arch/arm/arm64/sve.c
index 84a6dedc1fd7..feaca2cf647d 100644
--- a/xen/arch/arm/arm64/sve.c
+++ b/xen/arch/arm/arm64/sve.c
@@ -13,6 +13,9 @@
 #include 
 #include 
 
+/* opt_dom0_sve: allow Dom0 to use SVE and set maximum vector length. */
+int __initdata opt_dom0_sve;
+
 extern unsigned int sve_get_hw_vl(void);
 
 /*
@@ -152,6 +155,23 @@ void sve_restore_state(struct vcpu *v)
 sve_load_ctx(v->arch.vfp.sve_zreg_ctx_end, v->arch.vfp.fpregs, 1);
 }
 
+bool __init sve_domctl_vl_param(int val, unsigned int *out)
+{
+/*
+ * Negative SVE parameter value means to use the maximum supported
+ * vector length, otherwise if a positive value is provided, check if the
+ * vector length is a multiple of 128
+ */
+if ( val < 0 )
+*out = get_sys_vl_len();
+else if ( (val % SVE_VL_MULTIPLE_VAL) == 0 )
+

[PATCH v7 00/12] SVE feature for arm guests

2023-05-23 Thread Luca Fancellu
This serie is introducing the possibility for Dom0 and DomU guests to use
sve/sve2 instructions.

SVE feature introduces new instruction and registers to improve performances on
floating point operations.

The SVE feature is advertised using the ID_AA64PFR0_EL1 register, SVE field, and
when available the ID_AA64ZFR0_EL1 register provides additional information
about the implemented version and other SVE feature.

New registers added by the SVE feature are Z0-Z31, P0-P15, FFR, ZCR_ELx.

Z0-Z31 are scalable vector register whose size is implementation defined and
goes from 128 bits to maximum 2048, the term vector length will be used to refer
to this quantity.
P0-P15 are predicate registers and the size is the vector length divided by 8,
same size is the FFR (First Fault Register).
ZCR_ELx is a register that can control and restrict the maximum vector length
used by the  exception level and all the lower exception levels, so for
example EL3 can restrict the vector length usable by EL3,2,1,0.

The platform has a maximum implemented vector length, so for every value
written in ZCR register, if this value is above the implemented length, then the
lower value will be used. The RDVL instruction can be used to check what vector
length is the HW using after setting ZCR.

For an SVE guest, the V0-V31 registers are part of the Z0-Z31, so there is no
need to save them separately, saving Z0-Z31 will save implicitly also V0-V31.

SVE usage can be trapped using a flag in CPTR_EL2, hence in this serie the
register is added to the domain state, to be able to trap only the guests that
are not allowed to use SVE.

This serie is introducing a command line parameter to enable Dom0 to use SVE and
to set its maximum vector length that by default is 0 which means the guest is
not allowed to use SVE. Values from 128 to 2048 mean the guest can use SVE with
the selected value used as maximum allowed vector length (which could be lower
if the implemented one is lower).
For DomUs, an XL parameter with the same way of use is introduced and a dom0less
DTB binding is created.

The context switch is the most critical part because there can be big registers
to be saved, in this serie an easy approach is used and the context is
saved/restored every time for the guests that are allowed to use SVE.

Luca Fancellu (12):
  xen/arm: enable SVE extension for Xen
  xen/arm: add SVE vector length field to the domain
  xen/arm: Expose SVE feature to the guest
  xen/arm: add SVE exception class handling
  arm/sve: save/restore SVE context switch
  xen/common: add dom0 xen command line argument for Arm
  xen: enable Dom0 to use SVE feature
  xen/physinfo: encode Arm SVE vector length in arch_capabilities
  tools: add physinfo arch_capabilities handling for Arm
  xen/tools: add sve parameter in XL configuration
  xen/arm: add sve property for dom0less domUs
  xen/changelog: Add SVE and "dom0" options to the changelog for Arm

 CHANGELOG.md  |   3 +
 SUPPORT.md|   6 +
 docs/man/xl.cfg.5.pod.in  |  16 ++
 docs/misc/arm/device-tree/booting.txt |  16 ++
 docs/misc/xen-command-line.pandoc |  20 +-
 tools/golang/xenlight/helpers.gen.go  |   4 +
 tools/golang/xenlight/types.gen.go|  24 +++
 tools/include/libxl.h |  11 +
 .../include/xen-tools/arm-arch-capabilities.h |  28 +++
 tools/include/xen-tools/common-macros.h   |   2 +
 tools/libs/light/libxl.c  |   1 +
 tools/libs/light/libxl_arm.c  |  33 +++
 tools/libs/light/libxl_internal.h |   1 -
 tools/libs/light/libxl_types.idl  |  23 +++
 tools/ocaml/libs/xc/xenctrl.ml|   4 +-
 tools/ocaml/libs/xc/xenctrl.mli   |   4 +-
 tools/ocaml/libs/xc/xenctrl_stubs.c   |   8 +-
 tools/python/xen/lowlevel/xc/xc.c |   8 +-
 tools/xl/xl_info.c|   8 +
 tools/xl/xl_parse.c   |   8 +
 xen/arch/arm/Kconfig  |  10 +-
 xen/arch/arm/README.LinuxPrimitives   |  11 +
 xen/arch/arm/arm64/Makefile   |   1 +
 xen/arch/arm/arm64/cpufeature.c   |   7 +-
 xen/arch/arm/arm64/domctl.c   |   4 +
 xen/arch/arm/arm64/sve-asm.S  | 195 ++
 xen/arch/arm/arm64/sve.c  | 182 
 xen/arch/arm/arm64/vfp.c  |  79 ---
 xen/arch/arm/arm64/vsysreg.c  |  41 +++-
 xen/arch/arm/cpufeature.c |   6 +-
 xen/arch/arm/domain.c |  55 -
 xen/arch/arm/domain_build.c   |  66 ++
 xen/arch/arm/include/asm/arm64/sve.h  |  72 +++
 xen/arch/arm/include/asm/arm64/sysregs.h  |   4 +
 xen/arch/arm/include/asm/arm64/vfp.h  |  12 ++
 xen/arch/arm/include/asm/cpufeature.h |  14 ++
 

[PATCH v7 01/12] xen/arm: enable SVE extension for Xen

2023-05-23 Thread Luca Fancellu
Enable Xen to handle the SVE extension, add code in cpufeature module
to handle ZCR SVE register, disable trapping SVE feature on system
boot only when SVE resources are accessed.
While there, correct coding style for the comment on coprocessor
trapping.

Now cptr_el2 is part of the domain context and it will be restored
on context switch, this is a preparation for saving the SVE context
which will be part of VFP operations, so restore it before the call
to save VFP registers.
To save an additional isb barrier, restore cptr_el2 before an
existing isb barrier and move the call for saving VFP context after
that barrier. To keep a (mostly) specularity of ctxt_switch_to()
and ctxt_switch_from(), move vfp_save_state() up in the function.

Change the KConfig entry to make ARM64_SVE symbol selectable, by
default it will be not selected.

Create sve module and sve_asm.S that contains assembly routines for
the SVE feature, this code is inspired from linux and it uses
instruction encoding to be compatible with compilers that does not
support SVE, imported instructions are documented in
README.LinuxPrimitives.

Signed-off-by: Luca Fancellu 
---
Changes from v6:
 - modified licence, add emacs block, move vfp_save_state up in the
   function, add comments to CPTR_EL2 and vfp_restore_state, don't
   use variable in init_traps(), code style fixes,
   add entries to README.LinuxPrimitives (Julien)
 - vl_to_zcr is moved into sve.c module as changes to the series led
   to its usage only inside it, remove stub for compute_max_zcr and
   rely on compiler DCE.
Changes from v5:
 - Add R-by Bertrand
Changes from v4:
 - don't use fixed types in vl_to_zcr, forgot to address that in
   v3, by mistake I changed that in patch 2, fixing now (Jan)
Changes from v3:
 - no changes
Changes from v2:
 - renamed sve_asm.S in sve-asm.S, new files should not contain
   underscore in the name (Jan)
Changes from v1:
 - Add assert to vl_to_zcr, it is never called with vl==0, but just
   to be sure it won't in the future.
Changes from RFC:
 - Moved restoring of cptr before an existing barrier (Julien)
 - Marked the feature as unsupported for now (Julien)
 - Trap and un-trap only when using SVE resources in
   compute_max_zcr() (Julien)
---
 xen/arch/arm/Kconfig | 10 ++--
 xen/arch/arm/README.LinuxPrimitives  |  9 
 xen/arch/arm/arm64/Makefile  |  1 +
 xen/arch/arm/arm64/cpufeature.c  |  7 ++-
 xen/arch/arm/arm64/sve-asm.S | 48 +++
 xen/arch/arm/arm64/sve.c | 59 
 xen/arch/arm/cpufeature.c|  6 ++-
 xen/arch/arm/domain.c| 20 +---
 xen/arch/arm/include/asm/arm64/sve.h | 27 +++
 xen/arch/arm/include/asm/arm64/sysregs.h |  1 +
 xen/arch/arm/include/asm/cpufeature.h| 14 ++
 xen/arch/arm/include/asm/domain.h|  1 +
 xen/arch/arm/include/asm/processor.h |  2 +
 xen/arch/arm/setup.c |  5 +-
 xen/arch/arm/traps.c | 27 ++-
 15 files changed, 210 insertions(+), 27 deletions(-)
 create mode 100644 xen/arch/arm/arm64/sve-asm.S
 create mode 100644 xen/arch/arm/arm64/sve.c
 create mode 100644 xen/arch/arm/include/asm/arm64/sve.h

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 239d3aed3c7f..41f45d8d1203 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -112,11 +112,15 @@ config ARM64_PTR_AUTH
  This feature is not supported in Xen.
 
 config ARM64_SVE
-   def_bool n
+   bool "Enable Scalar Vector Extension support (UNSUPPORTED)" if 
UNSUPPORTED
depends on ARM_64
help
- Scalar Vector Extension support.
- This feature is not supported in Xen.
+ Scalar Vector Extension (SVE/SVE2) support for guests.
+
+ Please be aware that currently, enabling this feature will add 
latency on
+ VM context switch between SVE enabled guests, between not-enabled SVE
+ guests and SVE enabled guests and viceversa, compared to the time
+ required to switch between not-enabled SVE guests.
 
 config ARM64_MTE
def_bool n
diff --git a/xen/arch/arm/README.LinuxPrimitives 
b/xen/arch/arm/README.LinuxPrimitives
index 1d53e6a898da..76c8df29e416 100644
--- a/xen/arch/arm/README.LinuxPrimitives
+++ b/xen/arch/arm/README.LinuxPrimitives
@@ -62,6 +62,15 @@ done
 linux/arch/arm64/lib/clear_page.S   xen/arch/arm/arm64/lib/clear_page.S
 linux/arch/arm64/lib/copy_page.Sunused in Xen
 
+-
+
+SVE assembly macro: last sync @ v6.3.0 (last commit: 457391b03803)
+
+linux/arch/arm64/include/asm/fpsimdmacros.h   
xen/arch/arm/include/asm/arm64/sve-asm.S
+
+The following macros were taken from Linux:
+_check_general_reg, _check_num, _sve_rdvl
+
 =
 arm32
 

[PATCH v7 03/12] xen/arm: Expose SVE feature to the guest

2023-05-23 Thread Luca Fancellu
When a guest is allowed to use SVE, expose the SVE features through
the identification registers.

Signed-off-by: Luca Fancellu 
Acked-by: Julien Grall 
---
Changes from v6:
 - code style fix, add A-by Julien
Changes from v5:
 - given the move of is_sve_domain() in asm/arm64/sve.h, add the
   header to vsysreg.c
 - dropping Bertrand's R-by because of the change
Changes from v4:
 - no changes
Changes from v3:
 - no changes
Changes from v2:
 - no changes
Changes from v1:
 - No changes
Changes from RFC:
 - No changes
---
 xen/arch/arm/arm64/vsysreg.c | 41 ++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
index 758750983c11..fe31f7b3827f 100644
--- a/xen/arch/arm/arm64/vsysreg.c
+++ b/xen/arch/arm/arm64/vsysreg.c
@@ -18,6 +18,8 @@
 
 #include 
 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -295,7 +297,28 @@ void do_sysreg(struct cpu_user_regs *regs,
 GENERATE_TID3_INFO(MVFR0_EL1, mvfr, 0)
 GENERATE_TID3_INFO(MVFR1_EL1, mvfr, 1)
 GENERATE_TID3_INFO(MVFR2_EL1, mvfr, 2)
-GENERATE_TID3_INFO(ID_AA64PFR0_EL1, pfr64, 0)
+
+case HSR_SYSREG_ID_AA64PFR0_EL1:
+{
+register_t guest_reg_value = guest_cpuinfo.pfr64.bits[0];
+
+if ( is_sve_domain(v->domain) )
+{
+/* 4 is the SVE field width in id_aa64pfr0_el1 */
+uint64_t mask = GENMASK(ID_AA64PFR0_SVE_SHIFT + 4 - 1,
+ID_AA64PFR0_SVE_SHIFT);
+/* sysval is the sve field on the system */
+uint64_t sysval = cpuid_feature_extract_unsigned_field_width(
+system_cpuinfo.pfr64.bits[0],
+ID_AA64PFR0_SVE_SHIFT, 4);
+guest_reg_value &= ~mask;
+guest_reg_value |= (sysval << ID_AA64PFR0_SVE_SHIFT) & mask;
+}
+
+return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, 1,
+  guest_reg_value);
+}
+
 GENERATE_TID3_INFO(ID_AA64PFR1_EL1, pfr64, 1)
 GENERATE_TID3_INFO(ID_AA64DFR0_EL1, dbg64, 0)
 GENERATE_TID3_INFO(ID_AA64DFR1_EL1, dbg64, 1)
@@ -306,7 +329,21 @@ void do_sysreg(struct cpu_user_regs *regs,
 GENERATE_TID3_INFO(ID_AA64MMFR2_EL1, mm64, 2)
 GENERATE_TID3_INFO(ID_AA64AFR0_EL1, aux64, 0)
 GENERATE_TID3_INFO(ID_AA64AFR1_EL1, aux64, 1)
-GENERATE_TID3_INFO(ID_AA64ZFR0_EL1, zfr64, 0)
+
+case HSR_SYSREG_ID_AA64ZFR0_EL1:
+{
+/*
+ * When the guest has the SVE feature enabled, the whole 
id_aa64zfr0_el1
+ * needs to be exposed.
+ */
+register_t guest_reg_value = guest_cpuinfo.zfr64.bits[0];
+
+if ( is_sve_domain(v->domain) )
+guest_reg_value = system_cpuinfo.zfr64.bits[0];
+
+return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, 1,
+  guest_reg_value);
+}
 
 /*
  * Those cases are catching all Reserved registers trapped by TID3 which
-- 
2.34.1




[PATCH v7 02/12] xen/arm: add SVE vector length field to the domain

2023-05-23 Thread Luca Fancellu
Add sve_vl field to arch_domain and xen_arch_domainconfig struct,
to allow the domain to have an information about the SVE feature
and the number of SVE register bits that are allowed for this
domain.

sve_vl field is the vector length in bits divided by 128, this
allows to use less space in the structures.

The field is used also to allow or forbid a domain to use SVE,
because a value equal to zero means the guest is not allowed to
use the feature.

Check that the requested vector length is lower or equal to the
platform supported vector length, otherwise fail on domain
creation.

Check that only 64 bit domains have SVE enabled, otherwise fail.

Signed-off-by: Luca Fancellu 
---
Changes from v6:
 - Style fix, have is_sve_domain as static inline instead of macro
   (Julien)
Changes from v5:
 - Update commit message stating the interface ver. bump (Bertrand)
 - in struct arch_domain, protect sve_vl with CONFIG_ARM64_SVE,
   given the change, move also is_sve_domain() where it's protected
   inside sve.h and create a stub when the macro is not defined,
   protect the usage of sve_vl where needed.
   (Julien)
 - Add a check for 32 bit guest running on top of 64 bit host that
   have sve parameter enabled to stop the domain creation, added in
   construct_domain() of domain_build.c and subarch_do_domctl of
   domctl.c. (Julien)
Changes from v4:
 - Return 0 in get_sys_vl_len() if sve is not supported, code style fix,
   removed else if since the conditions can't fallthrough, removed not
   needed condition checking for VL bits validity because it's already
   covered, so delete is_vl_valid() function. (Jan)
Changes from v3:
 - don't use fixed types when not needed, use encoded value also in
   arch_domain so rename sve_vl_bits in sve_vl. (Jan)
 - rename domainconfig_decode_vl to sve_decode_vl because it will now
   be used also to decode from arch_domain value
 - change sve_vl from uint16_t to uint8_t and move it after "type" field
   to optimize space.
Changes from v2:
 - rename field in xen_arch_domainconfig from "sve_vl_bits" to
   "sve_vl" and use the implicit padding after gic_version to
   store it, now this field is the VL/128. (Jan)
 - Created domainconfig_decode_vl() function to decode the sve_vl
   field and use it as plain bits value inside arch_domain.
 - Changed commit message reflecting the changes
Changes from v1:
 - no changes
Changes from RFC:
 - restore zcr_el2 in sve_restore_state, that will be introduced
   later in this serie, so remove zcr_el2 related code from this
   patch and move everything to the later patch (Julien)
 - add explicit padding into struct xen_arch_domainconfig (Julien)
 - Don't lower down the vector length, just fail to create the
   domain. (Julien)
---
 xen/arch/arm/arm64/domctl.c  |  4 
 xen/arch/arm/arm64/sve.c | 12 +++
 xen/arch/arm/domain.c| 29 ++
 xen/arch/arm/domain_build.c  |  7 +++
 xen/arch/arm/include/asm/arm64/sve.h | 31 
 xen/arch/arm/include/asm/domain.h|  5 +
 xen/include/public/arch-arm.h|  2 ++
 7 files changed, 90 insertions(+)

diff --git a/xen/arch/arm/arm64/domctl.c b/xen/arch/arm/arm64/domctl.c
index 0de89b42c448..14fc622e9956 100644
--- a/xen/arch/arm/arm64/domctl.c
+++ b/xen/arch/arm/arm64/domctl.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 static long switch_mode(struct domain *d, enum domain_type type)
@@ -43,6 +44,9 @@ long subarch_do_domctl(struct xen_domctl *domctl, struct 
domain *d,
 case 32:
 if ( !cpu_has_el1_32 )
 return -EINVAL;
+/* SVE is not supported for 32 bit domain */
+if ( is_sve_domain(d) )
+return -EINVAL;
 return switch_mode(d, DOMAIN_32BIT);
 case 64:
 return switch_mode(d, DOMAIN_64BIT);
diff --git a/xen/arch/arm/arm64/sve.c b/xen/arch/arm/arm64/sve.c
index e05ccc38a896..a9144e48ef6b 100644
--- a/xen/arch/arm/arm64/sve.c
+++ b/xen/arch/arm/arm64/sve.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -49,6 +50,17 @@ register_t compute_max_zcr(void)
 return vl_to_zcr(hw_vl);
 }
 
+/* Get the system sanitized value for VL in bits */
+unsigned int get_sys_vl_len(void)
+{
+if ( !cpu_has_sve )
+return 0;
+
+/* ZCR_ELx len field is ((len + 1) * 128) = vector bits length */
+return ((system_cpuinfo.zcr64.bits[0] & ZCR_ELx_LEN_MASK) + 1U) *
+SVE_VL_MULTIPLE_VAL;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index d5ab15db46c4..6c22551b0ed2 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -13,6 +13,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -555,6 +556,8 @@ int arch_vcpu_create(struct vcpu *v)
 v->arch.vmpidr = MPIDR_SMP | vcpuid_to_vaffinity(v->vcpu_id);
 
 v->arch.cptr_el2 = 

RE: [PATCH v4 09/17] xen/arm: introduce a helper to parse device tree NUMA distance map

2023-05-23 Thread Henry Wang
Hi Jan,

> -Original Message-
> Subject: Re: [PATCH v4 09/17] xen/arm: introduce a helper to parse device
> tree NUMA distance map
> 
>  The [2] link you provided discusses NUMA_LOCAL_DISTANCE.
> >>>
> >>> I inferred the discussion as "we should try to keep consistent between the
> >> value
> >>> used in device tree and ACPI tables". Maybe my inference is wrong.
> >>>
>  Looking at
>  Linux'es Documentation/devicetree/numa.txt, there's no mention of an
>  upper bound on the distance values. It only says that on the diagonal
>  entries should be 10 (i.e. matching ACPI, without really saying so).
> >>>
> >>> I agree that the NUMA device tree binding is a little bit vague. So I 
> >>> cannot
> >>> say the case that you provided is not valid. I would like to ask Arm
> >> maintainers
> >>> (putting them into To:) opinion on this as I think I am not the one to
> decide
> >> the
> >>> expected behavior on Arm.
> >>>
> >>> Bertrand/Julien/Stefano: Would you please kindly share your opinion on
> >> which
> >>> value should be used as the default value of the node distance map? Do
> >> you
> >>> think reusing the "unreachable" distance, i.e. 0xFF, as the default node
> >> distance
> >>> is acceptable here? Thanks!
> >>
> >> My suggestion would be to, rather than rejecting values >= 0xff, to 
> >> saturate
> >> at 0xfe, while keeping 0xff for NUMA_NO_DISTANCE (and overall keeping
> >> things
> >> consistent with ACPI).
> >
> > Since it has been a while and there were no feedback from Arm
> maintainers,
> > I would like to follow your suggestions for v5. However while I was writing
> the
> > code for the "saturation", i.e., adding below snippet in
> numa_set_distance():
> > ```
> > if ( distance > NUMA_NO_DISTANCE )
> > distance = NUMA_MAX_DISTANCE;
> > ```
> > I noticed an issue which needs your clarification:
> > Currently, the default value of the distance map is NUMA_NO_DISTANCE,
> > which indicates the nodes are not reachable. IMHO, if in device tree, we do
> > saturations like above for ACPI invalid distances (distances >= 0xff), by
> saturating
> > the distance to 0xfe, we are making the unreachable nodes to reachable. I
> am
> > not sure if this will lead to problems. Do you have any more thoughts?
> Thanks!
> 
> I can only answer this with a question back: How is "unreachable"
> represented
> in DT? 

Exactly, that is also what we I am trying to find but failed. My understanding
is that, the spec of NUMA is defined in the ACPI, and the DT NUMA binding
only specifies how users can use DT to represent the same set of ACPI data,
instead of define another standard.

By looking into the existing implementation of NUMA for DT,
in Linux, from drivers/base/arch_numa.c: numa_set_distance(), there is a
"if ((u8)distance != distance)" then return. So I think at least in Linux the
distance value is consistent with the ACPI spec. 

> Or is "unreachable" simply expressed by the absence of any data?

Maybe I am wrong but I don't think so, as in the Linux implementation,
drivers/of/of_numa.c: of_numa_parse_distance_map_v1(), the for loop
"for (i = 0; i + 2 < entry_count; i += 3)" basically implies no fields should
be omitted in the distance map entry.

Kind regards,
Henry

> 
> Jan


[xen-unstable test] 180900: tolerable FAIL - PUSHED

2023-05-23 Thread osstest service owner
flight 180900 xen-unstable real [real]
flight 180911 xen-unstable real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/180900/
http://logs.test-lab.xenproject.org/osstest/logs/180911/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-qemuu-win7-amd64 12 windows-install fail pass in 
180911-retest

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stop  fail in 180911 like 180884
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180884
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 180884
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180884
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 180884
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 180884
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 180884
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 180884
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 180884
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180884
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180884
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180884
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-i386-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-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 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-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 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-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-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 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  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-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   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-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-libvirt 15 migrate-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

version targeted for testing:
 xen  c7908869ac26961a3919491705e521179ad3fc0e
baseline version:
 xen  753d903a6f2d1e68d98487d36449b5739c28d65a

Last test of basis   180884  2023-05-22 01:52:01 

Re: [PATCH RFC] build: respect top-level .config also for out-of-tree hypervisor builds

2023-05-23 Thread Jan Beulich
On 22.05.2023 17:49, Anthony PERARD wrote:
> On Mon, May 08, 2023 at 08:23:43AM +0200, Jan Beulich wrote:
>> On 05.05.2023 18:08, Anthony PERARD wrote:
>>> On Wed, Mar 15, 2023 at 03:58:59PM +0100, Jan Beulich wrote:
 With in-tree builds Config.mk includes a .config file (if present) from
 the top-level directory. Similar functionality is wanted with out-of-
 tree builds. Yet the concept of "top-level directory" becomes fuzzy in
 that case, because there is not really a requirement to have identical
 top-level directory structure in the output tree; in fact there's no
 need for anything top-level-ish there. Look for such a .config, but only
 if the tree layout matches (read: if the directory we're building in is
 named "xen").
>>>
>>> Well, as long as the "xen/" part of the repository is the only build
>>> system to be able to build out-of-srctree, there isn't going to be a
>>> top-level .config possible in the build tree, as such .config will be
>>> outside of the build tree. Reading outside of the build tree or source
>>> tree might be problematic.
>>>
>>> It's a possibility that some project might want to just build the
>>> hypervisor, and they happened to name the build tree "xen", but they
>>> also have a ".config" use for something else. Kconfig is using ".config"
>>> for other reason for example, like we do to build Xen.
>>
>> Right, that's what my first RFC remark is about.
>>
>>> It might be better to have a different name instead of ".config", and
>>> putting it in the build tree rather than the parent directory. Maybe
>>> ".xenbuild-config" ?
>>
>> Using a less ambiguous name is clearly an option. Putting the file in
>> the (Xen) build directory, otoh, imo isn't: In the hope that further
>> sub-trees would be enabled to build out-of-tree (qemu for instance in
>> principle can already, and I guess at least some of stubdom's sub-
>> packages also can), the present functionality of the top-level
>> .config (or whatever its new name) also controlling those builds would
>> better be retained.
> 
> I'm not sure what out-of-tree build for the whole tree will look like,
> but it probably going to be `/path/to/configure && make`. After that,
> Config.mk should know what kind of build it's doing, and probably only
> load ".config" in the build tree.

Except that the hypervisor build still isn't really connected to
./configure's results.

> In the meantime, it feels out of place
> for xen/Makefile or xen/Rules.mk to load a ".config" that would be
> present in the parent directory of the build dir.

Right, hence me looking for possible alternatives (and using this
approach only for the apparent lack thereof).

>>> I've been using the optional makefile named "xen-version" to adjust
>>> variable of the build system, with content like:
>>>
>>> export XEN_TARGET_ARCH=arm64
>>> export CROSS_COMPILE=aarch64-linux-gnu-
>>>
>>> so that I can have multiple build tree for different arch, and not have
>>> to do anything other than running make and do the expected build. Maybe
>>> that's not possible because for some reason, the build system still
>>> reconfigure some variable when entering a sub-directory, but that's a
>>> start.
>>
>> Hmm, interesting idea. I could (ab)use this at least in the short
>> term. Yet even then the file would further need including from
>> Rules.mk. Requiring all variables defined there to be exported isn't
>> a good idea, imo. Iirc not all make variables can usefully be
>> exported. For example, I have a local extension to CROSS_COMPILE in
>> place, which uses a variable with a dash in its name.
>>
 Signed-off-by: Jan Beulich 
 ---
 RFC: The directory name based heuristic of course isn't nice. But I
  couldn't think of anything better. Suggestions?
>>>
>>> I can only thing of looking at a file that is in the build-tree rather
>>> than outside of it. A file named ".xenbuild-config" proposed early for
>>> example.
>>>
 RFC: There also being a .config in the top-level source dir would be a
  little problematic: It would be included _after_ the one in the
  object tree. Yet if such a scenario is to be expected/supported at
  all, it makes more sense the other way around.
>>>
>>> Well, that would mean teaching Config.mk about out-of-tree build that
>>> part of the repository is capable of, or better would be to stop trying
>>> to read ".config" from Config.mk, and handle the file in the different
>>> sub-build systems.
>>
>> Hmm, is that a viable option? Or put differently: Wouldn't this mean doing
>> away with ./Config.mk altogether? Which in turn would mean no central
>> place anymore where XEN_TARGET_ARCH and friends could be overridden. (I
>> view this as a capability that we want to retain, if nothing else then for
>> the - see above - future option of allowing more than just xen/ to be
>> built out-of-tree.)
> 
> No, I'm not trying to get rid of Config.mk. There's a few thing in it
> that I'd like to 

Re: PVH Dom0 related UART failure

2023-05-23 Thread Jan Beulich
On 23.05.2023 00:20, Stefano Stabellini wrote:
> On Sat, 20 May 2023, Roger Pau Monné wrote:
>> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
>> index ec2e978a4e6b..0ff8e940fa8d 100644
>> --- a/xen/drivers/vpci/header.c
>> +++ b/xen/drivers/vpci/header.c
>> @@ -289,6 +289,13 @@ static int modify_bars(const struct pci_dev *pdev, 
>> uint16_t cmd, bool rom_only)
>>   */
>>  for_each_pdev ( pdev->domain, tmp )
>>  {
>> +if ( !tmp->vpci )
>> +{
>> +printk(XENLOG_G_WARNING "%pp: not handled by vPCI for %pd\n",
>> +   >sbdf, pdev->domain);
>> +continue;
>> +}
>> +
>>  if ( tmp == pdev )
>>  {
>>  /*
>> diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
>> index 652807a4a454..0baef3a8d3a1 100644
>> --- a/xen/drivers/vpci/vpci.c
>> +++ b/xen/drivers/vpci/vpci.c
>> @@ -72,7 +72,12 @@ int vpci_add_handlers(struct pci_dev *pdev)
>>  unsigned int i;
>>  int rc = 0;
>>  
>> -if ( !has_vpci(pdev->domain) )
>> +if ( !has_vpci(pdev->domain) ||
>> + /*
>> +  * Ignore RO and hidden devices, those are in use by Xen and vPCI
>> +  * won't work on them.
>> +  */
>> + pci_get_pdev(dom_xen, pdev->sbdf) )
>>  return 0;
>>  
>>  /* We should not get here twice for the same device. */
> 
> 
> Now this patch works! Thank you!! :-)
> 
> You can check the full logs here
> https://gitlab.com/xen-project/people/sstabellini/xen/-/jobs/4329259080
> 
> Is the patch ready to be upstreamed aside from the commit message?

I don't think so. vPCI ought to work on "r/o" devices. Out of curiosity,
have you also tried my (hackish and hence RFC) patch [1]?

Jan

[1] https://lists.xen.org/archives/html/xen-devel/2021-08/msg01489.html



Re: [PATCH v4 09/17] xen/arm: introduce a helper to parse device tree NUMA distance map

2023-05-23 Thread Jan Beulich
On 23.05.2023 08:31, Henry Wang wrote:
>> -Original Message-
>> Subject: Re: [PATCH v4 09/17] xen/arm: introduce a helper to parse device
>> tree NUMA distance map
>>
>>> +/* The default value in node_distance_map is
 NUMA_NO_DISTANCE
>> */
>>> +if ( opposite == NUMA_NO_DISTANCE )
>>
>> And the matrix you're reading from can't hold NUMA_NO_DISTANCE
 entries?
>> I ask because you don't check this above; you only check against
>> NUMA_LOCAL_DISTANCE.
>
> My understanding for the purpose of this part of code is to check if the
 opposite
> way distance has already been set, so we need to compare the opposite
 way
> distance with the default value NUMA_NO_DISTANCE here.
>
> Back to your question, I can see your point of the question. However I
>> don't
 think
> NUMA_NO_DISTANCE is a valid value to describe the node distance in the
 device
> tree. This is because I hunted down the previous discussions and found
>> [2]
 about
> we should try to keep consistent between the value used in device tree
>> and
 ACPI
> tables. From the ACPI spec, 0xFF, i.e. NUMA_NO_DISTANCE means
 unreachable.
> I think this is also the reason why NUMA_NO_DISTANCE can be used as
>> the
 default
> value of the distance map, otherwise we won't have any value to use.

 The [2] link you provided discusses NUMA_LOCAL_DISTANCE.
>>>
>>> I inferred the discussion as "we should try to keep consistent between the
>> value
>>> used in device tree and ACPI tables". Maybe my inference is wrong.
>>>
 Looking at
 Linux'es Documentation/devicetree/numa.txt, there's no mention of an
 upper bound on the distance values. It only says that on the diagonal
 entries should be 10 (i.e. matching ACPI, without really saying so).
>>>
>>> I agree that the NUMA device tree binding is a little bit vague. So I cannot
>>> say the case that you provided is not valid. I would like to ask Arm
>> maintainers
>>> (putting them into To:) opinion on this as I think I am not the one to 
>>> decide
>> the
>>> expected behavior on Arm.
>>>
>>> Bertrand/Julien/Stefano: Would you please kindly share your opinion on
>> which
>>> value should be used as the default value of the node distance map? Do
>> you
>>> think reusing the "unreachable" distance, i.e. 0xFF, as the default node
>> distance
>>> is acceptable here? Thanks!
>>
>> My suggestion would be to, rather than rejecting values >= 0xff, to saturate
>> at 0xfe, while keeping 0xff for NUMA_NO_DISTANCE (and overall keeping
>> things
>> consistent with ACPI).
> 
> Since it has been a while and there were no feedback from Arm maintainers,
> I would like to follow your suggestions for v5. However while I was writing 
> the
> code for the "saturation", i.e., adding below snippet in numa_set_distance():
> ```
> if ( distance > NUMA_NO_DISTANCE )
> distance = NUMA_MAX_DISTANCE;
> ```
> I noticed an issue which needs your clarification:
> Currently, the default value of the distance map is NUMA_NO_DISTANCE,
> which indicates the nodes are not reachable. IMHO, if in device tree, we do
> saturations like above for ACPI invalid distances (distances >= 0xff), by 
> saturating
> the distance to 0xfe, we are making the unreachable nodes to reachable. I am
> not sure if this will lead to problems. Do you have any more thoughts? Thanks!

I can only answer this with a question back: How is "unreachable" represented
in DT? Or is "unreachable" simply expressed by the absence of any data?

Jan



Re: [PATCH v2 00/10] x86: support AVX512-FP16

2023-05-23 Thread Jan Beulich
On 22.05.2023 18:25, Andrew Cooper wrote:
> On 03/04/2023 3:56 pm, Jan Beulich wrote:
>> While I (quite obviously) don't have any suitable hardware, Intel's
>> SDE allows testing the implementation. And since there's no new
>> state (registers) associated with this ISA extension, this should
>> suffice for integration.
> 
> I've given this a spin on a Sapphire Rapids system.
> 
> Relevant (AFAICT) bits of the log:
> 
> Testing vfpclasspsz $0x46,64(%edx),%k2...okay
> Testing vfpclassphz $0x46,128(%ecx),%k3...okay
> ...
> Testing avx512_fp16/all disp8 handling...okay
> Testing avx512_fp16/128 disp8 handling...okay
> ...
> Testing AVX512_FP16 f16 scal native execution...okay
> Testing AVX512_FP16 f16 scal 64-bit code sequence...okay
> Testing AVX512_FP16 f16 scal 32-bit code sequence...okay
> Testing AVX512_FP16 f16x32 native execution...okay
> Testing AVX512_FP16 f16x32 64-bit code sequence...okay
> Testing AVX512_FP16 f16x32 32-bit code sequence...okay
> Testing AVX512_FP16+VL f16x8 native execution...okay
> Testing AVX512_FP16+VL f16x8 64-bit code sequence...okay
> Testing AVX512_FP16+VL f16x8 32-bit code sequence...okay
> Testing AVX512_FP16+VL f16x16 native execution...okay
> Testing AVX512_FP16+VL f16x16 64-bit code sequence...okay
> Testing AVX512_FP16+VL f16x16 32-bit code sequence...okay
> 
> and it exits zero, so everything seems fine.
> 
> 
> One thing however, this series ups the minimum GCC version required to
> build the emulator at all:
> 
> make: Entering directory '/local/xen.git/tools/tests/x86_emulator'
> gcc: error: unrecognized command-line option ‘-mavx512fp16’; did you
> mean ‘-mavx512bf16’?
> Makefile:121: Test harness not built, use newer compiler than "gcc"
> (version 10) and an "{evex}" capable assembler
> 
> and I'm not sure we want to do this.  When upping the version of GCC but
> leaving binutils as-was does lead to a build of the harness without
> AVX512-FP16 active, which is the preferred behaviour here.

Well, this series on its own does, but I did notice the issue already.
Hence "x86emul: rework compiler probing in the test harness" [1].

Jan

[1] https://lists.xen.org/archives/html/xen-devel/2023-03/msg00123.html



Re: [XEN PATCH v8 03/22] tools: add Arm FF-A mediator

2023-05-23 Thread Jens Wiklander
On Thu, May 18, 2023 at 4:35 PM Anthony PERARD
 wrote:
>
> On Thu, Apr 13, 2023 at 09:14:05AM +0200, Jens Wiklander wrote:
> > Adds a new "ffa" value to the Enumeration "tee_type" to indicate if a
> > guest is trusted to use FF-A.
>
> Is "ffa" working yet in the hypervisor? (At this point in the patch
> series) I'm asking because the doc change is at the end of the patch
> series and this patch at the beginning.
>
> I feel like this patch would be better at the end of the series, just
> before the doc change, when the hypervisor is ready for it.

Makes sense, I'll move it.

>
> In any case:
> Acked-by: Anthony PERARD 

Thanks,
Jens

>
> Thanks,
>
> --
> Anthony PERARD



RE: [PATCH v4 09/17] xen/arm: introduce a helper to parse device tree NUMA distance map

2023-05-23 Thread Henry Wang
Hi Jan,

> -Original Message-
> Subject: Re: [PATCH v4 09/17] xen/arm: introduce a helper to parse device
> tree NUMA distance map
> 
> > +/* The default value in node_distance_map is
> >> NUMA_NO_DISTANCE
>  */
> > +if ( opposite == NUMA_NO_DISTANCE )
> 
>  And the matrix you're reading from can't hold NUMA_NO_DISTANCE
> >> entries?
>  I ask because you don't check this above; you only check against
>  NUMA_LOCAL_DISTANCE.
> >>>
> >>> My understanding for the purpose of this part of code is to check if the
> >> opposite
> >>> way distance has already been set, so we need to compare the opposite
> >> way
> >>> distance with the default value NUMA_NO_DISTANCE here.
> >>>
> >>> Back to your question, I can see your point of the question. However I
> don't
> >> think
> >>> NUMA_NO_DISTANCE is a valid value to describe the node distance in the
> >> device
> >>> tree. This is because I hunted down the previous discussions and found
> [2]
> >> about
> >>> we should try to keep consistent between the value used in device tree
> and
> >> ACPI
> >>> tables. From the ACPI spec, 0xFF, i.e. NUMA_NO_DISTANCE means
> >> unreachable.
> >>> I think this is also the reason why NUMA_NO_DISTANCE can be used as
> the
> >> default
> >>> value of the distance map, otherwise we won't have any value to use.
> >>
> >> The [2] link you provided discusses NUMA_LOCAL_DISTANCE.
> >
> > I inferred the discussion as "we should try to keep consistent between the
> value
> > used in device tree and ACPI tables". Maybe my inference is wrong.
> >
> >> Looking at
> >> Linux'es Documentation/devicetree/numa.txt, there's no mention of an
> >> upper bound on the distance values. It only says that on the diagonal
> >> entries should be 10 (i.e. matching ACPI, without really saying so).
> >
> > I agree that the NUMA device tree binding is a little bit vague. So I cannot
> > say the case that you provided is not valid. I would like to ask Arm
> maintainers
> > (putting them into To:) opinion on this as I think I am not the one to 
> > decide
> the
> > expected behavior on Arm.
> >
> > Bertrand/Julien/Stefano: Would you please kindly share your opinion on
> which
> > value should be used as the default value of the node distance map? Do
> you
> > think reusing the "unreachable" distance, i.e. 0xFF, as the default node
> distance
> > is acceptable here? Thanks!
> 
> My suggestion would be to, rather than rejecting values >= 0xff, to saturate
> at 0xfe, while keeping 0xff for NUMA_NO_DISTANCE (and overall keeping
> things
> consistent with ACPI).

Since it has been a while and there were no feedback from Arm maintainers,
I would like to follow your suggestions for v5. However while I was writing the
code for the "saturation", i.e., adding below snippet in numa_set_distance():
```
if ( distance > NUMA_NO_DISTANCE )
distance = NUMA_MAX_DISTANCE;
```
I noticed an issue which needs your clarification:
Currently, the default value of the distance map is NUMA_NO_DISTANCE,
which indicates the nodes are not reachable. IMHO, if in device tree, we do
saturations like above for ACPI invalid distances (distances >= 0xff), by 
saturating
the distance to 0xfe, we are making the unreachable nodes to reachable. I am
not sure if this will lead to problems. Do you have any more thoughts? Thanks!

Kind regards,
Henry

> 
> Jan