[ovmf test] 157018: all pass - PUSHED

[osstest service owner]

flight 157018 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/157018/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf e9d62effa37ea13fe04fc89b21d2de7776f183a2
baseline version:
 ovmf 388f9a9355ae7ab95a34ac2e9a3c608caf11b74a

Last test of basis   157012  2020-11-25 18:09:50 Z0 days
Testing same since   157018  2020-11-26 01:39:48 Z0 days1 attempts


People who touched revisions under test:
  gaoliming 
  Jiewen Yao 
  Liming Gao 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass



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

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

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   388f9a9355..e9d62effa3  e9d62effa37ea13fe04fc89b21d2de7776f183a2 -> 
xen-tested-master

[linux-linus test] 156996: regressions - FAIL

[osstest service owner]

flight 156996 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/156996/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemuu-ws16-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-xl-xsm7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-install fail REGR. vs. 
152332
 test-amd64-i386-xl-qemut-debianhvm-amd64  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-qemuu-rhel6hvm-intel  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332
 test-amd64-i386-examine   6 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemut-ws16-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-libvirt   7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-debianhvm-amd64  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-libvirt-xsm   7 xen-install  fail REGR. vs. 152332
 test-amd64-coresched-i386-xl  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-install fail REGR. vs. 
152332
 test-amd64-i386-qemuu-rhel6hvm-amd  7 xen-installfail REGR. vs. 152332
 test-amd64-i386-qemut-rhel6hvm-amd  7 xen-installfail REGR. vs. 152332
 test-amd64-i386-pair 10 xen-install/src_host fail REGR. vs. 152332
 test-amd64-i386-pair 11 xen-install/dst_host fail REGR. vs. 152332
 test-amd64-i386-xl-raw7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-freebsd10-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-xl-pvshim 7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332
 test-amd64-i386-xl-shadow 7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-freebsd10-i386  7 xen-installfail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-ovmf-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-xl-qemut-win7-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-win7-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-install fail REGR. 
vs. 152332
 test-amd64-i386-libvirt-pair 10 xen-install/src_host fail REGR. vs. 152332
 test-amd64-i386-libvirt-pair 11 xen-install/dst_host fail REGR. vs. 152332
 test-arm64-arm64-xl-xsm  12 debian-install   fail REGR. vs. 152332
 test-arm64-arm64-xl  10 host-ping-check-xen  fail REGR. vs. 152332
 test-arm64-arm64-examine  8 reboot   fail REGR. vs. 152332
 test-amd64-amd64-amd64-pvgrub 20 guest-stop  fail REGR. vs. 152332
 test-amd64-amd64-i386-pvgrub 20 guest-stop   fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-install fail REGR. 
vs. 152332
 test-amd64-i386-xl7 xen-install  fail REGR. vs. 152332
 test-arm64-arm64-libvirt-xsm  8 xen-boot fail REGR. vs. 152332

Tests which are failing intermittently (not blocking):
 test-arm64-arm64-xl-credit1   8 xen-boot fail in 156981 pass in 156996
 test-amd64-amd64-examine4 memdisk-try-append fail in 156981 pass in 156996
 test-arm64-arm64-xl-xsm   8 xen-boot fail in 156981 pass in 156996
 test-arm64-arm64-xl-seattle  10 host-ping-check-xenfail pass in 156981
 test-arm64-arm64-xl-credit2   8 xen-boot   fail pass in 156981

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl-credit1  11 leak-check/basis(11)fail blocked in 152332
 test-arm64-arm64-xl-seattle 11 leak-check/basis(11) fail in 156981 blocked in 
152332
 test-arm64-arm64-xl-credit2 11 leak-check/basis(11) fail in 156981 blocked in 
152332
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 152332
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 152332
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 152332
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 152332
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 152332
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 152332
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 152332
 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-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Edward Cree]

On 25/11/2020 00:32, Miguel Ojeda wrote:
> I have said *authoring* lines of *this* kind takes a minute per line.
> Specifically: lines fixing the fallthrough warning mechanically and
> repeatedly where the compiler tells you to, and doing so full-time for
> a month.

> It is useful since it makes intent clear.
To make the intent clear, you have to first be certain that you
 understand the intent; otherwise by adding either a break or a
 fallthrough to suppress the warning you are just destroying the
 information that "the intent of this code is unknown".
Figuring out the intent of a piece of unfamiliar code takes more
 than 1 minute; just because
case foo:
thing;
case bar:
break;
 produces identical code to
case foo:
thing;
break;
case bar:
break;
 doesn't mean that *either* is correct — maybe the author meant
 to write
case foo:
return thing;
case bar:
break;
 and by inserting that break you've destroyed the marker that
 would direct someone who knew what the code was about to look
 at that point in the code and spot the problem.
Thus, you *always* have to look at more than just the immediate
 mechanical context of the code, to make a proper judgement that
 yes, this was the intent.  If you think that that sort of thing
 can be done in an *average* time of one minute, then I hope you
 stay away from code I'm responsible for!
One minute would be an optimistic target for code that, as the
 maintainer, one is already somewhat familiar with.  For code
 that you're seeing for the first time, as is usually the case
 with the people doing these mechanical fix-a-warning patches,
 it's completely unrealistic.

A warning is only useful because it makes you *think* about the
 code.  If you suppress the warning without doing that thinking,
 then you made the warning useless; and if the warning made you
 think about code that didn't *need* it, then the warning was
 useless from the start.

So make your mind up: does Clang's stricter -Wimplicit-fallthrough
 flag up code that needs thought (in which case the fixes take
 effort both to author and to review) or does it flag up code
 that can be mindlessly "fixed" (in which case the warning is
 worthless)?  Proponents in this thread seem to be trying to
 have it both ways.

-ed

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Edward Cree]

On 24/11/2020 21:25, Kees Cook wrote:
> I still think this isn't right -- it's a case statement that runs off
> the end without an explicit flow control determination.

Proves too much — for instance
case foo:
case bar:
thing;
break;
 doesn't require a fallthrough; after case foo:, and afaik
 no-one is suggesting it should.  Yet it, too, is "a case
 statement that runs off the end without an explicit flow
 control determination".

-ed

Re: Xen on RP4

[Elliott Mitchell]

On Wed, Nov 25, 2020 at 10:57:31AM -0800, Stefano Stabellini wrote:
> On Tue, 24 Nov 2020, Elliott Mitchell wrote:
> > My testing section for Xen is:
> > xen_hypervisor /boot/xen-4.14-arm64.efi
> > xen_module /boot/vmlinuz-5.8.10-2rp4-6.1.7 
> > root=UUID=01234567-dead-beef-d13d-456789abcdef ro
> > devicetree /boot/dtb-5.8.10-2rp4-6.1.7
> > xen_module --nounzip /boot/initrd.img-5.8.10-2rp4-6.1.7
> > 
> > I've frankly got no idea how to ensure the correct device-tree is passed
> > to Xen.  Is GRUB's `devicetree` command correct when loading Xen?  Is a
> > device-tree matched to the Linux kernel appropriate for Xen?
> > 
> > (I'm guessing the second is "yes", but the first I don't have a clue)
> 
> Yes, devicetree is correct. I have not used the graphical output, so I
> cannot help you there but yes the best bet is to use the devicetree that
> comes with the kernel.

Well, I've now got everything together for a "proper" Debian Raspberry PI
installation.  Apparently since 5.2 (perhaps earlier, but 5.2 is
confirmed), Debian's kernel source packages have had their Raspberry PI
device-trees garbled.  I do have full untouched Linux kernel source
handy, but I tend to stick with the distribution until that proves
untenable.


> One thing I noticed is that you are missing some of the command line
> arguments for Xen and Linux in your grub config. For instance on the Xen
> line you want to have something like:
> 
> dom0_mem=1024M console=dtuart sync_console
> 
> And on the Linux line you might want to have:
> 
> console=tty0 console=hvc0

This plus adding the devicetree setting now turns into a report
elsewhere...


-- 
(\___(\___(\__  --=> 8-) EHM <=--  __/)___/)___/)
 \BS (| ehem+sig...@m5p.com  PGP 87145445 |)   /
  \_CS\   |  _  -O #include  O-   _  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445

[qemu-mainline test] 156994: regressions - FAIL

[osstest service owner]

flight 156994 qemu-mainline real [real]
flight 157019 qemu-mainline real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/156994/
http://logs.test-lab.xenproject.org/osstest/logs/157019/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-libvirt-vhd 19 guest-start/debian.repeat fail REGR. vs. 152631
 test-amd64-amd64-xl-qcow2   21 guest-start/debian.repeat fail REGR. vs. 152631
 test-armhf-armhf-xl-vhd 17 guest-start/debian.repeat fail REGR. vs. 152631

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds 20 guest-localmigrate/x10   fail REGR. vs. 152631

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

version targeted for testing:
 qemuucef64a0b347b8fda1a6f8b65ce435d4569c3969e
baseline version:
 qemuu1d806cef0e38b5db8347a8e12f214d543204a314

Last test of basis   152631  2020-08-20 09:07:46 Z   97 days
Failing since152659  2020-08-21 14:07:39 Z   96 days  203 attempts
Testing same since   156994  2020-11-24 18:09:41 Z1 days1 attempts


People who touched 

RE: [PATCH] xen/arm: Add Cortex-A73 erratum 858921 workaround

[Wei Chen]

Hi Stefano,

> -Original Message-
> From: Stefano Stabellini 
> Sent: 2020年11月26日 8:00
> To: Wei Chen 
> Cc: Julien Grall ; Penny Zheng ; xen-
> de...@lists.xenproject.org; sstabell...@kernel.org; Andre Przywara
> ; Bertrand Marquis ;
> Kaly Xin ; nd 
> Subject: RE: [PATCH] xen/arm: Add Cortex-A73 erratum 858921 workaround
> 
> Resuming this old thread.
> 
> On Fri, 13 Nov 2020, Wei Chen wrote:
> > > Hi,
> > >
> > > On 09/11/2020 08:21, Penny Zheng wrote:
> > > > CNTVCT_EL0 or CNTPCT_EL0 counter read in Cortex-A73 (all versions)
> > > > might return a wrong value when the counter crosses a 32bit boundary.
> > > >
> > > > Until now, there is no case for Xen itself to access CNTVCT_EL0,
> > > > and it also should be the Guest OS's responsibility to deal with
> > > > this part.
> > > >
> > > > But for CNTPCT, there exists several cases in Xen involving reading
> > > > CNTPCT, so a possible workaround is that performing the read twice,
> > > > and to return one or the other depending on whether a transition has
> > > > taken place.
> > > >
> > > > Signed-off-by: Penny Zheng 
> > >
> > > Acked-by: Julien Grall 
> > >
> > > On a related topic, do we need a fix similar to Linux commit
> > > 75a19a0202db "arm64: arch_timer: Ensure counter register reads occur
> > > with seqlock held"?
> > >
> >
> > I take a look at this Linux commit, it seems to prevent the seq-lock to be
> > speculated.  Using an enforce ordering instead of ISB after the read counter
> > operation seems to be for performance reasons.
> >
> > I have found that you had placed an ISB before read counter in get_cycles
> > to prevent counter value to be speculated. But you haven't placed the second
> > ISB after reading. Is it because we haven't used the get_cycles in seq lock
> > critical context (Maybe I didn't find the right place)? So should we need 
> > to fix it
> > now, or you prefer to fix it now for future usage?
> 
> Looking at the call sites, it doesn't look like we need any ISB after
> get_cycles as it is not used in any critical context. There is also a
> data dependency with the value returned by it.
> 
> So I am thinking we don't need any fix. At most we need an in-code comment?

I agree with you to add an in-code comment. It will remind us in future when we
use the get_cycles in critical context. Adding it now will probably only lead to
meaningless performance degradation. Because Xen may never use it in a similar
scenario.

BTW: 
Can we send a patch that just contains a pure comment : )

Regards,
Wei Chen

Re: [PATCH 61/78] zram: do not call set_blocksize

[Minchan Kim]

On Mon, Nov 16, 2020 at 03:57:52PM +0100, Christoph Hellwig wrote:
> set_blocksize is used by file systems to use their preferred buffer cache
> block size.  Block drivers should not set it.
> 
> Signed-off-by: Christoph Hellwig 
Acked-by: Minchan Kim 

Thanks.

> ---
>  drivers/block/zram/zram_drv.c | 11 +--
>  drivers/block/zram/zram_drv.h |  1 -
>  2 files changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 3641434a9b154d..d00b5761ec0b21 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -403,13 +403,10 @@ static void reset_bdev(struct zram *zram)
>   return;
>  
>   bdev = zram->bdev;
> - if (zram->old_block_size)
> - set_blocksize(bdev, zram->old_block_size);
>   blkdev_put(bdev, FMODE_READ|FMODE_WRITE|FMODE_EXCL);
>   /* hope filp_close flush all of IO */
>   filp_close(zram->backing_dev, NULL);
>   zram->backing_dev = NULL;
> - zram->old_block_size = 0;
>   zram->bdev = NULL;
>   zram->disk->fops = _devops;
>   kvfree(zram->bitmap);
> @@ -454,7 +451,7 @@ static ssize_t backing_dev_store(struct device *dev,
>   struct file *backing_dev = NULL;
>   struct inode *inode;
>   struct address_space *mapping;
> - unsigned int bitmap_sz, old_block_size = 0;
> + unsigned int bitmap_sz;
>   unsigned long nr_pages, *bitmap = NULL;
>   struct block_device *bdev = NULL;
>   int err;
> @@ -509,14 +506,8 @@ static ssize_t backing_dev_store(struct device *dev,
>   goto out;
>   }
>  
> - old_block_size = block_size(bdev);
> - err = set_blocksize(bdev, PAGE_SIZE);
> - if (err)
> - goto out;
> -
>   reset_bdev(zram);
>  
> - zram->old_block_size = old_block_size;
>   zram->bdev = bdev;
>   zram->backing_dev = backing_dev;
>   zram->bitmap = bitmap;
> diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> index f2fd46daa76045..712354a4207c77 100644
> --- a/drivers/block/zram/zram_drv.h
> +++ b/drivers/block/zram/zram_drv.h
> @@ -118,7 +118,6 @@ struct zram {
>   bool wb_limit_enable;
>   u64 bd_wb_limit;
>   struct block_device *bdev;
> - unsigned int old_block_size;
>   unsigned long *bitmap;
>   unsigned long nr_pages;
>  #endif
> -- 
> 2.29.2
> 

Re: [PATCH 60/78] zram: remove the claim mechanism

[Minchan Kim]

On Mon, Nov 16, 2020 at 03:57:51PM +0100, Christoph Hellwig wrote:
> The zram claim mechanism was added to ensure no new opens come in
> during teardown.  But the proper way to archive that is to call
> del_gendisk first, which takes care of all that.  Once del_gendisk
> is called in the right place, the reset side can also be simplified
> as no I/O can be outstanding on a block device that is not open.

It would be great if it makes the mess simple. Let me have a question
Please see below.

> 
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/block/zram/zram_drv.c | 76 ++-
>  1 file changed, 21 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 6d15d51cee2b7e..3641434a9b154d 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -1756,64 +1756,33 @@ static ssize_t disksize_store(struct device *dev,
>  static ssize_t reset_store(struct device *dev,
>   struct device_attribute *attr, const char *buf, size_t len)
>  {
> - int ret;
> - unsigned short do_reset;
> - struct zram *zram;
> + struct zram *zram = dev_to_zram(dev);
>   struct block_device *bdev;
> + unsigned short do_reset;
> + int ret = 0;
>  
>   ret = kstrtou16(buf, 10, _reset);
>   if (ret)
>   return ret;
> -
>   if (!do_reset)
>   return -EINVAL;
>  
> - zram = dev_to_zram(dev);
>   bdev = bdget_disk(zram->disk, 0);
>   if (!bdev)
>   return -ENOMEM;
>  
>   mutex_lock(>bd_mutex);
> - /* Do not reset an active device or claimed device */
> - if (bdev->bd_openers || zram->claim) {
> - mutex_unlock(>bd_mutex);
> - bdput(bdev);
> - return -EBUSY;
> - }
> -
> - /* From now on, anyone can't open /dev/zram[0-9] */
> - zram->claim = true;
> + if (bdev->bd_openers)
> + ret = -EBUSY;
> + else
> + zram_reset_device(zram);
>   mutex_unlock(>bd_mutex);
> -
> - /* Make sure all the pending I/O are finished */
> - fsync_bdev(bdev);
> - zram_reset_device(zram);
>   bdput(bdev);
>  
> - mutex_lock(>bd_mutex);
> - zram->claim = false;
> - mutex_unlock(>bd_mutex);
> -
> - return len;
> -}
> -
> -static int zram_open(struct block_device *bdev, fmode_t mode)
> -{
> - int ret = 0;
> - struct zram *zram;
> -
> - WARN_ON(!mutex_is_locked(>bd_mutex));
> -
> - zram = bdev->bd_disk->private_data;
> - /* zram was claimed to reset so open request fails */
> - if (zram->claim)
> - ret = -EBUSY;
> -
> - return ret;
> + return ret ? ret : len;
>  }
>  
>  static const struct block_device_operations zram_devops = {
> - .open = zram_open,
>   .submit_bio = zram_submit_bio,
>   .swap_slot_free_notify = zram_slot_free_notify,
>   .rw_page = zram_rw_page,
> @@ -1821,7 +1790,6 @@ static const struct block_device_operations zram_devops 
> = {
>  };
>  
>  static const struct block_device_operations zram_wb_devops = {
> - .open = zram_open,
>   .submit_bio = zram_submit_bio,
>   .swap_slot_free_notify = zram_slot_free_notify,
>   .owner = THIS_MODULE
> @@ -1972,34 +1940,32 @@ static int zram_add(void)
>   return ret;
>  }
>  
> -static int zram_remove(struct zram *zram)
> +static bool zram_busy(struct zram *zram)
>  {
>   struct block_device *bdev;
> + bool busy = false;
>  
>   bdev = bdget_disk(zram->disk, 0);
> - if (!bdev)
> - return -ENOMEM;
> -
> - mutex_lock(>bd_mutex);
> - if (bdev->bd_openers || zram->claim) {
> - mutex_unlock(>bd_mutex);
> + if (bdev) {
> + if (bdev->bd_openers)
> + busy = true;
>   bdput(bdev);
> - return -EBUSY;
>   }
>  
> - zram->claim = true;
> - mutex_unlock(>bd_mutex);
> + return busy;
> +}
>  
> - zram_debugfs_unregister(zram);
> +static int zram_remove(struct zram *zram)
> +{
> + if (zram_busy(zram))
> + return -EBUSY;
>  
> - /* Make sure all the pending I/O are finished */
> - fsync_bdev(bdev);
> + del_gendisk(zram->disk);
> + zram_debugfs_unregister(zram);
>   zram_reset_device(zram);
> - bdput(bdev);
>  
>   pr_info("Removed device: %s\n", zram->disk->disk_name);
>  
> - del_gendisk(zram->disk);
>   blk_cleanup_queue(zram->disk->queue);
>   put_disk(zram->disk);
>   kfree(zram);
> -- 
> 2.29.2
> 

With this patch, how deal with the race?

CPU 1 CPU 2

hot_remove_store
  zram_remove
zram_busy
  return -EBUSY
 open /dev/zram0
del_gendisk
zram_reset and destroy

Re: [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang

[Finn Thain]

On Wed, 25 Nov 2020, Nick Desaulniers wrote:

> On Wed, Nov 25, 2020 at 1:33 PM Finn Thain  wrote:
> >
> > Or do you think that a codebase can somehow satisfy multiple checkers 
> > and their divergent interpretations of the language spec?
> 
> Have we found any cases yet that are divergent? I don't think so. 

You mean, aside from -Wimplicit-fallthrough? I'm glad you asked. How about 
-Wincompatible-pointer-types and -Wframe-larger-than?

All of the following files have been affected by divergent diagnostics 
produced by clang and gcc.

arch/arm64/include/asm/neon-intrinsics.h
arch/powerpc/xmon/Makefile
drivers/gpu/drm/i915/Makefile
drivers/gpu/drm/i915/i915_utils.h
drivers/staging/media/atomisp/pci/atomisp_subdev.c
fs/ext4/super.c
include/trace/events/qla.h
net/mac80211/rate.c
tools/lib/string.c
tools/perf/util/setup.py
tools/scripts/Makefile.include

And if I searched for 'smatch' or 'coverity' instead of 'clang' I'd 
probably find more divergence.

Here are some of the relevant commits.

0738c8b5915c7eaf1e6007b441008e8f3b460443
9c87156cce5a63735d1218f0096a65c50a7a32aa
babaab2f473817f173a2d08e410c25abf5ed0f6b
065e5e559555e2f100bc95792a8ef1b609bbe130
93f56de259376d7e4fff2b2d104082e1fa66e237
6c4798d3f08b81c2c52936b10e0fa872590c96ae
b7a313d84e853049062011d78cb04b6decd12f5c
093b75ef5995ea35d7f6bdb6c7b32a42a1999813

And before you object, "but -Wconstant-logical-operand is a clang-only 
warning! it can't be divergent with gcc!", consider that the special cases 
added to deal with clang-only warnings have to be removed when gcc catches 
up, which is more churn. Now multiply that by the number of checkers you 
care about.

Re: [PATCH v3 07/12] automation: add alpine linux x86 build jobs

[Stefano Stabellini]

On Tue, 24 Nov 2020, Stefano Stabellini wrote:
> Allow failure for these jobs. Currently they fail because hvmloader
> doesn't build with musl. The failures don't block the pipeline.
> 
> Signed-off-by: Stefano Stabellini 
> ---
> This patch is probably required: 
> https://github.com/alpinelinux/aports/blob/master/main/xen/musl-hvmloader-fix-stdint.patch


We could simply disable the hvmloader build when it is not necessary.
When OVMF, SeaBios, and Rombios are all disabled, it doesn't make sense
to build hvmloader. If this assumption is correct, then the patch below
fixes the Alpine Linux build (as long as we pass --disable-seabios and
--disable-rombios appropriately)


diff --git a/config/Tools.mk.in b/config/Tools.mk.in
index 48bd9ab731..a6aada576f 100644
--- a/config/Tools.mk.in
+++ b/config/Tools.mk.in
@@ -55,6 +55,15 @@ CONFIG_QEMU_XEN := @qemu_xen@
 CONFIG_QEMUU_EXTRA_ARGS:= @EXTRA_QEMUU_CONFIGURE_ARGS@
 CONFIG_LIBNL:= @libnl@
 CONFIG_GOLANG   := @golang@
+ifeq ($(CONFIG_ROMBIOS),y)
+CONFIG_FIRMWARE=y
+endif
+ifeq ($(CONFIG_SEABIOS),y)
+CONFIG_FIRMWARE=y
+endif
+ifeq ($(CONFIG_OVMF),y)
+CONFIG_FIRMWARE=y
+endif
 
 CONFIG_SYSTEMD  := @systemd@
 SYSTEMD_CFLAGS  := @SYSTEMD_CFLAGS@
diff --git a/tools/Makefile b/tools/Makefile
index ed71474421..9821a7f5d5 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -14,7 +14,7 @@ SUBDIRS-y += examples
 SUBDIRS-y += hotplug
 SUBDIRS-y += xentrace
 SUBDIRS-$(CONFIG_XCUTILS) += xcutils
-SUBDIRS-$(CONFIG_X86) += firmware
+SUBDIRS-$(CONFIG_FIRMWARE) += firmware
 SUBDIRS-y += console
 SUBDIRS-y += xenmon
 SUBDIRS-y += xentop

RE: [PATCH] xen/arm: Add Cortex-A73 erratum 858921 workaround

[Stefano Stabellini]

Resuming this old thread.

On Fri, 13 Nov 2020, Wei Chen wrote:
> > Hi,
> > 
> > On 09/11/2020 08:21, Penny Zheng wrote:
> > > CNTVCT_EL0 or CNTPCT_EL0 counter read in Cortex-A73 (all versions)
> > > might return a wrong value when the counter crosses a 32bit boundary.
> > >
> > > Until now, there is no case for Xen itself to access CNTVCT_EL0,
> > > and it also should be the Guest OS's responsibility to deal with
> > > this part.
> > >
> > > But for CNTPCT, there exists several cases in Xen involving reading
> > > CNTPCT, so a possible workaround is that performing the read twice,
> > > and to return one or the other depending on whether a transition has
> > > taken place.
> > >
> > > Signed-off-by: Penny Zheng 
> > 
> > Acked-by: Julien Grall 
> > 
> > On a related topic, do we need a fix similar to Linux commit
> > 75a19a0202db "arm64: arch_timer: Ensure counter register reads occur
> > with seqlock held"?
> > 
> 
> I take a look at this Linux commit, it seems to prevent the seq-lock to be
> speculated.  Using an enforce ordering instead of ISB after the read counter
> operation seems to be for performance reasons.
> 
> I have found that you had placed an ISB before read counter in get_cycles
> to prevent counter value to be speculated. But you haven't placed the second
> ISB after reading. Is it because we haven't used the get_cycles in seq lock
> critical context (Maybe I didn't find the right place)? So should we need to 
> fix it
> now, or you prefer to fix it now for future usage?

Looking at the call sites, it doesn't look like we need any ISB after
get_cycles as it is not used in any critical context. There is also a
data dependency with the value returned by it.

So I am thinking we don't need any fix. At most we need an in-code comment?

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

[osstest service owner]

flight 156992 xen-unstable real [real]
flight 157014 xen-unstable real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/156992/
http://logs.test-lab.xenproject.org/osstest/logs/157014/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-examine  4 memdisk-try-append  fail pass in 157014-retest
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 18 
guest-start/debianhvm.repeat fail pass in 157014-retest

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

version targeted for testing:
 xen  8147e00e4fbfcc43b665dc6bf279b204c501ba04
baseline version:
 xen  b659a5cebd611dbe698e63c03485b5fe8cd964ad

Last test of basis   156975  2020-11-24 01:51:25 Z1 days
Testing same since   156992  2020-11-24 

Re: [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang

[Finn Thain]

On Wed, 25 Nov 2020, Nick Desaulniers wrote:

> On Wed, Nov 25, 2020 at 1:33 PM Finn Thain  
> wrote:
> >
> > Or do you think that a codebase can somehow satisfy multiple checkers 
> > and their divergent interpretations of the language spec?
> 
> Have we found any cases yet that are divergent? I don't think so.

There are many implementations, so I think you are guaranteed to find more 
divergence if you look. That's because the spec is full of language like 
this: "implementations are encouraged not to emit a diagnostic" and 
"implementations are encouraged to issue a diagnostic".

Some implementations will decide to not emit (under the premise that vast 
amounts of existing code would have to get patched until the compiler goes 
quiet) whereas other implementations will decide to emit (under the 
premise that the author is doing the checking and not the janitor or the 
packager).

> It sounds to me like GCC's cases it warns for is a subset of Clang's. 
> Having additional coverage with Clang then should ensure coverage for 
> both.
> 

If that claim were true, the solution would be simple. (It's not.)

For the benefit of projects that enable -Werror and projects that 
nominated gcc as their preferred compiler, clang would simply need a flag 
to enable conformance with gcc by suppressing those additional warnings 
that clang would normally produce.

This simple solution is, of course, completely unworkable, since it would 
force clang to copy some portion of gcc's logic (rewritten under LLVM's 
unique license) and then to track future changes to that portion of gcc 
indefinitely.

[ovmf test] 157012: all pass - PUSHED

[osstest service owner]

flight 157012 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/157012/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 388f9a9355ae7ab95a34ac2e9a3c608caf11b74a
baseline version:
 ovmf e7bd0dd26db7e56aa8ca70132d6ea916ee6f3db0

Last test of basis   156920  2020-11-21 09:10:55 Z4 days
Testing same since   157012  2020-11-25 18:09:50 Z0 days1 attempts


People who touched revisions under test:
  Laszlo Ersek 
  Michael D Kinney 
  Sean Brogan 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass



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

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

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   e7bd0dd26d..388f9a9355  388f9a9355ae7ab95a34ac2e9a3c608caf11b74a -> 
xen-tested-master

[libvirt test] 157000: regressions - FAIL

[osstest service owner]

flight 157000 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/157000/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf-libvirt   6 libvirt-buildfail REGR. vs. 151777
 build-amd64-libvirt   6 libvirt-buildfail REGR. vs. 151777
 build-i386-libvirt6 libvirt-buildfail REGR. vs. 151777
 build-arm64-libvirt   6 libvirt-buildfail REGR. vs. 151777

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a

version targeted for testing:
 libvirt  511013b57b50da7c800967cd990f8ae1ad5fa948
baseline version:
 libvirt  2c846fa6bcc11929c9fb857a22430fb9945654ad

Last test of basis   151777  2020-07-10 04:19:19 Z  138 days
Failing since151818  2020-07-11 04:18:52 Z  137 days  132 attempts
Testing same since   157000  2020-11-25 04:19:37 Z0 days1 attempts


People who touched revisions under test:
  Adolfo Jayme Barrientos 
  Aleksandr Alekseev 
  Andika Triwidada 
  Andrea Bolognani 
  Balázs Meskó 
  Barrett Schonefeld 
  Bastien Orivel 
  Bihong Yu 
  Binfeng Wu 
  Boris Fiuczynski 
  Brian Turek 
  Christian Ehrhardt 
  Christian Schoenebeck 
  Cole Robinson 
  Collin Walling 
  Cornelia Huck 
  Côme Borsoi 
  Daniel Henrique Barboza 
  Daniel Letai 
  Daniel P. Berrange 
  Daniel P. Berrangé 
  Erik Skultety 
  Fabian Freyer 
  Fangge Jin 
  Fedora Weblate Translation 
  Göran Uddeborg 
  Halil Pasic 
  Han Han 
  Hao Wang 
  Ian Wienand 
  Jamie Strandboge 
  Jamie Strandboge 
  Jean-Baptiste Holcroft 
  Jianan Gao 
  Jim Fehlig 
  Jin Yan 
  Jiri Denemark 
  Jonathan Watt 
  Jonathon Jongsma 
  Julio Faracco 
  Ján Tomko 
  Kashyap Chamarthy 
  Kevin Locke 
  Laine Stump 
  Liao Pingfang 
  Lin Ma 
  Lin Ma 
  Lin Ma 
  Marc Hartmayer 
  Marc-André Lureau 
  Marek Marczykowski-Górecki 
  Markus Schade 
  Martin Kletzander 
  Masayoshi Mizuma 
  Matt Coleman 
  Matt Coleman 
  Mauro Matteo Cascella 
  Michal Privoznik 
  Michał Smyk 
  Milo Casagrande 
  Neal Gompa 
  Nico Pache 
  Nikolay Shirokovskiy 
  Olaf Hering 
  Olesya Gerasimenko 
  Orion Poplawski 
  Patrick Magauran 
  Paulo de Rezende Pinatti 
  Pavel Hrdina 
  Peter Krempa 
  Pino Toscano 
  Pino Toscano 
  Piotr Drąg 
  Prathamesh Chavan 
  Ricky Tigg 
  Roman Bogorodskiy 
  Roman Bolshakov 
  Ryan Gahagan 
  Ryan Schmidt 
  Sam Hartman 
  Scott Shambarger 
  Sebastian Mitterle 
  Shaojun Yang 
  Simon Gaiser 
  Stefan Bader 
  Stefan Berger 
  Szymon Scholz 
  Thomas Huth 
  Tim Wiederhake 
  Tomáš Golembiovský 
  Wang Xin 
  Weblate 
  Yang Hang 
  Yanqiu Zhang 
  Yi Li 
  Yi Wang 
  Yuri Chornoivan 
  Zheng Chuan 
  zhenwei pi 
  Zhenyu Zheng 

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  fail
 build-arm64-libvirt  fail
 build-armhf-libvirt  fail
 build-i386-libvirt   fail
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm  

Re: [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang

[Nick Desaulniers]

On Wed, Nov 25, 2020 at 1:33 PM Finn Thain  wrote:
>
> Or do you think that a codebase can somehow satisfy multiple checkers and
> their divergent interpretations of the language spec?

Have we found any cases yet that are divergent? I don't think so.  It
sounds to me like GCC's cases it warns for is a subset of Clang's.
Having additional coverage with Clang then should ensure coverage for
both.

> > This is not a shiny new warning; it's already on for GCC and has existed
> > in both compilers for multiple releases.
> >
>
> Perhaps you're referring to the compiler feature that lead to the
> ill-fated, tree-wide /* fallthrough */ patch series.
>
> When the ink dries on the C23 language spec and the implementations figure
> out how to interpret it then sure, enforce the warning for new code -- the
> cost/benefit analysis is straight forward. However, the case for patching
> existing mature code is another story.

I don't think we need to wait for the ink to dry on the C23 language
spec to understand that implicit fallthrough is an obvious defect of
the C language.  While the kernel is a mature codebase, it's not
immune to bugs.  And its maturity has yet to slow its rapid pace of
development.
-- 
Thanks,
~Nick Desaulniers

Re: [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang

[Nick Desaulniers]

On Wed, Nov 25, 2020 at 8:24 AM Jakub Kicinski  wrote:
>
> Applying a real patch set and then getting a few follow ups the next day
> for trivial coding things like fallthrough missing or static missing,
> just because I didn't have the full range of compilers to check with
> before applying makes me feel pretty shitty, like I'm not doing a good
> job. YMMV.

I understand. Everyone feels that way, except maybe Bond villains and
robots.  My advice in that case is don't take it personally.  We're
working with a language that's more error prone relative to others.
While one would like to believe they are flawless, over time they
can't beat the aggregate statistics.  A balance between Imposter
Syndrome and Dunning Kruger is walked by all software developers, and
the fear of making mistakes in public is one of the number one reasons
folks don't take the plunge contributing to open source software or
even the kernel.  My advice to them is "don't sweat the small stuff."
-- 
Thanks,
~Nick Desaulniers

Re: [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang

[Finn Thain]

On Wed, 25 Nov 2020, Nick Desaulniers wrote:

> So developers and distributions using Clang can't have 
> -Wimplicit-fallthrough enabled because GCC is less strict (which has 
> been shown in this thread to lead to bugs)?  We'd like to have nice 
> things too, you know.
> 

Apparently the GCC developers don't want you to have "nice things" either. 
Do you think that the kernel should drop gcc in favour of clang?
Or do you think that a codebase can somehow satisfy multiple checkers and 
their divergent interpretations of the language spec?

> This is not a shiny new warning; it's already on for GCC and has existed 
> in both compilers for multiple releases.
> 

Perhaps you're referring to the compiler feature that lead to the 
ill-fated, tree-wide /* fallthrough */ patch series.

When the ink dries on the C23 language spec and the implementations figure 
out how to interpret it then sure, enforce the warning for new code -- the 
cost/benefit analysis is straight forward. However, the case for patching 
existing mature code is another story.

Re: [PATCH v4 3/3] ns16550: Gate all PCI code with CONFIG_X86

[Stefano Stabellini]

On Wed, 25 Nov 2020, Rahul Singh wrote:
> The NS16550 driver is assuming that NS16550 PCI card are usable if the
> architecture supports PCI (i.e. CONFIG_HAS_PCI=y). However, the code is
> very x86 focus and will fail to build on Arm (/!\ it is not all the
> errors):
> 
> ns16550.c: In function ‘ns16550_init_irq’:
> ns16550.c:726:21: error: implicit declaration of function ‘create_irq’;
> did you mean ‘release_irq’? [-Werror=implicit-function-declaration]
>   uart->irq = create_irq(0, false);
>   ^~
>   release_irq
> ns16550.c:726:21: error: nested extern declaration of ‘create_irq’
> [-Werror=nested-externs]
> ns16550.c: In function ‘ns16550_init_postirq’:
> ns16550.c:768:33: error: ‘mmio_ro_ranges’ undeclared (first use in this
> function); did you mean ‘mmio_handler’?
>rangeset_add_range(mmio_ro_ranges, uart->io_base,
>   ^~
>   mmio_handler
> ns16550.c:768:33: note: each undeclared identifier is reported only once
> for each function it appears in
> ns16550.c:780:20: error: variable ‘msi’ has initializer but incomplete
> type
>   struct msi_info msi = {
>  ^~~~
> 
> Enabling support for NS16550 PCI card on Arm would require more plumbing
> in addition to fixing the compilation error.
> 
> Arm systems tend to have platform UART available such as NS16550, PL011.
> So there are limited reasons to get NS16550 PCI support for now on Arm.
> 
> Guard all remaining PCI code that is not under x86 flag with CONFIG_X86.
> 
> No functional change intended.
> 
> Signed-off-by: Rahul Singh 

Acked-by: Stefano Stabellini 


> ---
> 
> Changes in v4:
> - As per the discussion guard all remaining PCI code with CONFIG_X86
> 
> ---
>  xen/drivers/char/ns16550.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index 9235d854fe..26e601857a 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -16,7 +16,7 @@
>  #include 
>  #include 
>  #include 
> -#ifdef CONFIG_HAS_PCI
> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
>  #include 
>  #include 
>  #include 
> @@ -51,7 +51,7 @@ static struct ns16550 {
>  unsigned int timeout_ms;
>  bool_t intr_works;
>  bool_t dw_usr_bsy;
> -#ifdef CONFIG_HAS_PCI
> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
>  /* PCI card parameters. */
>  bool_t pb_bdf_enable;   /* if =1, pb-bdf effective, port behind bridge */
>  bool_t ps_bdf_enable;   /* if =1, ps_bdf effective, port on pci card */
> @@ -66,7 +66,7 @@ static struct ns16550 {
>  #endif
>  } ns16550_com[2] = { { 0 } };
>  
> -#ifdef CONFIG_HAS_PCI
> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
>  struct ns16550_config {
>  u16 vendor_id;
>  u16 dev_id;
> @@ -256,7 +256,7 @@ static int ns16550_getc(struct serial_port *port, char 
> *pc)
>  
>  static void pci_serial_early_init(struct ns16550 *uart)
>  {
> -#ifdef CONFIG_HAS_PCI
> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
>  if ( !uart->ps_bdf_enable || uart->io_base >= 0x1 )
>  return;
>  
> @@ -355,7 +355,7 @@ static void __init ns16550_init_preirq(struct serial_port 
> *port)
>  
>  static void __init ns16550_init_irq(struct serial_port *port)
>  {
> -#ifdef CONFIG_HAS_PCI
> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
>  struct ns16550 *uart = port->uart;
>  
>  if ( uart->msi )
> @@ -397,7 +397,7 @@ static void __init ns16550_init_postirq(struct 
> serial_port *port)
>  uart->timeout_ms = max_t(
>  unsigned int, 1, (bits * uart->fifo_size * 1000) / uart->baud);
>  
> -#ifdef CONFIG_HAS_PCI
> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
>  if ( uart->bar || uart->ps_bdf_enable )
>  {
>  if ( uart->param && uart->param->mmio &&
> @@ -477,7 +477,7 @@ static void ns16550_suspend(struct serial_port *port)
>  
>  stop_timer(>timer);
>  
> -#ifdef CONFIG_HAS_PCI
> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
>  if ( uart->bar )
> uart->cr = pci_conf_read16(PCI_SBDF(0, uart->ps_bdf[0], 
> uart->ps_bdf[1],
>uart->ps_bdf[2]), PCI_COMMAND);
> @@ -486,7 +486,7 @@ static void ns16550_suspend(struct serial_port *port)
>  
>  static void _ns16550_resume(struct serial_port *port)
>  {
> -#ifdef CONFIG_HAS_PCI
> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
>  struct ns16550 *uart = port->uart;
>  
>  if ( uart->bar )
> -- 
> 2.17.1
> 

Re: [PATCH v4 2/3] xen/pci: solve compilation error on ARM with HAS_PCI enabled.

[Stefano Stabellini]

On Wed, 25 Nov 2020, Rahul Singh wrote:
> If mem-sharing, mem-paging, or log-dirty functionality is not enabled
> for architecture when HAS_PCI is enabled, the compiler will throw an
> error.
> 
> Move code to x86 specific file to fix compilation error.
> 
> Also, modify the code to use likely() in place of unlikley() for each
> condition to make code more optimized.
> 
> No functional change intended.
> 
> Signed-off-by: Rahul Singh 
> ---
> 
> Changes in v4:
> - fixed minor comments
> 
> ---
>  xen/drivers/passthrough/pci.c   |  8 +---
>  xen/drivers/passthrough/x86/iommu.c | 13 +
>  xen/include/xen/iommu.h |  2 ++
>  3 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
> index 3c6ab1bcb6..4c21655b7d 100644
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -20,7 +20,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -1418,12 +1417,7 @@ static int assign_device(struct domain *d, u16 seg, u8 
> bus, u8 devfn, u32 flag)
>  if ( !is_iommu_enabled(d) )
>  return 0;
>  
> -/* Prevent device assign if mem paging or mem sharing have been 
> - * enabled for this domain */
> -if ( d != dom_io &&
> - unlikely(mem_sharing_enabled(d) ||
> -  vm_event_check_ring(d->vm_event_paging) ||
> -  p2m_get_hostp2m(d)->global_logdirty) )
> +if( !arch_iommu_use_permitted(d) )

code style:

  if ( !arch_iommu_use_permitted(d) )

Re: [PATCH v4 1/3] xen/pci: Move x86 specific code to x86 directory.

[Stefano Stabellini]

On Wed, 25 Nov 2020, Rahul Singh wrote:
> passthrough/pci.c file is common for all architecture, but there is x86
> specific code in this file.
> 
> Move x86 specific code to the drivers/passthrough/io.c file to avoid
> compilation error for other architecture.
> 
> As drivers/passthrough/io.c is compiled only for x86 move it to
> x86 directory and rename it to hvm.c.
> 
> No functional change intended.
> 
> Signed-off-by: Rahul Singh 

Reviewed-by: Stefano Stabellini 


> ---
> 
> Changes in v4:
> - fixed compilation error when CONFIG_HVM is disabled 
> - remove iommu_update_ire_from_msi from the patch will send another patch
>   to fix.
> 
> ---
>  xen/drivers/passthrough/Makefile|  3 -
>  xen/drivers/passthrough/pci.c   | 71 +
>  xen/drivers/passthrough/x86/Makefile|  1 +
>  xen/drivers/passthrough/{io.c => x86/hvm.c} | 66 +++
>  xen/include/xen/pci.h   |  9 +++
>  5 files changed, 77 insertions(+), 73 deletions(-)
>  rename xen/drivers/passthrough/{io.c => x86/hvm.c} (95%)
> 
> diff --git a/xen/drivers/passthrough/Makefile 
> b/xen/drivers/passthrough/Makefile
> index e973e16c74..cc646612c7 100644
> --- a/xen/drivers/passthrough/Makefile
> +++ b/xen/drivers/passthrough/Makefile
> @@ -6,6 +6,3 @@ obj-$(CONFIG_ARM) += arm/
>  obj-y += iommu.o
>  obj-$(CONFIG_HAS_PCI) += pci.o
>  obj-$(CONFIG_HAS_DEVICE_TREE) += device_tree.o
> -
> -x86-$(CONFIG_HVM) := io.o
> -obj-$(CONFIG_X86) += $(x86-y)
> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
> index 51e584127e..3c6ab1bcb6 100644
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -14,9 +14,6 @@
>   * this program; If not, see .
>   */
>  
> -#include 
> -#include 
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -24,7 +21,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -842,71 +838,6 @@ int pci_remove_device(u16 seg, u8 bus, u8 devfn)
>  return ret;
>  }
>  
> -static int pci_clean_dpci_irq(struct domain *d,
> -  struct hvm_pirq_dpci *pirq_dpci, void *arg)
> -{
> -struct dev_intx_gsi_link *digl, *tmp;
> -
> -pirq_guest_unbind(d, dpci_pirq(pirq_dpci));
> -
> -if ( pt_irq_need_timer(pirq_dpci->flags) )
> -kill_timer(_dpci->timer);
> -
> -list_for_each_entry_safe ( digl, tmp, _dpci->digl_list, list )
> -{
> -list_del(>list);
> -xfree(digl);
> -}
> -
> -radix_tree_delete(>pirq_tree, dpci_pirq(pirq_dpci)->pirq);
> -
> -if ( !pt_pirq_softirq_active(pirq_dpci) )
> -return 0;
> -
> -domain_get_irq_dpci(d)->pending_pirq_dpci = pirq_dpci;
> -
> -return -ERESTART;
> -}
> -
> -static int pci_clean_dpci_irqs(struct domain *d)
> -{
> -struct hvm_irq_dpci *hvm_irq_dpci = NULL;
> -
> -if ( !is_iommu_enabled(d) )
> -return 0;
> -
> -if ( !is_hvm_domain(d) )
> -return 0;
> -
> -spin_lock(>event_lock);
> -hvm_irq_dpci = domain_get_irq_dpci(d);
> -if ( hvm_irq_dpci != NULL )
> -{
> -int ret = 0;
> -
> -if ( hvm_irq_dpci->pending_pirq_dpci )
> -{
> -if ( pt_pirq_softirq_active(hvm_irq_dpci->pending_pirq_dpci) )
> - ret = -ERESTART;
> -else
> - hvm_irq_dpci->pending_pirq_dpci = NULL;
> -}
> -
> -if ( !ret )
> -ret = pt_pirq_iterate(d, pci_clean_dpci_irq, NULL);
> -if ( ret )
> -{
> -spin_unlock(>event_lock);
> -return ret;
> -}
> -
> -hvm_domain_irq(d)->dpci = NULL;
> -free_hvm_irq_dpci(hvm_irq_dpci);
> -}
> -spin_unlock(>event_lock);
> -return 0;
> -}
> -
>  /* Caller should hold the pcidevs_lock */
>  static int deassign_device(struct domain *d, uint16_t seg, uint8_t bus,
> uint8_t devfn)
> @@ -966,7 +897,7 @@ int pci_release_devices(struct domain *d)
>  int ret;
>  
>  pcidevs_lock();
> -ret = pci_clean_dpci_irqs(d);
> +ret = arch_pci_clean_pirqs(d);
>  if ( ret )
>  {
>  pcidevs_unlock();
> diff --git a/xen/drivers/passthrough/x86/Makefile 
> b/xen/drivers/passthrough/x86/Makefile
> index a70cf9460d..69284a5d19 100644
> --- a/xen/drivers/passthrough/x86/Makefile
> +++ b/xen/drivers/passthrough/x86/Makefile
> @@ -1,2 +1,3 @@
>  obj-y += ats.o
>  obj-y += iommu.o
> +obj-$(CONFIG_HVM) += hvm.o
> diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/x86/hvm.c
> similarity index 95%
> rename from xen/drivers/passthrough/io.c
> rename to xen/drivers/passthrough/x86/hvm.c
> index 6b1305a3e5..41cfa2e200 100644
> --- a/xen/drivers/passthrough/io.c
> +++ b/xen/drivers/passthrough/x86/hvm.c
> @@ -1036,6 +1036,72 @@ unlock:
>  spin_unlock(>event_lock);
>  }
>  
> +static int pci_clean_dpci_irq(struct domain *d,
> +  struct 

Re: [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang

[Kees Cook]

On Tue, Nov 24, 2020 at 11:05:35PM -0800, James Bottomley wrote:
> Now, what we have seems to be about 6 cases (at least what's been shown
> in this thread) where a missing break would cause potentially user
> visible issues.  That means the value of this isn't zero, but it's not
> a no-brainer massive win either.  That's why I think asking what we've
> invested vs the return isn't a useless exercise.

The number is much higher[1]. If it were 6 in the entire history of the
kernel, I would agree with you. :) Some were fixed _before_ Gustavo's
effort too, which I also count towards the idea of "this is a dangerous
weakness in C, and now we have stopped it forever."

> But the broader point I'm making is just because the compiler people
> come up with a shiny new warning doesn't necessarily mean the problem
> it's detecting is one that causes us actual problems in the code base. 
> I'd really be happier if we had a theory about what classes of CVE or
> bug we could eliminate before we embrace the next new warning.

But we did! It was long ago justified and documented[2], and even links to
the CWE[3] for it. This wasn't random joy over discovering a new warning
we could turn on, this was turning on a warning that the compiler folks
finally gave us to handle an entire class of flaws. If we need to update
the code-base to address it not a useful debate -- that was settled
already, even if you're only discovering it now. :P. This last patch
set is about finishing that work for Clang, which is correctly even
more strict than GCC.

-Kees

[1] https://outflux.net/slides/2019/lss/kspp.pdf calls out specific
numbers (about 6.5% of the patches fixed missing breaks):
v4.19:  3 of 129
v4.20:  2 of  59
v5.0:   3 of  56
v5.1:  10 of 100
v5.2:   6 of  71
v5.3:   7 of  69

And in the history of the kernel, it's been an ongoing source of
flaws:

$ l --no-merges | grep -i 'missing break' | wc -l
185

The frequency of such errors being "naturally" found was pretty
steady until the static checkers started warning, and then it was
on the rise, but the full effort flushed the rest out, and now it's
dropped to almost zero:

  1 v2.6.12
  3 v2.6.16.28
  1 v2.6.17
  1 v2.6.19
  2 v2.6.21
  1 v2.6.22
  3 v2.6.24
  3 v2.6.29
  1 v2.6.32
  1 v2.6.33
  1 v2.6.35
  4 v2.6.36
  3 v2.6.38
  2 v2.6.39
  7 v3.0
  2 v3.1
  2 v3.2
  2 v3.3
  3 v3.4
  1 v3.5
  8 v3.6
  7 v3.7
  3 v3.8
  6 v3.9
  3 v3.10
  2 v3.11
  5 v3.12
  5 v3.13
  2 v3.14
  4 v3.15
  2 v3.16
  3 v3.17
  2 v3.18
  2 v3.19
  1 v4.0
  2 v4.1
  5 v4.2
  4 v4.5
  5 v4.7
  6 v4.8
  1 v4.9
  3 v4.10
  2 v4.11
  6 v4.12
  3 v4.13
  2 v4.14
  5 v4.15
  2 v4.16
  7 v4.18
  2 v4.19
  6 v4.20
  3 v5.0
 12 v5.1
  3 v5.2
  4 v5.3
  2 v5.4
  1 v5.8


And the reason it's fully zero, is because we still have the cases we're
cleaning up right now. Even this last one from v5.8 is specifically of
the same type this series addresses:

case 4:
color_index = TrueCModeIndex;
+   break;
default:
return;
}


[2] 
https://www.kernel.org/doc/html/latest/process/deprecated.html#implicit-switch-case-fall-through

All switch/case blocks must end in one of:

break;
fallthrough;
continue;
goto ;
return [expression];

[3] https://cwe.mitre.org/data/definitions/484.html

-- 
Kees Cook

[PATCH v2 4/6] xen: Delete xen_available() function

[Eduardo Habkost]

The function can be replaced with accel_available("xen").

Signed-off-by: Eduardo Habkost 
---
Cc: Paolo Bonzini 
Cc: qemu-de...@nongnu.org
Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: Paul Durrant 
Cc: xen-devel@lists.xenproject.org
Cc: Richard Henderson 
Cc: Claudio Fontana 
Cc: Roman Bolshakov 
---
 include/sysemu/arch_init.h | 2 --
 softmmu/arch_init.c| 9 -
 softmmu/vl.c   | 6 +++---
 3 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index b32ce1afa9..40ac8052b7 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -32,6 +32,4 @@ enum {
 
 extern const uint32_t arch_type;
 
-int xen_available(void);
-
 #endif
diff --git a/softmmu/arch_init.c b/softmmu/arch_init.c
index 79383c8db8..f4770931f5 100644
--- a/softmmu/arch_init.c
+++ b/softmmu/arch_init.c
@@ -49,12 +49,3 @@ int graphic_depth = 32;
 
 
 const uint32_t arch_type = QEMU_ARCH;
-
-int xen_available(void)
-{
-#ifdef CONFIG_XEN
-return 1;
-#else
-return 0;
-#endif
-}
diff --git a/softmmu/vl.c b/softmmu/vl.c
index e6e0ad5a92..74b6ebf1e4 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -3667,21 +3667,21 @@ void qemu_init(int argc, char **argv, char **envp)
 has_defaults = 0;
 break;
 case QEMU_OPTION_xen_domid:
-if (!(xen_available())) {
+if (!(accel_available("xen"))) {
 error_report("Option not supported for this target");
 exit(1);
 }
 xen_domid = atoi(optarg);
 break;
 case QEMU_OPTION_xen_attach:
-if (!(xen_available())) {
+if (!(accel_available("xen"))) {
 error_report("Option not supported for this target");
 exit(1);
 }
 xen_mode = XEN_ATTACH;
 break;
 case QEMU_OPTION_xen_domid_restrict:
-if (!(xen_available())) {
+if (!(accel_available("xen"))) {
 error_report("Option not supported for this target");
 exit(1);
 }
-- 
2.28.0

Re: [PATCH 25/45] block: reference struct block_device from struct hd_struct

[Tejun Heo]

Hello,

On Wed, Nov 25, 2020 at 05:45:15PM +0100, Christoph Hellwig wrote:
> On Tue, Nov 24, 2020 at 04:18:49PM -0500, Tejun Heo wrote:
> > Hello,
> > 
> > Please see lkml.kernel.org/r/x708btj5njtbc...@mtj.duckdns.org for a few nits
> > on the previous version.
> 
> Thanks, I've addressed the mapping_set_gfp mask nit and updated the
> commit log.  As Jan pointed also pointed out I think we do need the
> remove_inode_hash.

Agreed. It'd be nice to replace the stale comment.

> > Also, would it make sense to separate out lookup_sem removal? I *think* it's
> > there to ensure that the same bdev doesn't get associated with old and new
> > gendisks at the same time but can't wrap my head around how it works
> > exactly. I can see that this may not be needed once the lifetimes of gendisk
> > and block_devices are tied together but that may warrant a bit more
> > explanation.
> 
> Jan added lookup_sem in commit 56c0908c855afbb to prevent a three way
> race between del_gendisk and blkdev_open due to the weird bdev vs
> gendisk lifetime rules.  None of that can happen with the new lookup
> scheme.

Understood. I think it'd be worthwhile to note that in the commit log.

Thanks.

-- 
tejun

Re: [PATCH 23/45] block: remove i_bdev

[Tejun Heo]

Hello,

On Wed, Nov 25, 2020 at 05:29:26PM +0100, Christoph Hellwig wrote:
> > I was wondering whether losing the stale bdev flushing in bd_acquire() would
> > cause user-visible behavior changes but can't see how it would given that
> > userland has no way of holding onto a specific instance of block inode.
> > Maybe it's something worth mentioning in the commit message?
> 
> With stale bdev flushing do you mean the call to bd_forget if
> i_bdev exists but is unhashed?  It doesn't actually flush anything but

Yeah.

> just detaches the old bdev from the inode so that the new one can be
> attached.  That problem goes away by definition if we don't attach
> the bdev to the inode.

Yeah, I think so. Was just wondering whether the problem actually goes away.

Thanks.

-- 
tejun

Re: [PATCH v2 02/17] mm: introduce xvmalloc() et al and use for grant table allocations

[Stefano Stabellini]

On Wed, 25 Nov 2020, Jan Beulich wrote:
> On 25.11.2020 13:15, Julien Grall wrote:
> > On 23/11/2020 14:23, Jan Beulich wrote:
> >> All of the array allocations in grant_table_init() can exceed a page's
> >> worth of memory, which xmalloc()-based interfaces aren't really suitable
> >> for after boot. We also don't need any of these allocations to be
> >> physically contiguous.. Introduce interfaces dynamically switching
> >> between xmalloc() et al and vmalloc() et al, based on requested size,
> >> and use them instead.
> >>
> >> All the wrappers in the new header get cloned mostly verbatim from
> >> xmalloc.h, with the sole adjustment to switch unsigned long to size_t
> >> for sizes and to unsigned int for alignments.
> >>
> >> Signed-off-by: Jan Beulich 
> >> ---
> >> v2: Actually edit a copy-and-pasted comment in xvmalloc.h which was
> >>  meant to be edited from the beginning.
> >> ---
> >> I'm unconvinced of the mentioning of "physically contiguous" in the
> >> comment at the top of the new header: I don't think xmalloc() provides
> >> such a guarantee. Any use assuming so would look (latently) broken to
> >> me.
> > 
> > I haven't had the chance to reply to the first version about this. So I 
> > will reply here to avoid confusion.
> > 
> > I can at least spot one user in Arm that would use xmalloc() that way 
> > (see the allocation of itt_addr in arch/arm/gic-v3-its.c).
> 
> And I surely wouldn't have spotted this, even if I had tried
> to find "offenders", i.e. as said before not wanting to alter
> the behavior of existing code (beyond the explicit changes
> done here) was ...
> 
> > AFAIK, the memory is for the sole purpose of the ITS and should not be 
> > accessed by Xen. So I think we can replace by a new version of 
> > alloc_domheap_pages().
> > 
> > However, I still question the usefulness of introducing yet another way 
> > to allocate memory (we already have alloc_xenheap_pages(), xmalloc(), 
> > alloc_domheap_pages(), vmap()) if you think users cannot rely on 
> > xmalloc() to allocate memory physically contiguous.
> 
> ... the reason to introduce a separate new interface. Plus of
> course this parallels what Linux has.
> 
> > It definitely makes more difficult to figure out when to use xmalloc() 
> > vs xvalloc().
> 
> I don't see the difficulty:
> - if you need physically contiguous memory, use alloc_xen*_pages(),
> - if you know the allocation size is always no more than a page,
>   use xmalloc(),

What if you need memory physically contiguous but not necessarily an
order of pages, such as for instance 5200 bytes?

If xmalloc can't do physically contiguous allocations, we need something
else that does physically contiguous allocations not only at page
granularity, right?

The other issue is semantics. If xmalloc is unable to allocate more than
a page of contiguous memory, then it is identical to vmalloc from the
caller's point of view: both xmalloc and vmalloc return a virtual
address for an allocation that might not be physically contiguous.

Maybe we should get rid of xmalloc entirely and improve the
implementation of vmalloc so that it falls back to xmalloc for
sub-page allocations. Which in fact is almost the same thing that you
did.


> - if you know the allocation size is always more than a page, use
>   vmalloc(),
> - otherwise use xvmalloc(). Exceptions may of course apply, i.e.
> this is just a rule of thumb.
> 
> > I would like to hear an opinion from the other maintainers.
> 
> Let's hope at least one will voice theirs.

If we take a step back, I think we only really need two memory
allocators:

1) one that allocates physically contiguous memory
2) one that allocates non-physically contiguous memory

That's it, right?

In addition to that, I understand it could be convient to have a little
wrapper that automatically chooses between 1) and 2) depending on
circumstances.

But if the circumstance is just size < PAGE_SIZE then I don't think we
need any convenience wrappers: we should just be able to call 2), which
is vmalloc, once we improve the vmalloc implementation.

Or do you see any reasons to keep the current vmalloc implementation as
is for sub-page allocations?

Re: Xen on RP4

[Roman Shaposhnik]

On Tue, Nov 24, 2020 at 9:17 PM Elliott Mitchell  wrote:
>
> On Tue, Nov 24, 2020 at 08:45:32PM -0800, Roman Shaposhnik wrote:
> > On Tue, Nov 24, 2020 at 8:41 PM Elliott Mitchell  wrote:
> > >
> > > On Tue, Nov 24, 2020 at 08:01:32PM -0800, Roman Shaposhnik wrote:
> > > > On Tue, Nov 24, 2020 at 7:37 PM Elliott Mitchell  
> > > > wrote:
> > > > > Presently I'm using a 5.8 kernel with your patches and haven't seen
> > > > > graphical output under Xen with either boot stack.  I've confirmed 
> > > > > fully
> > > > > operational graphics without Xen on Tianocore, I've confirmed 
> > > > > operational
> > > > > virtual terminals with U-Boot and not Xen.
> > > > >
> > > > > I had been planning to wait a bit before moving to 5.9, but if that is
> > > > > the crucial ingredient I'll move early.
> > > >
> > > > We're still using 5.4 -- but it seems that the next LTS 5.10 is also 
> > > > functional.
> > > >
> > > > I can bet $10 whatever it is -- it is DT related ;-)
> > >
> > > Given how many of the pieces I'm assembling are alpha or beta level, I
> > > estimate a 50:50 chance on that.  Good odds it is device-tree, but good
> > > odds I grabbed a bad version of $something.
> > >
> > > I mostly wanted to know whether I was in completely uncharted territory
> > > and needed to wait for others to catch up, versus merely working in a
> > > situation where support is funky and I'm at an unknown location in
> > > charted territory.
> > >
> > > I'll be keeping the Tianocore setup available since Xen on ARM really
> > > /should/ allow ACPI...
> >
> > I don't think you're in the uncharted -- so perhaps a bit of debugging left.
> >
> > And, of course, alway feel free to compare what we do -- the image is
> > docker pull away.
>
> Actually, since device-tree is very much on my list of concerns, what is
> your Xen boot process setup like?
>
> Presently as previously mentioned I'm trying for
> U-Boot -> GRUB/EFI -> Xen.

Exactly the same. Here's what we put on a vfat partition:
 https://github.com/lf-edge/eve/tree/master/pkg/u-boot/rpi
and here's how u-boot is built:
 https://github.com/lf-edge/eve/blob/master/pkg/u-boot/Dockerfile

> According to the information I currently have
> the device-trees are often tied pretty closely to the kernel.  I'm also
> using GRUB 2.04 since that has proper support for loading Xen on ARM.

Yes. Our DT here:

https://github.com/lf-edge/eve/blob/master/pkg/u-boot/rpi/bcm2711-rpi-4-b.dtb
came from an honest build of our kernel (our build is still in flux -- hence
a quick hack of keeping a blob):
https://github.com/lf-edge/eve/blob/master/pkg/kernel/Dockerfile#L154

> The section of grub.cfg for Linux is roughly:
> linux /boot/vmlinuz-5.8.10-2rp4-6.1.7 
> root=UUID=01234567-dead-beef-d13d-456789abcdef ro
> devicetree /boot/dtb-5.8.10-2rp4-6.1.7
> initrd /boot/initrd.img-5.8.10-2rp4-6.1.7
>
> My testing section for Xen is:
> xen_hypervisor /boot/xen-4.14-arm64.efi
> xen_module /boot/vmlinuz-5.8.10-2rp4-6.1.7 
> root=UUID=01234567-dead-beef-d13d-456789abcdef ro
> devicetree /boot/dtb-5.8.10-2rp4-6.1.7
> xen_module --nounzip /boot/initrd.img-5.8.10-2rp4-6.1.7

Roughly the same -- but see Stefano's comment. More here:
https://github.com/lf-edge/eve/blob/master/pkg/grub/rootfs.cfg

> I've frankly got no idea how to ensure the correct device-tree is passed
> to Xen.  Is GRUB's `devicetree` command correct when loading Xen?  Is a
> device-tree matched to the Linux kernel appropriate for Xen?
>
> (I'm guessing the second is "yes", but the first I don't have a clue)

While you can use `devicetree`  in GRUB. E.g.:
https://github.com/lf-edge/eve/blob/master/pkg/grub/rootfs.cfg#L207
on EVE side we've only ever used it as an emergency override.

The happy path boot sequence preserves the DT that RPi bootloader
makes available to u-boot and it gets passed down the chain without
anybody doing anything.

Hope this helps.

Thanks,
Roman.

[xen-unstable-smoke test] 157009: tolerable all pass - PUSHED

[osstest service owner]

flight 157009 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/157009/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  181f2c224ccd0a2900d6ae94ec390a546731f593
baseline version:
 xen  fd7479b9aec25885cc17d33b326b9babae59faee

Last test of basis   157006  2020-11-25 12:00:29 Z0 days
Testing same since   157009  2020-11-25 15:00:53 Z0 days1 attempts


People who touched revisions under test:
  Jan Beulich 
  Julien Grall 
  Paul Durrant 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



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

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

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   fd7479b9ae..181f2c224c  181f2c224ccd0a2900d6ae94ec390a546731f593 -> smoke

Re: Xen on RP4

[Stefano Stabellini]

On Tue, 24 Nov 2020, Elliott Mitchell wrote:
> On Tue, Nov 24, 2020 at 08:45:32PM -0800, Roman Shaposhnik wrote:
> > On Tue, Nov 24, 2020 at 8:41 PM Elliott Mitchell  wrote:
> > >
> > > On Tue, Nov 24, 2020 at 08:01:32PM -0800, Roman Shaposhnik wrote:
> > > > On Tue, Nov 24, 2020 at 7:37 PM Elliott Mitchell  
> > > > wrote:
> > > > > Presently I'm using a 5.8 kernel with your patches and haven't seen
> > > > > graphical output under Xen with either boot stack.  I've confirmed 
> > > > > fully
> > > > > operational graphics without Xen on Tianocore, I've confirmed 
> > > > > operational
> > > > > virtual terminals with U-Boot and not Xen.
> > > > >
> > > > > I had been planning to wait a bit before moving to 5.9, but if that is
> > > > > the crucial ingredient I'll move early.
> > > >
> > > > We're still using 5.4 -- but it seems that the next LTS 5.10 is also 
> > > > functional.
> > > >
> > > > I can bet $10 whatever it is -- it is DT related ;-)
> > >
> > > Given how many of the pieces I'm assembling are alpha or beta level, I
> > > estimate a 50:50 chance on that.  Good odds it is device-tree, but good
> > > odds I grabbed a bad version of $something.
> > >
> > > I mostly wanted to know whether I was in completely uncharted territory
> > > and needed to wait for others to catch up, versus merely working in a
> > > situation where support is funky and I'm at an unknown location in
> > > charted territory.
> > >
> > > I'll be keeping the Tianocore setup available since Xen on ARM really
> > > /should/ allow ACPI...
> > 
> > I don't think you're in the uncharted -- so perhaps a bit of debugging left.
> > 
> > And, of course, alway feel free to compare what we do -- the image is
> > docker pull away.
> 
> Actually, since device-tree is very much on my list of concerns, what is
> your Xen boot process setup like?
> 
> Presently as previously mentioned I'm trying for
> U-Boot -> GRUB/EFI -> Xen.  According to the information I currently have
> the device-trees are often tied pretty closely to the kernel.  I'm also
> using GRUB 2.04 since that has proper support for loading Xen on ARM.
> 
> The section of grub.cfg for Linux is roughly:
> linux /boot/vmlinuz-5.8.10-2rp4-6.1.7 
> root=UUID=01234567-dead-beef-d13d-456789abcdef ro
> devicetree /boot/dtb-5.8.10-2rp4-6.1.7
> initrd /boot/initrd.img-5.8.10-2rp4-6.1.7
> 
> My testing section for Xen is:
> xen_hypervisor /boot/xen-4.14-arm64.efi
> xen_module /boot/vmlinuz-5.8.10-2rp4-6.1.7 
> root=UUID=01234567-dead-beef-d13d-456789abcdef ro
> devicetree /boot/dtb-5.8.10-2rp4-6.1.7
> xen_module --nounzip /boot/initrd.img-5.8.10-2rp4-6.1.7
> 
> I've frankly got no idea how to ensure the correct device-tree is passed
> to Xen.  Is GRUB's `devicetree` command correct when loading Xen?  Is a
> device-tree matched to the Linux kernel appropriate for Xen?
> 
> (I'm guessing the second is "yes", but the first I don't have a clue)

Yes, devicetree is correct. I have not used the graphical output, so I
cannot help you there but yes the best bet is to use the devicetree that
comes with the kernel.

One thing I noticed is that you are missing some of the command line
arguments for Xen and Linux in your grub config. For instance on the Xen
line you want to have something like:

dom0_mem=1024M console=dtuart sync_console

And on the Linux line you might want to have:

console=tty0 console=hvc0

Re: [PATCH v4 3/3] ns16550: Gate all PCI code with CONFIG_X86

[Rahul Singh]

Hello Julien,

> On 25 Nov 2020, at 6:16 pm, Rahul Singh  wrote:
> 
> The NS16550 driver is assuming that NS16550 PCI card are usable if the
> architecture supports PCI (i.e. CONFIG_HAS_PCI=y). However, the code is
> very x86 focus and will fail to build on Arm (/!\ it is not all the
> errors):
> 
> ns16550.c: In function ‘ns16550_init_irq’:
> ns16550.c:726:21: error: implicit declaration of function ‘create_irq’;
> did you mean ‘release_irq’? [-Werror=implicit-function-declaration]
>  uart->irq = create_irq(0, false);
>  ^~
>  release_irq
> ns16550.c:726:21: error: nested extern declaration of ‘create_irq’
> [-Werror=nested-externs]
> ns16550.c: In function ‘ns16550_init_postirq’:
> ns16550.c:768:33: error: ‘mmio_ro_ranges’ undeclared (first use in this
> function); did you mean ‘mmio_handler’?
>   rangeset_add_range(mmio_ro_ranges, uart->io_base,
>  ^~
>  mmio_handler
> ns16550.c:768:33: note: each undeclared identifier is reported only once
> for each function it appears in
> ns16550.c:780:20: error: variable ‘msi’ has initializer but incomplete
> type
>  struct msi_info msi = {
> ^~~~
> 
> Enabling support for NS16550 PCI card on Arm would require more plumbing
> in addition to fixing the compilation error.
> 
> Arm systems tend to have platform UART available such as NS16550, PL011.
> So there are limited reasons to get NS16550 PCI support for now on Arm.
> 
> Guard all remaining PCI code that is not under x86 flag with CONFIG_X86.
> 
> No functional change intended.
> 
> Signed-off-by: Rahul Singh 

Sorry I missed to add the signed off for the commit msg. I will send the next 
version once reviewed done.
Signed-off-by: Julien Grall 

Regards,
Rahul
> ---
> 
> Changes in v4:
> - As per the discussion guard all remaining PCI code with CONFIG_X86
> 
> ---
> xen/drivers/char/ns16550.c | 16 
> 1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index 9235d854fe..26e601857a 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -16,7 +16,7 @@
> #include 
> #include 
> #include 
> -#ifdef CONFIG_HAS_PCI
> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
> #include 
> #include 
> #include 
> @@ -51,7 +51,7 @@ static struct ns16550 {
> unsigned int timeout_ms;
> bool_t intr_works;
> bool_t dw_usr_bsy;
> -#ifdef CONFIG_HAS_PCI
> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
> /* PCI card parameters. */
> bool_t pb_bdf_enable;   /* if =1, pb-bdf effective, port behind bridge */
> bool_t ps_bdf_enable;   /* if =1, ps_bdf effective, port on pci card */
> @@ -66,7 +66,7 @@ static struct ns16550 {
> #endif
> } ns16550_com[2] = { { 0 } };
> 
> -#ifdef CONFIG_HAS_PCI
> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
> struct ns16550_config {
> u16 vendor_id;
> u16 dev_id;
> @@ -256,7 +256,7 @@ static int ns16550_getc(struct serial_port *port, char 
> *pc)
> 
> static void pci_serial_early_init(struct ns16550 *uart)
> {
> -#ifdef CONFIG_HAS_PCI
> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
> if ( !uart->ps_bdf_enable || uart->io_base >= 0x1 )
> return;
> 
> @@ -355,7 +355,7 @@ static void __init ns16550_init_preirq(struct serial_port 
> *port)
> 
> static void __init ns16550_init_irq(struct serial_port *port)
> {
> -#ifdef CONFIG_HAS_PCI
> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
> struct ns16550 *uart = port->uart;
> 
> if ( uart->msi )
> @@ -397,7 +397,7 @@ static void __init ns16550_init_postirq(struct 
> serial_port *port)
> uart->timeout_ms = max_t(
> unsigned int, 1, (bits * uart->fifo_size * 1000) / uart->baud);
> 
> -#ifdef CONFIG_HAS_PCI
> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
> if ( uart->bar || uart->ps_bdf_enable )
> {
> if ( uart->param && uart->param->mmio &&
> @@ -477,7 +477,7 @@ static void ns16550_suspend(struct serial_port *port)
> 
> stop_timer(>timer);
> 
> -#ifdef CONFIG_HAS_PCI
> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
> if ( uart->bar )
>uart->cr = pci_conf_read16(PCI_SBDF(0, uart->ps_bdf[0], 
> uart->ps_bdf[1],
>   uart->ps_bdf[2]), PCI_COMMAND);
> @@ -486,7 +486,7 @@ static void ns16550_suspend(struct serial_port *port)
> 
> static void _ns16550_resume(struct serial_port *port)
> {
> -#ifdef CONFIG_HAS_PCI
> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
> struct ns16550 *uart = port->uart;
> 
> if ( uart->bar )
> -- 
> 2.17.1
> 

[PATCH v4 3/3] ns16550: Gate all PCI code with CONFIG_X86

[Rahul Singh]

The NS16550 driver is assuming that NS16550 PCI card are usable if the
architecture supports PCI (i.e. CONFIG_HAS_PCI=y). However, the code is
very x86 focus and will fail to build on Arm (/!\ it is not all the
errors):

ns16550.c: In function ‘ns16550_init_irq’:
ns16550.c:726:21: error: implicit declaration of function ‘create_irq’;
did you mean ‘release_irq’? [-Werror=implicit-function-declaration]
  uart->irq = create_irq(0, false);
  ^~
  release_irq
ns16550.c:726:21: error: nested extern declaration of ‘create_irq’
[-Werror=nested-externs]
ns16550.c: In function ‘ns16550_init_postirq’:
ns16550.c:768:33: error: ‘mmio_ro_ranges’ undeclared (first use in this
function); did you mean ‘mmio_handler’?
   rangeset_add_range(mmio_ro_ranges, uart->io_base,
  ^~
  mmio_handler
ns16550.c:768:33: note: each undeclared identifier is reported only once
for each function it appears in
ns16550.c:780:20: error: variable ‘msi’ has initializer but incomplete
type
  struct msi_info msi = {
 ^~~~

Enabling support for NS16550 PCI card on Arm would require more plumbing
in addition to fixing the compilation error.

Arm systems tend to have platform UART available such as NS16550, PL011.
So there are limited reasons to get NS16550 PCI support for now on Arm.

Guard all remaining PCI code that is not under x86 flag with CONFIG_X86.

No functional change intended.

Signed-off-by: Rahul Singh 
---

Changes in v4:
- As per the discussion guard all remaining PCI code with CONFIG_X86

---
 xen/drivers/char/ns16550.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 9235d854fe..26e601857a 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -16,7 +16,7 @@
 #include 
 #include 
 #include 
-#ifdef CONFIG_HAS_PCI
+#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
 #include 
 #include 
 #include 
@@ -51,7 +51,7 @@ static struct ns16550 {
 unsigned int timeout_ms;
 bool_t intr_works;
 bool_t dw_usr_bsy;
-#ifdef CONFIG_HAS_PCI
+#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
 /* PCI card parameters. */
 bool_t pb_bdf_enable;   /* if =1, pb-bdf effective, port behind bridge */
 bool_t ps_bdf_enable;   /* if =1, ps_bdf effective, port on pci card */
@@ -66,7 +66,7 @@ static struct ns16550 {
 #endif
 } ns16550_com[2] = { { 0 } };
 
-#ifdef CONFIG_HAS_PCI
+#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
 struct ns16550_config {
 u16 vendor_id;
 u16 dev_id;
@@ -256,7 +256,7 @@ static int ns16550_getc(struct serial_port *port, char *pc)
 
 static void pci_serial_early_init(struct ns16550 *uart)
 {
-#ifdef CONFIG_HAS_PCI
+#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
 if ( !uart->ps_bdf_enable || uart->io_base >= 0x1 )
 return;
 
@@ -355,7 +355,7 @@ static void __init ns16550_init_preirq(struct serial_port 
*port)
 
 static void __init ns16550_init_irq(struct serial_port *port)
 {
-#ifdef CONFIG_HAS_PCI
+#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
 struct ns16550 *uart = port->uart;
 
 if ( uart->msi )
@@ -397,7 +397,7 @@ static void __init ns16550_init_postirq(struct serial_port 
*port)
 uart->timeout_ms = max_t(
 unsigned int, 1, (bits * uart->fifo_size * 1000) / uart->baud);
 
-#ifdef CONFIG_HAS_PCI
+#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
 if ( uart->bar || uart->ps_bdf_enable )
 {
 if ( uart->param && uart->param->mmio &&
@@ -477,7 +477,7 @@ static void ns16550_suspend(struct serial_port *port)
 
 stop_timer(>timer);
 
-#ifdef CONFIG_HAS_PCI
+#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
 if ( uart->bar )
uart->cr = pci_conf_read16(PCI_SBDF(0, uart->ps_bdf[0], uart->ps_bdf[1],
   uart->ps_bdf[2]), PCI_COMMAND);
@@ -486,7 +486,7 @@ static void ns16550_suspend(struct serial_port *port)
 
 static void _ns16550_resume(struct serial_port *port)
 {
-#ifdef CONFIG_HAS_PCI
+#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
 struct ns16550 *uart = port->uart;
 
 if ( uart->bar )
-- 
2.17.1

[PATCH v4 2/3] xen/pci: solve compilation error on ARM with HAS_PCI enabled.

[Rahul Singh]

If mem-sharing, mem-paging, or log-dirty functionality is not enabled
for architecture when HAS_PCI is enabled, the compiler will throw an
error.

Move code to x86 specific file to fix compilation error.

Also, modify the code to use likely() in place of unlikley() for each
condition to make code more optimized.

No functional change intended.

Signed-off-by: Rahul Singh 
---

Changes in v4:
- fixed minor comments

---
 xen/drivers/passthrough/pci.c   |  8 +---
 xen/drivers/passthrough/x86/iommu.c | 13 +
 xen/include/xen/iommu.h |  2 ++
 3 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 3c6ab1bcb6..4c21655b7d 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -20,7 +20,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -1418,12 +1417,7 @@ static int assign_device(struct domain *d, u16 seg, u8 
bus, u8 devfn, u32 flag)
 if ( !is_iommu_enabled(d) )
 return 0;
 
-/* Prevent device assign if mem paging or mem sharing have been 
- * enabled for this domain */
-if ( d != dom_io &&
- unlikely(mem_sharing_enabled(d) ||
-  vm_event_check_ring(d->vm_event_paging) ||
-  p2m_get_hostp2m(d)->global_logdirty) )
+if( !arch_iommu_use_permitted(d) )
 return -EXDEV;
 
 /* device_assigned() should already have cleared the device for assignment 
*/
diff --git a/xen/drivers/passthrough/x86/iommu.c 
b/xen/drivers/passthrough/x86/iommu.c
index f17b1820f4..cea1032b3d 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -308,6 +309,18 @@ struct page_info *iommu_alloc_pgtable(struct domain *d)
 return pg;
 }
 
+bool arch_iommu_use_permitted(const struct domain *d)
+{
+/*
+ * Prevent device assign if mem paging, mem sharing or log-dirty
+ * have been enabled for this domain.
+ */
+return d == dom_io ||
+   (likely(!mem_sharing_enabled(d)) &&
+likely(!vm_event_check_ring(d->vm_event_paging)) &&
+likely(!p2m_get_hostp2m(d)->global_logdirty));
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 191021870f..056eaa09fc 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -381,6 +381,8 @@ DECLARE_PER_CPU(bool_t, iommu_dont_flush_iotlb);
 extern struct spinlock iommu_pt_cleanup_lock;
 extern struct page_list_head iommu_pt_cleanup_list;
 
+bool arch_iommu_use_permitted(const struct domain *d);
+
 #endif /* _IOMMU_H_ */
 
 /*
-- 
2.17.1

[PATCH v4 1/3] xen/pci: Move x86 specific code to x86 directory.

[Rahul Singh]

passthrough/pci.c file is common for all architecture, but there is x86
specific code in this file.

Move x86 specific code to the drivers/passthrough/io.c file to avoid
compilation error for other architecture.

As drivers/passthrough/io.c is compiled only for x86 move it to
x86 directory and rename it to hvm.c.

No functional change intended.

Signed-off-by: Rahul Singh 
---

Changes in v4:
- fixed compilation error when CONFIG_HVM is disabled 
- remove iommu_update_ire_from_msi from the patch will send another patch
  to fix.

---
 xen/drivers/passthrough/Makefile|  3 -
 xen/drivers/passthrough/pci.c   | 71 +
 xen/drivers/passthrough/x86/Makefile|  1 +
 xen/drivers/passthrough/{io.c => x86/hvm.c} | 66 +++
 xen/include/xen/pci.h   |  9 +++
 5 files changed, 77 insertions(+), 73 deletions(-)
 rename xen/drivers/passthrough/{io.c => x86/hvm.c} (95%)

diff --git a/xen/drivers/passthrough/Makefile b/xen/drivers/passthrough/Makefile
index e973e16c74..cc646612c7 100644
--- a/xen/drivers/passthrough/Makefile
+++ b/xen/drivers/passthrough/Makefile
@@ -6,6 +6,3 @@ obj-$(CONFIG_ARM) += arm/
 obj-y += iommu.o
 obj-$(CONFIG_HAS_PCI) += pci.o
 obj-$(CONFIG_HAS_DEVICE_TREE) += device_tree.o
-
-x86-$(CONFIG_HVM) := io.o
-obj-$(CONFIG_X86) += $(x86-y)
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 51e584127e..3c6ab1bcb6 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -14,9 +14,6 @@
  * this program; If not, see .
  */
 
-#include 
-#include 
-#include 
 #include 
 #include 
 #include 
@@ -24,7 +21,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -842,71 +838,6 @@ int pci_remove_device(u16 seg, u8 bus, u8 devfn)
 return ret;
 }
 
-static int pci_clean_dpci_irq(struct domain *d,
-  struct hvm_pirq_dpci *pirq_dpci, void *arg)
-{
-struct dev_intx_gsi_link *digl, *tmp;
-
-pirq_guest_unbind(d, dpci_pirq(pirq_dpci));
-
-if ( pt_irq_need_timer(pirq_dpci->flags) )
-kill_timer(_dpci->timer);
-
-list_for_each_entry_safe ( digl, tmp, _dpci->digl_list, list )
-{
-list_del(>list);
-xfree(digl);
-}
-
-radix_tree_delete(>pirq_tree, dpci_pirq(pirq_dpci)->pirq);
-
-if ( !pt_pirq_softirq_active(pirq_dpci) )
-return 0;
-
-domain_get_irq_dpci(d)->pending_pirq_dpci = pirq_dpci;
-
-return -ERESTART;
-}
-
-static int pci_clean_dpci_irqs(struct domain *d)
-{
-struct hvm_irq_dpci *hvm_irq_dpci = NULL;
-
-if ( !is_iommu_enabled(d) )
-return 0;
-
-if ( !is_hvm_domain(d) )
-return 0;
-
-spin_lock(>event_lock);
-hvm_irq_dpci = domain_get_irq_dpci(d);
-if ( hvm_irq_dpci != NULL )
-{
-int ret = 0;
-
-if ( hvm_irq_dpci->pending_pirq_dpci )
-{
-if ( pt_pirq_softirq_active(hvm_irq_dpci->pending_pirq_dpci) )
- ret = -ERESTART;
-else
- hvm_irq_dpci->pending_pirq_dpci = NULL;
-}
-
-if ( !ret )
-ret = pt_pirq_iterate(d, pci_clean_dpci_irq, NULL);
-if ( ret )
-{
-spin_unlock(>event_lock);
-return ret;
-}
-
-hvm_domain_irq(d)->dpci = NULL;
-free_hvm_irq_dpci(hvm_irq_dpci);
-}
-spin_unlock(>event_lock);
-return 0;
-}
-
 /* Caller should hold the pcidevs_lock */
 static int deassign_device(struct domain *d, uint16_t seg, uint8_t bus,
uint8_t devfn)
@@ -966,7 +897,7 @@ int pci_release_devices(struct domain *d)
 int ret;
 
 pcidevs_lock();
-ret = pci_clean_dpci_irqs(d);
+ret = arch_pci_clean_pirqs(d);
 if ( ret )
 {
 pcidevs_unlock();
diff --git a/xen/drivers/passthrough/x86/Makefile 
b/xen/drivers/passthrough/x86/Makefile
index a70cf9460d..69284a5d19 100644
--- a/xen/drivers/passthrough/x86/Makefile
+++ b/xen/drivers/passthrough/x86/Makefile
@@ -1,2 +1,3 @@
 obj-y += ats.o
 obj-y += iommu.o
+obj-$(CONFIG_HVM) += hvm.o
diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/x86/hvm.c
similarity index 95%
rename from xen/drivers/passthrough/io.c
rename to xen/drivers/passthrough/x86/hvm.c
index 6b1305a3e5..41cfa2e200 100644
--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/x86/hvm.c
@@ -1036,6 +1036,72 @@ unlock:
 spin_unlock(>event_lock);
 }
 
+static int pci_clean_dpci_irq(struct domain *d,
+  struct hvm_pirq_dpci *pirq_dpci, void *arg)
+{
+struct dev_intx_gsi_link *digl, *tmp;
+
+pirq_guest_unbind(d, dpci_pirq(pirq_dpci));
+
+if ( pt_irq_need_timer(pirq_dpci->flags) )
+kill_timer(_dpci->timer);
+
+list_for_each_entry_safe ( digl, tmp, _dpci->digl_list, list )
+{
+list_del(>list);
+xfree(digl);
+}
+
+radix_tree_delete(>pirq_tree, 

[PATCH v4 0/3] xen/arm: Make PCI passthrough code non-x86 specific

[Rahul Singh]

This patch series is v4 of preparatory work to make PCI passthrough code
non-x86 specific.

Rahul Singh (3):
  xen/pci: Move x86 specific code to x86 directory.
  xen/pci: solve compilation error on ARM with HAS_PCI enabled.
  ns16550: Gate all PCI code with CONFIG_X86

 xen/drivers/char/ns16550.c  | 16 ++---
 xen/drivers/passthrough/Makefile|  3 -
 xen/drivers/passthrough/pci.c   | 79 +
 xen/drivers/passthrough/x86/Makefile|  1 +
 xen/drivers/passthrough/{io.c => x86/hvm.c} | 66 +
 xen/drivers/passthrough/x86/iommu.c | 13 
 xen/include/xen/iommu.h |  2 +
 xen/include/xen/pci.h   |  9 +++
 8 files changed, 101 insertions(+), 88 deletions(-)
 rename xen/drivers/passthrough/{io.c => x86/hvm.c} (95%)

-- 
2.17.1

Re: [PATCH RFC 4/6] xen/arm: mm: Allow other mapping size in xen_pt_update_entry()

[Julien Grall]

On 24/11/2020 18:13, Bertrand Marquis wrote:

Hi Julien,


Hi Bertrand,


On 19 Nov 2020, at 19:07, Julien Grall  wrote:

From: Julien Grall 

At the moment, xen_pt_update_entry() only supports mapping at level 3
(i.e 4KB mapping). While this is fine for most of the runtime helper,
the boot code will require to use superpage mapping.

We don't want to allow superpage mapping by default as some of the
callers may expect small mappings (i.e populate_pt_range()) or even
expect to unmap only a part of a superpage.

To keep the code simple, a new flag _PAGE_BLOCK is introduced to
allow the caller to enable superpage mapping.

As the code doesn't support all the combinations, xen_pt_check_entry()
is extended to take into account the cases we don't support when
using block mapping:
- Replacing a table with a mapping. This may happen if region was
first mapped with 4KB mapping and then later on replaced with a 2MB
(or 1GB mapping)
- Removing/modify a table. This may happen if a caller try to remove a
region with _PAGE_BLOCK set when it was created without it

Note that the current restriction mean that the caller must ensure that
_PAGE_BLOCK is consistently set/cleared across all the updates on a
given virtual region. This ought to be fine with the expected use-cases.

More rework will be necessary if we wanted to remove the restrictions.

Note that nr_mfns is now marked const as it is used for flushing the
TLBs and we don't want it to be modified.

Signed-off-by: Julien Grall 



First I did test the serie on Arm and so far it was working properly.


Thanks for the testing and...



I only have some remarks because even if the code is right, I think
some parts of the code are not easy to read...


... I am always open for suggestion :).


---

This patch is necessary for upcoming changes in the MM code. I would
like to remove most of the open-coding update of the page-tables as they
are not easy to properly fix/extend. For instance, always mapping
xenheap mapping with 1GB superpage is plain wrong because:
- RAM regions are not always 1GB aligned (such as on RPI 4) and we
may end up to map MMIO with cacheable attributes
- RAM may contain reserved regions should either not be mapped
---
xen/arch/arm/mm.c  | 87 ++
xen/include/asm-arm/page.h |  4 ++
2 files changed, 73 insertions(+), 18 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 59f8a3f15fd1..af0f12b6e6d3 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1060,9 +1060,10 @@ static int xen_pt_next_level(bool read_only, unsigned 
int level,
}

/* Sanity check of the entry */
-static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
+static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int level,
+   unsigned int flags)
{
-/* Sanity check when modifying a page. */
+/* Sanity check when modifying an entry. */
 if ( (flags & _PAGE_PRESENT) && mfn_eq(mfn, INVALID_MFN) )
 {
 /* We don't allow modifying an invalid entry. */
@@ -1072,6 +1073,13 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, 
unsigned int flags)
 return false;
 }

+/* We don't allow modifying a table entry */
+if ( !lpae_is_mapping(entry, level) )
+{
+mm_printk("Modifying a table entry is not allowed.\n");
+return false;
+}
+
 /* We don't allow changing memory attributes. */
 if ( entry.pt.ai != PAGE_AI_MASK(flags) )
 {
@@ -1087,7 +1095,7 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, 
unsigned int flags)
 return false;
 }
 }
-/* Sanity check when inserting a page */
+/* Sanity check when inserting a mapping */
 else if ( flags & _PAGE_PRESENT )
 {
 /* We should be here with a valid MFN. */
@@ -1096,18 +1104,28 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, 
unsigned int flags)
 /* We don't allow replacing any valid entry. */
 if ( lpae_is_valid(entry) )
 {
-mm_printk("Changing MFN for a valid entry is not allowed (%#"PRI_mfn" -> 
%#"PRI_mfn").\n",
-  mfn_x(lpae_get_mfn(entry)), mfn_x(mfn));
+if ( lpae_is_mapping(entry, level) )
+mm_printk("Changing MFN for a valid entry is not allowed (%#"PRI_mfn" -> 
%#"PRI_mfn").\n",
+  mfn_x(lpae_get_mfn(entry)), mfn_x(mfn));
+else
+mm_printk("Trying to replace a table with a mapping.\n");
 return false;
 }
 }
-/* Sanity check when removing a page. */
+/* Sanity check when removing a mapping. */
 else if ( (flags & (_PAGE_PRESENT|_PAGE_POPULATE)) == 0 )
 {
 /* We should be here with an invalid MFN. */
 ASSERT(mfn_eq(mfn, INVALID_MFN));

-/* We don't allow removing page with contiguous bit set. */
+   

Re: [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang

[Miguel Ojeda]

On Wed, Nov 25, 2020 at 5:24 PM Jakub Kicinski  wrote:
>
> And just to spell it out,
>
> case ENUM_VALUE1:
> bla();
> break;
> case ENUM_VALUE2:
> bla();
> default:
> break;
>
> is a fairly idiomatic way of indicating that not all values of the enum
> are expected to be handled by the switch statement.

It looks like a benign typo to me -- `ENUM_VALUE2` does not follow the
same pattern like `ENUM_VALUE1`. To me, the presence of the `default`
is what indicates (explicitly) that not everything is handled.

> Applying a real patch set and then getting a few follow ups the next day
> for trivial coding things like fallthrough missing or static missing,
> just because I didn't have the full range of compilers to check with
> before applying makes me feel pretty shitty, like I'm not doing a good
> job. YMMV.

The number of compilers, checkers, static analyzers, tests, etc. we
use keeps going up. That, indeed, means maintainers will miss more
things (unless maintainers do more work than before). But catching
bugs before they happen is *not* a bad thing.

Perhaps we could encourage more rebasing in -next (while still giving
credit to bots and testers) to avoid having many fixing commits
afterwards, but that is orthogonal.

I really don't think we should encourage the feeling that a maintainer
is doing a bad job if they don't catch everything on their reviews.
Any review is worth it. Maintainers, in the end, are just the
"guaranteed" reviewers that decide when the code looks reasonable
enough. They should definitely not feel pressured to be perfect.

Cheers,
Miguel

Re: [PATCH] tools/libs: Simplify internal *.pc files

[Andrew Cooper]

On 25/11/2020 14:49, Andrew Cooper wrote:
> diff --git a/tools/Rules.mk b/tools/Rules.mk
> index f61da81f4a..5d92ff0699 100644
> --- a/tools/Rules.mk
> +++ b/tools/Rules.mk
> @@ -184,7 +184,7 @@ $(PKG_CONFIG_DIR)/%.pc: Makefile 
> $(XEN_ROOT)/tools/Rules.mk $(PKG_CONFIG_DIR)
>   echo "Description: $(PKG_CONFIG_DESC)"; \
>   echo "Version: $(PKG_CONFIG_VERSION)"; \
>   echo "Cflags: -I\$${includedir} $(CFLAGS_xeninclude)"; \
> - echo "Libs: -L\$${libdir} $(PKG_CONFIG_USELIBS) -l$(PKG_CONFIG_LIB)"; \
> + echo "Libs: -L\$${libdir} $(sort $(PKG_CONFIG_USELIBS)) 
> -l$(PKG_CONFIG_LIB)"; \
>   echo "Libs.private: $(PKG_CONFIG_LIBSPRIV)"; \
>   echo "Requires.private: $(PKG_CONFIG_REQPRIV)"; \
>   } > $@

Actually, it occurs to me that this would be better in libs.mk as

PKG_CONFIG_USELIBS := $(sort $(SHLIB_libxen$(LIBNAME)))

in case we gain any further uses of PKG_CONFIG_USELIBS

~Andrew

[xen-4.14-testing test] 156989: tolerable FAIL - PUSHED

[osstest service owner]

flight 156989 xen-4.14-testing real [real]
flight 157010 xen-4.14-testing real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/156989/
http://logs.test-lab.xenproject.org/osstest/logs/157010/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-arm64-arm64-xl-xsm   8 xen-bootfail pass in 157010-retest

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

version targeted for testing:
 xen  0057b1f8fa79abe8272690341db54b064c8f2b7f
baseline version:
 xen  d101b417b784a26326fc7800a79cc539ba570b79

Last test of basis   156716  2020-11-12 12:17:49 Z   13 days
Testing same since   156989  2020-11-24 13:36:54 Z1 days1 attempts


People who touched revisions under test:
  Jan Beulich 

jobs:
 build-amd64-xsm  

Re: [PATCH 23/45] block: remove i_bdev

[Christoph Hellwig]

On Tue, Nov 24, 2020 at 02:37:05PM -0500, Tejun Heo wrote:
> On Tue, Nov 24, 2020 at 02:27:29PM +0100, Christoph Hellwig wrote:
> > Switch the block device lookup interfaces to directly work with a dev_t
> > so that struct block_device references are only acquired by the
> > blkdev_get variants (and the blk-cgroup special case).  This means that
> > we not don't need an extra reference in the inode and can generally
>  ^
>  now
> > simplify handling of struct block_device to keep the lookups contained
> > in the core block layer code.
> > 
> > Signed-off-by: Christoph Hellwig 
> ...
> > @@ -1689,14 +1599,12 @@ static int blkdev_open(struct inode * inode, struct 
> > file * filp)
> > if ((filp->f_flags & O_ACCMODE) == 3)
> > filp->f_mode |= FMODE_WRITE_IOCTL;
> >  
> > -   bdev = bd_acquire(inode);
> > -   if (bdev == NULL)
> > -   return -ENOMEM;
> > -
> > +   bdev = blkdev_get_by_dev(inode->i_rdev, filp->f_mode, filp);
> > +   if (IS_ERR(bdev))
> > +   return PTR_ERR(bdev);
> > filp->f_mapping = bdev->bd_inode->i_mapping;
> > filp->f_wb_err = filemap_sample_wb_err(filp->f_mapping);
> > -
> > -   return blkdev_get(bdev, filp->f_mode, filp);
> > +   return 0;
> >  }
> 
> I was wondering whether losing the stale bdev flushing in bd_acquire() would
> cause user-visible behavior changes but can't see how it would given that
> userland has no way of holding onto a specific instance of block inode.
> Maybe it's something worth mentioning in the commit message?

With stale bdev flushing do you mean the call to bd_forget if
i_bdev exists but is unhashed?  It doesn't actually flush anything but
just detaches the old bdev from the inode so that the new one can be
attached.  That problem goes away by definition if we don't attach
the bdev to the inode.

Re: [PATCH] tools/libs: Simplify internal *.pc files

[Bertrand Marquis]

Hi Andrew,

> On 25 Nov 2020, at 14:49, Andrew Cooper  wrote:
> 
> The internal package config file for libxenlight reads (reformatted to avoid
> exceeding the SMTP 998-character line length):
> 
>  Libs: -L${libdir}
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/evtchn
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/call
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/evtchn
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/gnttab
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/foreignmemory
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/call
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/devicemodel
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/ctrl
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/store
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/call
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/hypfs
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/evtchn
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/call
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/evtchn
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/gnttab
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/foreignmemory
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
>  
> -Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
>  
> 

[PATCH AUTOSEL 4.4 4/8] x86/xen: don't unbind uninitialized lock_kicker_irq

[Sasha Levin]

From: Brian Masney 

[ Upstream commit 65cae18882f943215d0505ddc7e70495877308e6 ]

When booting a hyperthreaded system with the kernel parameter
'mitigations=auto,nosmt', the following warning occurs:

WARNING: CPU: 0 PID: 1 at drivers/xen/events/events_base.c:1112 
unbind_from_irqhandler+0x4e/0x60
...
Hardware name: Xen HVM domU, BIOS 4.2.amazon 08/24/2006
...
Call Trace:
 xen_uninit_lock_cpu+0x28/0x62
 xen_hvm_cpu_die+0x21/0x30
 takedown_cpu+0x9c/0xe0
 ? trace_suspend_resume+0x60/0x60
 cpuhp_invoke_callback+0x9a/0x530
 _cpu_up+0x11a/0x130
 cpu_up+0x7e/0xc0
 bringup_nonboot_cpus+0x48/0x50
 smp_init+0x26/0x79
 kernel_init_freeable+0xea/0x229
 ? rest_init+0xaa/0xaa
 kernel_init+0xa/0x106
 ret_from_fork+0x35/0x40

The secondary CPUs are not activated with the nosmt mitigations and only
the primary thread on each CPU core is used. In this situation,
xen_hvm_smp_prepare_cpus(), and more importantly xen_init_lock_cpu(), is
not called, so the lock_kicker_irq is not initialized for the secondary
CPUs. Let's fix this by exiting early in xen_uninit_lock_cpu() if the
irq is not set to avoid the warning from above for each secondary CPU.

Signed-off-by: Brian Masney 
Link: https://lore.kernel.org/r/2020110709.631442-1-bmas...@redhat.com
Reviewed-by: Juergen Gross 
Signed-off-by: Boris Ostrovsky 
Signed-off-by: Sasha Levin 
---
 arch/x86/xen/spinlock.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index 85872a08994a1..e9fc0f7df0da8 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -301,10 +301,20 @@ void xen_init_lock_cpu(int cpu)
 
 void xen_uninit_lock_cpu(int cpu)
 {
+   int irq;
+
if (!xen_pvspin)
return;
 
-   unbind_from_irqhandler(per_cpu(lock_kicker_irq, cpu), NULL);
+   /*
+* When booting the kernel with 'mitigations=auto,nosmt', the secondary
+* CPUs are not activated, and lock_kicker_irq is not initialized.
+*/
+   irq = per_cpu(lock_kicker_irq, cpu);
+   if (irq == -1)
+   return;
+
+   unbind_from_irqhandler(irq, NULL);
per_cpu(lock_kicker_irq, cpu) = -1;
kfree(per_cpu(irq_name, cpu));
per_cpu(irq_name, cpu) = NULL;
-- 
2.27.0

[PATCH AUTOSEL 4.9 05/10] x86/xen: don't unbind uninitialized lock_kicker_irq

[Sasha Levin]

From: Brian Masney 

[ Upstream commit 65cae18882f943215d0505ddc7e70495877308e6 ]

When booting a hyperthreaded system with the kernel parameter
'mitigations=auto,nosmt', the following warning occurs:

WARNING: CPU: 0 PID: 1 at drivers/xen/events/events_base.c:1112 
unbind_from_irqhandler+0x4e/0x60
...
Hardware name: Xen HVM domU, BIOS 4.2.amazon 08/24/2006
...
Call Trace:
 xen_uninit_lock_cpu+0x28/0x62
 xen_hvm_cpu_die+0x21/0x30
 takedown_cpu+0x9c/0xe0
 ? trace_suspend_resume+0x60/0x60
 cpuhp_invoke_callback+0x9a/0x530
 _cpu_up+0x11a/0x130
 cpu_up+0x7e/0xc0
 bringup_nonboot_cpus+0x48/0x50
 smp_init+0x26/0x79
 kernel_init_freeable+0xea/0x229
 ? rest_init+0xaa/0xaa
 kernel_init+0xa/0x106
 ret_from_fork+0x35/0x40

The secondary CPUs are not activated with the nosmt mitigations and only
the primary thread on each CPU core is used. In this situation,
xen_hvm_smp_prepare_cpus(), and more importantly xen_init_lock_cpu(), is
not called, so the lock_kicker_irq is not initialized for the secondary
CPUs. Let's fix this by exiting early in xen_uninit_lock_cpu() if the
irq is not set to avoid the warning from above for each secondary CPU.

Signed-off-by: Brian Masney 
Link: https://lore.kernel.org/r/2020110709.631442-1-bmas...@redhat.com
Reviewed-by: Juergen Gross 
Signed-off-by: Boris Ostrovsky 
Signed-off-by: Sasha Levin 
---
 arch/x86/xen/spinlock.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index 8d2c6f071dccf..44bf8a22c97b8 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -98,10 +98,20 @@ void xen_init_lock_cpu(int cpu)
 
 void xen_uninit_lock_cpu(int cpu)
 {
+   int irq;
+
if (!xen_pvspin)
return;
 
-   unbind_from_irqhandler(per_cpu(lock_kicker_irq, cpu), NULL);
+   /*
+* When booting the kernel with 'mitigations=auto,nosmt', the secondary
+* CPUs are not activated, and lock_kicker_irq is not initialized.
+*/
+   irq = per_cpu(lock_kicker_irq, cpu);
+   if (irq == -1)
+   return;
+
+   unbind_from_irqhandler(irq, NULL);
per_cpu(lock_kicker_irq, cpu) = -1;
kfree(per_cpu(irq_name, cpu));
per_cpu(irq_name, cpu) = NULL;
-- 
2.27.0

[PATCH AUTOSEL 4.14 05/12] x86/xen: don't unbind uninitialized lock_kicker_irq

[Sasha Levin]

From: Brian Masney 

[ Upstream commit 65cae18882f943215d0505ddc7e70495877308e6 ]

When booting a hyperthreaded system with the kernel parameter
'mitigations=auto,nosmt', the following warning occurs:

WARNING: CPU: 0 PID: 1 at drivers/xen/events/events_base.c:1112 
unbind_from_irqhandler+0x4e/0x60
...
Hardware name: Xen HVM domU, BIOS 4.2.amazon 08/24/2006
...
Call Trace:
 xen_uninit_lock_cpu+0x28/0x62
 xen_hvm_cpu_die+0x21/0x30
 takedown_cpu+0x9c/0xe0
 ? trace_suspend_resume+0x60/0x60
 cpuhp_invoke_callback+0x9a/0x530
 _cpu_up+0x11a/0x130
 cpu_up+0x7e/0xc0
 bringup_nonboot_cpus+0x48/0x50
 smp_init+0x26/0x79
 kernel_init_freeable+0xea/0x229
 ? rest_init+0xaa/0xaa
 kernel_init+0xa/0x106
 ret_from_fork+0x35/0x40

The secondary CPUs are not activated with the nosmt mitigations and only
the primary thread on each CPU core is used. In this situation,
xen_hvm_smp_prepare_cpus(), and more importantly xen_init_lock_cpu(), is
not called, so the lock_kicker_irq is not initialized for the secondary
CPUs. Let's fix this by exiting early in xen_uninit_lock_cpu() if the
irq is not set to avoid the warning from above for each secondary CPU.

Signed-off-by: Brian Masney 
Link: https://lore.kernel.org/r/2020110709.631442-1-bmas...@redhat.com
Reviewed-by: Juergen Gross 
Signed-off-by: Boris Ostrovsky 
Signed-off-by: Sasha Levin 
---
 arch/x86/xen/spinlock.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index 2527540051ff0..e22ee24396158 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -99,10 +99,20 @@ void xen_init_lock_cpu(int cpu)
 
 void xen_uninit_lock_cpu(int cpu)
 {
+   int irq;
+
if (!xen_pvspin)
return;
 
-   unbind_from_irqhandler(per_cpu(lock_kicker_irq, cpu), NULL);
+   /*
+* When booting the kernel with 'mitigations=auto,nosmt', the secondary
+* CPUs are not activated, and lock_kicker_irq is not initialized.
+*/
+   irq = per_cpu(lock_kicker_irq, cpu);
+   if (irq == -1)
+   return;
+
+   unbind_from_irqhandler(irq, NULL);
per_cpu(lock_kicker_irq, cpu) = -1;
kfree(per_cpu(irq_name, cpu));
per_cpu(irq_name, cpu) = NULL;
-- 
2.27.0

[PATCH AUTOSEL 4.19 07/15] x86/xen: don't unbind uninitialized lock_kicker_irq

[Sasha Levin]

From: Brian Masney 

[ Upstream commit 65cae18882f943215d0505ddc7e70495877308e6 ]

When booting a hyperthreaded system with the kernel parameter
'mitigations=auto,nosmt', the following warning occurs:

WARNING: CPU: 0 PID: 1 at drivers/xen/events/events_base.c:1112 
unbind_from_irqhandler+0x4e/0x60
...
Hardware name: Xen HVM domU, BIOS 4.2.amazon 08/24/2006
...
Call Trace:
 xen_uninit_lock_cpu+0x28/0x62
 xen_hvm_cpu_die+0x21/0x30
 takedown_cpu+0x9c/0xe0
 ? trace_suspend_resume+0x60/0x60
 cpuhp_invoke_callback+0x9a/0x530
 _cpu_up+0x11a/0x130
 cpu_up+0x7e/0xc0
 bringup_nonboot_cpus+0x48/0x50
 smp_init+0x26/0x79
 kernel_init_freeable+0xea/0x229
 ? rest_init+0xaa/0xaa
 kernel_init+0xa/0x106
 ret_from_fork+0x35/0x40

The secondary CPUs are not activated with the nosmt mitigations and only
the primary thread on each CPU core is used. In this situation,
xen_hvm_smp_prepare_cpus(), and more importantly xen_init_lock_cpu(), is
not called, so the lock_kicker_irq is not initialized for the secondary
CPUs. Let's fix this by exiting early in xen_uninit_lock_cpu() if the
irq is not set to avoid the warning from above for each secondary CPU.

Signed-off-by: Brian Masney 
Link: https://lore.kernel.org/r/2020110709.631442-1-bmas...@redhat.com
Reviewed-by: Juergen Gross 
Signed-off-by: Boris Ostrovsky 
Signed-off-by: Sasha Levin 
---
 arch/x86/xen/spinlock.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index 717b4847b473f..6fffb86a32add 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -101,10 +101,20 @@ void xen_init_lock_cpu(int cpu)
 
 void xen_uninit_lock_cpu(int cpu)
 {
+   int irq;
+
if (!xen_pvspin)
return;
 
-   unbind_from_irqhandler(per_cpu(lock_kicker_irq, cpu), NULL);
+   /*
+* When booting the kernel with 'mitigations=auto,nosmt', the secondary
+* CPUs are not activated, and lock_kicker_irq is not initialized.
+*/
+   irq = per_cpu(lock_kicker_irq, cpu);
+   if (irq == -1)
+   return;
+
+   unbind_from_irqhandler(irq, NULL);
per_cpu(lock_kicker_irq, cpu) = -1;
kfree(per_cpu(irq_name, cpu));
per_cpu(irq_name, cpu) = NULL;
-- 
2.27.0

[PATCH AUTOSEL 5.4 10/23] x86/xen: don't unbind uninitialized lock_kicker_irq

[Sasha Levin]

From: Brian Masney 

[ Upstream commit 65cae18882f943215d0505ddc7e70495877308e6 ]

When booting a hyperthreaded system with the kernel parameter
'mitigations=auto,nosmt', the following warning occurs:

WARNING: CPU: 0 PID: 1 at drivers/xen/events/events_base.c:1112 
unbind_from_irqhandler+0x4e/0x60
...
Hardware name: Xen HVM domU, BIOS 4.2.amazon 08/24/2006
...
Call Trace:
 xen_uninit_lock_cpu+0x28/0x62
 xen_hvm_cpu_die+0x21/0x30
 takedown_cpu+0x9c/0xe0
 ? trace_suspend_resume+0x60/0x60
 cpuhp_invoke_callback+0x9a/0x530
 _cpu_up+0x11a/0x130
 cpu_up+0x7e/0xc0
 bringup_nonboot_cpus+0x48/0x50
 smp_init+0x26/0x79
 kernel_init_freeable+0xea/0x229
 ? rest_init+0xaa/0xaa
 kernel_init+0xa/0x106
 ret_from_fork+0x35/0x40

The secondary CPUs are not activated with the nosmt mitigations and only
the primary thread on each CPU core is used. In this situation,
xen_hvm_smp_prepare_cpus(), and more importantly xen_init_lock_cpu(), is
not called, so the lock_kicker_irq is not initialized for the secondary
CPUs. Let's fix this by exiting early in xen_uninit_lock_cpu() if the
irq is not set to avoid the warning from above for each secondary CPU.

Signed-off-by: Brian Masney 
Link: https://lore.kernel.org/r/2020110709.631442-1-bmas...@redhat.com
Reviewed-by: Juergen Gross 
Signed-off-by: Boris Ostrovsky 
Signed-off-by: Sasha Levin 
---
 arch/x86/xen/spinlock.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index 6deb49094c605..d817b7c862a62 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -93,10 +93,20 @@ void xen_init_lock_cpu(int cpu)
 
 void xen_uninit_lock_cpu(int cpu)
 {
+   int irq;
+
if (!xen_pvspin)
return;
 
-   unbind_from_irqhandler(per_cpu(lock_kicker_irq, cpu), NULL);
+   /*
+* When booting the kernel with 'mitigations=auto,nosmt', the secondary
+* CPUs are not activated, and lock_kicker_irq is not initialized.
+*/
+   irq = per_cpu(lock_kicker_irq, cpu);
+   if (irq == -1)
+   return;
+
+   unbind_from_irqhandler(irq, NULL);
per_cpu(lock_kicker_irq, cpu) = -1;
kfree(per_cpu(irq_name, cpu));
per_cpu(irq_name, cpu) = NULL;
-- 
2.27.0

[PATCH AUTOSEL 5.9 10/33] x86/xen: don't unbind uninitialized lock_kicker_irq

[Sasha Levin]

From: Brian Masney 

[ Upstream commit 65cae18882f943215d0505ddc7e70495877308e6 ]

When booting a hyperthreaded system with the kernel parameter
'mitigations=auto,nosmt', the following warning occurs:

WARNING: CPU: 0 PID: 1 at drivers/xen/events/events_base.c:1112 
unbind_from_irqhandler+0x4e/0x60
...
Hardware name: Xen HVM domU, BIOS 4.2.amazon 08/24/2006
...
Call Trace:
 xen_uninit_lock_cpu+0x28/0x62
 xen_hvm_cpu_die+0x21/0x30
 takedown_cpu+0x9c/0xe0
 ? trace_suspend_resume+0x60/0x60
 cpuhp_invoke_callback+0x9a/0x530
 _cpu_up+0x11a/0x130
 cpu_up+0x7e/0xc0
 bringup_nonboot_cpus+0x48/0x50
 smp_init+0x26/0x79
 kernel_init_freeable+0xea/0x229
 ? rest_init+0xaa/0xaa
 kernel_init+0xa/0x106
 ret_from_fork+0x35/0x40

The secondary CPUs are not activated with the nosmt mitigations and only
the primary thread on each CPU core is used. In this situation,
xen_hvm_smp_prepare_cpus(), and more importantly xen_init_lock_cpu(), is
not called, so the lock_kicker_irq is not initialized for the secondary
CPUs. Let's fix this by exiting early in xen_uninit_lock_cpu() if the
irq is not set to avoid the warning from above for each secondary CPU.

Signed-off-by: Brian Masney 
Link: https://lore.kernel.org/r/2020110709.631442-1-bmas...@redhat.com
Reviewed-by: Juergen Gross 
Signed-off-by: Boris Ostrovsky 
Signed-off-by: Sasha Levin 
---
 arch/x86/xen/spinlock.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index 799f4eba0a621..043c73dfd2c98 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -93,10 +93,20 @@ void xen_init_lock_cpu(int cpu)
 
 void xen_uninit_lock_cpu(int cpu)
 {
+   int irq;
+
if (!xen_pvspin)
return;
 
-   unbind_from_irqhandler(per_cpu(lock_kicker_irq, cpu), NULL);
+   /*
+* When booting the kernel with 'mitigations=auto,nosmt', the secondary
+* CPUs are not activated, and lock_kicker_irq is not initialized.
+*/
+   irq = per_cpu(lock_kicker_irq, cpu);
+   if (irq == -1)
+   return;
+
+   unbind_from_irqhandler(irq, NULL);
per_cpu(lock_kicker_irq, cpu) = -1;
kfree(per_cpu(irq_name, cpu));
per_cpu(irq_name, cpu) = NULL;
-- 
2.27.0

[PATCH] tools/libs: Simplify internal *.pc files

[Andrew Cooper]

The internal package config file for libxenlight reads (reformatted to avoid
exceeding the SMTP 998-character line length):

  Libs: -L${libdir}
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/evtchn
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/call
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/evtchn
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/gnttab
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/foreignmemory
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/call
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/devicemodel
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/ctrl
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/store
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/call
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/hypfs
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/evtchn
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/call
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/evtchn
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/gnttab
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/foreignmemory
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toollog
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/toolcore
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/call
  
-Wl,-rpath-link=/local/security/xen.git/tools/libs/light/../../../tools/libs/devicemodel
  

[xen-unstable-smoke test] 157006: tolerable all pass - PUSHED

[osstest service owner]

flight 157006 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/157006/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  fd7479b9aec25885cc17d33b326b9babae59faee
baseline version:
 xen  9b156bcc3ffcc7949edd4460b718a241e87ae302

Last test of basis   156991  2020-11-24 14:01:23 Z1 days
Testing same since   157006  2020-11-25 12:00:29 Z0 days1 attempts


People who touched revisions under test:
  Bertrand Marquis 
  Julien Grall 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



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

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

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   9b156bcc3f..fd7479b9ae  fd7479b9aec25885cc17d33b326b9babae59faee -> smoke

Xen 4.15: Proposed release schedule

[Ian Jackson]

(resending because the first one had corrupted email headers;
 please reply to this one and not the previous one)

Hi.  I've done a little bit of consultation with previous release
managers, and reviewed various list archives and calendars.  These
consultations seemed to suggest some folklore that wasn't captured in
our process doc - hence the proposed patch, below.

I would like to tentatively propose the following schedule and
policies for Xen 4.15.

If you have opinions, please comment as soon as you can so that we can
have an open dialogue.  Comments must be submitted at the very latest
by 1700 UTC on Wednesday the 2nd of December.

Having never done this before, I am particularly interested in
comments from previous release managers.

** DRAFT **

  Friday 8th JanuaryLast posting date

Patches adding new features should be posted to the mailing list
by this cate, although perhaps not in their final version.

  Friday 22nd January   Feature freeze
 
Patches adding new features should be committed by this date.
Straightforward bugfixes may continue to be accepted by
maintainers.

  Friday 12th February **tentatve**   Code freeze

Bugfixes only, all changes to be approved by the Release Manager.

  Week of 12th March **tentative**   Release
(probably Tuesday or Wednesday)

Any patches containing substantial refactoring are to treated as
new features, even if they intent is to fix bugs.

Freeze exceptions will not be routine, but may be granted in
exceptional cases for small changes on the basis of risk assessment.
Large series will not get exceptions.  Contributors *must not* rely on
getting, or expect, a freeze exception.

Chinese New Year falls around the 11th-19th of February this year.  In
my plan above, that falls within the hard code freeze period.  If we
don't manage to get the tree to an acceptable quality level by the
tentative codefreeze and release dates above, these dates will slip.

I have not yet started tracking "big ticket" items, and bugs.  I
expect to start doing that starting after Christmas.  NB the primary
responsibility for driving a feature's progress to meet the release
schedule, lies with the feature's proponents.

If as a feature proponent you feel your feature is at risk and there
is something the Xen Project could do to help, please consult me or
the Community Manager.  In such situations please reach out earlier
rather than later.

** END OF DRAFT **

Thanks,
Ian.

>From b34f4ddace0b8d76d8c340a46288a2db79c99460 Mon Sep 17 00:00:00 2001
From: Ian Jackson 
To: xen-devel@lists.xenproject.org
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Julien Grall 
Cc: Stefano Stabellini 
Date: Wed, 25 Nov 2020 13:22:08 +
Subject: [PATCH] xen-release-management doc: More info on schedule
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This documents our practice, established in 2018
  https://lists.xen.org/archives/html/xen-devel/2018-07/msg02240.html
et seq

CC: Jürgen Groß 
CC: Paul Durrant 
CC: Wei Liu 
Signed-off-by: Ian Jackson 
---
 docs/process/xen-release-management.pandoc | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/docs/process/xen-release-management.pandoc 
b/docs/process/xen-release-management.pandoc
index e1aa1eda8f..a5d70fed67 100644
--- a/docs/process/xen-release-management.pandoc
+++ b/docs/process/xen-release-management.pandoc
@@ -15,8 +15,10 @@ that they can have an idea what to expect from the Release 
Manager.
 
 # Xen release cycle
 
-The Xen hypervisor project now releases every 8 months. The actual release date
-depends on a lot of factors.
+The Xen hypervisor project now releases every 8 months.  We aim to
+release in the first half of March/July/November.  These dates have
+been chosen to avoid major holidays and cultural events; if one
+release slips, ideally the previous release cycle would be shortened.
 
 We can roughly divide one release into two periods. The development period
 and the freeze period. The former is 6 months long and the latter is about 2
@@ -33,6 +35,12 @@ During freeze period, the tree is closed for new features. 
Only bug fixes are
 accepted. This period can be shorter or longer than 2 months. If it ends up
 longer than 2 months, it eats into the next development period.
 
+The precise release schedule depends on a lot of factors and needs to
+be set afresh by the Release Manager in each release cycle.  When the
+release is in March, particular consideration should be given to the
+Chinese New Year holidaty which will then typically occur curing the
+freeze, so the freeze should probably be extended to compensate.
+
 # The different roles in a Xen release
 
 ## Release Manager
-- 
2.20.1

Xen 4.15: Proposed release schedule

[Ian Jackson]

Andrew Cooper ,
George Dunlap ,
Jan Beulich ,
Julien Grall ,
Stefano Stabellini ,
=?iso-8859-1?Q?J=FCrgen_Gro=DF?= ,
Paul Durrant ,
Wei Liu 
FCC: ~/mail/Outbound
--text follows this line--
Hi.  I've done a little bit of consultation with previous release
managers, and reviewed various list archives and calendars.  These
consultations seemed to suggest some folklore that wasn't captured in
our process doc - hence the proposed patch, below.

I would like to tentatively propose the following schedule and
policies for Xen 4.15.

If you have opinions, please comment as soon as you can so that we can
have an open dialogue.  Comments must be submitted at the very latest
by 1700 UTC on Wednesday the 2nd of December.

Having never done this before, I am particularly interested in
comments from previous release managers.

** DRAFT **

  Friday 8th JanuaryLast posting date

Patches adding new features should be posted to the mailing list
by this cate, although perhaps not in their final version.

  Friday 22nd January   Feature freeze
 
Patches adding new features should be committed by this date.
Straightforward bugfixes may continue to be accepted by
maintainers.

  Friday 12th February **tentatve**   Code freeze

Bugfixes only, all changes to be approved by the Release Manager.

  Week of 12th March **tentative**   Release
(probably Tuesday or Wednesday)

Any patches containing substantial refactoring are to treated as
new features, even if they intent is to fix bugs.

Freeze exceptions will not be routine, but may be granted in
exceptional cases for small changes on the basis of risk assessment.
Large series will not get exceptions.  Contributors *must not* rely on
getting, or expect, a freeze exception.

Chinese New Year falls around the 11th-19th of February this year.  In
my plan above, that falls within the hard code freeze period.  If we
don't manage to get the tree to an acceptable quality level by the
tentative codefreeze and release dates above, these dates will slip.

I have not yet started tracking "big ticket" items, and bugs.  I
expect to start doing that starting after Christmas.  NB the primary
responsibility for driving a feature's progress to meet the release
schedule, lies with the feature's proponents.

If as a feature proponent you feel your feature is at risk and there
is something the Xen Project could do to help, please consult me or
the Community Manager.  In such situations please reach out earlier
rather than later.

** END OF DRAFT **

Thanks,
Ian.

>From b34f4ddace0b8d76d8c340a46288a2db79c99460 Mon Sep 17 00:00:00 2001
From: Ian Jackson 
To: xen-devel@lists.xenproject.org
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Julien Grall 
Cc: Stefano Stabellini 
Date: Wed, 25 Nov 2020 13:22:08 +
Subject: [PATCH] xen-release-management doc: More info on schedule
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This documents our practice, established in 2018
  https://lists.xen.org/archives/html/xen-devel/2018-07/msg02240.html
et seq

CC: Jürgen Groß 
CC: Paul Durrant 
CC: Wei Liu 
Signed-off-by: Ian Jackson 
---
 docs/process/xen-release-management.pandoc | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/docs/process/xen-release-management.pandoc 
b/docs/process/xen-release-management.pandoc
index e1aa1eda8f..a5d70fed67 100644
--- a/docs/process/xen-release-management.pandoc
+++ b/docs/process/xen-release-management.pandoc
@@ -15,8 +15,10 @@ that they can have an idea what to expect from the Release 
Manager.
 
 # Xen release cycle
 
-The Xen hypervisor project now releases every 8 months. The actual release date
-depends on a lot of factors.
+The Xen hypervisor project now releases every 8 months.  We aim to
+release in the first half of March/July/November.  These dates have
+been chosen to avoid major holidays and cultural events; if one
+release slips, ideally the previous release cycle would be shortened.
 
 We can roughly divide one release into two periods. The development period
 and the freeze period. The former is 6 months long and the latter is about 2
@@ -33,6 +35,12 @@ During freeze period, the tree is closed for new features. 
Only bug fixes are
 accepted. This period can be shorter or longer than 2 months. If it ends up
 longer than 2 months, it eats into the next development period.
 
+The precise release schedule depends on a lot of factors and needs to
+be set afresh by the Release Manager in each release cycle.  When the
+release is in March, particular consideration should be given to the
+Chinese New Year holidaty which will then typically occur curing the
+freeze, so the freeze should probably be extended to compensate.
+
 # The different roles in a Xen release
 
 ## Release Manager
-- 
2.20.1

Re: [PATCH 12/45] block: remove a superflous check in blkpg_do_ioctl

[Jan Kara]

On Tue 24-11-20 14:27:18, Christoph Hellwig wrote:
> sector_t is now always a u64, so this check is not needed.
> 
> Signed-off-by: Christoph Hellwig 

Looks good. You can add:

Reviewed-by: Jan Kara 

Honza

> ---
>  block/ioctl.c | 9 -
>  1 file changed, 9 deletions(-)
> 
> diff --git a/block/ioctl.c b/block/ioctl.c
> index 6b785181344fe1..0c09bb7a6ff35f 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -35,15 +35,6 @@ static int blkpg_do_ioctl(struct block_device *bdev,
>   start = p.start >> SECTOR_SHIFT;
>   length = p.length >> SECTOR_SHIFT;
>  
> - /* check for fit in a hd_struct */
> - if (sizeof(sector_t) < sizeof(long long)) {
> - long pstart = start, plength = length;
> -
> - if (pstart != start || plength != length || pstart < 0 ||
> - plength < 0 || p.pno > 65535)
> - return -EINVAL;
> - }
> -
>   switch (op) {
>   case BLKPG_ADD_PARTITION:
>   /* check if partition is aligned to blocksize */
> -- 
> 2.29.2
> 
-- 
Jan Kara 
SUSE Labs, CR

Re: [PATCH 08/45] loop: do not call set_blocksize

[Jan Kara]

On Tue 24-11-20 14:27:14, Christoph Hellwig wrote:
> set_blocksize is used by file systems to use their preferred buffer cache
> block size.  Block drivers should not set it.
> 
> Signed-off-by: Christoph Hellwig 

Looks good to me. You can add:

Reviewed-by: Jan Kara 

Honza

> ---
>  drivers/block/loop.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 9a27d4f1c08aac..b42c728620c9e4 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -1164,9 +1164,6 @@ static int loop_configure(struct loop_device *lo, 
> fmode_t mode,
>   size = get_loop_size(lo, file);
>   loop_set_size(lo, size);
>  
> - set_blocksize(bdev, S_ISBLK(inode->i_mode) ?
> -   block_size(inode->i_bdev) : PAGE_SIZE);
> -
>   lo->lo_state = Lo_bound;
>   if (part_shift)
>   lo->lo_flags |= LO_FLAGS_PARTSCAN;
> -- 
> 2.29.2
> 
-- 
Jan Kara 
SUSE Labs, CR

Re: [PATCH v2 02/17] mm: introduce xvmalloc() et al and use for grant table allocations

[Jan Beulich]

On 25.11.2020 13:15, Julien Grall wrote:
> On 23/11/2020 14:23, Jan Beulich wrote:
>> All of the array allocations in grant_table_init() can exceed a page's
>> worth of memory, which xmalloc()-based interfaces aren't really suitable
>> for after boot. We also don't need any of these allocations to be
>> physically contiguous.. Introduce interfaces dynamically switching
>> between xmalloc() et al and vmalloc() et al, based on requested size,
>> and use them instead.
>>
>> All the wrappers in the new header get cloned mostly verbatim from
>> xmalloc.h, with the sole adjustment to switch unsigned long to size_t
>> for sizes and to unsigned int for alignments.
>>
>> Signed-off-by: Jan Beulich 
>> ---
>> v2: Actually edit a copy-and-pasted comment in xvmalloc.h which was
>>  meant to be edited from the beginning.
>> ---
>> I'm unconvinced of the mentioning of "physically contiguous" in the
>> comment at the top of the new header: I don't think xmalloc() provides
>> such a guarantee. Any use assuming so would look (latently) broken to
>> me.
> 
> I haven't had the chance to reply to the first version about this. So I 
> will reply here to avoid confusion.
> 
> I can at least spot one user in Arm that would use xmalloc() that way 
> (see the allocation of itt_addr in arch/arm/gic-v3-its.c).

And I surely wouldn't have spotted this, even if I had tried
to find "offenders", i.e. as said before not wanting to alter
the behavior of existing code (beyond the explicit changes
done here) was ...

> AFAIK, the memory is for the sole purpose of the ITS and should not be 
> accessed by Xen. So I think we can replace by a new version of 
> alloc_domheap_pages().
> 
> However, I still question the usefulness of introducing yet another way 
> to allocate memory (we already have alloc_xenheap_pages(), xmalloc(), 
> alloc_domheap_pages(), vmap()) if you think users cannot rely on 
> xmalloc() to allocate memory physically contiguous.

... the reason to introduce a separate new interface. Plus of
course this parallels what Linux has.

> It definitely makes more difficult to figure out when to use xmalloc() 
> vs xvalloc().

I don't see the difficulty:
- if you need physically contiguous memory, use alloc_xen*_pages(),
- if you know the allocation size is always no more than a page,
  use xmalloc(),
- if you know the allocation size is always more than a page, use
  vmalloc(),
- otherwise use xvmalloc(). Exceptions may of course apply, i.e.
this is just a rule of thumb.

> I would like to hear an opinion from the other maintainers.

Let's hope at least one will voice theirs.

Jan

[xen-4.13-testing test] 156988: tolerable FAIL - PUSHED

[osstest service owner]

flight 156988 xen-4.13-testing real [real]
flight 157005 xen-4.13-testing real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/156988/
http://logs.test-lab.xenproject.org/osstest/logs/157005/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail pass 
in 157005-retest

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

version targeted for testing:
 xen  5e4914e60da9a8dfdc00e839278f40c87525b8ae
baseline version:
 xen  d4c0483c0b87768cd9b95542e98111e4c098d57f

Last test of basis   156636  2020-11-10 18:06:32 Z   14 days
Testing same since   156988  2020-11-24 13:36:44 Z0 days1 attempts


People who touched revisions under test:
  Jan Beulich 

jobs:
 

Re: [PATCH 06/45] zram: remove the claim mechanism

[Jan Kara]

On Tue 24-11-20 14:27:12, Christoph Hellwig wrote:
> The zram claim mechanism was added to ensure no new opens come in
> during teardown.  But the proper way to archive that is to call
  ^^^ achieve

> del_gendisk first, which takes care of all that.  Once del_gendisk
> is called in the right place, the reset side can also be simplified
> as no I/O can be outstanding on a block device that is not open.
> 
> Signed-off-by: Christoph Hellwig 

Otherwise I didn't find anything obviously wrong with the patch but I don't
feel confident enough with zram to really give you my reviewed-by on this
one.

Honza

> ---
>  drivers/block/zram/zram_drv.c | 72 ---
>  1 file changed, 15 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 6d15d51cee2b7e..2e6d75ec1afddb 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -1756,64 +1756,33 @@ static ssize_t disksize_store(struct device *dev,
>  static ssize_t reset_store(struct device *dev,
>   struct device_attribute *attr, const char *buf, size_t len)
>  {
> - int ret;
> - unsigned short do_reset;
> - struct zram *zram;
> + struct zram *zram = dev_to_zram(dev);
>   struct block_device *bdev;
> + unsigned short do_reset;
> + int ret = 0;
>  
>   ret = kstrtou16(buf, 10, _reset);
>   if (ret)
>   return ret;
> -
>   if (!do_reset)
>   return -EINVAL;
>  
> - zram = dev_to_zram(dev);
>   bdev = bdget_disk(zram->disk, 0);
>   if (!bdev)
>   return -ENOMEM;
>  
>   mutex_lock(>bd_mutex);
> - /* Do not reset an active device or claimed device */
> - if (bdev->bd_openers || zram->claim) {
> - mutex_unlock(>bd_mutex);
> - bdput(bdev);
> - return -EBUSY;
> - }
> -
> - /* From now on, anyone can't open /dev/zram[0-9] */
> - zram->claim = true;
> + if (bdev->bd_openers)
> + ret = -EBUSY;
> + else
> + zram_reset_device(zram);
>   mutex_unlock(>bd_mutex);
> -
> - /* Make sure all the pending I/O are finished */
> - fsync_bdev(bdev);
> - zram_reset_device(zram);
>   bdput(bdev);
>  
> - mutex_lock(>bd_mutex);
> - zram->claim = false;
> - mutex_unlock(>bd_mutex);
> -
> - return len;
> -}
> -
> -static int zram_open(struct block_device *bdev, fmode_t mode)
> -{
> - int ret = 0;
> - struct zram *zram;
> -
> - WARN_ON(!mutex_is_locked(>bd_mutex));
> -
> - zram = bdev->bd_disk->private_data;
> - /* zram was claimed to reset so open request fails */
> - if (zram->claim)
> - ret = -EBUSY;
> -
> - return ret;
> + return ret ? ret : len;
>  }
>  
>  static const struct block_device_operations zram_devops = {
> - .open = zram_open,
>   .submit_bio = zram_submit_bio,
>   .swap_slot_free_notify = zram_slot_free_notify,
>   .rw_page = zram_rw_page,
> @@ -1821,7 +1790,6 @@ static const struct block_device_operations zram_devops 
> = {
>  };
>  
>  static const struct block_device_operations zram_wb_devops = {
> - .open = zram_open,
>   .submit_bio = zram_submit_bio,
>   .swap_slot_free_notify = zram_slot_free_notify,
>   .owner = THIS_MODULE
> @@ -1974,32 +1942,22 @@ static int zram_add(void)
>  
>  static int zram_remove(struct zram *zram)
>  {
> - struct block_device *bdev;
> -
> - bdev = bdget_disk(zram->disk, 0);
> - if (!bdev)
> - return -ENOMEM;
> + struct block_device *bdev = bdget_disk(zram->disk, 0);
>  
> - mutex_lock(>bd_mutex);
> - if (bdev->bd_openers || zram->claim) {
> - mutex_unlock(>bd_mutex);
> + if (bdev) {
> + if (bdev->bd_openers) {
> + bdput(bdev);
> + return -EBUSY;
> + }
>   bdput(bdev);
> - return -EBUSY;
>   }
>  
> - zram->claim = true;
> - mutex_unlock(>bd_mutex);
> -
> + del_gendisk(zram->disk);
>   zram_debugfs_unregister(zram);
> -
> - /* Make sure all the pending I/O are finished */
> - fsync_bdev(bdev);
>   zram_reset_device(zram);
> - bdput(bdev);
>  
>   pr_info("Removed device: %s\n", zram->disk->disk_name);
>  
> - del_gendisk(zram->disk);
>   blk_cleanup_queue(zram->disk->queue);
>   put_disk(zram->disk);
>   kfree(zram);
> -- 
> 2.29.2
> 
-- 
Jan Kara 
SUSE Labs, CR

Re: [PATCH 05/45] mtip32xx: remove the call to fsync_bdev on removal

[Jan Kara]

On Tue 24-11-20 14:27:11, Christoph Hellwig wrote:
> del_gendisk already calls fsync_bdev for every partition, no need
> to do this twice.
> 
> Signed-off-by: Christoph Hellwig 

Makes sense to me. You can add:

Reviewed-by: Jan Kara 

Honza

> ---
>  drivers/block/mtip32xx/mtip32xx.c | 15 ---
>  drivers/block/mtip32xx/mtip32xx.h |  2 --
>  2 files changed, 17 deletions(-)
> 
> diff --git a/drivers/block/mtip32xx/mtip32xx.c 
> b/drivers/block/mtip32xx/mtip32xx.c
> index 153e2cdecb4d40..53ac59d19ae530 100644
> --- a/drivers/block/mtip32xx/mtip32xx.c
> +++ b/drivers/block/mtip32xx/mtip32xx.c
> @@ -3687,7 +3687,6 @@ static int mtip_block_initialize(struct driver_data *dd)
>   /* Enable the block device and add it to /dev */
>   device_add_disk(>pdev->dev, dd->disk, NULL);
>  
> - dd->bdev = bdget_disk(dd->disk, 0);
>   /*
>* Now that the disk is active, initialize any sysfs attributes
>* managed by the protocol layer.
> @@ -3721,9 +3720,6 @@ static int mtip_block_initialize(struct driver_data *dd)
>   return rv;
>  
>  kthread_run_error:
> - bdput(dd->bdev);
> - dd->bdev = NULL;
> -
>   /* Delete our gendisk. This also removes the device from /dev */
>   del_gendisk(dd->disk);
>  
> @@ -3804,14 +3800,6 @@ static int mtip_block_remove(struct driver_data *dd)
>   blk_mq_tagset_busy_iter(>tags, mtip_no_dev_cleanup, dd);
>   blk_mq_unquiesce_queue(dd->queue);
>  
> - /*
> -  * Delete our gendisk structure. This also removes the device
> -  * from /dev
> -  */
> - if (dd->bdev) {
> - bdput(dd->bdev);
> - dd->bdev = NULL;
> - }
>   if (dd->disk) {
>   if (test_bit(MTIP_DDF_INIT_DONE_BIT, >dd_flag))
>   del_gendisk(dd->disk);
> @@ -4206,9 +4194,6 @@ static void mtip_pci_remove(struct pci_dev *pdev)
>   } while (atomic_read(>irq_workers_active) != 0 &&
>   time_before(jiffies, to));
>  
> - if (!dd->sr)
> - fsync_bdev(dd->bdev);
> -
>   if (atomic_read(>irq_workers_active) != 0) {
>   dev_warn(>pdev->dev,
>   "Completion workers still active!\n");
> diff --git a/drivers/block/mtip32xx/mtip32xx.h 
> b/drivers/block/mtip32xx/mtip32xx.h
> index e22a7f0523bf30..88f4206310e4c8 100644
> --- a/drivers/block/mtip32xx/mtip32xx.h
> +++ b/drivers/block/mtip32xx/mtip32xx.h
> @@ -463,8 +463,6 @@ struct driver_data {
>  
>   int isr_binding;
>  
> - struct block_device *bdev;
> -
>   struct list_head online_list; /* linkage for online list */
>  
>   struct list_head remove_list; /* linkage for removing list */
> -- 
> 2.29.2
> 
-- 
Jan Kara 
SUSE Labs, CR

Re: [PATCH 04/45] fs: simplify freeze_bdev/thaw_bdev

[Jan Kara]

On Tue 24-11-20 14:27:10, Christoph Hellwig wrote:
> Store the frozen superblock in struct block_device to avoid the awkward
> interface that can return a sb only used a cookie, an ERR_PTR or NULL.
> 
> Signed-off-by: Christoph Hellwig 

Some comments below...

> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index d8664f5c1ff669..60492620d51866 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -548,55 +548,47 @@ EXPORT_SYMBOL(fsync_bdev);
>   * count down in thaw_bdev(). When it becomes 0, thaw_bdev() will unfreeze
>   * actually.
>   */
> -struct super_block *freeze_bdev(struct block_device *bdev)
> +int freeze_bdev(struct block_device *bdev)
>  {
>   struct super_block *sb;
>   int error = 0;
>  
>   mutex_lock(>bd_fsfreeze_mutex);
> - if (++bdev->bd_fsfreeze_count > 1) {
> - /*
> -  * We don't even need to grab a reference - the first call
> -  * to freeze_bdev grab an active reference and only the last
> -  * thaw_bdev drops it.
> -  */
> - sb = get_super(bdev);
> - if (sb)
> - drop_super(sb);
> - mutex_unlock(>bd_fsfreeze_mutex);
> - return sb;
> - }
> + if (++bdev->bd_fsfreeze_count > 1)
> + goto done;
>  
>   sb = get_active_super(bdev);
>   if (!sb)
> - goto out;
> + goto sync;
>   if (sb->s_op->freeze_super)
>   error = sb->s_op->freeze_super(sb);
>   else
>   error = freeze_super(sb);
> + deactivate_super(sb);
> +
>   if (error) {
> - deactivate_super(sb);
>   bdev->bd_fsfreeze_count--;
> - mutex_unlock(>bd_fsfreeze_mutex);
> - return ERR_PTR(error);
> + goto done;
>   }
> - deactivate_super(sb);
> - out:
> + bdev->bd_fsfreeze_sb = sb;
> +
> +sync:
>   sync_blockdev(bdev);
> +done:
>   mutex_unlock(>bd_fsfreeze_mutex);
> - return sb;  /* thaw_bdev releases s->s_umount */
> + return error;   /* thaw_bdev releases s->s_umount */

The comment about thaw_bdev() seems to be stale? At least I don't see what
it's speaking about...

>  }
>  EXPORT_SYMBOL(freeze_bdev);
>  
>  /**
>   * thaw_bdev  -- unlock filesystem
>   * @bdev:blockdevice to unlock
> - * @sb:  associated superblock
>   *
>   * Unlocks the filesystem and marks it writeable again after freeze_bdev().
>   */
> -int thaw_bdev(struct block_device *bdev, struct super_block *sb)
> +int thaw_bdev(struct block_device *bdev)
>  {
> + struct super_block *sb;
>   int error = -EINVAL;
>  
>   mutex_lock(>bd_fsfreeze_mutex);
> @@ -607,6 +599,7 @@ int thaw_bdev(struct block_device *bdev, struct 
> super_block *sb)
>   if (--bdev->bd_fsfreeze_count > 0)
>   goto out;
>  
> + sb = bdev->bd_fsfreeze_sb;
>   if (!sb)
>   goto out;
>  
> @@ -618,7 +611,7 @@ int thaw_bdev(struct block_device *bdev, struct 
> super_block *sb)
>   bdev->bd_fsfreeze_count++;
>  out:
>   mutex_unlock(>bd_fsfreeze_mutex);
> - return error;
> + return 0;

But we now won't return -EINVAL if this gets called e.g. with
bd_fsfreeze_count == 0, right?

Honza
-- 
Jan Kara 
SUSE Labs, CR

Re: [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang

[Nick Desaulniers]

On Tue, Nov 24, 2020 at 11:05 PM James Bottomley
 wrote:
>
> On Tue, 2020-11-24 at 13:32 -0800, Kees Cook wrote:
> > We already enable -Wimplicit-fallthrough globally, so that's not the
> > discussion. The issue is that Clang is (correctly) even more strict
> > than GCC for this, so these are the remaining ones to fix for full
> > Clang coverage too.
> >
> > People have spent more time debating this already than it would have
> > taken to apply the patches. :)
>
> You mean we've already spent 90% of the effort to come this far so we
> might as well go the remaining 10% because then at least we get some
> return? It's certainly a clinching argument in defence procurement ...

So developers and distributions using Clang can't have
-Wimplicit-fallthrough enabled because GCC is less strict (which has
been shown in this thread to lead to bugs)?  We'd like to have nice
things too, you know.

I even agree that most of the churn comes from

case 0:
  ++x;
default:
  break;

which I have a patch for: https://reviews.llvm.org/D91895.  I agree
that can never lead to bugs.  But that's not the sole case of this
series, just most of them.

Though, note how the reviewer (C++ spec editor and clang front end
owner) in https://reviews.llvm.org/D91895 even asks in that review how
maybe a new flag would be more appropriate for a watered
down/stylistic variant of the existing behavior.  And if the current
wording of Documentation/process/deprecated.rst around "fallthrough"
is a straightforward rule of thumb, I kind of agree with him.

>
> > This is about robustness and language wrangling. It's a big code-
> > base, and this is the price of our managing technical debt for
> > permanent robustness improvements. (The numbers I ran from Gustavo's
> > earlier patches were that about 10% of the places adjusted were
> > identified as legitimate bugs being fixed. This final series may be
> > lower, but there are still bugs being found from it -- we need to
> > finish this and shut the door on it for good.)
>
> I got my six patches by analyzing the lwn.net report of the fixes that
> was cited which had 21 of which 50% didn't actually change the emitted
> code, and 25% didn't have a user visible effect.
>
> But the broader point I'm making is just because the compiler people
> come up with a shiny new warning doesn't necessarily mean the problem

That's not what this is though; you're attacking a strawman.  I'd
encourage you to bring that up when that actually occurs, unlike this
case since it's actively hindering getting -Wimplicit-fallthrough
enabled for Clang.  This is not a shiny new warning; it's already on
for GCC and has existed in both compilers for multiple releases.

And I'll also note that warnings are warnings and not errors because
they cannot be proven to be bugs in 100% of cases, but they have led
to bugs in the past.  They require a human to review their intent and
remove ambiguities.  If 97% of cases would end in a break ("Expert C
Programming: Deep C Secrets" - Peter van der Linden), then it starts
to look to me like a language defect; certainly an incorrectly chosen
default.  But the compiler can't know those 3% were intentional,
unless you're explicit for those exceptional cases.

> it's detecting is one that causes us actual problems in the code base.
> I'd really be happier if we had a theory about what classes of CVE or
> bug we could eliminate before we embrace the next new warning.

We don't generally file CVEs and waiting for them to occur might be
too reactive, but I agree that pointing to some additional
documentation in commit messages about how a warning could lead to a
bug would make it clearer to reviewers why being able to enable it
treewide, even if there's no bug in their particular subsystem, is in
the general interest of the commons.

On Mon, Nov 23, 2020 at 7:58 AM James Bottomley
 wrote:
>
> We're also complaining about the inability to recruit maintainers:
>
> https://www.theregister.com/2020/06/30/hard_to_find_linux_maintainers_says_torvalds/
>
> And burn out:
>
> http://antirez.com/news/129
>
> The whole crux of your argument seems to be maintainers' time isn't
> important so we should accept all trivial patches ... I'm pushing back
> on that assumption in two places, firstly the valulessness of the time
> and secondly that all trivial patches are valuable.

It's critical to the longevity of any open source project that there
are not single points of failure.  If someone is not expendable or
replaceable (or claims to be) then that's a risk to the project and a
bottleneck.  Not having a replacement in training or some form of
redundancy is short sighted.

If trivial patches are adding too much to your workload, consider
training a co-maintainer or asking for help from one of your reviewers
whom you trust.  I don't doubt it's hard to find maintainers, but
existing maintainers should go out of their way to entrust
co-maintainers especially when they find their workload becomes too
high.  And 

Re: [PATCH 03/45] fs: remove get_super_thawed and get_super_exclusive_thawed

[Jan Kara]

On Tue 24-11-20 14:27:09, Christoph Hellwig wrote:
> Just open code the wait in the only caller of both functions.
> 
> Signed-off-by: Christoph Hellwig 

Fine by me. You can add:

Reviewed-by: Jan Kara 

Honza

> ---
>  fs/internal.h  |  2 ++
>  fs/quota/quota.c   | 31 +---
>  fs/super.c | 51 ++
>  include/linux/fs.h |  4 +---
>  4 files changed, 29 insertions(+), 59 deletions(-)
> 
> diff --git a/fs/internal.h b/fs/internal.h
> index a7cd0f64faa4ab..47be21dfeebef5 100644
> --- a/fs/internal.h
> +++ b/fs/internal.h
> @@ -114,7 +114,9 @@ extern struct file *alloc_empty_file_noaccount(int, const 
> struct cred *);
>   */
>  extern int reconfigure_super(struct fs_context *);
>  extern bool trylock_super(struct super_block *sb);
> +struct super_block *__get_super(struct block_device *bdev, bool excl);
>  extern struct super_block *user_get_super(dev_t);
> +void put_super(struct super_block *sb);
>  extern bool mount_capable(struct fs_context *);
>  
>  /*
> diff --git a/fs/quota/quota.c b/fs/quota/quota.c
> index 9af95c7a0bbe3c..f3d32b0d9008f2 100644
> --- a/fs/quota/quota.c
> +++ b/fs/quota/quota.c
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include "compat.h"
> +#include "../internal.h"
>  
>  static int check_quotactl_permission(struct super_block *sb, int type, int 
> cmd,
>qid_t id)
> @@ -868,6 +869,7 @@ static struct super_block *quotactl_block(const char 
> __user *special, int cmd)
>   struct block_device *bdev;
>   struct super_block *sb;
>   struct filename *tmp = getname(special);
> + bool excl = false, thawed = false;
>  
>   if (IS_ERR(tmp))
>   return ERR_CAST(tmp);
> @@ -875,17 +877,32 @@ static struct super_block *quotactl_block(const char 
> __user *special, int cmd)
>   putname(tmp);
>   if (IS_ERR(bdev))
>   return ERR_CAST(bdev);
> - if (quotactl_cmd_onoff(cmd))
> - sb = get_super_exclusive_thawed(bdev);
> - else if (quotactl_cmd_write(cmd))
> - sb = get_super_thawed(bdev);
> - else
> - sb = get_super(bdev);
> +
> + if (quotactl_cmd_onoff(cmd)) {
> + excl = true;
> + thawed = true;
> + } else if (quotactl_cmd_write(cmd)) {
> + thawed = true;
> + }
> +
> +retry:
> + sb = __get_super(bdev, excl);
> + if (thawed && sb && sb->s_writers.frozen != SB_UNFROZEN) {
> + if (excl)
> + up_write(>s_umount);
> + else
> + up_read(>s_umount);
> + wait_event(sb->s_writers.wait_unfrozen,
> +sb->s_writers.frozen == SB_UNFROZEN);
> + put_super(sb);
> + goto retry;
> + }
> +
>   bdput(bdev);
>   if (!sb)
>   return ERR_PTR(-ENODEV);
> -
>   return sb;
> +
>  #else
>   return ERR_PTR(-ENODEV);
>  #endif
> diff --git a/fs/super.c b/fs/super.c
> index 98bb0629ee108e..343e5c1e538d2a 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -307,7 +307,7 @@ static void __put_super(struct super_block *s)
>   *   Drops a temporary reference, frees superblock if there's no
>   *   references left.
>   */
> -static void put_super(struct super_block *sb)
> +void put_super(struct super_block *sb)
>  {
>   spin_lock(_lock);
>   __put_super(sb);
> @@ -740,7 +740,7 @@ void iterate_supers_type(struct file_system_type *type,
>  
>  EXPORT_SYMBOL(iterate_supers_type);
>  
> -static struct super_block *__get_super(struct block_device *bdev, bool excl)
> +struct super_block *__get_super(struct block_device *bdev, bool excl)
>  {
>   struct super_block *sb;
>  
> @@ -789,53 +789,6 @@ struct super_block *get_super(struct block_device *bdev)
>  }
>  EXPORT_SYMBOL(get_super);
>  
> -static struct super_block *__get_super_thawed(struct block_device *bdev,
> -   bool excl)
> -{
> - while (1) {
> - struct super_block *s = __get_super(bdev, excl);
> - if (!s || s->s_writers.frozen == SB_UNFROZEN)
> - return s;
> - if (!excl)
> - up_read(>s_umount);
> - else
> - up_write(>s_umount);
> - wait_event(s->s_writers.wait_unfrozen,
> -s->s_writers.frozen == SB_UNFROZEN);
> - put_super(s);
> - }
> -}
> -
> -/**
> - *   get_super_thawed - get thawed superblock of a device
> - *   @bdev: device to get the superblock for
> - *
> - *   Scans the superblock list and finds the superblock of the file system
> - *   mounted on the device. The superblock is returned once it is thawed
> - *   (or immediately if it was not frozen). %NULL is returned if no match
> - *   is found.
> - */
> -struct super_block *get_super_thawed(struct block_device *bdev)
> -{
> -

Re: [PATCH 02/45] filemap: consistently use ->f_mapping over ->i_mapping

[Jan Kara]

On Tue 24-11-20 14:27:08, Christoph Hellwig wrote:
> Use file->f_mapping in all remaining places that have a struct file
> available to properly handle the case where inode->i_mapping !=
> file_inode(file)->i_mapping.
> 
> Signed-off-by: Christoph Hellwig 

Looks good to me. You can add:

Reviewed-by: Jan Kara 

Honza

> ---
>  mm/filemap.c | 13 ++---
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/filemap.c b/mm/filemap.c
> index d5e7c2029d16b4..4f583489aa3c2a 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -2886,14 +2886,14 @@ EXPORT_SYMBOL(filemap_map_pages);
>  
>  vm_fault_t filemap_page_mkwrite(struct vm_fault *vmf)
>  {
> + struct address_space *mapping = vmf->vma->vm_file->f_mapping;
>   struct page *page = vmf->page;
> - struct inode *inode = file_inode(vmf->vma->vm_file);
>   vm_fault_t ret = VM_FAULT_LOCKED;
>  
> - sb_start_pagefault(inode->i_sb);
> + sb_start_pagefault(mapping->host->i_sb);
>   file_update_time(vmf->vma->vm_file);
>   lock_page(page);
> - if (page->mapping != inode->i_mapping) {
> + if (page->mapping != mapping) {
>   unlock_page(page);
>   ret = VM_FAULT_NOPAGE;
>   goto out;
> @@ -2906,7 +2906,7 @@ vm_fault_t filemap_page_mkwrite(struct vm_fault *vmf)
>   set_page_dirty(page);
>   wait_for_stable_page(page);
>  out:
> - sb_end_pagefault(inode->i_sb);
> + sb_end_pagefault(mapping->host->i_sb);
>   return ret;
>  }
>  
> @@ -3149,10 +3149,9 @@ void dio_warn_stale_pagecache(struct file *filp)
>  {
>   static DEFINE_RATELIMIT_STATE(_rs, 86400 * HZ, DEFAULT_RATELIMIT_BURST);
>   char pathname[128];
> - struct inode *inode = file_inode(filp);
>   char *path;
>  
> - errseq_set(>i_mapping->wb_err, -EIO);
> + errseq_set(>f_mapping->wb_err, -EIO);
>   if (__ratelimit(&_rs)) {
>   path = file_path(filp, pathname, sizeof(pathname));
>   if (IS_ERR(path))
> @@ -3179,7 +3178,7 @@ generic_file_direct_write(struct kiocb *iocb, struct 
> iov_iter *from)
>  
>   if (iocb->ki_flags & IOCB_NOWAIT) {
>   /* If there are pages to writeback, return */
> - if (filemap_range_has_page(inode->i_mapping, pos,
> + if (filemap_range_has_page(file->f_mapping, pos,
>  pos + write_len - 1))
>   return -EAGAIN;
>   } else {
> -- 
> 2.29.2
> 
-- 
Jan Kara 
SUSE Labs, CR

Re: [PATCH v2 02/17] mm: introduce xvmalloc() et al and use for grant table allocations

[Julien Grall]

Hi Jan,

On 23/11/2020 14:23, Jan Beulich wrote:

All of the array allocations in grant_table_init() can exceed a page's
worth of memory, which xmalloc()-based interfaces aren't really suitable
for after boot. We also don't need any of these allocations to be
physically contiguous.. Introduce interfaces dynamically switching
between xmalloc() et al and vmalloc() et al, based on requested size,
and use them instead.

All the wrappers in the new header get cloned mostly verbatim from
xmalloc.h, with the sole adjustment to switch unsigned long to size_t
for sizes and to unsigned int for alignments.

Signed-off-by: Jan Beulich 
---
v2: Actually edit a copy-and-pasted comment in xvmalloc.h which was
 meant to be edited from the beginning.
---
I'm unconvinced of the mentioning of "physically contiguous" in the
comment at the top of the new header: I don't think xmalloc() provides
such a guarantee. Any use assuming so would look (latently) broken to
me.


I haven't had the chance to reply to the first version about this. So I 
will reply here to avoid confusion.


I can at least spot one user in Arm that would use xmalloc() that way 
(see the allocation of itt_addr in arch/arm/gic-v3-its.c).


AFAIK, the memory is for the sole purpose of the ITS and should not be 
accessed by Xen. So I think we can replace by a new version of 
alloc_domheap_pages().


However, I still question the usefulness of introducing yet another way 
to allocate memory (we already have alloc_xenheap_pages(), xmalloc(), 
alloc_domheap_pages(), vmap()) if you think users cannot rely on 
xmalloc() to allocate memory physically contiguous.


It definitely makes more difficult to figure out when to use xmalloc() 
vs xvalloc().


I would like to hear an opinion from the other maintainers.



--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -37,7 +37,7 @@
  #include 
  #include 
  #include 
-#include 
+#include 
  #include 
  #include 
  #include 
@@ -1925,27 +1925,28 @@ int grant_table_init(struct domain *d, i
  d->grant_table = gt;
  
  /* Active grant table. */

-gt->active = xzalloc_array(struct active_grant_entry *,
-   max_nr_active_grant_frames(gt));
+gt->active = xvzalloc_array(struct active_grant_entry *,
+max_nr_active_grant_frames(gt));
  if ( gt->active == NULL )
  goto out;
  
  /* Tracking of mapped foreign frames table */

  if ( gt->max_maptrack_frames )
  {
-gt->maptrack = vzalloc(gt->max_maptrack_frames * 
sizeof(*gt->maptrack));
+gt->maptrack = xvzalloc_array(struct grant_mapping *,
+  gt->max_maptrack_frames);
  if ( gt->maptrack == NULL )
  goto out;
  }
  
  /* Shared grant table. */

-gt->shared_raw = xzalloc_array(void *, gt->max_grant_frames);
+gt->shared_raw = xvzalloc_array(void *, gt->max_grant_frames);
  if ( gt->shared_raw == NULL )
  goto out;
  
  /* Status pages for grant table - for version 2 */

-gt->status = xzalloc_array(grant_status_t *,
-   grant_to_status_frames(gt->max_grant_frames));
+gt->status = xvzalloc_array(grant_status_t *,
+grant_to_status_frames(gt->max_grant_frames));
  if ( gt->status == NULL )
  goto out;
  
@@ -3870,19 +3871,19 @@ grant_table_destroy(
  
  for ( i = 0; i < nr_grant_frames(t); i++ )

  free_xenheap_page(t->shared_raw[i]);
-xfree(t->shared_raw);
+xvfree(t->shared_raw);
  
  for ( i = 0; i < nr_maptrack_frames(t); i++ )

  free_xenheap_page(t->maptrack[i]);
-vfree(t->maptrack);
+xvfree(t->maptrack);
  
  for ( i = 0; i < nr_active_grant_frames(t); i++ )

  free_xenheap_page(t->active[i]);
-xfree(t->active);
+xvfree(t->active);
  
  for ( i = 0; i < nr_status_frames(t); i++ )

  free_xenheap_page(t->status[i]);
-xfree(t->status);
+xvfree(t->status);
  
  xfree(t);

  d->grant_table = NULL;
--- a/xen/common/vmap.c
+++ b/xen/common/vmap.c
@@ -7,6 +7,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  
  static DEFINE_SPINLOCK(vm_lock);

@@ -301,11 +302,29 @@ void *vzalloc(size_t size)
  return p;
  }
  
-void vfree(void *va)

+static void _vfree(const void *va, unsigned int pages, enum vmap_region type)
  {
-unsigned int i, pages;
+unsigned int i;
  struct page_info *pg;
  PAGE_LIST_HEAD(pg_list);
+
+ASSERT(pages);
+
+for ( i = 0; i < pages; i++ )
+{
+pg = vmap_to_page(va + i * PAGE_SIZE);
+ASSERT(pg);
+page_list_add(pg, _list);
+}
+vunmap(va);
+
+while ( (pg = page_list_remove_head(_list)) != NULL )
+free_domheap_page(pg);
+}
+
+void vfree(void *va)
+{
+unsigned int pages;
  enum vmap_region type = VMAP_DEFAULT;
  
  if ( !va )

@@ -317,18 +336,71 @@ void vfree(void *va)
  type = 

RE: [PATCH v4 1/3] domctl: introduce a new domain create flag, XEN_DOMCTL_CDF_disable_fifo, ...

[Durrant, Paul]

> -Original Message-
> From: Jan Beulich 
> Sent: 25 November 2020 11:51
> To: p...@xen.org
> Cc: Durrant, Paul ; Elnikety, Eslam 
> ; 'Christian Lindig'
> ; 'David Scott' ; 'Ian Jackson' 
> ;
> 'Wei Liu' ; 'Andrew Cooper' ; 
> 'George Dunlap'
> ; 'Julien Grall' ; 'Stefano 
> Stabellini'
> ; xen-devel@lists.xenproject.org
> Subject: RE: [EXTERNAL] [PATCH v4 1/3] domctl: introduce a new domain create 
> flag,
> XEN_DOMCTL_CDF_disable_fifo, ...
> 
> CAUTION: This email originated from outside of the organization. Do not click 
> links or open
> attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> On 25.11.2020 12:10, Paul Durrant wrote:
> >> From: Jan Beulich 
> >> Sent: 25 November 2020 09:20
> >>
> >> On 24.11.2020 20:17, Paul Durrant wrote:
> >>> From: Paul Durrant 
> >>>
> >>> ...to control the visibility of the FIFO event channel operations
> >>> (EVTCHNOP_init_control, EVTCHNOP_expand_array, and EVTCHNOP_set_priority) 
> >>> to
> >>> the guest.
> >>>
> >>> These operations were added to the public header in commit d2d50c2f308f
> >>> ("evtchn: add FIFO-based event channel ABI") and the first implementation
> >>> appeared in the two subsequent commits: edc8872aeb4a ("evtchn: implement
> >>> EVTCHNOP_set_priority and add the set_priority hook") and 88910061ec61
> >>> ("evtchn: add FIFO-based event channel hypercalls and port ops"). Prior to
> >>> that, a guest issuing those operations would receive a return value of
> >>> -ENOSYS (not implemented) from Xen. Guests aware of the FIFO operations 
> >>> but
> >>> running on an older (pre-4.4) Xen would fall back to using the 2-level 
> >>> event
> >>> channel interface upon seeing this return value.
> >>>
> >>> Unfortunately the uncontrolable appearance of these new operations in Xen 
> >>> 4.4
> >>> onwards has implications for hibernation of some Linux guests. During 
> >>> resume
> >>> from hibernation, there are two kernels involved: the "boot" kernel and 
> >>> the
> >>> "resume" kernel. The guest boot kernel may default to use FIFO operations 
> >>> and
> >>> instruct Xen via EVTCHNOP_init_control to switch from 2-level to FIFO. On 
> >>> the
> >>> other hand, the resume kernel keeps assuming 2-level, because it was 
> >>> hibernated
> >>> on a version of Xen that did not support the FIFO operations.
> >>
> >> And the alternative of the boot kernel issuing EVTCHNOP_reset has
> >> other unwanted consequences. Maybe worth mentioning here, as
> >> otherwise this would look like the obvious way to return to 2-level
> >> mode?
> >>
> >> Also, why can't the boot kernel be instructed to avoid engaging
> >> FIFO mode?
> >>
> >
> > Both of those are, of course, viable alternatives if the guest can be
> > modified. The problem we need to work around is guest that are already
> > out there and cannot be updated.
> 
> Making use of EVTCHNOP_reset indeed would require a change to the
> kernel. But Linux has a command line option to suppress use of
> FIFO event channels, so I can't see why the boot kernel couldn't
> be passed this flag without any modification to the binary.
> 

I'm sure that was considered and found not to be feasible in our use-case. 
(Likely the command line is as much baked into the guest image as the kernel 
itself).

> >>> To maintain compatibility it is necessary to make Xen behave as it did
> >>> before the new operations were added and hence the code in this patch 
> >>> ensures
> >>> that, if XEN_DOMCTL_CDF_disable_fifo is set, the FIFO event channel 
> >>> operations
> >>> will again result in -ENOSYS being returned to the guest.
> >>
> >> Are there indeed dependencies on the precise return value anywhere?
> >> If so, the generally inappropriate use (do_event_channel_op()'s
> >> default case really would also need switching) would want a brief
> >> comment, so it'll be understood by readers that this isn't code to
> >> derive other code from. If not, -EPERM or -EACCES perhaps?
> >>
> >
> > The patch, as stated, is reverting behaviour and so the -ENOSYS really
> > needs to stay since it is essentially ABI now. I am not aware of guest
> > code that will, in fact, die if it sees -EPERM or -EACCES instead but
> > there may be such code. The only safe thing to do is to make things
> > look like the used to.
> 
> I don't think specific error codes can be considered "ABI". Not
> the least because, if there are multiple causes for an error, it
> ought to be undefined which error gets returned. A guest not
> falling back to 2-level on _any_ error here is basically setting
> itself up for eventual failure because of e.g. getting back
> -ENOMEM. Or someone deciding to add an XSM check to the function.
> 

I'm not disagreeing that depending on -ENOSYS is a bad idea but, before FIFO 
came along, that's what the guest would see and that is what it ought to see 
again if we want to truly obscure the interface (which is the stated aim here).

> As said, I'm of the opinion that the other -ENOSYS ought to be
> 

Re: [PATCH 11/20] block: reference struct block_device from struct hd_struct

[Tejun Heo]

Hey, Jan,

On Wed, Nov 25, 2020 at 12:40:44PM +0100, Jan Kara wrote:
> > I don't think this is necessary now that the bdev and inode lifetimes are
> > one. Before, punching out the association early was necessary because we
> > could be in a situation where we can successfully look up a part from idr
> > and then try to pin the associated disk which may already be freed. With the
> > new code, the lookup is through the inode whose lifetime is one and the same
> > with gendisk, so use-after-free isn't possible and __blkdev_get() will
> > reliably reject such open attempts.
> 
> I think the remove_inode_hash() call is actually still needed. Consider a
> situation when the disk is unplugged, gendisk gets destroyed, bdev still
> lives on (e.g. because it is still open). Device gets re-plugged, gendisk
> for the same device number gets created. But we really need new bdev for
> this because from higher level POV this is completely new device. And the
> old bdev needs to live on as long as it is open. So IMO we still need to
> just unhash the inode and leave it lingering in the background.

You're absolutely right. I was only thinking about the lifetime problem
described in the comment. So, it just needs an updated comment there, I
think.

Thanks.

-- 
tejun

Re: [PATCH] xen/arm: Add workaround for Cortex-A55 erratum #1530923

[Bertrand Marquis]

> On 25 Nov 2020, at 11:26, Julien Grall  wrote:
> 
> 
> 
> On 24/11/2020 17:44, Stefano Stabellini wrote:
>> On Tue, 24 Nov 2020, Rahul Singh wrote:
 On 24 Nov 2020, at 11:12 am, Bertrand Marquis  
 wrote:
 
 On the Cortex A55, TLB entries can be allocated by a speculative AT
 instruction. If this is happening during a guest context switch with an
 inconsistent page table state in the guest, TLBs with wrong values might
 be allocated.
 The ARM64_WORKAROUND_AT_SPECULATE workaround is used as for erratum
 1165522 on Cortex A76 or Neoverse N1.
 
 This change is also introducing the MIDR identifier for the Cortex-A55.
 
 Signed-off-by: Bertrand Marquis 
>>> 
>>> Reviewed-by: Rahul Singh 
>> Reviewed-by: Stefano Stabellini 
> 
> Acked-by: Julien Grall 
> 
> And committed.

Thanks :-)

Cheers
Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall

Re: [PATCH v2 01/17] mm: check for truncation in vmalloc_type()

[Julien Grall]

On 23/11/2020 14:23, Jan Beulich wrote:

While it's currently implied from the checking xmalloc_array() does,
let's make this more explicit in the function itself. As a result both
involved local variables don't need to have size_t type anymore. This
brings them in line with the rest of the code in this file.

Requested-by: Julien Grall 
Signed-off-by: Jan Beulich 


Acked-by: Julien Grall 

Cheers,

--
Julien Grall

Re: [PATCH v4 1/3] domctl: introduce a new domain create flag, XEN_DOMCTL_CDF_disable_fifo, ...

[Jan Beulich]

On 25.11.2020 12:10, Paul Durrant wrote:
>> From: Jan Beulich 
>> Sent: 25 November 2020 09:20
>>
>> On 24.11.2020 20:17, Paul Durrant wrote:
>>> From: Paul Durrant 
>>>
>>> ...to control the visibility of the FIFO event channel operations
>>> (EVTCHNOP_init_control, EVTCHNOP_expand_array, and EVTCHNOP_set_priority) to
>>> the guest.
>>>
>>> These operations were added to the public header in commit d2d50c2f308f
>>> ("evtchn: add FIFO-based event channel ABI") and the first implementation
>>> appeared in the two subsequent commits: edc8872aeb4a ("evtchn: implement
>>> EVTCHNOP_set_priority and add the set_priority hook") and 88910061ec61
>>> ("evtchn: add FIFO-based event channel hypercalls and port ops"). Prior to
>>> that, a guest issuing those operations would receive a return value of
>>> -ENOSYS (not implemented) from Xen. Guests aware of the FIFO operations but
>>> running on an older (pre-4.4) Xen would fall back to using the 2-level event
>>> channel interface upon seeing this return value.
>>>
>>> Unfortunately the uncontrolable appearance of these new operations in Xen 
>>> 4.4
>>> onwards has implications for hibernation of some Linux guests. During resume
>>> from hibernation, there are two kernels involved: the "boot" kernel and the
>>> "resume" kernel. The guest boot kernel may default to use FIFO operations 
>>> and
>>> instruct Xen via EVTCHNOP_init_control to switch from 2-level to FIFO. On 
>>> the
>>> other hand, the resume kernel keeps assuming 2-level, because it was 
>>> hibernated
>>> on a version of Xen that did not support the FIFO operations.
>>
>> And the alternative of the boot kernel issuing EVTCHNOP_reset has
>> other unwanted consequences. Maybe worth mentioning here, as
>> otherwise this would look like the obvious way to return to 2-level
>> mode?
>>
>> Also, why can't the boot kernel be instructed to avoid engaging
>> FIFO mode?
>>
> 
> Both of those are, of course, viable alternatives if the guest can be
> modified. The problem we need to work around is guest that are already
> out there and cannot be updated.

Making use of EVTCHNOP_reset indeed would require a change to the
kernel. But Linux has a command line option to suppress use of
FIFO event channels, so I can't see why the boot kernel couldn't
be passed this flag without any modification to the binary.

>>> To maintain compatibility it is necessary to make Xen behave as it did
>>> before the new operations were added and hence the code in this patch 
>>> ensures
>>> that, if XEN_DOMCTL_CDF_disable_fifo is set, the FIFO event channel 
>>> operations
>>> will again result in -ENOSYS being returned to the guest.
>>
>> Are there indeed dependencies on the precise return value anywhere?
>> If so, the generally inappropriate use (do_event_channel_op()'s
>> default case really would also need switching) would want a brief
>> comment, so it'll be understood by readers that this isn't code to
>> derive other code from. If not, -EPERM or -EACCES perhaps?
>>
> 
> The patch, as stated, is reverting behaviour and so the -ENOSYS really
> needs to stay since it is essentially ABI now. I am not aware of guest
> code that will, in fact, die if it sees -EPERM or -EACCES instead but
> there may be such code. The only safe thing to do is to make things
> look like the used to.

I don't think specific error codes can be considered "ABI". Not
the least because, if there are multiple causes for an error, it
ought to be undefined which error gets returned. A guest not
falling back to 2-level on _any_ error here is basically setting
itself up for eventual failure because of e.g. getting back
-ENOMEM. Or someone deciding to add an XSM check to the function.

As said, I'm of the opinion that the other -ENOSYS ought to be
replaced as well, which of course would be precluded if this was
considered "ABI".

Jan

RE: [PATCH v4 1/3] domctl: introduce a new domain create flag, XEN_DOMCTL_CDF_disable_fifo, ...

[Durrant, Paul]

> -Original Message-
> From: Andrew Cooper 
> Sent: 25 November 2020 11:31
> To: Paul Durrant ; xen-devel@lists.xenproject.org
> Cc: Durrant, Paul ; Elnikety, Eslam 
> ; Christian Lindig
> ; David Scott ; Ian Jackson 
> ; Wei
> Liu ; George Dunlap ; Jan Beulich 
> ; Julien
> Grall ; Stefano Stabellini 
> Subject: RE: [EXTERNAL] [PATCH v4 1/3] domctl: introduce a new domain create 
> flag,
> XEN_DOMCTL_CDF_disable_fifo, ...
> 
> CAUTION: This email originated from outside of the organization. Do not click 
> links or open
> attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> On 24/11/2020 19:17, Paul Durrant wrote:
> > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> > index 666aeb71bf1b..70701c59d053 100644
> > --- a/xen/include/public/domctl.h
> > +++ b/xen/include/public/domctl.h
> > @@ -70,9 +70,11 @@ struct xen_domctl_createdomain {
> >  #define XEN_DOMCTL_CDF_iommu  (1U<<_XEN_DOMCTL_CDF_iommu)
> >  #define _XEN_DOMCTL_CDF_nested_virt   6
> >  #define XEN_DOMCTL_CDF_nested_virt(1U << _XEN_DOMCTL_CDF_nested_virt)
> > +#define _XEN_DOMCTL_CDF_disable_fifo  7
> > +#define XEN_DOMCTL_CDF_disable_fifo   (1U << _XEN_DOMCTL_CDF_disable_fifo)
> 
> The sense is backwards.  It should be a "permit the use of FIFO"
> control.  If the code had been written this way to begin with, the bug
> you found wouldn't have existed.
> 
> Given that there is not currently a way to disable FIFO, you can
> probably do without an enumeration of whether the hypervisor supports it
> or not.
> 

Ok, I can reverse the sense.

I found another one that we ought to control in a similar way... the per-cpu 
evtchn upcalls. AFAIK only the Windows PV drivers make use of it (and I can 
arrange to squash that with a registry flag) but it really falls into the same 
category as FIFO... so maybe we need a separate bit-field for these sorts of 
thing?

  Paul

> ~Andrew

Re: [PATCH 11/20] block: reference struct block_device from struct hd_struct

[Jan Kara]

Hello!

On Tue 24-11-20 11:59:49, Tejun Heo wrote:
> > diff --git a/block/partitions/core.c b/block/partitions/core.c
> > index a02e224115943d..0ba0bf44b88af3 100644
> > --- a/block/partitions/core.c
> > +++ b/block/partitions/core.c
> > @@ -340,12 +340,11 @@ void delete_partition(struct hd_struct *part)
> > device_del(part_to_dev(part));
> >  
> > /*
> > -* Remove gendisk pointer from idr so that it cannot be looked up
> > -* while RCU period before freeing gendisk is running to prevent
> > -* use-after-free issues. Note that the device number stays
> > -* "in-use" until we really free the gendisk.
> > +* Remove the block device from the inode hash, so that it cannot be
> > +* looked up while waiting for the RCU grace period.
> >  */
> > -   blk_invalidate_devt(part_devt(part));
> > +   remove_inode_hash(part->bdev->bd_inode);
> 
> I don't think this is necessary now that the bdev and inode lifetimes are
> one. Before, punching out the association early was necessary because we
> could be in a situation where we can successfully look up a part from idr
> and then try to pin the associated disk which may already be freed. With the
> new code, the lookup is through the inode whose lifetime is one and the same
> with gendisk, so use-after-free isn't possible and __blkdev_get() will
> reliably reject such open attempts.

I think the remove_inode_hash() call is actually still needed. Consider a
situation when the disk is unplugged, gendisk gets destroyed, bdev still
lives on (e.g. because it is still open). Device gets re-plugged, gendisk
for the same device number gets created. But we really need new bdev for
this because from higher level POV this is completely new device. And the
old bdev needs to live on as long as it is open. So IMO we still need to
just unhash the inode and leave it lingering in the background.

Honza
-- 
Jan Kara 
SUSE Labs, CR

Re: [PATCH v4 1/3] domctl: introduce a new domain create flag, XEN_DOMCTL_CDF_disable_fifo, ...

[Andrew Cooper]

On 24/11/2020 19:17, Paul Durrant wrote:
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index 666aeb71bf1b..70701c59d053 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -70,9 +70,11 @@ struct xen_domctl_createdomain {
>  #define XEN_DOMCTL_CDF_iommu  (1U<<_XEN_DOMCTL_CDF_iommu)
>  #define _XEN_DOMCTL_CDF_nested_virt   6
>  #define XEN_DOMCTL_CDF_nested_virt(1U << _XEN_DOMCTL_CDF_nested_virt)
> +#define _XEN_DOMCTL_CDF_disable_fifo  7
> +#define XEN_DOMCTL_CDF_disable_fifo   (1U << _XEN_DOMCTL_CDF_disable_fifo)

The sense is backwards.  It should be a "permit the use of FIFO"
control.  If the code had been written this way to begin with, the bug
you found wouldn't have existed.

Given that there is not currently a way to disable FIFO, you can
probably do without an enumeration of whether the hypervisor supports it
or not.

~Andrew

Re: [PATCH] xen/arm: Add workaround for Cortex-A55 erratum #1530923

[Julien Grall]

On 24/11/2020 17:44, Stefano Stabellini wrote:

On Tue, 24 Nov 2020, Rahul Singh wrote:

On 24 Nov 2020, at 11:12 am, Bertrand Marquis  wrote:

On the Cortex A55, TLB entries can be allocated by a speculative AT
instruction. If this is happening during a guest context switch with an
inconsistent page table state in the guest, TLBs with wrong values might
be allocated.
The ARM64_WORKAROUND_AT_SPECULATE workaround is used as for erratum
1165522 on Cortex A76 or Neoverse N1.

This change is also introducing the MIDR identifier for the Cortex-A55.

Signed-off-by: Bertrand Marquis 


Reviewed-by: Rahul Singh 


Reviewed-by: Stefano Stabellini 


Acked-by: Julien Grall 

And committed.

Cheers,

--
Julien Grall

Re: [PATCH] evtchn: double per-channel locking can't hit identical channels

[Julien Grall]

Hi Jan,

On 24/11/2020 17:03, Jan Beulich wrote:

Inter-domain channels can't possibly be bound to themselves, there's
always a 2nd channel involved, even when this is a loopback into the
same domain. As a result we can drop one conditional each from the two
involved functions.

With this, the number of evtchn_write_lock() invocations can also be
shrunk by half, swapping the two incoming function arguments instead.

Signed-off-by: Jan Beulich 


Acked-by: Julien Grall 

Cheers,

--
Julien Grall

RE: [PATCH v4 1/3] domctl: introduce a new domain create flag, XEN_DOMCTL_CDF_disable_fifo, ...

[Paul Durrant]

> -Original Message-
> From: Jan Beulich 
> Sent: 25 November 2020 09:20
> To: Paul Durrant 
> Cc: Paul Durrant ; Eslam Elnikety ; 
> Christian Lindig
> ; David Scott ; Ian Jackson 
> ; Wei
> Liu ; Andrew Cooper ; George Dunlap 
> ;
> Julien Grall ; Stefano Stabellini ; 
> xen-
> de...@lists.xenproject.org
> Subject: Re: [PATCH v4 1/3] domctl: introduce a new domain create flag, 
> XEN_DOMCTL_CDF_disable_fifo,
> ...
> 
> On 24.11.2020 20:17, Paul Durrant wrote:
> > From: Paul Durrant 
> >
> > ...to control the visibility of the FIFO event channel operations
> > (EVTCHNOP_init_control, EVTCHNOP_expand_array, and EVTCHNOP_set_priority) to
> > the guest.
> >
> > These operations were added to the public header in commit d2d50c2f308f
> > ("evtchn: add FIFO-based event channel ABI") and the first implementation
> > appeared in the two subsequent commits: edc8872aeb4a ("evtchn: implement
> > EVTCHNOP_set_priority and add the set_priority hook") and 88910061ec61
> > ("evtchn: add FIFO-based event channel hypercalls and port ops"). Prior to
> > that, a guest issuing those operations would receive a return value of
> > -ENOSYS (not implemented) from Xen. Guests aware of the FIFO operations but
> > running on an older (pre-4.4) Xen would fall back to using the 2-level event
> > channel interface upon seeing this return value.
> >
> > Unfortunately the uncontrolable appearance of these new operations in Xen 
> > 4.4
> > onwards has implications for hibernation of some Linux guests. During resume
> > from hibernation, there are two kernels involved: the "boot" kernel and the
> > "resume" kernel. The guest boot kernel may default to use FIFO operations 
> > and
> > instruct Xen via EVTCHNOP_init_control to switch from 2-level to FIFO. On 
> > the
> > other hand, the resume kernel keeps assuming 2-level, because it was 
> > hibernated
> > on a version of Xen that did not support the FIFO operations.
> 
> And the alternative of the boot kernel issuing EVTCHNOP_reset has
> other unwanted consequences. Maybe worth mentioning here, as
> otherwise this would look like the obvious way to return to 2-level
> mode?
> 
> Also, why can't the boot kernel be instructed to avoid engaging
> FIFO mode?
> 

Both of those are, of course, viable alternatives if the guest can be modified. 
The problem we need to work around is guest that are already out there and 
cannot be updated.

> > To maintain compatibility it is necessary to make Xen behave as it did
> > before the new operations were added and hence the code in this patch 
> > ensures
> > that, if XEN_DOMCTL_CDF_disable_fifo is set, the FIFO event channel 
> > operations
> > will again result in -ENOSYS being returned to the guest.
> 
> Are there indeed dependencies on the precise return value anywhere?
> If so, the generally inappropriate use (do_event_channel_op()'s
> default case really would also need switching) would want a brief
> comment, so it'll be understood by readers that this isn't code to
> derive other code from. If not, -EPERM or -EACCES perhaps?
> 

The patch, as stated, is reverting behaviour and so the -ENOSYS really needs to 
stay since it is essentially ABI now. I am not aware of guest code that will, 
in fact, die if it sees -EPERM or -EACCES instead but there may be such code. 
The only safe thing to do is to make things look like the used to.

> Also, now that we gain a runtime control, do we perhaps also want a
> build time one?

Yes, a Kconfig flag to compile out the code seems like a worthy addition. I'll 
spin up a follow-up patch as soon as I get time.

  Paul

> 
> > Signed-off-by: Paul Durrant 
> > Signed-off-by: Eslam Elnikety 
> 
> Are this order as well as the From: tag above correct? Or
> alternatively, are there actually any pieces left at all from
> Eslam's earlier patch?
> 
> > v4:
> >  - New in v4
> 
> (Just as an aside: That's quite interesting for a previously
> standalone patch. I guess that patch was really split, considering
> you've retained Eslam's S-o-b? But perhaps there are different ways
> to look at things ...)
> 
> > --- a/xen/include/public/domctl.h
> > +++ b/xen/include/public/domctl.h
> > @@ -70,9 +70,11 @@ struct xen_domctl_createdomain {
> >  #define XEN_DOMCTL_CDF_iommu  (1U<<_XEN_DOMCTL_CDF_iommu)
> >  #define _XEN_DOMCTL_CDF_nested_virt   6
> >  #define XEN_DOMCTL_CDF_nested_virt(1U << _XEN_DOMCTL_CDF_nested_virt)
> > +#define _XEN_DOMCTL_CDF_disable_fifo  7
> > +#define XEN_DOMCTL_CDF_disable_fifo   (1U << _XEN_DOMCTL_CDF_disable_fifo)
> 
> Despite getting longish, I think this needs "evtchn" somewhere in
> the name. To keep size bounded, maybe XEN_DOMCTL_CDF_no_fifo_evtchn?
> 
> >  /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
> > -#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_nested_virt
> > +#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_disable_fifo
> 
> While not directly related to this patch, I'm puzzled by the
> presence of this constant: I've not been able to find any use of
> it. In 

RE: [PATCH v4 1/3] domctl: introduce a new domain create flag, XEN_DOMCTL_CDF_disable_fifo, ...

[Durrant, Paul]

> -Original Message-
> From: Jan Beulich 
> Sent: 25 November 2020 09:36
> To: Andrew Cooper 
> Cc: Durrant, Paul ; Elnikety, Eslam 
> ; Christian Lindig
> ; David Scott ; Ian Jackson 
> ; Wei
> Liu ; George Dunlap ; Julien Grall 
> ; Stefano
> Stabellini ; xen-devel@lists.xenproject.org; Paul 
> Durrant 
> Subject: RE: [EXTERNAL] [PATCH v4 1/3] domctl: introduce a new domain create 
> flag,
> XEN_DOMCTL_CDF_disable_fifo, ...
> 
> CAUTION: This email originated from outside of the organization. Do not click 
> links or open
> attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> On 25.11.2020 10:20, Jan Beulich wrote:
> > On 24.11.2020 20:17, Paul Durrant wrote:
> >> --- a/xen/include/public/domctl.h
> >> +++ b/xen/include/public/domctl.h
> >> @@ -70,9 +70,11 @@ struct xen_domctl_createdomain {
> >>  #define XEN_DOMCTL_CDF_iommu  (1U<<_XEN_DOMCTL_CDF_iommu)
> >>  #define _XEN_DOMCTL_CDF_nested_virt   6
> >>  #define XEN_DOMCTL_CDF_nested_virt(1U << _XEN_DOMCTL_CDF_nested_virt)
> >> +#define _XEN_DOMCTL_CDF_disable_fifo  7
> >> +#define XEN_DOMCTL_CDF_disable_fifo   (1U << _XEN_DOMCTL_CDF_disable_fifo)
> >
> > Despite getting longish, I think this needs "evtchn" somewhere in
> > the name. To keep size bounded, maybe XEN_DOMCTL_CDF_no_fifo_evtchn?
> >

I'm ok with that name; I'll send a v5.

> >>  /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
> >> -#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_nested_virt
> >> +#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_disable_fifo
> >
> > While not directly related to this patch, I'm puzzled by the
> > presence of this constant: I've not been able to find any use of
> > it. In particular you did have a need to modify
> > sanitise_domain_config().
> 
> So it was you to introduce this, right away without any user, in
> 7fb0e134f8c6 ("tools/ocaml: abi: Use formal conversion and check
> in more places"). The only reference is from what I regard as a
> comment (I don't speak any ocaml, so I may be wrong). Could you
> clarify why we need to maintain this constant?
> 

I can't remember the exact sequence of events but it became apparent at some 
point that the ocaml bindings were out of sync and they rely on a list of 
domain create flags where the number has to match the bit-shift value in 
domctl.h (among other things). Thus there is an auto-generated header called 
"xenctrl_abi_check.h" which is included by xenctrl_stubs.c. This header is 
generated from xenctrl.ml by the perl script "abi-check" and it relies the 
XEN_DOMCTL_CDF_MAX constant to form part of the checks it generates.

As an example, here is the generated header with this patch applied:

// found ocaml type x86_arch_emulation_flags at xenctrl.ml:38
BUILD_BUG_ON( XEN_X86_EMU_LAPIC  != (1u << 0)  );
BUILD_BUG_ON( XEN_X86_EMU_HPET   != (1u << 1)  );
BUILD_BUG_ON( XEN_X86_EMU_PM != (1u << 2)  );
BUILD_BUG_ON( XEN_X86_EMU_RTC!= (1u << 3)  );
BUILD_BUG_ON( XEN_X86_EMU_IOAPIC != (1u << 4)  );
BUILD_BUG_ON( XEN_X86_EMU_PIC!= (1u << 5)  );
BUILD_BUG_ON( XEN_X86_EMU_VGA!= (1u << 6)  );
BUILD_BUG_ON( XEN_X86_EMU_IOMMU  != (1u << 7)  );
BUILD_BUG_ON( XEN_X86_EMU_PIT!= (1u << 8)  );
BUILD_BUG_ON( XEN_X86_EMU_USE_PIRQ   != (1u << 9)  );
BUILD_BUG_ON( XEN_X86_EMU_VPCI   != (1u << 10) );
BUILD_BUG_ON( XEN_X86_EMU_ALL!= (1u << 11)-1u );
// found ocaml type domain_create_flag at xenctrl.ml:60
BUILD_BUG_ON( XEN_DOMCTL_CDF_hvm != (1u << 0)  );
BUILD_BUG_ON( XEN_DOMCTL_CDF_hap != (1u << 1)  );
BUILD_BUG_ON( XEN_DOMCTL_CDF_s3_integrity!= (1u << 2)  );
BUILD_BUG_ON( XEN_DOMCTL_CDF_oos_off != (1u << 3)  );
BUILD_BUG_ON( XEN_DOMCTL_CDF_xs_domain   != (1u << 4)  );
BUILD_BUG_ON( XEN_DOMCTL_CDF_iommu   != (1u << 5)  );
BUILD_BUG_ON( XEN_DOMCTL_CDF_nested_virt != (1u << 6)  );
BUILD_BUG_ON( XEN_DOMCTL_CDF_disable_fifo!= (1u << 7)  );
BUILD_BUG_ON( XEN_DOMCTL_CDF_MAX != (1u << 7)  );
// found ocaml type domain_create_iommu_opts at xenctrl.ml:70
BUILD_BUG_ON( XEN_DOMCTL_IOMMU_no_sharept!= (1u << 0)  );
BUILD_BUG_ON( XEN_DOMCTL_IOMMU_MAX   != (1u << 0)  );
// found ocaml type physinfo_cap_flag at xenctrl.ml:113
BUILD_BUG_ON( XEN_SYSCTL_PHYSCAP_hvm != (1u << 0)  );
BUILD_BUG_ON( XEN_SYSCTL_PHYSCAP_pv  != (1u << 1)  );
BUILD_BUG_ON( XEN_SYSCTL_PHYSCAP_directio!= (1u << 2)  );
BUILD_BUG_ON( XEN_SYSCTL_PHYSCAP_hap != (1u << 3)  );
BUILD_BUG_ON( XEN_SYSCTL_PHYSCAP_shadow  != (1u << 4)  );
BUILD_BUG_ON( XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share != (1u << 5)  );
BUILD_BUG_ON( XEN_SYSCTL_PHYSCAP_MAX != (1u << 5)  );

  Paul

[PATCH v8 3/3] xen/events: do some cleanups in evtchn_fifo_set_pending()

[Juergen Gross]

evtchn_fifo_set_pending() can be simplified a little bit.

Suggested-by: Jan Beulich 
Signed-off-by: Juergen Gross 
---
V8:
- new patch
---
 xen/common/event_fifo.c | 34 +++---
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/xen/common/event_fifo.c b/xen/common/event_fifo.c
index 443593c3b3..77609539b1 100644
--- a/xen/common/event_fifo.c
+++ b/xen/common/event_fifo.c
@@ -175,6 +175,18 @@ static void evtchn_fifo_set_pending(struct vcpu *v, struct 
evtchn *evtchn)
 return;
 }
 
+/*
+ * Control block not mapped.  The guest must not unmask an
+ * event until the control block is initialized, so we can
+ * just drop the event.
+ */
+if ( unlikely(!v->evtchn_fifo->control_block) )
+{
+printk(XENLOG_G_WARNING
+   "%pv has no FIFO event channel control block\n", v);
+return;
+}
+
 /*
  * Lock all queues related to the event channel (in case of a queue change
  * this might be two).
@@ -233,25 +245,8 @@ static void evtchn_fifo_set_pending(struct vcpu *v, struct 
evtchn *evtchn)
  * Link the event if it unmasked and not already linked.
  */
 if ( !guest_test_bit(d, EVTCHN_FIFO_MASKED, word) &&
- !guest_test_bit(d, EVTCHN_FIFO_LINKED, word) )
+ !guest_test_and_set_bit(d, EVTCHN_FIFO_LINKED, word) )
 {
-event_word_t *tail_word;
-
-/*
- * Control block not mapped.  The guest must not unmask an
- * event until the control block is initialized, so we can
- * just drop the event.
- */
-if ( unlikely(!v->evtchn_fifo->control_block) )
-{
-printk(XENLOG_G_WARNING
-   "%pv has no FIFO event channel control block\n", v);
-goto unlock;
-}
-
-if ( guest_test_and_set_bit(d, EVTCHN_FIFO_LINKED, word) )
-goto unlock;
-
 /*
  * If this event was a tail, the old queue is now empty and
  * its tail must be invalidated to prevent adding an event to
@@ -286,6 +281,8 @@ static void evtchn_fifo_set_pending(struct vcpu *v, struct 
evtchn *evtchn)
 linked = false;
 if ( q->tail )
 {
+event_word_t *tail_word;
+
 tail_word = evtchn_fifo_word_from_port(d, q->tail);
 linked = evtchn_fifo_set_link(d, tail_word, port);
 }
@@ -294,7 +291,6 @@ static void evtchn_fifo_set_pending(struct vcpu *v, struct 
evtchn *evtchn)
 q->tail = port;
 }
 
- unlock:
 if ( q != old_q )
 spin_unlock(_q->lock);
 spin_unlock_irqrestore(>lock, flags);
-- 
2.26.2

[PATCH v8 1/3] xen/events: modify struct evtchn layout

[Juergen Gross]

In order to avoid latent races when updating an event channel put
xen_consumer and pending fields in different bytes. This is no problem
right now, but especially the pending indicator isn't used only when
initializing an event channel (unlike xen_consumer), so any future
addition to this byte would need to be done with a potential race kept
in mind.

At the same time move some other fields around to have less implicit
paddings and to keep related fields more closely together.

Finally switch struct evtchn to no longer use fixed sized types where
not needed.

Signed-off-by: Juergen Gross 
---
V7:
- new patch

V8:
- retain xen_consumer to be a bitfield (Jan Beulich)
- switch to non fixed sizes types

Signed-off-by: Juergen Gross 
---
 xen/common/event_fifo.c |  4 ++--
 xen/include/xen/sched.h | 34 ++
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/xen/common/event_fifo.c b/xen/common/event_fifo.c
index 79090c04ca..f39e61105f 100644
--- a/xen/common/event_fifo.c
+++ b/xen/common/event_fifo.c
@@ -200,7 +200,7 @@ static void evtchn_fifo_set_pending(struct vcpu *v, struct 
evtchn *evtchn)
  */
 if ( unlikely(!word) )
 {
-evtchn->pending = 1;
+evtchn->pending = true;
 return;
 }
 
@@ -535,7 +535,7 @@ static void setup_ports(struct domain *d, unsigned int 
prev_evtchns)
 evtchn = evtchn_from_port(d, port);
 
 if ( guest_test_bit(d, port, _info(d, evtchn_pending)) )
-evtchn->pending = 1;
+evtchn->pending = true;
 
 evtchn_fifo_set_priority(d, evtchn, EVTCHN_FIFO_PRIORITY_DEFAULT);
 }
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index a345cc01f8..7afbae7dd1 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -93,31 +93,33 @@ struct evtchn
 #define ECS_PIRQ 4 /* Channel is bound to a physical IRQ line.   */
 #define ECS_VIRQ 5 /* Channel is bound to a virtual IRQ line.*/
 #define ECS_IPI  6 /* Channel is bound to a virtual IPI line.*/
-u8  state; /* ECS_* */
-u8  xen_consumer:XEN_CONSUMER_BITS; /* Consumer in Xen if nonzero */
-u8  pending:1;
-u16 notify_vcpu_id;/* VCPU for local delivery notification */
-u32 port;
+unsigned char state;   /* ECS_* */
+#ifndef NDEBUG
+unsigned char old_state; /* State when taking lock in write mode. */
+#endif
+unsigned char xen_consumer:XEN_CONSUMER_BITS; /* Consumer in Xen if != 0 */
+unsigned int port;
 union {
 struct {
 domid_t remote_domid;
-} unbound; /* state == ECS_UNBOUND */
+} unbound;  /* state == ECS_UNBOUND */
 struct {
 evtchn_port_t  remote_port;
 struct domain *remote_dom;
-} interdomain; /* state == ECS_INTERDOMAIN */
+} interdomain;  /* state == ECS_INTERDOMAIN */
 struct {
-u32irq;
+unsigned int   irq;
 evtchn_port_t  next_port;
 evtchn_port_t  prev_port;
-} pirq;/* state == ECS_PIRQ */
-u16 virq;  /* state == ECS_VIRQ */
+} pirq; /* state == ECS_PIRQ */
+unsigned int virq;  /* state == ECS_VIRQ */
 } u;
-u8 priority;
-#ifndef NDEBUG
-u8 old_state;  /* State when taking lock in write mode. */
-#endif
-u32 fifo_lastq;/* Data for fifo events identifying last queue. */
+
+bool pending;  /* FIFO event channels only. */
+unsigned char priority;/* FIFO event channels only. */
+unsigned short notify_vcpu_id; /* VCPU for local delivery notification */
+uint32_t fifo_lastq;   /* Data for identifying last queue. */
+
 #ifdef CONFIG_XSM
 union {
 #ifdef XSM_NEED_GENERIC_EVTCHN_SSID
@@ -133,7 +135,7 @@ struct evtchn
  * allocations, and on 64-bit platforms with only FLASK enabled,
  * reduces the size of struct evtchn.
  */
-u32 flask_sid;
+uint32_t flask_sid;
 #endif
 } ssid;
 #endif
-- 
2.26.2

[PATCH v8 0/3] xen/events: further locking adjustments

[Juergen Gross]

This is an add-on of my event channel locking series.

Juergen Gross (3):
  xen/events: modify struct evtchn layout
  xen/events: rework fifo queue locking
  xen/events: do some cleanups in evtchn_fifo_set_pending()

 xen/common/event_fifo.c | 152 +---
 xen/include/xen/sched.h |  34 -
 2 files changed, 98 insertions(+), 88 deletions(-)

-- 
2.26.2

[PATCH v8 2/3] xen/events: rework fifo queue locking

[Juergen Gross]

Two cpus entering evtchn_fifo_set_pending() for the same event channel
can race in case the first one gets interrupted after setting
EVTCHN_FIFO_PENDING and when the other one manages to set
EVTCHN_FIFO_LINKED before the first one is testing that bit. This can
lead to evtchn_check_pollers() being called before the event is put
properly into the queue, resulting eventually in the guest not seeing
the event pending and thus blocking forever afterwards.

Note that commit 5f2df45ead7c1195 ("xen/evtchn: rework per event channel
lock") made the race just more obvious, while the fifo event channel
implementation had this race from the beginning when an unmask operation
was running in parallel with an event channel send operation.

Using a spinlock for the per event channel lock is problematic due to
some paths needing to take the lock are called with interrupts off, so
the lock would need to disable interrupts, which in turn breaks some
use cases related to vm events.

For avoiding this race the queue locking in evtchn_fifo_set_pending()
needs to be reworked to cover the test of EVTCHN_FIFO_PENDING,
EVTCHN_FIFO_MASKED and EVTCHN_FIFO_LINKED, too. Additionally when an
event channel needs to change queues both queues need to be locked
initially.

Reported-by: Jan Beulich 
Fixes: 5f2df45ead7c1195 ("xen/evtchn: rework per event channel lock")
Fixes: de6acb78bf0e137c ("evtchn: use a per-event channel lock for sending 
events")
Signed-off-by: Juergen Gross 
---
V7:
- new patch

V8:
- update commit message (Jan Beulich)
- update double locking (Jan Beulich)
- add comment (Jan Beulich)
- fix handling when not getting lock (Jan Beulich)
- add const (Jan Beulich)

Signed-off-by: Juergen Gross 
---
 xen/common/event_fifo.c | 128 ++--
 1 file changed, 70 insertions(+), 58 deletions(-)

diff --git a/xen/common/event_fifo.c b/xen/common/event_fifo.c
index f39e61105f..443593c3b3 100644
--- a/xen/common/event_fifo.c
+++ b/xen/common/event_fifo.c
@@ -87,38 +87,6 @@ static void evtchn_fifo_init(struct domain *d, struct evtchn 
*evtchn)
  d->domain_id, evtchn->port);
 }
 
-static struct evtchn_fifo_queue *lock_old_queue(const struct domain *d,
-struct evtchn *evtchn,
-unsigned long *flags)
-{
-struct vcpu *v;
-struct evtchn_fifo_queue *q, *old_q;
-unsigned int try;
-union evtchn_fifo_lastq lastq;
-
-for ( try = 0; try < 3; try++ )
-{
-lastq.raw = read_atomic(>fifo_lastq);
-v = d->vcpu[lastq.last_vcpu_id];
-old_q = >evtchn_fifo->queue[lastq.last_priority];
-
-spin_lock_irqsave(_q->lock, *flags);
-
-v = d->vcpu[lastq.last_vcpu_id];
-q = >evtchn_fifo->queue[lastq.last_priority];
-
-if ( old_q == q )
-return old_q;
-
-spin_unlock_irqrestore(_q->lock, *flags);
-}
-
-gprintk(XENLOG_WARNING,
-"dom%d port %d lost event (too many queue changes)\n",
-d->domain_id, evtchn->port);
-return NULL;
-}  
-
 static int try_set_link(event_word_t *word, event_word_t *w, uint32_t link)
 {
 event_word_t new, old;
@@ -190,6 +158,9 @@ static void evtchn_fifo_set_pending(struct vcpu *v, struct 
evtchn *evtchn)
 event_word_t *word;
 unsigned long flags;
 bool_t was_pending;
+struct evtchn_fifo_queue *q, *old_q;
+unsigned int try;
+bool linked = true;
 
 port = evtchn->port;
 word = evtchn_fifo_word_from_port(d, port);
@@ -204,17 +175,67 @@ static void evtchn_fifo_set_pending(struct vcpu *v, 
struct evtchn *evtchn)
 return;
 }
 
+/*
+ * Lock all queues related to the event channel (in case of a queue change
+ * this might be two).
+ * It is mandatory to do that before setting and testing the PENDING bit
+ * and to hold the current queue lock until the event has put into the
+ * list of pending events in order to avoid waking up a guest without the
+ * event being visibly pending in the guest.
+ */
+for ( try = 0; try < 4; try++ )
+{
+union evtchn_fifo_lastq lastq;
+const struct vcpu *old_v;
+
+lastq.raw = read_atomic(>fifo_lastq);
+old_v = d->vcpu[lastq.last_vcpu_id];
+
+q = >evtchn_fifo->queue[evtchn->priority];
+old_q = _v->evtchn_fifo->queue[lastq.last_priority];
+
+if ( q == old_q )
+spin_lock_irqsave(>lock, flags);
+else if ( q < old_q )
+{
+spin_lock_irqsave(>lock, flags);
+spin_lock(_q->lock);
+}
+else
+{
+spin_lock_irqsave(_q->lock, flags);
+spin_lock(>lock);
+}
+
+lastq.raw = read_atomic(>fifo_lastq);
+old_v = d->vcpu[lastq.last_vcpu_id];
+if ( q == >evtchn_fifo->queue[evtchn->priority] &&
+ old_q == _v->evtchn_fifo->queue[lastq.last_priority] )
+break;
+
+if 

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Andy Shevchenko]

On Mon, Nov 23, 2020 at 10:39 PM James Bottomley
 wrote:
> On Mon, 2020-11-23 at 19:56 +0100, Miguel Ojeda wrote:
> > On Mon, Nov 23, 2020 at 4:58 PM James Bottomley
> >  wrote:

...

> > But if we do the math, for an author, at even 1 minute per line
> > change and assuming nothing can be automated at all, it would take 1
> > month of work. For maintainers, a couple of trivial lines is noise
> > compared to many other patches.
>
> So you think a one line patch should take one minute to produce ... I
> really don't think that's grounded in reality.  I suppose a one line
> patch only takes a minute to merge with b4 if no-one reviews or tests
> it, but that's not really desirable.

In my practice most of the one line patches were either to fix or to
introduce quite interesting issues.
1 minute is 2-3 orders less than usually needed for such patches.
That's why I don't like churn produced by people who often even didn't
compile their useful contributions.

-- 
With Best Regards,
Andy Shevchenko

Vtpm in Windows 10

[psarpol]

Hi,  Is there a way to mount a vtpm 2.0 in a Windows 10 VM ? Thanks Paul

[xen-unstable-coverity test] 157003: all pass - PUSHED

[osstest service owner]

flight 157003 xen-unstable-coverity real [real]
http://logs.test-lab.xenproject.org/osstest/logs/157003/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 xen  9b156bcc3ffcc7949edd4460b718a241e87ae302
baseline version:
 xen  b659a5cebd611dbe698e63c03485b5fe8cd964ad

Last test of basis   156941  2020-11-22 09:18:30 Z3 days
Testing same since   157003  2020-11-25 09:18:28 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Jan Beulich 
  Juergen Gross 
  Roger Pau Monné 

jobs:
 coverity-amd64   pass



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

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

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   b659a5cebd..9b156bcc3f  9b156bcc3ffcc7949edd4460b718a241e87ae302 -> 
coverity-tested/smoke

Re: [PATCH v4 1/3] domctl: introduce a new domain create flag, XEN_DOMCTL_CDF_disable_fifo, ...

[Jan Beulich]

On 25.11.2020 10:20, Jan Beulich wrote:
> On 24.11.2020 20:17, Paul Durrant wrote:
>> --- a/xen/include/public/domctl.h
>> +++ b/xen/include/public/domctl.h
>> @@ -70,9 +70,11 @@ struct xen_domctl_createdomain {
>>  #define XEN_DOMCTL_CDF_iommu  (1U<<_XEN_DOMCTL_CDF_iommu)
>>  #define _XEN_DOMCTL_CDF_nested_virt   6
>>  #define XEN_DOMCTL_CDF_nested_virt(1U << _XEN_DOMCTL_CDF_nested_virt)
>> +#define _XEN_DOMCTL_CDF_disable_fifo  7
>> +#define XEN_DOMCTL_CDF_disable_fifo   (1U << _XEN_DOMCTL_CDF_disable_fifo)
> 
> Despite getting longish, I think this needs "evtchn" somewhere in
> the name. To keep size bounded, maybe XEN_DOMCTL_CDF_no_fifo_evtchn?
> 
>>  /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
>> -#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_nested_virt
>> +#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_disable_fifo
> 
> While not directly related to this patch, I'm puzzled by the
> presence of this constant: I've not been able to find any use of
> it. In particular you did have a need to modify
> sanitise_domain_config().

So it was you to introduce this, right away without any user, in
7fb0e134f8c6 ("tools/ocaml: abi: Use formal conversion and check
in more places"). The only reference is from what I regard as a
comment (I don't speak any ocaml, so I may be wrong). Could you
clarify why we need to maintain this constant?

Thanks, Jan

Re: [PATCH v4 1/3] domctl: introduce a new domain create flag, XEN_DOMCTL_CDF_disable_fifo, ...

[Jan Beulich]

On 24.11.2020 20:17, Paul Durrant wrote:
> From: Paul Durrant 
> 
> ...to control the visibility of the FIFO event channel operations
> (EVTCHNOP_init_control, EVTCHNOP_expand_array, and EVTCHNOP_set_priority) to
> the guest.
> 
> These operations were added to the public header in commit d2d50c2f308f
> ("evtchn: add FIFO-based event channel ABI") and the first implementation
> appeared in the two subsequent commits: edc8872aeb4a ("evtchn: implement
> EVTCHNOP_set_priority and add the set_priority hook") and 88910061ec61
> ("evtchn: add FIFO-based event channel hypercalls and port ops"). Prior to
> that, a guest issuing those operations would receive a return value of
> -ENOSYS (not implemented) from Xen. Guests aware of the FIFO operations but
> running on an older (pre-4.4) Xen would fall back to using the 2-level event
> channel interface upon seeing this return value.
> 
> Unfortunately the uncontrolable appearance of these new operations in Xen 4.4
> onwards has implications for hibernation of some Linux guests. During resume
> from hibernation, there are two kernels involved: the "boot" kernel and the
> "resume" kernel. The guest boot kernel may default to use FIFO operations and
> instruct Xen via EVTCHNOP_init_control to switch from 2-level to FIFO. On the
> other hand, the resume kernel keeps assuming 2-level, because it was 
> hibernated
> on a version of Xen that did not support the FIFO operations.

And the alternative of the boot kernel issuing EVTCHNOP_reset has
other unwanted consequences. Maybe worth mentioning here, as
otherwise this would look like the obvious way to return to 2-level
mode?

Also, why can't the boot kernel be instructed to avoid engaging
FIFO mode?

> To maintain compatibility it is necessary to make Xen behave as it did
> before the new operations were added and hence the code in this patch ensures
> that, if XEN_DOMCTL_CDF_disable_fifo is set, the FIFO event channel operations
> will again result in -ENOSYS being returned to the guest.

Are there indeed dependencies on the precise return value anywhere?
If so, the generally inappropriate use (do_event_channel_op()'s
default case really would also need switching) would want a brief
comment, so it'll be understood by readers that this isn't code to
derive other code from. If not, -EPERM or -EACCES perhaps?

Also, now that we gain a runtime control, do we perhaps also want a
build time one?

> Signed-off-by: Paul Durrant 
> Signed-off-by: Eslam Elnikety 

Are this order as well as the From: tag above correct? Or
alternatively, are there actually any pieces left at all from
Eslam's earlier patch?

> v4:
>  - New in v4

(Just as an aside: That's quite interesting for a previously
standalone patch. I guess that patch was really split, considering
you've retained Eslam's S-o-b? But perhaps there are different ways
to look at things ...)

> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -70,9 +70,11 @@ struct xen_domctl_createdomain {
>  #define XEN_DOMCTL_CDF_iommu  (1U<<_XEN_DOMCTL_CDF_iommu)
>  #define _XEN_DOMCTL_CDF_nested_virt   6
>  #define XEN_DOMCTL_CDF_nested_virt(1U << _XEN_DOMCTL_CDF_nested_virt)
> +#define _XEN_DOMCTL_CDF_disable_fifo  7
> +#define XEN_DOMCTL_CDF_disable_fifo   (1U << _XEN_DOMCTL_CDF_disable_fifo)

Despite getting longish, I think this needs "evtchn" somewhere in
the name. To keep size bounded, maybe XEN_DOMCTL_CDF_no_fifo_evtchn?

>  /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
> -#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_nested_virt
> +#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_disable_fifo

While not directly related to this patch, I'm puzzled by the
presence of this constant: I've not been able to find any use of
it. In particular you did have a need to modify
sanitise_domain_config().

Jan

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Sean Young]

On Mon, Nov 23, 2020 at 07:58:06AM -0800, James Bottomley wrote:
> On Mon, 2020-11-23 at 15:19 +0100, Miguel Ojeda wrote:
> > On Sun, Nov 22, 2020 at 11:36 PM James Bottomley
> >  wrote:
> > > It's not about the risk of the changes it's about the cost of
> > > implementing them.  Even if you discount the producer time (which
> > > someone gets to pay for, and if I were the engineering manager, I'd
> > > be unhappy about), the review/merge/rework time is pretty
> > > significant in exchange for six minor bug fixes.  Fine, when a new
> > > compiler warning comes along it's certainly reasonable to see if we
> > > can benefit from it and the fact that the compiler people think
> > > it's worthwhile is enough evidence to assume this initially.  But
> > > at some point you have to ask whether that assumption is supported
> > > by the evidence we've accumulated over the time we've been using
> > > it.  And if the evidence doesn't support it perhaps it is time to
> > > stop the experiment.
> > 
> > Maintainers routinely review 1-line trivial patches, not to mention
> > internal API changes, etc.
> 
> We're also complaining about the inability to recruit maintainers:
> 
> https://www.theregister.com/2020/06/30/hard_to_find_linux_maintainers_says_torvalds/
> 
> And burn out:
> 
> http://antirez.com/news/129
> 
> The whole crux of your argument seems to be maintainers' time isn't
> important so we should accept all trivial patches ... I'm pushing back
> on that assumption in two places, firstly the valulessness of the time
> and secondly that all trivial patches are valuable.

You're assuming burn out or recruitment problems is due to patch workload
or too many "trivial" patches.

In my experience, "other maintainers" is by far the biggest cause of
burn out for my kernel maintenance work.

Certainly arguing with a maintainer about some obviously-correct patch
series must be a good example of this.


Sean

[PATCH 5/5] x86: don't build unused entry code when !PV32

[Jan Beulich]

Except for the initial part of cstar_enter compat/entry.S is all dead
code in this case. Further, along the lines of the PV conditionals we
already have in entry.S, make code PV32-conditional there too (to a
fair part because this code actually references compat/entry.S).

Signed-off-by: Jan Beulich 
---
TBD: I'm on the fence of whether (in a separate patch) to also make
 conditional struct pv_domain's is_32bit field.

--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -9,7 +9,7 @@
 #include 
 #endif
 #include 
-#ifdef CONFIG_PV
+#ifdef CONFIG_PV32
 #include 
 #endif
 #include 
@@ -102,19 +102,21 @@ void __dummy__(void)
 BLANK();
 #endif
 
-#ifdef CONFIG_PV
+#ifdef CONFIG_PV32
 OFFSET(DOMAIN_is_32bit_pv, struct domain, arch.pv.is_32bit);
 BLANK();
 
-OFFSET(VCPUINFO_upcall_pending, struct vcpu_info, evtchn_upcall_pending);
-OFFSET(VCPUINFO_upcall_mask, struct vcpu_info, evtchn_upcall_mask);
-BLANK();
-
 OFFSET(COMPAT_VCPUINFO_upcall_pending, struct compat_vcpu_info, 
evtchn_upcall_pending);
 OFFSET(COMPAT_VCPUINFO_upcall_mask, struct compat_vcpu_info, 
evtchn_upcall_mask);
 BLANK();
 #endif
 
+#ifdef CONFIG_PV
+OFFSET(VCPUINFO_upcall_pending, struct vcpu_info, evtchn_upcall_pending);
+OFFSET(VCPUINFO_upcall_mask, struct vcpu_info, evtchn_upcall_mask);
+BLANK();
+#endif
+
 OFFSET(CPUINFO_guest_cpu_user_regs, struct cpu_info, guest_cpu_user_regs);
 OFFSET(CPUINFO_verw_sel, struct cpu_info, verw_sel);
 OFFSET(CPUINFO_current_vcpu, struct cpu_info, current_vcpu);
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -29,8 +29,6 @@ ENTRY(entry_int82)
 mov   %rsp, %rdi
 call  do_entry_int82
 
-#endif /* CONFIG_PV32 */
-
 /* %rbx: struct vcpu */
 ENTRY(compat_test_all_events)
 ASSERT_NOT_IN_ATOMIC
@@ -197,6 +195,8 @@ ENTRY(cr4_pv32_restore)
 xor   %eax, %eax
 ret
 
+#endif /* CONFIG_PV32 */
+
 .section .text.entry, "ax", @progbits
 
 /* See lstar_enter for entry register state. */
@@ -230,6 +230,13 @@ ENTRY(cstar_enter)
 sti
 
 movq  STACK_CPUINFO_FIELD(current_vcpu)(%rbx), %rbx
+
+#ifndef CONFIG_PV32
+
+jmp   switch_to_kernel
+
+#else
+
 movq  VCPU_domain(%rbx),%rcx
 cmpb  $0,DOMAIN_is_32bit_pv(%rcx)
 jeswitch_to_kernel
@@ -393,3 +400,5 @@ compat_crash_page_fault:
 jmp   .Lft14
 .previous
 _ASM_EXTABLE(.Lft14, .Lfx14)
+
+#endif /* CONFIG_PV32 */
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -328,8 +328,10 @@ UNLIKELY_END(sysenter_gpf)
 movq  VCPU_domain(%rbx),%rdi
 movq  %rax,TRAPBOUNCE_eip(%rdx)
 movb  %cl,TRAPBOUNCE_flags(%rdx)
+#ifdef CONFIG_PV32
 cmpb  $0, DOMAIN_is_32bit_pv(%rdi)
 jne   compat_sysenter
+#endif
 jmp   .Lbounce_exception
 
 ENTRY(int80_direct_trap)
@@ -370,6 +372,7 @@ UNLIKELY_END(msi_check)
 mov0x80 * TRAPINFO_sizeof + TRAPINFO_eip(%rsi), %rdi
 movzwl 0x80 * TRAPINFO_sizeof + TRAPINFO_cs (%rsi), %ecx
 
+#ifdef CONFIG_PV32
 mov   %ecx, %edx
 and   $~3, %edx
 
@@ -378,6 +381,10 @@ UNLIKELY_END(msi_check)
 
 test  %rdx, %rdx
 jzint80_slow_path
+#else
+test  %rdi, %rdi
+jzint80_slow_path
+#endif
 
 /* Construct trap_bounce from trap_ctxt[0x80]. */
 lea   VCPU_trap_bounce(%rbx), %rdx
@@ -390,8 +397,10 @@ UNLIKELY_END(msi_check)
 lea   (, %rcx, TBF_INTERRUPT), %ecx
 mov   %cl, TRAPBOUNCE_flags(%rdx)
 
+#ifdef CONFIG_PV32
 cmpb  $0, DOMAIN_is_32bit_pv(%rax)
 jne   compat_int80_direct_trap
+#endif
 
 call  create_bounce_frame
 jmp   test_all_events
@@ -541,12 +550,16 @@ ENTRY(dom_crash_sync_extable)
 GET_STACK_END(ax)
 leaq  STACK_CPUINFO_FIELD(guest_cpu_user_regs)(%rax),%rsp
 # create_bounce_frame() temporarily clobbers CS.RPL. Fix up.
+#ifdef CONFIG_PV32
 movq  STACK_CPUINFO_FIELD(current_vcpu)(%rax), %rax
 movq  VCPU_domain(%rax),%rax
 cmpb  $0, DOMAIN_is_32bit_pv(%rax)
 sete  %al
 leal  (%rax,%rax,2),%eax
 orb   %al,UREGS_cs(%rsp)
+#else
+orb   $3, UREGS_cs(%rsp)
+#endif
 xorl  %edi,%edi
 jmp   asm_domain_crash_synchronous /* Does not return */
 .popsection
@@ -562,11 +575,15 @@ ENTRY(ret_from_intr)
 GET_CURRENT(bx)
 testb $3, UREGS_cs(%rsp)
 jzrestore_all_xen
+#ifdef CONFIG_PV32
 movq  VCPU_domain(%rbx), %rax
 cmpb  $0, DOMAIN_is_32bit_pv(%rax)
 jetest_all_events
 jmp   compat_test_all_events
 #else
+jmp   test_all_events
+#endif
+#else
 ASSERT_CONTEXT_IS_XEN
 jmp   restore_all_xen
 #endif
@@ -652,7 +669,7 @@ handle_exception_saved:
 testb $X86_EFLAGS_IF>>8,UREGS_eflags+1(%rsp)
 jzexception_with_ints_disabled
 
-#ifdef CONFIG_PV
+#if 

[PATCH 4/5] x86: hypercall vector is unused when !PV32

[Jan Beulich]

This vector can be used as an ordinary interrupt handling one in this
case. To be sure no references are left, make the #define itself
conditional.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -436,8 +436,12 @@ int __init init_irq_data(void)
 irq_to_desc(irq)->irq = irq;
 
 #ifdef CONFIG_PV
-/* Never allocate the hypercall vector or Linux/BSD fast-trap vector. */
+/* Never allocate the Linux/BSD fast-trap vector. */
 set_bit(LEGACY_SYSCALL_VECTOR, used_vectors);
+#endif
+
+#ifdef CONFIG_PV32
+/* Never allocate the hypercall vector. */
 set_bit(HYPERCALL_VECTOR, used_vectors);
 #endif
 
--- a/xen/arch/x86/pv/traps.c
+++ b/xen/arch/x86/pv/traps.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 
+#ifdef CONFIG_PV32
 void do_entry_int82(struct cpu_user_regs *regs)
 {
 if ( unlikely(untrusted_msi) )
@@ -37,6 +38,7 @@ void do_entry_int82(struct cpu_user_regs
 
 pv_hypercall(regs);
 }
+#endif
 
 void pv_inject_event(const struct x86_event *event)
 {
@@ -155,9 +157,11 @@ static void nmi_softirq(void)
 
 void __init pv_trap_init(void)
 {
+#ifdef CONFIG_PV32
 /* The 32-on-64 hypercall vector is only accessible from ring 1. */
 _set_gate(idt_table + HYPERCALL_VECTOR,
   SYS_DESC_trap_gate, 1, entry_int82);
+#endif
 
 /* Fast trap for int80 (faster than taking the #GP-fixup path). */
 _set_gate(idt_table + LEGACY_SYSCALL_VECTOR, SYS_DESC_trap_gate, 3,
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -11,6 +11,8 @@
 #include 
 #include 
 
+#ifdef CONFIG_PV32
+
 ENTRY(entry_int82)
 ASM_CLAC
 pushq $0
@@ -27,6 +29,8 @@ ENTRY(entry_int82)
 mov   %rsp, %rdi
 call  do_entry_int82
 
+#endif /* CONFIG_PV32 */
+
 /* %rbx: struct vcpu */
 ENTRY(compat_test_all_events)
 ASSERT_NOT_IN_ATOMIC
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -982,8 +982,10 @@ autogen_stubs: /* Automatically generate
 .rept X86_NR_VECTORS
 
 /* Common interrupts, heading towards do_IRQ(). */
-#ifdef CONFIG_PV
+#if defined(CONFIG_PV32)
 .if vec >= FIRST_IRQ_VECTOR && vec != HYPERCALL_VECTOR && vec != 
LEGACY_SYSCALL_VECTOR
+#elif defined(CONFIG_PV)
+.if vec >= FIRST_IRQ_VECTOR && vec != LEGACY_SYSCALL_VECTOR
 #else
 .if vec >= FIRST_IRQ_VECTOR
 #endif
--- a/xen/include/asm-x86/mach-default/irq_vectors.h
+++ b/xen/include/asm-x86/mach-default/irq_vectors.h
@@ -22,7 +22,10 @@
 #define FIRST_LEGACY_VECTOR FIRST_DYNAMIC_VECTOR
 #define LAST_LEGACY_VECTOR  (FIRST_LEGACY_VECTOR + 0xf)
 
-#define HYPERCALL_VECTOR   0x82
+#ifdef CONFIG_PV32
+#define HYPERCALL_VECTOR0x82
+#endif
+
 #define LEGACY_SYSCALL_VECTOR   0x80
 
 /*

[PATCH 3/5] x86/build: restrict contents of asm-offsets.h when !HVM / !PV

[Jan Beulich]

This file has a long dependencies list (through asm-offsets.[cs]) and a
long list of dependents. IOW if any of the former changes, all of the
latter will be rebuilt, even if there's no actual change to the
generated file. Therefore avoid producing symbols we don't actually
need, depending on configuration.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -84,6 +84,7 @@ void __dummy__(void)
 DEFINE(_VGCF_syscall_disables_events,  _VGCF_syscall_disables_events);
 BLANK();
 
+#ifdef CONFIG_HVM
 OFFSET(VCPU_svm_vmcb_pa, struct vcpu, arch.hvm.svm.vmcb_pa);
 OFFSET(VCPU_svm_vmcb, struct vcpu, arch.hvm.svm.vmcb);
 BLANK();
@@ -99,6 +100,7 @@ void __dummy__(void)
 OFFSET(VCPU_nhvm_p2m, struct vcpu, arch.hvm.nvcpu.nv_p2m);
 OFFSET(VCPU_nsvm_hap_enabled, struct vcpu, 
arch.hvm.nvcpu.u.nsvm.ns_hap_enabled);
 BLANK();
+#endif
 
 #ifdef CONFIG_PV
 OFFSET(DOMAIN_is_32bit_pv, struct domain, arch.pv.is_32bit);
@@ -128,6 +130,7 @@ void __dummy__(void)
 DEFINE(CPUINFO_sizeof, sizeof(struct cpu_info));
 BLANK();
 
+#ifdef CONFIG_PV
 OFFSET(TRAPINFO_eip, struct trap_info, address);
 OFFSET(TRAPINFO_cs, struct trap_info, cs);
 OFFSET(TRAPINFO_flags, struct trap_info, flags);
@@ -139,6 +142,7 @@ void __dummy__(void)
 OFFSET(TRAPBOUNCE_cs, struct trap_bounce, cs);
 OFFSET(TRAPBOUNCE_eip, struct trap_bounce, eip);
 BLANK();
+#endif
 
 OFFSET(VCPUMSR_spec_ctrl_raw, struct vcpu_msrs, spec_ctrl.raw);
 BLANK();

[PATCH 2/5] x86/build: limit #include-ing by asm-offsets.c

[Jan Beulich]

This file has a long dependencies list and asm-offsets.h, generated from
it, has a long list of dependents. IOW if any of the former changes, all
of the latter will be rebuilt, even if there's no actual change to the
generated file. Therefore avoid including headers we don't actually need
(generally or configuration dependent).

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -5,11 +5,13 @@
  */
 #define COMPILE_OFFSETS
 
+#ifdef CONFIG_PERF_COUNTERS
 #include 
+#endif
 #include 
-#include 
+#ifdef CONFIG_PV
 #include 
-#include 
+#endif
 #include 
 #include 
 #include 
@@ -101,7 +103,6 @@ void __dummy__(void)
 #ifdef CONFIG_PV
 OFFSET(DOMAIN_is_32bit_pv, struct domain, arch.pv.is_32bit);
 BLANK();
-#endif
 
 OFFSET(VCPUINFO_upcall_pending, struct vcpu_info, evtchn_upcall_pending);
 OFFSET(VCPUINFO_upcall_mask, struct vcpu_info, evtchn_upcall_mask);
@@ -110,6 +111,7 @@ void __dummy__(void)
 OFFSET(COMPAT_VCPUINFO_upcall_pending, struct compat_vcpu_info, 
evtchn_upcall_pending);
 OFFSET(COMPAT_VCPUINFO_upcall_mask, struct compat_vcpu_info, 
evtchn_upcall_mask);
 BLANK();
+#endif
 
 OFFSET(CPUINFO_guest_cpu_user_regs, struct cpu_info, guest_cpu_user_regs);
 OFFSET(CPUINFO_verw_sel, struct cpu_info, verw_sel);

[PATCH 1/5] x86/build: limit rebuilding of asm-offsets.h

[Jan Beulich]

This file has a long dependencies list (through asm-offsets.s) and a
long list of dependents. IOW if any of the former changes, all of the
latter will be rebuilt, even if there's no actual change to the
generated file. This is the primary scenario we have the move-if-changed
macro for.

Since debug information may easily cause the file contents to change in
benign ways, also avoid emitting this into the output file.

Finally already before this change *.new files needed including in what
gets removed by the "clean" target.

Signed-off-by: Jan Beulich 
---
Perhaps Arm would want doing the same. In fact perhaps the rules should
be unified by moving to common code?

--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -235,7 +235,8 @@ efi/buildid.o efi/relocs-dummy.o: $(BASE
 efi/buildid.o efi/relocs-dummy.o: ;
 
 asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c 
$(BASEDIR)/include/asm-x86/asm-macros.h
-   $(CC) $(filter-out -Wa$(comma)% -flto,$(c_flags)) -S -o $@ $<
+   $(CC) $(filter-out -Wa$(comma)% -flto,$(c_flags)) -S -g0 -o $@.new $<
+   $(call move-if-changed,$@.new,$@)
 
 asm-macros.i: CFLAGS-y += -D__ASSEMBLY__ -P
 
@@ -262,7 +263,7 @@ efi/mkreloc: efi/mkreloc.c
 
 .PHONY: clean
 clean::
-   rm -f asm-offsets.s *.lds boot/*.o boot/*~ boot/core boot/mkelf32
+   rm -f asm-offsets.s *.lds *.new boot/*.o boot/*~ boot/core boot/mkelf32
rm -f asm-macros.i $(BASEDIR)/include/asm-x86/asm-macros.*
rm -f $(BASEDIR)/.xen-syms.[0-9]* boot/.*.d $(BASEDIR)/.xen.elf32
rm -f $(BASEDIR)/.xen.efi.[0-9]* efi/*.efi efi/mkreloc

[PATCH 0/5] x86: asm-offsets.h and !PV32 adjustments

[Jan Beulich]

The 2nd and 3rd patches here effectively called for the latter
two to also be created, hence why they all live together.

1: build: limit rebuilding of asm-offsets.s
2: build: limit #include-ing by asm-offsets.h
3: build: restrict contents of asm-offsets.h when !HVM / !PV
4: hypercall vector is unused when !PV32
5: don't build unused entry code when !PV32

Jan

Re: [PATCH v7 3/3] xen/events: rework fifo queue locking

[Jürgen Groß]

On 25.11.20 09:25, Jan Beulich wrote:

On 25.11.2020 09:08, Jürgen Groß wrote:

On 25.11.20 08:42, Jan Beulich wrote:

On 25.11.2020 06:23, Jürgen Groß wrote:

On 24.11.20 17:32, Jan Beulich wrote:

On 24.11.2020 15:49, Jürgen Groß wrote:

On 24.11.20 15:02, Jan Beulich wrote:

On 24.11.2020 08:01, Juergen Gross wrote:

Two cpus entering evtchn_fifo_set_pending() for the same event channel
can race in case the first one gets interrupted after setting
EVTCHN_FIFO_PENDING and when the other one manages to set
EVTCHN_FIFO_LINKED before the first one is testing that bit. This can
lead to evtchn_check_pollers() being called before the event is put
properly into the queue, resulting eventually in the guest not seeing
the event pending and thus blocking forever afterwards.

Note that commit 5f2df45ead7c1195 ("xen/evtchn: rework per event channel
lock") made the race just more obvious, while the fifo event channel
implementation had this race from the beginning when an unmask operation
was running in parallel with an event channel send operation.


Ah yes, but then also only for inter-domain channels, as it was
only in that case that the "wrong" domain's event lock was held.
IOW there was a much earlier change already where this issue
got widened (when the per-channel locking got introduced). This
then got reduced to the original scope by XSA-343's adding of
locking to evtchn_unmask(). (Not sure how much of this history
wants actually adding here. I'm writing it down not the least to
make sure I have a complete enough picture.)


I think we both agree that this race was possible for quite some time.
And I even think one customer bug I've been looking into recently
might be exactly this problem (a dom0 was occasionally hanging in
cross-cpu function calls, but switching to 2-level events made the
problem disappear).


IPIs weren't affected earlier on (i.e. in any released version),
if my analysis above is correct.


I don't think it is correct.

An unmask operation in parallel with set_pending will have had the
same race for IPIs.


Why? When FIFO locks were introduced, the event lock got acquired
around the call to evtchn_unmask(), and IPIs got sent with that
lock similarly held. Likewise after XSA-343 evtchn_unmask() as
well as the sending of IPIs acquire the per-channel lock (which at
that point was still an ordinary spin lock).


Oh, I think we are talking about different paths.

I'm talking about EVTCHNOP_unmask. There is no lock involved when
calling evtchn_unmask().


Above I said "When FIFO locks were introduced, the event lock got
acquired around the call to evtchn_unmask()" and further "Likewise
after XSA-343 evtchn_unmask() ..." I can't see how we're talking
of different paths here. The situation has changed from back then
(lock in the callers of evtchn_unmask()) to after XSA-343 (lock in
evtchn_unmask()), but there was suitable locking. There was a
(large) window in time prior to XSA-343 where there was indeed no
locking, but that was introduced by the conversion to per-channel
locks and addressed by one of the XSA-343 changes. The issue then
got re-introduced by converting spin_lock() to read_lock().


Okay, then I misunderstood your claim. I thought you meant there was
always suitable locking before the rwlock change. So I need to modify
the Fixes: tag.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH v7 3/3] xen/events: rework fifo queue locking

[Jan Beulich]

On 25.11.2020 09:08, Jürgen Groß wrote:
> On 25.11.20 08:42, Jan Beulich wrote:
>> On 25.11.2020 06:23, Jürgen Groß wrote:
>>> On 24.11.20 17:32, Jan Beulich wrote:
 On 24.11.2020 15:49, Jürgen Groß wrote:
> On 24.11.20 15:02, Jan Beulich wrote:
>> On 24.11.2020 08:01, Juergen Gross wrote:
>>> Two cpus entering evtchn_fifo_set_pending() for the same event channel
>>> can race in case the first one gets interrupted after setting
>>> EVTCHN_FIFO_PENDING and when the other one manages to set
>>> EVTCHN_FIFO_LINKED before the first one is testing that bit. This can
>>> lead to evtchn_check_pollers() being called before the event is put
>>> properly into the queue, resulting eventually in the guest not seeing
>>> the event pending and thus blocking forever afterwards.
>>>
>>> Note that commit 5f2df45ead7c1195 ("xen/evtchn: rework per event channel
>>> lock") made the race just more obvious, while the fifo event channel
>>> implementation had this race from the beginning when an unmask operation
>>> was running in parallel with an event channel send operation.
>>
>> Ah yes, but then also only for inter-domain channels, as it was
>> only in that case that the "wrong" domain's event lock was held.
>> IOW there was a much earlier change already where this issue
>> got widened (when the per-channel locking got introduced). This
>> then got reduced to the original scope by XSA-343's adding of
>> locking to evtchn_unmask(). (Not sure how much of this history
>> wants actually adding here. I'm writing it down not the least to
>> make sure I have a complete enough picture.)
>
> I think we both agree that this race was possible for quite some time.
> And I even think one customer bug I've been looking into recently
> might be exactly this problem (a dom0 was occasionally hanging in
> cross-cpu function calls, but switching to 2-level events made the
> problem disappear).

 IPIs weren't affected earlier on (i.e. in any released version),
 if my analysis above is correct.
>>>
>>> I don't think it is correct.
>>>
>>> An unmask operation in parallel with set_pending will have had the
>>> same race for IPIs.
>>
>> Why? When FIFO locks were introduced, the event lock got acquired
>> around the call to evtchn_unmask(), and IPIs got sent with that
>> lock similarly held. Likewise after XSA-343 evtchn_unmask() as
>> well as the sending of IPIs acquire the per-channel lock (which at
>> that point was still an ordinary spin lock).
> 
> Oh, I think we are talking about different paths.
> 
> I'm talking about EVTCHNOP_unmask. There is no lock involved when
> calling evtchn_unmask().

Above I said "When FIFO locks were introduced, the event lock got
acquired around the call to evtchn_unmask()" and further "Likewise
after XSA-343 evtchn_unmask() ..." I can't see how we're talking
of different paths here. The situation has changed from back then
(lock in the callers of evtchn_unmask()) to after XSA-343 (lock in
evtchn_unmask()), but there was suitable locking. There was a
(large) window in time prior to XSA-343 where there was indeed no
locking, but that was introduced by the conversion to per-channel
locks and addressed by one of the XSA-343 changes. The issue then
got re-introduced by converting spin_lock() to read_lock().

Jan

Re: [PATCH v7 3/3] xen/events: rework fifo queue locking

[Jürgen Groß]

On 25.11.20 08:42, Jan Beulich wrote:

On 25.11.2020 06:23, Jürgen Groß wrote:

On 24.11.20 17:32, Jan Beulich wrote:

On 24.11.2020 15:49, Jürgen Groß wrote:

On 24.11.20 15:02, Jan Beulich wrote:

On 24.11.2020 08:01, Juergen Gross wrote:

Two cpus entering evtchn_fifo_set_pending() for the same event channel
can race in case the first one gets interrupted after setting
EVTCHN_FIFO_PENDING and when the other one manages to set
EVTCHN_FIFO_LINKED before the first one is testing that bit. This can
lead to evtchn_check_pollers() being called before the event is put
properly into the queue, resulting eventually in the guest not seeing
the event pending and thus blocking forever afterwards.

Note that commit 5f2df45ead7c1195 ("xen/evtchn: rework per event channel
lock") made the race just more obvious, while the fifo event channel
implementation had this race from the beginning when an unmask operation
was running in parallel with an event channel send operation.


Ah yes, but then also only for inter-domain channels, as it was
only in that case that the "wrong" domain's event lock was held.
IOW there was a much earlier change already where this issue
got widened (when the per-channel locking got introduced). This
then got reduced to the original scope by XSA-343's adding of
locking to evtchn_unmask(). (Not sure how much of this history
wants actually adding here. I'm writing it down not the least to
make sure I have a complete enough picture.)


I think we both agree that this race was possible for quite some time.
And I even think one customer bug I've been looking into recently
might be exactly this problem (a dom0 was occasionally hanging in
cross-cpu function calls, but switching to 2-level events made the
problem disappear).


IPIs weren't affected earlier on (i.e. in any released version),
if my analysis above is correct.


I don't think it is correct.

An unmask operation in parallel with set_pending will have had the
same race for IPIs.


Why? When FIFO locks were introduced, the event lock got acquired
around the call to evtchn_unmask(), and IPIs got sent with that
lock similarly held. Likewise after XSA-343 evtchn_unmask() as
well as the sending of IPIs acquire the per-channel lock (which at
that point was still an ordinary spin lock).


Oh, I think we are talking about different paths.

I'm talking about EVTCHNOP_unmask. There is no lock involved when
calling evtchn_unmask().




Additionally when an
event channel needs to change queues both queues need to be locked
initially.


Since this was (afaict) intentionally not the case before, I
think I would want to see a word spent on the "why", perhaps
better in a code comment than here. Even more so that you
delete a respective comment justifying the possible race as
permissible. And I have to admit right now I'm still uncertain
both ways, i.e. I neither have a clear understanding of why it
would have been considered fine the other way around before,
nor why the double locking is strictly needed.


I need the double locking to avoid someone entering the locked region
when dropping the lock for the old queue and taking the one for the
new queue, as this would open the same race window again.


Well, that's what have already said. Thing is that the code
prior to your change gives the impression as if this race was
benign.


The race regarding a queue change, yes. But not the race I'm fixing with
this patch. I need to make sure that only one caller is inside the big
if clause for a specific event. And dropping the lock inside this clause
would violate that assumption.


IOW the presumed wrong assumption back then was that the function
would always be called with a lock already held which excludes
the region to be entered twice for the same channel. But - was
this a wrong assumption at the time? Thinking about this again I
now actually come to the conclusion that my analysis above was
wrong in the other direction: Even inter-domain channels did have
consistent locking (of the other side's event lock), preventing
any such race there. Which implies that imo one of the Fixes: tags
wants dropping, as the race became possible only when "downgrading"
some of the involved locks to rw ones. Obviously my "evtchn:
convert vIRQ lock to an r/w one" then extends this race to vIRQ-s.


No. See my remark regarding unmask.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys


OpenPGP_signature
Description: OpenPGP digital signature