[libvirt test] 159693: regressions - FAIL

[osstest service owner]

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

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  eaaf9397f40a7a893a04ee86676478cca3c80d9d
baseline version:
 libvirt  2c846fa6bcc11929c9fb857a22430fb9945654ad

Last test of basis   151777  2020-07-10 04:19:19 Z  231 days
Failing since151818  2020-07-11 04:18:52 Z  230 days  223 attempts
Testing same since   159693  2021-02-26 04:18:55 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 
  BiaoXiang Ye 
  Bihong Yu 
  Binfeng Wu 
  Boris Fiuczynski 
  Brian Turek 
  Bruno Haible 
  Christian Ehrhardt 
  Christian Schoenebeck 
  Cole Robinson 
  Collin Walling 
  Cornelia Huck 
  Cédric Bosdonnat 
  Côme Borsoi 
  Daniel Henrique Barboza 
  Daniel Letai 
  Daniel P. Berrange 
  Daniel P. Berrangé 
  Dmytro Linkin 
  Eiichi Tsukata 
  Erik Skultety 
  Fabian Affolter 
  Fabian Freyer 
  Fangge Jin 
  Farhan Ali 
  Fedora Weblate Translation 
  gongwei 
  Guoyi Tu
  Göran Uddeborg 
  Halil Pasic 
  Han Han 
  Hao Wang 
  Helmut Grohne 
  Ian Wienand 
  Jakob Meng 
  Jamie Strandboge 
  Jamie Strandboge 
  Jan Kuparinen 
  Jean-Baptiste Holcroft 
  Jianan Gao 
  Jim Fehlig 
  Jin Yan 
  Jiri Denemark 
  John Ferlan 
  Jonathan Watt 
  Jonathon Jongsma 
  Julio Faracco 
  Ján Tomko 
  Kashyap Chamarthy 
  Kevin Locke 
  Kristina Hanicova 
  Laine Stump 
  Laszlo Ersek 
  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 
  Meina Li 
  Michal Privoznik 
  Michał Smyk 
  Milo Casagrande 
  Moshe Levi 
  Muha Aliss 
  Neal Gompa 
  Nick Shyrokovskiy 
  Nickys Music Group 
  Nico Pache 
  Nikolay Shirokovskiy 
  Olaf Hering 
  Olesya Gerasimenko 
  Orion Poplawski 
  Pany 
  Patrick Magauran 
  Paulo de Rezende Pinatti 
  Pavel Hrdina 
  Peng Liang 
  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 
  Shalini Chellathurai Saroja 
  Shaojun Yang 
  Shi Lei 
  Simon Gaiser 
  Stefan Bader 
  Stefan Berger 
  Stefan Berger 
  Szymon Scholz 
  Thomas Huth 
  Tim Wiederhake 
  Tomáš Golembiovský 
  Tomáš Janoušek 
  Tuguoyi 
  Ville Skyttä 
  Wang Xin 
  Weblate 
  Yalei Li <274268...@qq.com>
  Yalei Li 
  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 

Re: [PATCH for-next 2/6] xen/common: Guard iommu symbols with CONFIG_HAS_PASSTHROUGH

[Jan Beulich]

On 26.02.2021 03:54, Connor Davis wrote:
> On Thu, Feb 25, 2021 at 04:45:15PM +0100, Jan Beulich wrote:
>> On 25.02.2021 16:24, Connor Davis wrote:
>>> --- a/xen/common/domain.c
>>> +++ b/xen/common/domain.c
>>> @@ -501,7 +501,9 @@ static int sanitise_domain_config(struct 
>>> xen_domctl_createdomain *config)
>>>  return -EINVAL;
>>>  }
>>>  
>>> +#ifdef CONFIG_HAS_PASSTHROUGH
>>>  if ( !iommu_enabled )
>>> +#endif
>>>  {
>>>  dprintk(XENLOG_INFO, "IOMMU requested but not available\n");
>>>  return -EINVAL;
>>
>> Where possible - to avoid such #ifdef-ary - the symbol instead should
>> get #define-d to a sensible value ("false" in this case) in the header.
>> The other cases here may indeed need to remain as you have them.
>>
> Do you prefer the #define in the same function near the if or
> somwhere near the top of the file?

Neither, if I understand you correctly. It should be in the same header
where the extern declaration lives, for the whole code base to consume.

Jan

Re: [PATCH for-4.15 5/5] tools/xenstored: Silence coverity when using xs_state_* structures

[Jürgen Groß]

On 25.02.21 18:41, Julien Grall wrote:

From: Julien Grall 

Coverity will report unitialized values for every use of xs_state_*
structures in the save part. This can be prevented by using the [0]
rather than [] to define variable length array.

Coverity-ID: 1472398
Coverity-ID: 1472397
Coverity-ID: 1472396
Coverity-ID: 1472395
Signed-off-by: Julien Grall 


Sorry, but Coverity is clearly wrong here.

Should we really modify our code to work around bugs in external
static code analyzers?


Juergen



---

 From my understanding, the tools and the hypervisor already rely on GNU
extensions. So the change should be fine.

If not, we can use the same approach as XEN_FLEX_ARRAY_DIM.
---
  tools/xenstore/include/xenstore_state.h | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/xenstore/include/xenstore_state.h 
b/tools/xenstore/include/xenstore_state.h
index ae0d053c8ffc..407d9e920c0f 100644
--- a/tools/xenstore/include/xenstore_state.h
+++ b/tools/xenstore/include/xenstore_state.h
@@ -86,7 +86,7 @@ struct xs_state_connection {
  uint16_t data_in_len;/* Number of unprocessed bytes read from conn. */
  uint16_t data_resp_len;  /* Size of partial response pending for conn. */
  uint32_t data_out_len;   /* Number of bytes not yet written to conn. */
-uint8_t  data[]; /* Pending data (read, written) + 0-7 pad bytes. 
*/
+uint8_t  data[0]; /* Pending data (read, written) + 0-7 pad bytes. 
*/
  };
  
  /* Watch: */

@@ -94,7 +94,7 @@ struct xs_state_watch {
  uint32_t conn_id;   /* Connection this watch is associated with. */
  uint16_t path_length;   /* Number of bytes of path watched (incl. 0). */
  uint16_t token_length;  /* Number of bytes of watch token (incl. 0). */
-uint8_t data[]; /* Path bytes, token bytes, 0-7 pad bytes. */
+uint8_t data[0];/* Path bytes, token bytes, 0-7 pad bytes. */
  };
  
  /* Transaction: */

@@ -125,7 +125,7 @@ struct xs_state_node {
  #define XS_STATE_NODE_TA_WRITTEN  0x0002
  uint16_t perm_n;/* Number of permissions (0 in TA: node deleted). 
*/
  /* Permissions (first is owner, has full access). */
-struct xs_state_node_perm perms[];
+struct xs_state_node_perm perms[0];
  /* Path and data follows, plus 0-7 pad bytes. */
  };
  #endif /* XENSTORE_STATE_H */





OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys


OpenPGP_signature
Description: OpenPGP digital signature

RE: [PATCH v3 1/2][4.15] VMX: delay p2m insertion of APIC access page

[Tian, Kevin]

> From: Jan Beulich 
> Sent: Thursday, February 25, 2021 4:44 PM
> 
> On 22.02.2021 11:56, Jan Beulich wrote:
> > --- a/xen/arch/x86/hvm/vmx/vmx.c
> > +++ b/xen/arch/x86/hvm/vmx/vmx.c
> > @@ -428,6 +428,14 @@ static void vmx_domain_relinquish_resour
> >  vmx_free_vlapic_mapping(d);
> >  }
> >
> > +static void domain_creation_finished(struct domain *d)
> > +{
> > +if ( has_vlapic(d) && !mfn_eq(d->arch.hvm.vmx.apic_access_mfn,
> _mfn(0)) &&
> > + set_mmio_p2m_entry(d, gaddr_to_gfn(APIC_DEFAULT_PHYS_BASE),
> > +d->arch.hvm.vmx.apic_access_mfn, 
> > PAGE_ORDER_4K) )
> > +domain_crash(d);
> > +}
> 
> Having noticed that in patch 2 I also need to arrange for
> ept_get_entry_emt() to continue to return WB for this page, I'm
> inclined to add a respective assertion here. Would anyone object
> to me doing so?
> 
> Kevin, Jun - I'd like this to also serve as a ping for an ack
> (with or without the suggested ASSERT() addition).
> 

Reviewed-by: Kevin Tian 

Re: [PATCH for-4.15 4/5] tools/xenstore-control: Don't leak buf in live_update_start()

[Jürgen Groß]

On 25.02.21 18:41, Julien Grall wrote:

From: Julien Grall 

All the error paths but one will free buf. Cover the remaining path so
buf can't be leaked.

This bug was discovered and resolved using Coverity Static Analysis
Security Testing (SAST) by Synopsys, Inc.

Fixes: 7f97193e6aa8 ("tools/xenstore: add live update command to 
xenstore-control")
Signed-off-by: Julien Grall 


Reviewed-by: Juergen Gross 


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH for-4.15 3/5] tools/xenstored: control: Store the save filename in lu_dump_state

[Jürgen Groß]

On 25.02.21 18:41, Julien Grall wrote:

From: Julien Grall 

The function lu_close_dump_state() will use talloc_asprintf() without
checking whether the allocation succeeded. In the unlikely case we are
out of memory, we would dereference a NULL pointer.

As we already computed the filename in lu_get_dump_state(), we can store
the name in the lu_dump_state. This is avoiding to deal with memory file
in the close path and also reduce the risk to use the different
filename.

This bug was discovered and resolved using Coverity Static Analysis
Security Testing (SAST) by Synopsys, Inc.

Fixes: c0dc6a3e7c41 ("tools/xenstore: read internal state when doing live 
upgrade")
Signed-off-by: Julien Grall 


Reviewed-by: Juergen Gross 


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH for-4.15 2/5] tools/xenstored: Avoid unnecessary talloc_strdup() in do_lu_start()

[Jürgen Groß]

On 25.02.21 18:41, Julien Grall wrote:

From: Julien Grall 

At the moment, the return of talloc_strdup() is not checked. This means
we may dereference a NULL pointer if the allocation failed.

However, it is pointless to allocate the memory as send_reply() will
copy the data to a different buffer. So drop the use of talloc_strdup().

This bug was discovered and resolved using Coverity Static Analysis
Security Testing (SAST) by Synopsys, Inc.

Fixes: af216a99fb4a ("tools/xenstore: add the basic framework for doing the live 
update")
Signed-off-by: Julien Grall 


Reviewed-by: Juergen Gross 


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH for-4.15 1/5] tools/xenstored: Avoid unnecessary talloc_strdup() in do_control_lu()

[Jürgen Groß]

On 25.02.21 18:41, Julien Grall wrote:

From: Julien Grall 

At the moment, the return of talloc_strdup() is not checked. This means
we may dereference a NULL pointer if the allocation failed.

However, it is pointless to allocate the memory as send_reply() will
copy the data to a different buffer. So drop the use of talloc_strdup().

This bug was discovered and resolved using Coverity Static Analysis
Security Testing (SAST) by Synopsys, Inc.

Fixes: fecab256d474 ("tools/xenstore: add basic live-update command parsing")
Signed-off-by: Julien Grall 


Reviewed-by: Juergen Gross 


Juergen



OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH for-next 6/6] automation: add container for riscv64 builds

[Connor Davis]

On Thu, Feb 25, 2021 at 04:31:13PM -0800, Stefano Stabellini wrote:
> On Thu, 25 Feb 2021, Connor Davis wrote:
> > Add a container for cross-compiling xen to riscv64.
> > This just includes the cross-compiler and necessary packages for
> > building xen itself (packages for tools, stubdoms, etc., can be
> > added later).
> > 
> > To build xen in the container run the following:
> > 
> > $ make XEN_TARGET_ARCH=riscv64 SUBSYSTEMS=xen
> 
> The container build failed for me with:
> 
> error: failed to initialize alpm library
> (could not find or read directory: /var/lib/pacman/)
> The command '/bin/sh -c pacman --noconfirm -Syu pixman python sh' 
> returned a non-zero code: 255
> 

Ooof, apparently archlinux/base has been broken for a
few weeks now due to interactions between faccessat2 and the
host's libseccomp version [0]. This thread [1] suggests the next
docker release will have a fix.

As a temporary workaround I got the attached patch to work
(based on this one[2]) if you want to give that a go.

Connor

[0] https://bugs.archlinux.org/task/69563
[1] https://github.com/actions/virtual-environments/issues/2658
[2] https://github.com/MiguelNdeCarvalho/docker-baseimage-archlinux/pull/8/files
diff --git a/automation/build/archlinux/riscv64.dockerfile b/automation/build/archlinux/riscv64.dockerfile
index d94048b6c3..5b3c3b9e3b 100644
--- a/automation/build/archlinux/riscv64.dockerfile
+++ b/automation/build/archlinux/riscv64.dockerfile
@@ -2,6 +2,11 @@ FROM archlinux/base
 LABEL maintainer.name="The Xen Project" \
   maintainer.email="xen-devel@lists.xenproject.org"
 
+RUN patched_glibc=glibc-linux4-2.33-4-x86_64.pkg.tar.zst && \
+curl -LO "https://repo.archlinuxcn.org/x86_64/$patched_glibc"; && \
+bsdtar -C / -xvf "$patched_glibc" && \
+sed -i 's/#IgnorePkg   =/IgnorePkg   = glibc/' /etc/pacman.conf
+
 # Packages needed for the build
 RUN pacman --noconfirm -Syu \
 base-devel \
@@ -26,6 +31,9 @@ RUN git clone --recursive -j$(nproc) --progress https://github.com/riscv/riscv-g
 
 # Add compiler path
 ENV PATH=/opt/riscv/bin/:${PATH}
+ENV CROSS_COMPILE=riscv64-unknown-linux-gnu-
+ENV XEN_TARGET_ARCH=riscv64
+ENV SUBSYSTEMS=xen
 
 RUN useradd --create-home user
 USER user

Re: [PATCH v4 12/14] swiotlb: Add restricted DMA alloc/free support.

[Christoph Hellwig]

On Fri, Feb 26, 2021 at 12:17:50PM +0800, Claire Chang wrote:
> Do you think I should fix this and rebase on the latest linux-next
> now? I wonder if there are more factor and clean up coming and I
> should wait after that.

Here is my preferred plan:

 1) wait for my series to support the min alignment in swiotlb to
land in Linus tree
 2) I'll resend my series with the further swiotlb cleanup and
refactoring, which includes a slightly rebased version of your
patch to add the io_tlb_mem structure
 3) resend your series on top of that as a baseline

This is my current WIP tree for 2:

  http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/swiotlb-struct

Re: [PATCH] virtio-gpu: Respect graphics update interval for EDID

[Akihiko Odaki]

2021年2月25日(木) 20:46 Gerd Hoffmann :
>
>   Hi,
>
> > > Because of the wasted frames I'd like this to be an option you can
> > > enable when needed.  For the majority of use cases this seems to be
> > > no problem ...
> >
> > I see blinks with GNOME on Wayland on Ubuntu 20.04 and virtio-gpu with
> > the EDID change included in this patch.
>
> /me looks closely at the patch again.
>
> So you update the edid dynamically, each time the refresh rate changes.
> Problem with that approach is software doesn't expect edid to change
> dynamically because on physical hardware it is static information about
> the connected monitor.
>
> So what the virtio-gpu guest driver does is emulate a monitor hotplug
> event to notify userspace.  If you resize the qemu window on the host
> it'll look like the monitor with the old window size was unplugged and
> a new monitor with the new window size got plugged instead, so gnome
> shell goes adapt the display resolution to the new virtual monitor size.
>
> The blink you are seeing probably comes from gnome-shell processing the
> monitor hotplug event.
>
> We could try to skip generating a monitor hotplug event in case only the
> refresh rate did change.  That would fix the blink, but it would also
> have the effect that nobody will notice the update.
>
> Bottom line:  I think making the edid refresh rate configurable might be
> useful, but changing it dynamically most likely isn't.
>
> take care,
>   Gerd
>

The "hotplug" implementation is probably what other combinations of
devices and guests will do if they want to respond to the changes of
the refresh rate, or display mode in general. That makes telling the
dynamic refresh rate to guests infeasible.

As you wrote, making the refresh rate configurable should be still
useful, and I think matching it to the backend physical display is
even better. GTK, the sole implementer of gfx_update_interval in my
patch reports the refresh rate of the physical display the window
resides in. It means the value may change when the physical display
changes its refresh rate, which should be rare if it does, or the
window moves to another physical display.

In the former case, there is nothing different from implementing a
physical display driver for guests so there should be no problem. The
latter case is similar to how the changes of the window size, which is
also part of display mode, is delivered to guests and they should be
consistent. The only inconsistency I see in my patch is that the
refresh rate change has no throttling while the window size change
has. I don't think it is problematic because it should be rare to move
the window across physical displays, but I can implement one if you
don't agree or know other cases the refresh rate frequently changes.

Regards,
Akihiko Odaki

Re: [PATCH v4 12/14] swiotlb: Add restricted DMA alloc/free support.

[Claire Chang]

> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index fd9c1bd183ac..8b77fd64199e 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -836,6 +836,40 @@ late_initcall(swiotlb_create_default_debugfs);
>  #endif
>
>  #ifdef CONFIG_DMA_RESTRICTED_POOL
> +struct page *dev_swiotlb_alloc(struct device *dev, size_t size, gfp_t gfp)
> +{
> +   struct swiotlb *swiotlb;
> +   phys_addr_t tlb_addr;
> +   unsigned int index;
> +
> +   /* dev_swiotlb_alloc can be used only in the context which permits 
> sleeping. */
> +   if (!dev->dev_swiotlb || !gfpflags_allow_blocking(gfp))

Just noticed that !gfpflags_allow_blocking(gfp) shouldn't be here.

Hi Christoph,

Do you think I should fix this and rebase on the latest linux-next
now? I wonder if there are more factor and clean up coming and I
should wait after that.

Thanks,
Claire

[xen-unstable test] 159671: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemut-ws16-amd64  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-xl-qemuu-debianhvm-amd64  8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-shadow 8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-qemut-rhel6hvm-intel  8 xen-boot fail REGR. vs. 159475
 test-xtf-amd64-amd64-3  19 xtf/test-pv32pae-selftest fail REGR. vs. 159475
 test-xtf-amd64-amd64-1  19 xtf/test-pv32pae-selftest fail REGR. vs. 159475
 test-amd64-i386-xl-qemut-debianhvm-amd64  8 xen-boot fail REGR. vs. 159475
 test-xtf-amd64-amd64-4  19 xtf/test-pv32pae-selftest fail REGR. vs. 159475
 test-xtf-amd64-amd64-2  19 xtf/test-pv32pae-selftest fail REGR. vs. 159475
 test-xtf-amd64-amd64-5  19 xtf/test-pv32pae-selftest fail REGR. vs. 159475
 test-amd64-i386-libvirt-xsm   8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-raw8 xen-boot fail REGR. vs. 159475
 test-amd64-coresched-i386-xl  8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-migrupgrade  13 xen-boot/dst_hostfail REGR. vs. 159475
 test-amd64-i386-xl-xsm8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-qemuu-ovmf-amd64  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 8 xen-boot fail REGR. vs. 
159475
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-qemut-rhel6hvm-amd  8 xen-boot   fail REGR. vs. 159475
 test-amd64-i386-xl8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-libvirt-pair 12 xen-boot/src_hostfail REGR. vs. 159475
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 8 xen-boot fail REGR. vs. 
159475
 test-amd64-i386-libvirt-pair 13 xen-boot/dst_hostfail REGR. vs. 159475
 test-amd64-i386-qemuu-rhel6hvm-amd  8 xen-boot   fail REGR. vs. 159475
 test-amd64-i386-xl-qemut-win7-amd64  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-freebsd10-amd64  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-freebsd10-i386  8 xen-boot   fail REGR. vs. 159475
 test-amd64-i386-examine   8 reboot   fail REGR. vs. 159475
 test-amd64-i386-libvirt   8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-livepatch 8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 8 xen-boot fail REGR. vs. 
159475
 test-amd64-i386-qemuu-rhel6hvm-intel  8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-qemuu-ws16-amd64  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-pair 12 xen-boot/src_hostfail REGR. vs. 159475
 test-amd64-i386-pair 13 xen-boot/dst_hostfail REGR. vs. 159475
 test-amd64-i386-xl-pvshim 8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 8 xen-boot fail REGR. vs. 
159475
 test-amd64-i386-xl-qemuu-win7-amd64  8 xen-boot  fail REGR. vs. 159475

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 159475
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 159475
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 159475
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 159475
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 159475
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 159475
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 159475
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  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-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
 te

Re: [PATCH for-next 4/6] xen: Fix build when !CONFIG_GRANT_TABLE

[Connor Davis]

On Thu, Feb 25, 2021 at 04:53:23PM +0100, Jan Beulich wrote:
> On 25.02.2021 16:24, Connor Davis wrote:
> > --- a/xen/include/xen/grant_table.h
> > +++ b/xen/include/xen/grant_table.h
> > @@ -66,6 +66,8 @@ int gnttab_acquire_resource(
> >  
> >  #define opt_max_grant_frames 0
> >  
> > +struct grant_table {};
> > +
> >  static inline int grant_table_init(struct domain *d,
> > int max_grant_frames,
> > int max_maptrack_frames)
> 
> You shouldn't actually declare a struct, all you need is to
> move the forward decl further up in the file ahead of the
> #ifdef.

Thanks, will fix this in v2.

Connor

Re: [PATCH for-next 5/6] xen: Add files needed for minimal riscv build

[Connor Davis]

On Thu, Feb 25, 2021 at 11:14:53PM +, Andrew Cooper wrote:
> On 25/02/2021 15:24, Connor Davis wrote:
> > Add the minimum code required to get xen to build with
> > XEN_TARGET_ARCH=riscv64. It is minimal in the sense that every file and
> > function added is required for a successful build, given the .config
> > generated from riscv64_defconfig. The function implementations are just
> > stubs; actual implmentations will need to be added later.
> >
> > Signed-off-by: Connor Davis 
> > ---
> >  config/riscv64.mk|   7 +
> >  xen/Makefile |   8 +-
> >  xen/arch/riscv/Kconfig   |  54 
> >  xen/arch/riscv/Kconfig.debug |   0
> >  xen/arch/riscv/Makefile  |  57 
> >  xen/arch/riscv/README.source |  19 ++
> >  xen/arch/riscv/Rules.mk  |  13 +
> >  xen/arch/riscv/arch.mk   |   7 +
> >  xen/arch/riscv/configs/riscv64_defconfig |  12 +
> >  xen/arch/riscv/delay.c   |  16 +
> >  xen/arch/riscv/domain.c  | 144 +
> >  xen/arch/riscv/domctl.c  |  36 +++
> >  xen/arch/riscv/guestcopy.c   |  57 
> >  xen/arch/riscv/head.S|   6 +
> >  xen/arch/riscv/irq.c |  78 +
> >  xen/arch/riscv/lib/Makefile  |   1 +
> >  xen/arch/riscv/lib/find_next_bit.c   | 284 +
> >  xen/arch/riscv/mm.c  |  93 ++
> >  xen/arch/riscv/p2m.c | 150 +
> >  xen/arch/riscv/percpu.c  |  17 +
> >  xen/arch/riscv/platforms/Kconfig |  31 ++
> >  xen/arch/riscv/riscv64/asm-offsets.c |  31 ++
> >  xen/arch/riscv/setup.c   |  27 ++
> >  xen/arch/riscv/shutdown.c|  28 ++
> >  xen/arch/riscv/smp.c |  35 +++
> >  xen/arch/riscv/smpboot.c |  34 ++
> >  xen/arch/riscv/sysctl.c  |  33 ++
> >  xen/arch/riscv/time.c|  35 +++
> >  xen/arch/riscv/traps.c   |  35 +++
> >  xen/arch/riscv/vm_event.c|  39 +++
> >  xen/arch/riscv/xen.lds.S | 113 +++
> >  xen/drivers/char/serial.c|   1 +
> >  xen/include/asm-riscv/altp2m.h   |  39 +++
> >  xen/include/asm-riscv/asm.h  |  77 +
> >  xen/include/asm-riscv/asm_defns.h|  24 ++
> >  xen/include/asm-riscv/atomic.h   | 204 
> >  xen/include/asm-riscv/bitops.h   | 331 
> >  xen/include/asm-riscv/bug.h  |  54 
> >  xen/include/asm-riscv/byteorder.h|  16 +
> >  xen/include/asm-riscv/cache.h|  24 ++
> >  xen/include/asm-riscv/cmpxchg.h  | 382 +++
> >  xen/include/asm-riscv/compiler_types.h   |  32 ++
> >  xen/include/asm-riscv/config.h   | 110 +++
> >  xen/include/asm-riscv/cpufeature.h   |  17 +
> >  xen/include/asm-riscv/csr.h  | 219 +
> >  xen/include/asm-riscv/current.h  |  47 +++
> >  xen/include/asm-riscv/debugger.h |  15 +
> >  xen/include/asm-riscv/delay.h|  15 +
> >  xen/include/asm-riscv/desc.h |  12 +
> >  xen/include/asm-riscv/device.h   |  15 +
> >  xen/include/asm-riscv/div64.h|  23 ++
> >  xen/include/asm-riscv/domain.h   |  50 +++
> >  xen/include/asm-riscv/event.h|  42 +++
> >  xen/include/asm-riscv/fence.h|  12 +
> >  xen/include/asm-riscv/flushtlb.h |  34 ++
> >  xen/include/asm-riscv/grant_table.h  |  12 +
> >  xen/include/asm-riscv/guest_access.h |  41 +++
> >  xen/include/asm-riscv/guest_atomics.h|  60 
> >  xen/include/asm-riscv/hardirq.h  |  27 ++
> >  xen/include/asm-riscv/hypercall.h|  12 +
> >  xen/include/asm-riscv/init.h |  42 +++
> >  xen/include/asm-riscv/io.h   | 283 +
> >  xen/include/asm-riscv/iocap.h|  13 +
> >  xen/include/asm-riscv/iommu.h|  46 +++
> >  xen/include/asm-riscv/irq.h  |  58 
> >  xen/include/asm-riscv/mem_access.h   |   4 +
> >  xen/include/asm-riscv/mm.h   | 246 +++
> >  xen/include/asm-riscv/monitor.h  |  65 
> >  xen/include/asm-riscv/nospec.h   |  25 ++
> >  xen/include/asm-riscv/numa.h |  41 +++
> >  xen/include/asm-riscv/p2m.h  | 218 +
> >  xen/include/asm-riscv/page-bits.h|  11 +
> >  xen/include/asm-riscv/page.h |  73 +
> >  xen/include/asm-riscv/paging.h   |  15 +
> >  xen/include/asm-riscv/pci.h  |  31 ++
> >  xen/include/asm-riscv/percpu.h   |  33 ++
> >  xen/include/asm-riscv/processor.h|  59 
> >  xen/include/asm-riscv/random.h   |   9 +
> >  xen/include/asm-riscv/regs.h |  23 ++
> >

Re: [PATCH for-next 3/6] xen/sched: Fix build when NR_CPUS == 1

[Connor Davis]

On Thu, Feb 25, 2021 at 04:50:02PM +0100, Jan Beulich wrote:
> On 25.02.2021 16:24, Connor Davis wrote:
> > Return from cpu_schedule_up when either cpu is 0 or
> > NR_CPUS == 1. This fixes the following:
> > 
> > core.c: In function 'cpu_schedule_up':
> > core.c:2769:19: error: array subscript 1 is above array bounds
> > of 'struct vcpu *[1]' [-Werror=array-bounds]
> >  2769 | if ( idle_vcpu[cpu] == NULL )
> >   |
> > 
> > Signed-off-by: Connor Davis 
> > ---
> >  xen/common/sched/core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
> > index 9745a77eee..f5ec65bf9b 100644
> > --- a/xen/common/sched/core.c
> > +++ b/xen/common/sched/core.c
> > @@ -2763,7 +2763,7 @@ static int cpu_schedule_up(unsigned int cpu)
> >  cpumask_set_cpu(cpu, &sched_res_mask);
> >  
> >  /* Boot CPU is dealt with later in scheduler_init(). */
> > -if ( cpu == 0 )
> > +if ( cpu == 0 || NR_CPUS == 1 )
> >  return 0;
> >  
> >  if ( idle_vcpu[cpu] == NULL )
> 
> I'm not convinced a compiler warning is due here, and in turn
> I'm not sure we want/need to work around this the way you do.

It seems like a reasonable warning to me, but of course I'm open
to dealing with it in a different way.

> First question is whether that's just a specific compiler
> version that's flawed. If it's not just a special case (e.g.

The docker container uses gcc 10.2.0 from
https://github.com/riscv/riscv-gnu-toolchain

> some unreleased version) we may want to think of possible
> alternatives - the addition looks really odd to me.
> 
> Jan

Connor

Re: [PATCH for-next 3/6] xen/sched: Fix build when NR_CPUS == 1

[Connor Davis]

On Thu, Feb 25, 2021 at 02:55:45PM -0800, Bob Eshleman wrote:
> On 2/25/21 7:24 AM, Connor Davis wrote:
> > Return from cpu_schedule_up when either cpu is 0 or
> > NR_CPUS == 1. This fixes the following:
> > 
> > core.c: In function 'cpu_schedule_up':
> > core.c:2769:19: error: array subscript 1 is above array bounds
> > of 'struct vcpu *[1]' [-Werror=array-bounds]
> >  2769 | if ( idle_vcpu[cpu] == NULL )
> >   |
> > 
> > Signed-off-by: Connor Davis 
> > ---
> >  xen/common/sched/core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
> > index 9745a77eee..f5ec65bf9b 100644
> > --- a/xen/common/sched/core.c
> > +++ b/xen/common/sched/core.c
> > @@ -2763,7 +2763,7 @@ static int cpu_schedule_up(unsigned int cpu)
> >  cpumask_set_cpu(cpu, &sched_res_mask);
> >  
> >  /* Boot CPU is dealt with later in scheduler_init(). */
> > -if ( cpu == 0 )
> > +if ( cpu == 0 || NR_CPUS == 1 )
> >  return 0;
> >  
> >  if ( idle_vcpu[cpu] == NULL )
> > 
> 
> Interesting.  I wonder when this changed in GCC.
> 
> I haven't yet seen this issue compiling with:
>   NR_CPUS=1
>   ARCH=riscv64
>   riscv64-unknown-linux-gnu-gcc (GCC) 10.1.0
> 
> Which version of GCC are you seeing emit this?

The one from cloned from github.com/riscv/riscv-gnu-toolchain
in the docker container uses 10.2.0

Connor

Re: [PATCH for-next 2/6] xen/common: Guard iommu symbols with CONFIG_HAS_PASSTHROUGH

[Connor Davis]

On Thu, Feb 25, 2021 at 04:45:15PM +0100, Jan Beulich wrote:
> On 25.02.2021 16:24, Connor Davis wrote:
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -501,7 +501,9 @@ static int sanitise_domain_config(struct 
> > xen_domctl_createdomain *config)
> >  return -EINVAL;
> >  }
> >  
> > +#ifdef CONFIG_HAS_PASSTHROUGH
> >  if ( !iommu_enabled )
> > +#endif
> >  {
> >  dprintk(XENLOG_INFO, "IOMMU requested but not available\n");
> >  return -EINVAL;
> 
> Where possible - to avoid such #ifdef-ary - the symbol instead should
> get #define-d to a sensible value ("false" in this case) in the header.
> The other cases here may indeed need to remain as you have them.
> 
Do you prefer the #define in the same function near the if or
somwhere near the top of the file?

Connor

Re: [PATCH for-next 5/6] xen: Add files needed for minimal riscv build

[Stefano Stabellini]

On Thu, 25 Feb 2021, Andrew Cooper wrote:
> On 25/02/2021 15:24, Connor Davis wrote:
> > Add the minimum code required to get xen to build with
> > XEN_TARGET_ARCH=riscv64. It is minimal in the sense that every file and
> > function added is required for a successful build, given the .config
> > generated from riscv64_defconfig. The function implementations are just
> > stubs; actual implmentations will need to be added later.
> >
> > Signed-off-by: Connor Davis 

This is awesome, Connor! I am glad you are continuing this work and
I am really looking forward to have it in the tree.


> > ---
> >  config/riscv64.mk|   7 +
> >  xen/Makefile |   8 +-
> >  xen/arch/riscv/Kconfig   |  54 
> >  xen/arch/riscv/Kconfig.debug |   0
> >  xen/arch/riscv/Makefile  |  57 
> >  xen/arch/riscv/README.source |  19 ++
> >  xen/arch/riscv/Rules.mk  |  13 +
> >  xen/arch/riscv/arch.mk   |   7 +
> >  xen/arch/riscv/configs/riscv64_defconfig |  12 +
> >  xen/arch/riscv/delay.c   |  16 +
> >  xen/arch/riscv/domain.c  | 144 +
> >  xen/arch/riscv/domctl.c  |  36 +++
> >  xen/arch/riscv/guestcopy.c   |  57 
> >  xen/arch/riscv/head.S|   6 +
> >  xen/arch/riscv/irq.c |  78 +
> >  xen/arch/riscv/lib/Makefile  |   1 +
> >  xen/arch/riscv/lib/find_next_bit.c   | 284 +
> >  xen/arch/riscv/mm.c  |  93 ++
> >  xen/arch/riscv/p2m.c | 150 +
> >  xen/arch/riscv/percpu.c  |  17 +
> >  xen/arch/riscv/platforms/Kconfig |  31 ++
> >  xen/arch/riscv/riscv64/asm-offsets.c |  31 ++
> >  xen/arch/riscv/setup.c   |  27 ++
> >  xen/arch/riscv/shutdown.c|  28 ++
> >  xen/arch/riscv/smp.c |  35 +++
> >  xen/arch/riscv/smpboot.c |  34 ++
> >  xen/arch/riscv/sysctl.c  |  33 ++
> >  xen/arch/riscv/time.c|  35 +++
> >  xen/arch/riscv/traps.c   |  35 +++
> >  xen/arch/riscv/vm_event.c|  39 +++
> >  xen/arch/riscv/xen.lds.S | 113 +++
> >  xen/drivers/char/serial.c|   1 +
> >  xen/include/asm-riscv/altp2m.h   |  39 +++
> >  xen/include/asm-riscv/asm.h  |  77 +
> >  xen/include/asm-riscv/asm_defns.h|  24 ++
> >  xen/include/asm-riscv/atomic.h   | 204 
> >  xen/include/asm-riscv/bitops.h   | 331 
> >  xen/include/asm-riscv/bug.h  |  54 
> >  xen/include/asm-riscv/byteorder.h|  16 +
> >  xen/include/asm-riscv/cache.h|  24 ++
> >  xen/include/asm-riscv/cmpxchg.h  | 382 +++
> >  xen/include/asm-riscv/compiler_types.h   |  32 ++
> >  xen/include/asm-riscv/config.h   | 110 +++
> >  xen/include/asm-riscv/cpufeature.h   |  17 +
> >  xen/include/asm-riscv/csr.h  | 219 +
> >  xen/include/asm-riscv/current.h  |  47 +++
> >  xen/include/asm-riscv/debugger.h |  15 +
> >  xen/include/asm-riscv/delay.h|  15 +
> >  xen/include/asm-riscv/desc.h |  12 +
> >  xen/include/asm-riscv/device.h   |  15 +
> >  xen/include/asm-riscv/div64.h|  23 ++
> >  xen/include/asm-riscv/domain.h   |  50 +++
> >  xen/include/asm-riscv/event.h|  42 +++
> >  xen/include/asm-riscv/fence.h|  12 +
> >  xen/include/asm-riscv/flushtlb.h |  34 ++
> >  xen/include/asm-riscv/grant_table.h  |  12 +
> >  xen/include/asm-riscv/guest_access.h |  41 +++
> >  xen/include/asm-riscv/guest_atomics.h|  60 
> >  xen/include/asm-riscv/hardirq.h  |  27 ++
> >  xen/include/asm-riscv/hypercall.h|  12 +
> >  xen/include/asm-riscv/init.h |  42 +++
> >  xen/include/asm-riscv/io.h   | 283 +
> >  xen/include/asm-riscv/iocap.h|  13 +
> >  xen/include/asm-riscv/iommu.h|  46 +++
> >  xen/include/asm-riscv/irq.h  |  58 
> >  xen/include/asm-riscv/mem_access.h   |   4 +
> >  xen/include/asm-riscv/mm.h   | 246 +++
> >  xen/include/asm-riscv/monitor.h  |  65 
> >  xen/include/asm-riscv/nospec.h   |  25 ++
> >  xen/include/asm-riscv/numa.h |  41 +++
> >  xen/include/asm-riscv/p2m.h  | 218 +
> >  xen/include/asm-riscv/page-bits.h|  11 +
> >  xen/include/asm-riscv/page.h |  73 +
> >  xen/include/asm-riscv/paging.h   |  15 +
> >  xen/include/asm-riscv/pci.h  |  31 ++
> >  xen/include/asm-riscv/percpu.h   |  33 ++
> >  xen/include/asm-riscv/processor.h|  59 
> >  xen/i

Re: [PATCH for-next 6/6] automation: add container for riscv64 builds

[Stefano Stabellini]

On Thu, 25 Feb 2021, Connor Davis wrote:
> Add a container for cross-compiling xen to riscv64.
> This just includes the cross-compiler and necessary packages for
> building xen itself (packages for tools, stubdoms, etc., can be
> added later).
> 
> To build xen in the container run the following:
> 
> $ make XEN_TARGET_ARCH=riscv64 SUBSYSTEMS=xen
> 
> Signed-off-by: Connor Davis 

The container build failed for me with:

Creating user git (git daemon user) with uid 977 and gid 977.
:: Running post-transaction hooks...
( 1/13) Creating system user accounts...
( 2/13) Updating journal message catalog...
( 3/13) Reloading system manager configuration...
  Skipped: Current root is not booted.
( 4/13) Updating udev hardware database...
( 5/13) Applying kernel sysctl settings...
  Skipped: Current root is not booted.
( 6/13) Creating temporary files...
/usr/lib/tmpfiles.d/journal-nocow.conf:26: Failed to resolve specifier: 
uninitialized /etc detected, skipping
All rules containing unresolvable specifiers will be skipped.
( 7/13) Reloading device manager configuration...
  Skipped: Device manager is not running.
( 8/13) Arming ConditionNeedsUpdate...
( 9/13) Rebuilding certificate stores...
(10/13) Reloading system bus configuration...
  Skipped: Current root is not booted.
(11/13) Warn about old perl modules
(12/13) Cleaning up package cache...
(13/13) Updating the info directory file...
Removing intermediate container 81e02adffada
 ---> 575bfaafc6af
Step 4/9 : RUN pacman --noconfirm -Syu pixman python sh
 ---> Running in 9010bd7932b5
error: failed to initialize alpm library
(could not find or read directory: /var/lib/pacman/)
The command '/bin/sh -c pacman --noconfirm -Syu pixman python sh' 
returned a non-zero code: 255


> ---
>  automation/build/archlinux/riscv64.dockerfile | 32 +++
>  automation/scripts/containerize   |  1 +
>  2 files changed, 33 insertions(+)
>  create mode 100644 automation/build/archlinux/riscv64.dockerfile
> 
> diff --git a/automation/build/archlinux/riscv64.dockerfile 
> b/automation/build/archlinux/riscv64.dockerfile
> new file mode 100644
> index 00..d94048b6c3
> --- /dev/null
> +++ b/automation/build/archlinux/riscv64.dockerfile
> @@ -0,0 +1,32 @@
> +FROM archlinux/base
> +LABEL maintainer.name="The Xen Project" \
> +  maintainer.email="xen-devel@lists.xenproject.org"
> +
> +# Packages needed for the build
> +RUN pacman --noconfirm -Syu \
> +base-devel \
> +gcc \
> +git
> +
> +# Packages needed for QEMU
> +RUN pacman --noconfirm -Syu \
> +pixman \
> +python \
> +sh
> +
> +# There is a regression in GDB that causes an assertion error
> +# when setting breakpoints, use this commit until it is fixed!
> +RUN git clone --recursive -j$(nproc) --progress 
> https://github.com/riscv/riscv-gnu-toolchain && \
> +cd riscv-gnu-toolchain/riscv-gdb && \
> +git checkout 1dd588507782591478882a891f64945af9e2b86c && \
> +cd  .. && \
> +./configure --prefix=/opt/riscv && \
> +make linux -j$(nproc) && \
> +rm -R /riscv-gnu-toolchain
> +
> +# Add compiler path
> +ENV PATH=/opt/riscv/bin/:${PATH}
> +
> +RUN useradd --create-home user
> +USER user
> +WORKDIR /build
> diff --git a/automation/scripts/containerize b/automation/scripts/containerize
> index da45baed4e..1901e8c0ef 100755
> --- a/automation/scripts/containerize
> +++ b/automation/scripts/containerize
> @@ -25,6 +25,7 @@ die() {
>  BASE="registry.gitlab.com/xen-project/xen"
>  case "_${CONTAINER}" in
>  _archlinux|_arch) CONTAINER="${BASE}/archlinux:current" ;;
> +_riscv64) CONTAINER="${BASE}/archlinux:riscv64" ;;
>  _centos7) CONTAINER="${BASE}/centos:7" ;;
>  _centos72) CONTAINER="${BASE}/centos:7.2" ;;
>  _fedora) CONTAINER="${BASE}/fedora:29";;
> -- 
> 2.27.0
> 
> 

[PATCH] xen/arm: introduce XENFEAT_direct_mapped and XENFEAT_not_direct_mapped

[Stefano Stabellini]

Newer Xen versions expose two Xen feature flags to tell us if the domain
is directly mapped or not. Only when a domain is directly mapped it
makes sense to enable swiotlb-xen on ARM.

Introduce a function on ARM to check the new Xen feature flags and also
to deal with the legacy case. Call the function xen_swiotlb_detect.

Also rename the existing pci_xen_swiotlb_detect on x86 to
xen_swiotlb_detect so that we can share a common function declaration.

Signed-off-by: Stefano Stabellini 
---

This is the corresponding Xen patch under review:
https://marc.info/?l=xen-devel&m=161421618217686

We don't *have to* make the x86 function and the ARM function exactly
the same, but I thought it would be much nicer if we did. However, we
can't really call it pci_* on ARM as there is no PCI necessarily.

---
 arch/arm/xen/mm.c  | 14 +-
 arch/arm64/mm/dma-mapping.c|  2 +-
 arch/x86/include/asm/xen/swiotlb-xen.h |  4 ++--
 arch/x86/kernel/pci-swiotlb.c  |  2 +-
 arch/x86/xen/pci-swiotlb-xen.c |  6 +++---
 include/xen/interface/features.h   |  7 +++
 include/xen/swiotlb-xen.h  |  6 ++
 7 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index 467fa225c3d0..f8e5acbef05d 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -135,10 +135,22 @@ void xen_destroy_contiguous_region(phys_addr_t pstart, 
unsigned int order)
return;
 }
 
+int __init xen_swiotlb_detect(void)
+{
+   if (!xen_domain())
+   return 0;
+   if (xen_feature(XENFEAT_direct_mapped))
+   return 1;
+   /* legacy case */
+   if (!xen_feature(XENFEAT_not_direct_mapped) && xen_initial_domain())
+   return 1;
+   return 0;
+}
+
 static int __init xen_mm_init(void)
 {
struct gnttab_cache_flush cflush;
-   if (!xen_initial_domain())
+   if (!xen_swiotlb_detect())
return 0;
xen_swiotlb_init(1, false);
 
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 93e87b287556..4bf1dd3eb041 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -53,7 +53,7 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 
size,
iommu_setup_dma_ops(dev, dma_base, size);
 
 #ifdef CONFIG_XEN
-   if (xen_initial_domain())
+   if (xen_swiotlb_detect())
dev->dma_ops = &xen_swiotlb_dma_ops;
 #endif
 }
diff --git a/arch/x86/include/asm/xen/swiotlb-xen.h 
b/arch/x86/include/asm/xen/swiotlb-xen.h
index 6b56d0d45d15..494694744844 100644
--- a/arch/x86/include/asm/xen/swiotlb-xen.h
+++ b/arch/x86/include/asm/xen/swiotlb-xen.h
@@ -2,14 +2,14 @@
 #ifndef _ASM_X86_SWIOTLB_XEN_H
 #define _ASM_X86_SWIOTLB_XEN_H
 
+#include 
+
 #ifdef CONFIG_SWIOTLB_XEN
 extern int xen_swiotlb;
-extern int __init pci_xen_swiotlb_detect(void);
 extern void __init pci_xen_swiotlb_init(void);
 extern int pci_xen_swiotlb_init_late(void);
 #else
 #define xen_swiotlb (0)
-static inline int __init pci_xen_swiotlb_detect(void) { return 0; }
 static inline void __init pci_xen_swiotlb_init(void) { }
 static inline int pci_xen_swiotlb_init_late(void) { return -ENXIO; }
 #endif
diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
index c2cfa5e7c152..c18eb6629326 100644
--- a/arch/x86/kernel/pci-swiotlb.c
+++ b/arch/x86/kernel/pci-swiotlb.c
@@ -30,7 +30,7 @@ int __init pci_swiotlb_detect_override(void)
return swiotlb;
 }
 IOMMU_INIT_FINISH(pci_swiotlb_detect_override,
- pci_xen_swiotlb_detect,
+ xen_swiotlb_detect,
  pci_swiotlb_init,
  pci_swiotlb_late_init);
 
diff --git a/arch/x86/xen/pci-swiotlb-xen.c b/arch/x86/xen/pci-swiotlb-xen.c
index 19ae3e4fe4e9..0a35657eeb85 100644
--- a/arch/x86/xen/pci-swiotlb-xen.c
+++ b/arch/x86/xen/pci-swiotlb-xen.c
@@ -21,12 +21,12 @@
 int xen_swiotlb __read_mostly;
 
 /*
- * pci_xen_swiotlb_detect - set xen_swiotlb to 1 if necessary
+ * xen_swiotlb_detect - set xen_swiotlb to 1 if necessary
  *
  * This returns non-zero if we are forced to use xen_swiotlb (by the boot
  * option).
  */
-int __init pci_xen_swiotlb_detect(void)
+int __init xen_swiotlb_detect(void)
 {
 
if (!xen_pv_domain())
@@ -90,7 +90,7 @@ int pci_xen_swiotlb_init_late(void)
 }
 EXPORT_SYMBOL_GPL(pci_xen_swiotlb_init_late);
 
-IOMMU_INIT_FINISH(pci_xen_swiotlb_detect,
+IOMMU_INIT_FINISH(xen_swiotlb_detect,
  NULL,
  pci_xen_swiotlb_init,
  NULL);
diff --git a/include/xen/interface/features.h b/include/xen/interface/features.h
index 6d1384abfbdf..f0d00bb0ac63 100644
--- a/include/xen/interface/features.h
+++ b/include/xen/interface/features.h
@@ -83,6 +83,13 @@
  */
 #define XENFEAT_linux_rsdp_unrestricted   15
 
+/*
+ * A direct-mapped (or 1:1 mapped) domain is a domain for which its
+ * local pages have gfn == mfn.
+ */
+#define XENFEAT_not_direct_map

[xen-unstable bisection] complete test-xtf-amd64-amd64-4

[osstest service owner]

branch xen-unstable
xenbranch xen-unstable
job test-xtf-amd64-amd64-4
testid xtf/test-pv32pae-selftest

Tree: linux git://xenbits.xen.org/linux-pvops.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git
Tree: xtf git://xenbits.xen.org/xtf.git

*** Found and reproduced problem changeset ***

  Bug is in tree:  xen git://xenbits.xen.org/xen.git
  Bug introduced:  4dc1815991420b809ce18dddfdf9c0af48944204
  Bug not present: 2d824791504f4119f04f95bafffec2e37d319c25
  Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/159687/


  commit 4dc1815991420b809ce18dddfdf9c0af48944204
  Author: Jan Beulich 
  Date:   Fri Feb 19 17:19:56 2021 +0100
  
  x86/PV: harden guest memory accesses against speculative abuse
  
  Inspired by
  
https://lore.kernel.org/lkml/f12e7d3cecf41b2c29734ea45a393be21d4a8058.1597848273.git.jpoim...@redhat.com/
  and prior work in that area of x86 Linux, suppress speculation with
  guest specified pointer values by suitably masking the addresses to
  non-canonical space in case they fall into Xen's virtual address range.
  
  Introduce a new Kconfig control.
  
  Note that it is necessary in such code to avoid using "m" kind operands:
  If we didn't, there would be no guarantee that the register passed to
  guest_access_mask_ptr is also the (base) one used for the memory access.
  
  As a minor unrelated change in get_unsafe_asm() the unnecessary "itype"
  parameter gets dropped and the XOR on the fixup path gets changed to be
  a 32-bit one in all cases: This way we avoid pointless REX.W or operand
  size overrides, or writes to partial registers.
  
  Requested-by: Andrew Cooper 
  Signed-off-by: Jan Beulich 
  Reviewed-by: Roger Pau Monné 
  Release-Acked-by: Ian Jackson 


For bisection revision-tuple graph see:
   
http://logs.test-lab.xenproject.org/osstest/results/bisect/xen-unstable/test-xtf-amd64-amd64-4.xtf--test-pv32pae-selftest.html
Revision IDs in each graph node refer, respectively, to the Trees above.


Running cs-bisection-step 
--graph-out=/home/logs/results/bisect/xen-unstable/test-xtf-amd64-amd64-4.xtf--test-pv32pae-selftest
 --summary-out=tmp/159687.bisection-summary --basis-template=159475 
--blessings=real,real-bisect,real-retry xen-unstable test-xtf-amd64-amd64-4 
xtf/test-pv32pae-selftest
Searching for failure / basis pass:
 159652 fail [host=godello0] / 159475 ok.
Failure / basis pass flights: 159652 / 159475
(tree with no url: minios)
(tree with no url: ovmf)
(tree with no url: seabios)
Tree: linux git://xenbits.xen.org/linux-pvops.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git
Tree: xtf git://xenbits.xen.org/xtf.git
Latest c3038e718a19fc596f7b1baba0f83d5146dc7784 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
60390ccb8b9b2dbf85010f8b47779bb231aa2533 
8ab15139728a8efd3ebbb60beb16a958a6a93fa1
Basis pass c3038e718a19fc596f7b1baba0f83d5146dc7784 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
e8185c5f01c68f7d29d23a4a91bc1be1ff2cc1ca 
8ab15139728a8efd3ebbb60beb16a958a6a93fa1
Generating revisions with ./adhoc-revtuple-generator  
git://xenbits.xen.org/linux-pvops.git#c3038e718a19fc596f7b1baba0f83d5146dc7784-c3038e718a19fc596f7b1baba0f83d5146dc7784
 
git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860
 
git://xenbits.xen.org/qemu-xen-traditional.git#3d273dd05e51e5a1ffba3d98c7437ee84e8f8764-3d273dd05e51e5a1ffba3d98c7437ee84e8f8764
 git://xenbits.xen.org/qemu-xen.git#7ea428895af2840d85c524f0bd11a38\
 aac308308-7ea428895af2840d85c524f0bd11a38aac308308 
git://xenbits.xen.org/xen.git#e8185c5f01c68f7d29d23a4a91bc1be1ff2cc1ca-60390ccb8b9b2dbf85010f8b47779bb231aa2533
 
git://xenbits.xen.org/xtf.git#8ab15139728a8efd3ebbb60beb16a958a6a93fa1-8ab15139728a8efd3ebbb60beb16a958a6a93fa1
Loaded 5001 nodes in revision graph
Searching for test results:
 159475 pass c3038e718a19fc596f7b1baba0f83d5146dc7784 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
e8185c5f01c68f7d29d23a4a91bc1be1ff2cc1ca 
8ab15139728a8efd3ebbb60beb16a958a6a93fa1
 159487 [host=godello1]
 159491 [host=albana0]
 159508 [host=albana1]
 159526 [host=huxelrebe1]
 159540 [host=elbling0]
 159559 [host=fiano1]
 159576 [host=chardonnay0]
 159602 [host=elbling1]
 159626 [host=chardonnay1]
 159669 [host=chardonnay1]
 159652 fail c3038e718a19fc596f

Re: [PATCH v1] xen: ACPI: Get rid of ACPICA message printing

[Boris Ostrovsky]

On 2/24/21 1:47 PM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki 
>
> The ACPI_DEBUG_PRINT() macro is used in a few places in
> xen-acpi-cpuhotplug.c and xen-acpi-memhotplug.c for printing debug
> messages, but that is questionable, because that macro belongs to
> ACPICA and it should not be used elsewhere.  In addition,
> ACPI_DEBUG_PRINT() requires special enabling to allow it to actually
> print the message and the _COMPONENT symbol generally needed for
> that is not defined in any of the files in question.
>
> For this reason, replace all of the ACPI_DEBUG_PRINT() instances in
> the Xen code with acpi_handle_debug() (with the additional benefit
> that the source object can be identified more easily after this
> change) and drop the ACPI_MODULE_NAME() definitions that are only
> used by the ACPICA message printing macros from that code.
>
> Signed-off-by: Rafael J. Wysocki 


Reviewed-by: Boris Ostrovsky 

Re: [PATCH for-next 5/6] xen: Add files needed for minimal riscv build

[Andrew Cooper]

On 25/02/2021 15:24, Connor Davis wrote:
> Add the minimum code required to get xen to build with
> XEN_TARGET_ARCH=riscv64. It is minimal in the sense that every file and
> function added is required for a successful build, given the .config
> generated from riscv64_defconfig. The function implementations are just
> stubs; actual implmentations will need to be added later.
>
> Signed-off-by: Connor Davis 
> ---
>  config/riscv64.mk|   7 +
>  xen/Makefile |   8 +-
>  xen/arch/riscv/Kconfig   |  54 
>  xen/arch/riscv/Kconfig.debug |   0
>  xen/arch/riscv/Makefile  |  57 
>  xen/arch/riscv/README.source |  19 ++
>  xen/arch/riscv/Rules.mk  |  13 +
>  xen/arch/riscv/arch.mk   |   7 +
>  xen/arch/riscv/configs/riscv64_defconfig |  12 +
>  xen/arch/riscv/delay.c   |  16 +
>  xen/arch/riscv/domain.c  | 144 +
>  xen/arch/riscv/domctl.c  |  36 +++
>  xen/arch/riscv/guestcopy.c   |  57 
>  xen/arch/riscv/head.S|   6 +
>  xen/arch/riscv/irq.c |  78 +
>  xen/arch/riscv/lib/Makefile  |   1 +
>  xen/arch/riscv/lib/find_next_bit.c   | 284 +
>  xen/arch/riscv/mm.c  |  93 ++
>  xen/arch/riscv/p2m.c | 150 +
>  xen/arch/riscv/percpu.c  |  17 +
>  xen/arch/riscv/platforms/Kconfig |  31 ++
>  xen/arch/riscv/riscv64/asm-offsets.c |  31 ++
>  xen/arch/riscv/setup.c   |  27 ++
>  xen/arch/riscv/shutdown.c|  28 ++
>  xen/arch/riscv/smp.c |  35 +++
>  xen/arch/riscv/smpboot.c |  34 ++
>  xen/arch/riscv/sysctl.c  |  33 ++
>  xen/arch/riscv/time.c|  35 +++
>  xen/arch/riscv/traps.c   |  35 +++
>  xen/arch/riscv/vm_event.c|  39 +++
>  xen/arch/riscv/xen.lds.S | 113 +++
>  xen/drivers/char/serial.c|   1 +
>  xen/include/asm-riscv/altp2m.h   |  39 +++
>  xen/include/asm-riscv/asm.h  |  77 +
>  xen/include/asm-riscv/asm_defns.h|  24 ++
>  xen/include/asm-riscv/atomic.h   | 204 
>  xen/include/asm-riscv/bitops.h   | 331 
>  xen/include/asm-riscv/bug.h  |  54 
>  xen/include/asm-riscv/byteorder.h|  16 +
>  xen/include/asm-riscv/cache.h|  24 ++
>  xen/include/asm-riscv/cmpxchg.h  | 382 +++
>  xen/include/asm-riscv/compiler_types.h   |  32 ++
>  xen/include/asm-riscv/config.h   | 110 +++
>  xen/include/asm-riscv/cpufeature.h   |  17 +
>  xen/include/asm-riscv/csr.h  | 219 +
>  xen/include/asm-riscv/current.h  |  47 +++
>  xen/include/asm-riscv/debugger.h |  15 +
>  xen/include/asm-riscv/delay.h|  15 +
>  xen/include/asm-riscv/desc.h |  12 +
>  xen/include/asm-riscv/device.h   |  15 +
>  xen/include/asm-riscv/div64.h|  23 ++
>  xen/include/asm-riscv/domain.h   |  50 +++
>  xen/include/asm-riscv/event.h|  42 +++
>  xen/include/asm-riscv/fence.h|  12 +
>  xen/include/asm-riscv/flushtlb.h |  34 ++
>  xen/include/asm-riscv/grant_table.h  |  12 +
>  xen/include/asm-riscv/guest_access.h |  41 +++
>  xen/include/asm-riscv/guest_atomics.h|  60 
>  xen/include/asm-riscv/hardirq.h  |  27 ++
>  xen/include/asm-riscv/hypercall.h|  12 +
>  xen/include/asm-riscv/init.h |  42 +++
>  xen/include/asm-riscv/io.h   | 283 +
>  xen/include/asm-riscv/iocap.h|  13 +
>  xen/include/asm-riscv/iommu.h|  46 +++
>  xen/include/asm-riscv/irq.h  |  58 
>  xen/include/asm-riscv/mem_access.h   |   4 +
>  xen/include/asm-riscv/mm.h   | 246 +++
>  xen/include/asm-riscv/monitor.h  |  65 
>  xen/include/asm-riscv/nospec.h   |  25 ++
>  xen/include/asm-riscv/numa.h |  41 +++
>  xen/include/asm-riscv/p2m.h  | 218 +
>  xen/include/asm-riscv/page-bits.h|  11 +
>  xen/include/asm-riscv/page.h |  73 +
>  xen/include/asm-riscv/paging.h   |  15 +
>  xen/include/asm-riscv/pci.h  |  31 ++
>  xen/include/asm-riscv/percpu.h   |  33 ++
>  xen/include/asm-riscv/processor.h|  59 
>  xen/include/asm-riscv/random.h   |   9 +
>  xen/include/asm-riscv/regs.h |  23 ++
>  xen/include/asm-riscv/setup.h|  14 +
>  xen/include/asm-riscv/smp.h  |  46 +++
>  xen/include/asm-riscv/softirq.h  |  16 +
>  xen/include/asm-riscv/spinlock.h |  12 +
>  xen/include/asm-riscv/string.

Re: [PATCH for-next 3/6] xen/sched: Fix build when NR_CPUS == 1

[Bob Eshleman]

On 2/25/21 7:24 AM, Connor Davis wrote:
> Return from cpu_schedule_up when either cpu is 0 or
> NR_CPUS == 1. This fixes the following:
> 
> core.c: In function 'cpu_schedule_up':
> core.c:2769:19: error: array subscript 1 is above array bounds
> of 'struct vcpu *[1]' [-Werror=array-bounds]
>  2769 | if ( idle_vcpu[cpu] == NULL )
>   |
> 
> Signed-off-by: Connor Davis 
> ---
>  xen/common/sched/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
> index 9745a77eee..f5ec65bf9b 100644
> --- a/xen/common/sched/core.c
> +++ b/xen/common/sched/core.c
> @@ -2763,7 +2763,7 @@ static int cpu_schedule_up(unsigned int cpu)
>  cpumask_set_cpu(cpu, &sched_res_mask);
>  
>  /* Boot CPU is dealt with later in scheduler_init(). */
> -if ( cpu == 0 )
> +if ( cpu == 0 || NR_CPUS == 1 )
>  return 0;
>  
>  if ( idle_vcpu[cpu] == NULL )
> 

Interesting.  I wonder when this changed in GCC.

I haven't yet seen this issue compiling with:
  NR_CPUS=1
  ARCH=riscv64
  riscv64-unknown-linux-gnu-gcc (GCC) 10.1.0

Which version of GCC are you seeing emit this?

- Bob

Re: [PATCH for-4.15] xen/sched: Add missing memory barrier in vcpu_block()

[Stefano Stabellini]

On Thu, 25 Feb 2021, Julien Grall wrote:
> On 23/02/2021 13:24, Ash Wilding wrote:
> > Hi Julien,
> 
> Hi Ash,
> 
> > Thanks for looking at this,
> > 
> > > vcpu_block() is now gaining an smp_mb__after_atomic() to prevent the
> > > CPU to read any information about local events before the flag
> > > _VPF_blocked is set.
> > 
> > Reviewed-by: Ash Wilding 
> 
> Thanks!
> 
> > 
> > 
> > As an aside,
> > 
> > > I couldn't convince myself whether the Arm implementation of
> > > local_events_need_delivery() contains enough barrier to prevent the
> > > re-ordering. However, I don't think we want to play with devil here
> > > as the function may be optimized in the future.
> > 
> > Agreed.
> > 
> > The vgic_vcpu_pending_irq() and vgic_evtchn_irq_pending() in the call
> > path of local_events_need_delivery() both call spin_lock_irqsave(),
> > which has an arch_lock_acquire_barrier() in its call path.
> > 
> > That just happens to map to a heavier smp_mb() on Arm right now, but
> > relying on this behaviour would be shaky; I can imagine a future update
> > to arch_lock_acquire_barrier() that relaxes it down to just acquire
> > semantics like its name implies (for example an LSE-based lock_acquire()
> > using LDUMAXA),in which case any code incorrectly relying on that full
> > barrier behaviour may break. I'm guessing this is what you meant by the
> > function may be optimized in future?
> 
> That's one of the optimization I had in mind. The other one is we may find a
> way to remove the spinlocks, so the barriers would disappear completely.
> 
> > 
> > Do we know whether there is an expectation for previous loads/stores
> > to have been observed before local_events_need_delivery()? I'm wondering
> > whether it would make sense to have an smb_mb() at the start of the
> > *_nomask() variant in local_events_need_delivery()'s call path.
> 
> That's a good question :). For Arm, there are 4 users of
> local_events_need_delivery():
>   1) do_poll()
>   2) vcpu_block()
>   3) hypercall_preempt_check()
>   4) general_preempt_check()
> 
> 3 and 4 are used for breaking down long running operations. I guess we would
> want to have an accurate view of the pending events and therefore we would
> need a memory barrier to prevent the loads happening too early.
> 
> In this case, I think the smp_mb() would want to be part of the
> hypercall_preempt_check() and general_preempt_check().
> 
> Therefore, I think we want to avoid the extra barrier in
> local_events_need_delivery(). Instead, we should require the caller to take
> care of the ordering if needed.
> 
> This would have benefits any new architecture as the common code would already
> contain the appropriate barriers.
> 
> @Stefano, what do you think?

I am thinking the same way as you. Also because it is cleaner if it is
the one who writes that also takes care of any barriers/flushes needed.

In this case it is vcpu_block that is writing _VPF_blocked and knows
that it has to be seen before local_events_need_delivery(). It is easier
to keep track of it if the barrier is in vcpu_block together with the
set_bit call.

Re: [PATCH for-4.15] xen/vgic: Implement write to ISPENDR in vGICv{2, 3}

[Stefano Stabellini]

On Thu, 25 Feb 2021, Julien Grall wrote:
> On 22/02/2021 13:45, Bertrand Marquis wrote:
> > Hi Julien,
> > 
> > > On 20 Feb 2021, at 14:04, Julien Grall  wrote:
> > > 
> > > From: Julien Grall 
> > > 
> > > Currently, Xen will send a data abort to a guest trying to write to the
> > > ISPENDR.
> > > 
> > > Unfortunately, recent version of Linux (at least 5.9+) will start
> > > writing to the register if the interrupt needs to be re-triggered
> > > (see the callback irq_retrigger). This can happen when a driver (such as
> > > the xgbe network driver on AMD Seattle) re-enable an interrupt:
> > > 
> > > (XEN) d0v0: vGICD: unhandled word write 0x000400 to ISPENDR44
> > > [...]
> > > [   25.635837] Unhandled fault at 0x80001000522c
> > > [...]
> > > [   25.818716]  gic_retrigger+0x2c/0x38
> > > [   25.822361]  irq_startup+0x78/0x138
> > > [   25.825920]  __enable_irq+0x70/0x80
> > > [   25.829478]  enable_irq+0x50/0xa0
> > > [   25.832864]  xgbe_one_poll+0xc8/0xd8
> > > [   25.836509]  net_rx_action+0x110/0x3a8
> > > [   25.840328]  __do_softirq+0x124/0x288
> > > [   25.844061]  irq_exit+0xe0/0xf0
> > > [   25.847272]  __handle_domain_irq+0x68/0xc0
> > > [   25.851442]  gic_handle_irq+0xa8/0xe0
> > > [   25.855171]  el1_irq+0xb0/0x180
> > > [   25.858383]  arch_cpu_idle+0x18/0x28
> > > [   25.862028]  default_idle_call+0x24/0x5c
> > > [   25.866021]  do_idle+0x204/0x278
> > > [   25.869319]  cpu_startup_entry+0x24/0x68
> > > [   25.873313]  rest_init+0xd4/0xe4
> > > [   25.876611]  arch_call_rest_init+0x10/0x1c
> > > [   25.880777]  start_kernel+0x5b8/0x5ec
> > > 
> > > As a consequence, the OS may become unusable.
> > > 
> > > Implementing the write part of ISPENDR is somewhat easy. For
> > > virtual interrupt, we only need to inject the interrupt again.
> > > 
> > > For physical interrupt, we need to be more careful as the de-activation
> > > of the virtual interrupt will be propagated to the physical distributor.
> > > For simplicity, the physical interrupt will be set pending so the
> > > workflow will not differ from a "real" interrupt.
> > > 
> > > Longer term, we could possible directly activate the physical interrupt
> > > and avoid taking an exception to inject the interrupt to the domain.
> > > (This is the approach taken by the new vGIC based on KVM).
> > > 
> > > Signed-off-by: Julien Grall 
> > 
> > This is something which will not be done by a guest very often so I think
> > your
> > implementation actually makes it simpler and reduce possibilities of race
> > conditions
> > so I am not even sure that the XXX comment is needed.
> 
> I think the XXX is useful as if someone notice an issue with the code, then
> they know what they could try.
> 
> I am open to suggestion how we could keep track of potential improvement.

It is worth capturing it somewhere. Maybe the commit message is better
than as an in-code comment?

Either way it is fine by me and feel free to make that kind of change on
commit.

[ovmf test] 159676: all pass - PUSHED

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 7f34681c488aee2563eaa2afcc6a2c8aa7c5b912
baseline version:
 ovmf 35f87da8a2debd443ac842db0a3794b17914a8f4

Last test of basis   159640  2021-02-24 17:10:46 Z1 days
Testing same since   159676  2021-02-25 16:11:45 Z0 days1 attempts


People who touched revisions under test:
  Li, Walon 
  Walon Li 

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
   35f87da8a2..7f34681c48  7f34681c488aee2563eaa2afcc6a2c8aa7c5b912 -> 
xen-tested-master

Re: [PATCH] xen: introduce XENFEAT_direct_mapped and XENFEAT_not_direct_mapped

[Stefano Stabellini]

On Thu, 25 Feb 2021, Jan Beulich wrote:
> On 25.02.2021 02:22, Stefano Stabellini wrote:
> > --- a/xen/include/public/features.h
> > +++ b/xen/include/public/features.h
> > @@ -114,6 +114,13 @@
> >   */
> >  #define XENFEAT_linux_rsdp_unrestricted   15
> >  
> > +/*
> > + * A direct-mapped (or 1:1 mapped) domain is a domain for which its
> > + * local pages have gfn == mfn.
> > + */
> > +#define XENFEAT_not_direct_mapped   16
> > +#define XENFEAT_direct_mapped   17
> 
> Why two new values? Absence of XENFEAT_direct_mapped requires
> implying not-direct-mapped by the consumer anyway, doesn't it?

That's because if we add both flags we can avoid all unpleasant guessing
games in the guest kernel.

If one flag or the other flag is set, we can make an informed decision.

But if neither flag is set, it means we are running on an older Xen,
and we fall back on the current checks.


> Further, quoting xen/mm.h: "For a non-translated guest which
> is aware of Xen, gfn == mfn." This to me implies that PV would
> need to get XENFEAT_direct_mapped set; not sure whether this
> simply means x86'es is_domain_direct_mapped() is wrong, but if
> it is, uses elsewhere in the code would likely need changing.

That's a good point, I didn't think about x86 PV. I think the two flags
are needed for autotranslated guests. I don't know for sure what is best
for non-autotranslated guests.

Maybe we could say that XENFEAT_not_direct_mapped and
XENFEAT_direct_mapped only apply to XENFEAT_auto_translated_physmap
guests. And it would match the implementation of
is_domain_direct_mapped().

For non XENFEAT_auto_translated_physmap guests we could either do:

- neither flag is set
- set XENFEAT_direct_mapped (without changing the implementation of
  is_domain_direct_mapped)

What do you think? I am happy either way.


> Also, nit: Please keep the right sides aligned with #define-s
> higher up in the file.

OK

[PATCH 1/3] tools/hvmloader: Drop machelf include as well

[Andrew Cooper]

The logic behind switching to elfstructs applies to sun builds as well.

Fixes: 81b2b328a2 ("hvmloader: use Xen private header for elf structs")
Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Roger Pau Monné 
CC: Wei Liu 
CC: Ian Jackson 
---
 tools/firmware/hvmloader/32bitbios_support.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tools/firmware/hvmloader/32bitbios_support.c 
b/tools/firmware/hvmloader/32bitbios_support.c
index e726946a7b..6f28fb6bde 100644
--- a/tools/firmware/hvmloader/32bitbios_support.c
+++ b/tools/firmware/hvmloader/32bitbios_support.c
@@ -22,9 +22,6 @@
 
 #include 
 #include 
-#ifdef __sun__
-#include 
-#endif
 
 #include "util.h"
 #include "config.h"
-- 
2.11.0

[PATCH for-4.15 0/3] Build firmware as freestanding

[Andrew Cooper]

This fixes a bug we've had for ages, which even ended up being documented as
an inappropriate build dependency.

For 4.15.  I'm tempted to suggest that it wants backporting to the stable
branches as well.

Andrew Cooper (3):
  tools/hvmloader: Drop machelf include as well
  tools/firmware: Build firmware as -ffreestanding
  automation: Annotate that a 32bit libc is no longer a dependency

 .travis.yml  | 1 -
 README   | 3 ---
 automation/build/archlinux/current.dockerfile| 1 +
 automation/build/centos/7.2.dockerfile   | 1 +
 automation/build/centos/7.dockerfile | 1 +
 automation/build/debian/jessie.dockerfile| 1 +
 automation/build/debian/stretch.dockerfile   | 1 +
 automation/build/debian/unstable.dockerfile  | 1 +
 automation/build/fedora/29.dockerfile| 1 +
 automation/build/suse/opensuse-leap.dockerfile   | 1 +
 automation/build/suse/opensuse-tumbleweed.dockerfile | 1 +
 automation/build/ubuntu/bionic.dockerfile| 1 +
 automation/build/ubuntu/focal.dockerfile | 1 +
 automation/build/ubuntu/trusty.dockerfile| 1 +
 automation/build/ubuntu/xenial.dockerfile| 1 +
 tools/firmware/Rules.mk  | 2 +-
 tools/firmware/hvmloader/32bitbios_support.c | 5 +
 17 files changed, 15 insertions(+), 9 deletions(-)

-- 
2.11.0

[PATCH 2/3] tools/firmware: Build firmware as -ffreestanding

[Andrew Cooper]

firmware should always have been -ffreestanding, as it doesn't execute in the
host environment.

inttypes.h isn't a freestanding header, but the 32bitbios_support.c only wants
the stdint.h types so switch to the more appropriate include.

This removes the build time dependency on a 32bit libc just to compile the
hvmloader and friends.

Update README and the TravisCI configuration.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Roger Pau Monné 
CC: Wei Liu 
CC: Ian Jackson 

For 4.15.  Build tested in Travis (Ubuntu) and XenServer (CentOS) - no change
in the compiled HVMLoader binary.  I'm currently rebuilding the containers
locally to check Arch, Debian and OpenSUSE, but don't anticipate any problems.

This does not resolve the build issue on Alpine.  Exactly what to do there is
still TBC, but Roger has opened a bug with Apline concerning their GCC
packaging.
---
 .travis.yml  | 1 -
 README   | 3 ---
 tools/firmware/Rules.mk  | 2 +-
 tools/firmware/hvmloader/32bitbios_support.c | 2 +-
 4 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 15ca9e9047..2362475f7a 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -58,7 +58,6 @@ addons:
 - acpica-tools
 - bin86
 - bcc
-- libc6-dev-i386
 - libnl-3-dev
 - ocaml-nox
 - libfindlib-ocaml-dev
diff --git a/README b/README
index 33cdf6b826..5167bb1708 100644
--- a/README
+++ b/README
@@ -62,9 +62,6 @@ provided by your OS distributor:
 * GNU bison and GNU flex
 * GNU gettext
 * ACPI ASL compiler (iasl)
-* Libc multiarch package (e.g. libc6-dev-i386 / glibc-devel.i686).
-  Required when building on a 64-bit platform to build
-  32-bit components which are enabled on a default build.
 
 In addition to the above there are a number of optional build
 prerequisites. Omitting these will cause the related features to be
diff --git a/tools/firmware/Rules.mk b/tools/firmware/Rules.mk
index 26bbddccd4..93abcabc67 100644
--- a/tools/firmware/Rules.mk
+++ b/tools/firmware/Rules.mk
@@ -16,4 +16,4 @@ CFLAGS += -Werror
 $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
 
 # Extra CFLAGS suitable for an embedded type of environment.
-CFLAGS += -fno-builtin -msoft-float
+CFLAGS += -fno-builtin -msoft-float -ffreestanding
diff --git a/tools/firmware/hvmloader/32bitbios_support.c 
b/tools/firmware/hvmloader/32bitbios_support.c
index 6f28fb6bde..cee3804888 100644
--- a/tools/firmware/hvmloader/32bitbios_support.c
+++ b/tools/firmware/hvmloader/32bitbios_support.c
@@ -20,7 +20,7 @@
  * this program; If not, see .
  */
 
-#include 
+#include 
 #include 
 
 #include "util.h"
-- 
2.11.0

[PATCH 3/3] automation: Annotate that a 32bit libc is no longer a dependency

[Andrew Cooper]

We can't drop the 32bit libc from the existing containers, because they are
used on older Xen branches as well.

However, we can avoid the dependency being propagated into newer conainers
derived from our dockerfiles.

No functional change.

Signed-off-by: Andrew Cooper 
---
CC: Doug Goldstein 
CC: Wei Liu 
CC: Ian Jackson 

For 4.15.  Documentation changes only
---
 automation/build/archlinux/current.dockerfile| 1 +
 automation/build/centos/7.2.dockerfile   | 1 +
 automation/build/centos/7.dockerfile | 1 +
 automation/build/debian/jessie.dockerfile| 1 +
 automation/build/debian/stretch.dockerfile   | 1 +
 automation/build/debian/unstable.dockerfile  | 1 +
 automation/build/fedora/29.dockerfile| 1 +
 automation/build/suse/opensuse-leap.dockerfile   | 1 +
 automation/build/suse/opensuse-tumbleweed.dockerfile | 1 +
 automation/build/ubuntu/bionic.dockerfile| 1 +
 automation/build/ubuntu/focal.dockerfile | 1 +
 automation/build/ubuntu/trusty.dockerfile| 1 +
 automation/build/ubuntu/xenial.dockerfile| 1 +
 13 files changed, 13 insertions(+)

diff --git a/automation/build/archlinux/current.dockerfile 
b/automation/build/archlinux/current.dockerfile
index d8fbebaf79..d46fc9d9ca 100644
--- a/automation/build/archlinux/current.dockerfile
+++ b/automation/build/archlinux/current.dockerfile
@@ -20,6 +20,7 @@ RUN pacman -S --refresh --sysupgrade --noconfirm 
--noprogressbar --needed \
 iasl \
 inetutils \
 iproute \
+# lib32-glibc for Xen < 4.15
 lib32-glibc \
 libaio \
 libcacard \
diff --git a/automation/build/centos/7.2.dockerfile 
b/automation/build/centos/7.2.dockerfile
index c2f46b694c..af672a0be1 100644
--- a/automation/build/centos/7.2.dockerfile
+++ b/automation/build/centos/7.2.dockerfile
@@ -34,6 +34,7 @@ RUN rpm --rebuilddb && \
 yajl-devel \
 pixman-devel \
 glibc-devel \
+# glibc-devel.i686 for Xen < 4.15
 glibc-devel.i686 \
 make \
 binutils \
diff --git a/automation/build/centos/7.dockerfile 
b/automation/build/centos/7.dockerfile
index e37d9d743a..5f83c97d0c 100644
--- a/automation/build/centos/7.dockerfile
+++ b/automation/build/centos/7.dockerfile
@@ -32,6 +32,7 @@ RUN yum -y install \
 yajl-devel \
 pixman-devel \
 glibc-devel \
+# glibc-devel.i686 for Xen < 4.15
 glibc-devel.i686 \
 make \
 binutils \
diff --git a/automation/build/debian/jessie.dockerfile 
b/automation/build/debian/jessie.dockerfile
index 1232b9e204..808d6272e4 100644
--- a/automation/build/debian/jessie.dockerfile
+++ b/automation/build/debian/jessie.dockerfile
@@ -31,6 +31,7 @@ RUN apt-get update && \
 bin86 \
 bcc \
 liblzma-dev \
+# libc6-dev-i386 for Xen < 4.15
 libc6-dev-i386 \
 libnl-3-dev \
 ocaml-nox \
diff --git a/automation/build/debian/stretch.dockerfile 
b/automation/build/debian/stretch.dockerfile
index 32742f7f39..e3bace1f87 100644
--- a/automation/build/debian/stretch.dockerfile
+++ b/automation/build/debian/stretch.dockerfile
@@ -32,6 +32,7 @@ RUN apt-get update && \
 bin86 \
 bcc \
 liblzma-dev \
+# libc6-dev-i386 for Xen < 4.15
 libc6-dev-i386 \
 libnl-3-dev \
 ocaml-nox \
diff --git a/automation/build/debian/unstable.dockerfile 
b/automation/build/debian/unstable.dockerfile
index aeb4f3448b..9a10ee08d6 100644
--- a/automation/build/debian/unstable.dockerfile
+++ b/automation/build/debian/unstable.dockerfile
@@ -32,6 +32,7 @@ RUN apt-get update && \
 bin86 \
 bcc \
 liblzma-dev \
+# libc6-dev-i386 for Xen < 4.15
 libc6-dev-i386 \
 libnl-3-dev \
 ocaml-nox \
diff --git a/automation/build/fedora/29.dockerfile 
b/automation/build/fedora/29.dockerfile
index 6a4e5b0413..5482952523 100644
--- a/automation/build/fedora/29.dockerfile
+++ b/automation/build/fedora/29.dockerfile
@@ -25,6 +25,7 @@ RUN dnf -y install \
 yajl-devel \
 pixman-devel \
 glibc-devel \
+# glibc-devel.i686 for Xen < 4.15
 glibc-devel.i686 \
 make \
 binutils \
diff --git a/automation/build/suse/opensuse-leap.dockerfile 
b/automation/build/suse/opensuse-leap.dockerfile
index c60c13c943..685dd5d7fd 100644
--- a/automation/build/suse/opensuse-leap.dockerfile
+++ b/automation/build/suse/opensuse-leap.dockerfile
@@ -26,6 +26,7 @@ RUN zypper install -y --no-recommends \
 git \
 glib2-devel \
 glibc-devel \
+# glibc-devel-32bit for Xen < 4.15
 glibc-devel-32bit \
 gzip \
 hostname \
diff --git a/automation/build/suse/opensuse-tumbleweed.dockerfile 
b/automation/build/suse/opensuse-tumbleweed.dockerfile
index 084cce0921..061173e751 100644
--- a/automation/build/suse/opensuse-tumbleweed

[linux-linus test] 159662: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-xsm7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 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-ws16-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-install fail REGR. 
vs. 152332
 test-amd64-i386-xl-qemut-debianhvm-amd64  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-examine   6 xen-install  fail REGR. vs. 152332
 test-amd64-i386-libvirt   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-xl-qemut-ws16-amd64  7 xen-install   fail 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-coresched-i386-xl  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-qemuu-rhel6hvm-amd  7 xen-installfail REGR. vs. 152332
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-install fail REGR. vs. 
152332
 test-amd64-i386-libvirt-xsm   7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-debianhvm-amd64  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-qemut-rhel6hvm-amd  7 xen-installfail 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-raw7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-freebsd10-i386  7 xen-installfail REGR. vs. 152332
 test-amd64-i386-xl-shadow 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-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-qemuu-ovmf-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-examine  8 reboot   fail REGR. vs. 152332
 test-arm64-arm64-xl   8 xen-boot fail REGR. vs. 152332
 test-arm64-arm64-xl-xsm  12 debian-install   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-arm64-arm64-xl-seattle   8 xen-boot fail REGR. vs. 152332
 test-arm64-arm64-libvirt-xsm  8 xen-boot fail REGR. vs. 152332
 test-arm64-arm64-xl-credit2   8 xen-boot fail REGR. vs. 152332
 test-arm64-arm64-xl-credit1   8 xen-boot fail REGR. vs. 152332

Tests which did not succeed, but are not blocking:
 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-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail 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-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 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-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-check  

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

[osstest service owner]

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

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  fc8fb368515391374e5f170a1e07205d914bc14a
baseline version:
 xen  067935804a8e7a33ff7170a2db8ce94bb46d9a63

Last test of basis   159668  2021-02-25 13:01:28 Z0 days
Testing same since   159674  2021-02-25 16:01:33 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Hubert Jasudowicz 
  Jan Beulich 
  Roger Pau Monné 

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



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   067935804a..fc8fb36851  fc8fb368515391374e5f170a1e07205d914bc14a -> smoke

[qemu-mainline test] 159660: regressions - FAIL

[osstest service owner]

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

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

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

version targeted for testing:
 qemuu7ef8134565dccf9186d5eabd7dbb4ecae6dead87
baseline version:
 qemuu1d806cef0e38b5db8347a8e12f214d543204a314

Last test of basis   152631  2020-08-20 09:07:46 Z  189 days
Failing since152659  2020-08-21 14:07:39 Z  188 days  363 attempts
Testing same since   159563  2021-02-22 23:37:57 Z2 days5 attempts


425 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm 

Re: [PATCH for-4.15 0/5] xenstore: Address coverity issues in the LiveUpdate code

[Ian Jackson]

Julien Grall writes ("Re: [PATCH for-4.15 0/5] xenstore: Address coverity 
issues in the LiveUpdate code"):
> On 25/02/2021 17:54, Ian Jackson wrote:
> > Julien Grall writes ("[PATCH for-4.15 0/5] xenstore: Address coverity 
> > issues in the LiveUpdate code"):
> >>tools/xenstored: Silence coverity when using xs_state_* structures
> > 
> > For this I can't see a reason to give a release ack ?  See also Andy's
> > comments.
> 
> I don't have a reason for this one as it is so far just silencing 
> Coverity. Sorry I should have mention that this one is not really 4.15 
> material.

No problem, thanks for the fixes!

Ian.

Re: [PATCH for-4.15 0/5] xenstore: Address coverity issues in the LiveUpdate code

[Julien Grall]

Hi Ian,

On 25/02/2021 17:54, Ian Jackson wrote:

Julien Grall writes ("[PATCH for-4.15 0/5] xenstore: Address coverity issues in the 
LiveUpdate code"):

   tools/xenstored: Avoid unnecessary talloc_strdup() in do_control_lu()
   tools/xenstored: Avoid unnecessary talloc_strdup() in do_lu_start()
   tools/xenstored: control: Store the save filename in lu_dump_state
   tools/xenstore-control: Don't leak buf in live_update_start()


These four are actual bugfixes:

Release-Acked-by: Ian Jackson 


Thanks!




   tools/xenstored: Silence coverity when using xs_state_* structures


For this I can't see a reason to give a release ack ?  See also Andy's
comments.


I don't have a reason for this one as it is so far just silencing 
Coverity. Sorry I should have mention that this one is not really 4.15 
material.


Cheers,

--
Julien Grall

Re: [PATCH for-4.15] x86/dmop: Properly fail for PV guests

[Ian Jackson]

Andrew Cooper writes ("Re: [PATCH for-4.15] x86/dmop: Properly fail for PV 
guests"):
> On 25/02/2021 17:22, Ian Jackson wrote:
> > Andrew Cooper writes ("[PATCH for-4.15] x86/dmop: Properly fail for PV 
> > guests"):
> >> The current code has an early exit for PV guests, but it returns 0 having 
> >> done
> >> nothing.
> > Reviewed-by: Ian Jackson 
> 
> Thanks.

Release-Acked-by: Ian Jackson 

[PATCH for-4.15 0/5] xenstore: Address coverity issues in the LiveUpdate code

[Ian Jackson]

Julien Grall writes ("[PATCH for-4.15 0/5] xenstore: Address coverity issues in 
the LiveUpdate code"):
>   tools/xenstored: Avoid unnecessary talloc_strdup() in do_control_lu()
>   tools/xenstored: Avoid unnecessary talloc_strdup() in do_lu_start()
>   tools/xenstored: control: Store the save filename in lu_dump_state
>   tools/xenstore-control: Don't leak buf in live_update_start()

These four are actual bugfixes:

Release-Acked-by: Ian Jackson 

>   tools/xenstored: Silence coverity when using xs_state_* structures

For this I can't see a reason to give a release ack ?  See also Andy's
comments.

Ian.

Re: [PATCH for-4.15 5/5] tools/xenstored: Silence coverity when using xs_state_* structures

[Julien Grall]

Hi Andrew,

On 25/02/2021 17:47, Andrew Cooper wrote:

On 25/02/2021 17:41, Julien Grall wrote:

From: Julien Grall 

Coverity will report unitialized values for every use of xs_state_*
structures in the save part. This can be prevented by using the [0]
rather than [] to define variable length array.

Coverity-ID: 1472398
Coverity-ID: 1472397
Coverity-ID: 1472396
Coverity-ID: 1472395
Signed-off-by: Julien Grall 

---

 From my understanding, the tools and the hypervisor already rely on GNU
extensions. So the change should be fine.

If not, we can use the same approach as XEN_FLEX_ARRAY_DIM.


Linux has recently purged the use of [0] because it causes sizeof() to
do unsafe things.


Do you have a link to the Linux thread?



Flexible array members is a C99 standard - are we sure that Coverity is
doing something wrong with them?
I have run coverity with one of the structure switched to [0] and it 
removed the unitialized warning for that specific one.


So clearly coverity is not happy with []. Although, I am not sure why.

Do you have a suggestion how to approach the problem?

Cheers,

--
Julien Grall

Re: [PATCH for-4.15 5/5] tools/xenstored: Silence coverity when using xs_state_* structures

[Andrew Cooper]

On 25/02/2021 17:41, Julien Grall wrote:
> From: Julien Grall 
>
> Coverity will report unitialized values for every use of xs_state_*
> structures in the save part. This can be prevented by using the [0]
> rather than [] to define variable length array.
>
> Coverity-ID: 1472398
> Coverity-ID: 1472397
> Coverity-ID: 1472396
> Coverity-ID: 1472395
> Signed-off-by: Julien Grall 
>
> ---
>
> From my understanding, the tools and the hypervisor already rely on GNU
> extensions. So the change should be fine.
>
> If not, we can use the same approach as XEN_FLEX_ARRAY_DIM.

Linux has recently purged the use of [0] because it causes sizeof() to
do unsafe things.

Flexible array members is a C99 standard - are we sure that Coverity is
doing something wrong with them?

~Andrew

[PATCH for-4.15 4/5] tools/xenstore-control: Don't leak buf in live_update_start()

[Julien Grall]

From: Julien Grall 

All the error paths but one will free buf. Cover the remaining path so
buf can't be leaked.

This bug was discovered and resolved using Coverity Static Analysis
Security Testing (SAST) by Synopsys, Inc.

Fixes: 7f97193e6aa8 ("tools/xenstore: add live update command to 
xenstore-control")
Signed-off-by: Julien Grall 
---
 tools/xenstore/xenstore_control.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/xenstore/xenstore_control.c 
b/tools/xenstore/xenstore_control.c
index f6f4626c0656..548363ee7094 100644
--- a/tools/xenstore/xenstore_control.c
+++ b/tools/xenstore/xenstore_control.c
@@ -44,8 +44,10 @@ static int live_update_start(struct xs_handle *xsh, bool 
force, unsigned int to)
 return 1;
 
 ret = strdup("BUSY");
-if (!ret)
+if (!ret) {
+free(buf);
 return 1;
+}
 
 for (time_start = time(NULL); time(NULL) - time_start < to;) {
 free(ret);
-- 
2.17.1

[PATCH for-4.15 5/5] tools/xenstored: Silence coverity when using xs_state_* structures

[Julien Grall]

From: Julien Grall 

Coverity will report unitialized values for every use of xs_state_*
structures in the save part. This can be prevented by using the [0]
rather than [] to define variable length array.

Coverity-ID: 1472398
Coverity-ID: 1472397
Coverity-ID: 1472396
Coverity-ID: 1472395
Signed-off-by: Julien Grall 

---

>From my understanding, the tools and the hypervisor already rely on GNU
extensions. So the change should be fine.

If not, we can use the same approach as XEN_FLEX_ARRAY_DIM.
---
 tools/xenstore/include/xenstore_state.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/xenstore/include/xenstore_state.h 
b/tools/xenstore/include/xenstore_state.h
index ae0d053c8ffc..407d9e920c0f 100644
--- a/tools/xenstore/include/xenstore_state.h
+++ b/tools/xenstore/include/xenstore_state.h
@@ -86,7 +86,7 @@ struct xs_state_connection {
 uint16_t data_in_len;/* Number of unprocessed bytes read from conn. */
 uint16_t data_resp_len;  /* Size of partial response pending for conn. */
 uint32_t data_out_len;   /* Number of bytes not yet written to conn. */
-uint8_t  data[]; /* Pending data (read, written) + 0-7 pad bytes. 
*/
+uint8_t  data[0]; /* Pending data (read, written) + 0-7 pad bytes. 
*/
 };
 
 /* Watch: */
@@ -94,7 +94,7 @@ struct xs_state_watch {
 uint32_t conn_id;   /* Connection this watch is associated with. */
 uint16_t path_length;   /* Number of bytes of path watched (incl. 0). */
 uint16_t token_length;  /* Number of bytes of watch token (incl. 0). */
-uint8_t data[]; /* Path bytes, token bytes, 0-7 pad bytes. */
+uint8_t data[0];/* Path bytes, token bytes, 0-7 pad bytes. */
 };
 
 /* Transaction: */
@@ -125,7 +125,7 @@ struct xs_state_node {
 #define XS_STATE_NODE_TA_WRITTEN  0x0002
 uint16_t perm_n;/* Number of permissions (0 in TA: node deleted). 
*/
 /* Permissions (first is owner, has full access). */
-struct xs_state_node_perm perms[];
+struct xs_state_node_perm perms[0];
 /* Path and data follows, plus 0-7 pad bytes. */
 };
 #endif /* XENSTORE_STATE_H */
-- 
2.17.1

[PATCH for-4.15 3/5] tools/xenstored: control: Store the save filename in lu_dump_state

[Julien Grall]

From: Julien Grall 

The function lu_close_dump_state() will use talloc_asprintf() without
checking whether the allocation succeeded. In the unlikely case we are
out of memory, we would dereference a NULL pointer.

As we already computed the filename in lu_get_dump_state(), we can store
the name in the lu_dump_state. This is avoiding to deal with memory file
in the close path and also reduce the risk to use the different
filename.

This bug was discovered and resolved using Coverity Static Analysis
Security Testing (SAST) by Synopsys, Inc.

Fixes: c0dc6a3e7c41 ("tools/xenstore: read internal state when doing live 
upgrade")
Signed-off-by: Julien Grall 
---
 tools/xenstore/xenstored_control.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/tools/xenstore/xenstored_control.c 
b/tools/xenstore/xenstored_control.c
index 8eb57827765c..653890f2d9e0 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -16,6 +16,7 @@ Interactive commands for Xen Store Daemon.
 along with this program; If not, see .
 */
 
+#include 
 #include 
 #include 
 #include 
@@ -74,6 +75,7 @@ struct lu_dump_state {
unsigned int size;
 #ifndef __MINIOS__
int fd;
+   char *filename;
 #endif
 };
 
@@ -399,17 +401,16 @@ static void lu_dump_close(FILE *fp)
 
 static void lu_get_dump_state(struct lu_dump_state *state)
 {
-   char *filename;
struct stat statbuf;
 
state->size = 0;
 
-   filename = talloc_asprintf(NULL, "%s/state_dump", xs_daemon_rootdir());
-   if (!filename)
+   state->filename = talloc_asprintf(NULL, "%s/state_dump",
+ xs_daemon_rootdir());
+   if (!state->filename)
barf("Allocation failure");
 
-   state->fd = open(filename, O_RDONLY);
-   talloc_free(filename);
+   state->fd = open(state->filename, O_RDONLY);
if (state->fd < 0)
return;
if (fstat(state->fd, &statbuf) != 0)
@@ -431,14 +432,13 @@ static void lu_get_dump_state(struct lu_dump_state *state)
 
 static void lu_close_dump_state(struct lu_dump_state *state)
 {
-   char *filename;
+   assert(state->filename != NULL);
 
munmap(state->buf, state->size);
close(state->fd);
 
-   filename = talloc_asprintf(NULL, "%s/state_dump", xs_daemon_rootdir());
-   unlink(filename);
-   talloc_free(filename);
+   unlink(state->filename);
+   talloc_free(state->filename);
 }
 
 static char *lu_exec(const void *ctx, int argc, char **argv)
-- 
2.17.1

[PATCH for-4.15 0/5] xenstore: Address coverity issues in the LiveUpdate code

[Julien Grall]

From: Julien Grall 

Hi all,

The AWS coverity instance spotted a few issues that could either
leak memory and derefence NULL pointer.

All the patches are candidate for 4.15 as they are hardening XenStored
code. The changes are low risks.

Cheers,

Julien Grall (5):
  tools/xenstored: Avoid unnecessary talloc_strdup() in do_control_lu()
  tools/xenstored: Avoid unnecessary talloc_strdup() in do_lu_start()
  tools/xenstored: control: Store the save filename in lu_dump_state
  tools/xenstore-control: Don't leak buf in live_update_start()
  tools/xenstored: Silence coverity when using xs_state_* structures

 tools/xenstore/include/xenstore_state.h |  6 +++---
 tools/xenstore/xenstore_control.c   |  4 +++-
 tools/xenstore/xenstored_control.c  | 26 +++--
 3 files changed, 17 insertions(+), 19 deletions(-)

-- 
2.17.1

[PATCH for-4.15 1/5] tools/xenstored: Avoid unnecessary talloc_strdup() in do_control_lu()

[Julien Grall]

From: Julien Grall 

At the moment, the return of talloc_strdup() is not checked. This means
we may dereference a NULL pointer if the allocation failed.

However, it is pointless to allocate the memory as send_reply() will
copy the data to a different buffer. So drop the use of talloc_strdup().

This bug was discovered and resolved using Coverity Static Analysis
Security Testing (SAST) by Synopsys, Inc.

Fixes: fecab256d474 ("tools/xenstore: add basic live-update command parsing")
Signed-off-by: Julien Grall 
---
 tools/xenstore/xenstored_control.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/xenstore/xenstored_control.c 
b/tools/xenstore/xenstored_control.c
index f10beaf85eb4..e8a501acdb62 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -691,7 +691,6 @@ static const char *lu_start(const void *ctx, struct 
connection *conn,
 static int do_control_lu(void *ctx, struct connection *conn,
 char **vec, int num)
 {
-   const char *resp;
const char *ret = NULL;
unsigned int i;
bool force = false;
@@ -734,8 +733,7 @@ static int do_control_lu(void *ctx, struct connection *conn,
 
if (!ret)
ret = "OK";
-   resp = talloc_strdup(ctx, ret);
-   send_reply(conn, XS_CONTROL, resp, strlen(resp) + 1);
+   send_reply(conn, XS_CONTROL, ret, strlen(ret) + 1);
return 0;
 }
 #endif
-- 
2.17.1

[PATCH for-4.15 2/5] tools/xenstored: Avoid unnecessary talloc_strdup() in do_lu_start()

[Julien Grall]

From: Julien Grall 

At the moment, the return of talloc_strdup() is not checked. This means
we may dereference a NULL pointer if the allocation failed.

However, it is pointless to allocate the memory as send_reply() will
copy the data to a different buffer. So drop the use of talloc_strdup().

This bug was discovered and resolved using Coverity Static Analysis
Security Testing (SAST) by Synopsys, Inc.

Fixes: af216a99fb4a ("tools/xenstore: add the basic framework for doing the 
live update")
Signed-off-by: Julien Grall 
---
 tools/xenstore/xenstored_control.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/xenstore/xenstored_control.c 
b/tools/xenstore/xenstored_control.c
index e8a501acdb62..8eb57827765c 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -638,7 +638,6 @@ static bool do_lu_start(struct delayed_request *req)
 {
time_t now = time(NULL);
const char *ret;
-   char *resp;
 
if (!lu_check_lu_allowed()) {
if (now < lu_status->started_at + lu_status->timeout)
@@ -660,8 +659,7 @@ static bool do_lu_start(struct delayed_request *req)
  out:
talloc_free(lu_status);
 
-   resp = talloc_strdup(req->in, ret);
-   send_reply(lu_status->conn, XS_CONTROL, resp, strlen(resp) + 1);
+   send_reply(lu_status->conn, XS_CONTROL, ret, strlen(ret) + 1);
 
return true;
 }
-- 
2.17.1

Re: [PATCH for-4.15] x86/dmop: Properly fail for PV guests

[Andrew Cooper]

On 25/02/2021 17:22, Ian Jackson wrote:
> Andrew Cooper writes ("[PATCH for-4.15] x86/dmop: Properly fail for PV 
> guests"):
>> The current code has an early exit for PV guests, but it returns 0 having 
>> done
>> nothing.
> Reviewed-by: Ian Jackson 

Thanks.

>
>> diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
>> index 5bc172a5d4..612749442e 100644
>> --- a/xen/arch/x86/hvm/dm.c
>> +++ b/xen/arch/x86/hvm/dm.c
>> @@ -365,6 +365,7 @@ int dm_op(const struct dmop_args *op_args)
>>  if ( rc )
>>  return rc;
>>  
>> +rc = -EINVAL;
>>  if ( !is_hvm_domain(d) )
>>  goto out;
> Is this style, of setting rc outside the if, the standard hypervisor
> style ?

If you think the cyclomatic complexity is bad in libxl...

This is the prevailing style in this function.  Its common, but not
universal style.

~Andrew

Re: [PATCH 00/14] deprecations: remove many old deprecations

[Jim Fehlig]

Adding xen-devel and Ian to cc.

On 2/24/21 6:11 AM, Daniel P. Berrangé wrote:

The following features have been deprecated for well over the 2
release cycle we promise


This reminded me of a bug report we received late last year when updating to 
5.2.0. 'virsh setvcpus' suddenly stopped working for Xen HVM guests. Turns out 
libxl uses cpu-add under the covers.




   ``-usbdevice`` (since 2.10.0)
   ``-drive file=3Djson:{...{'driver':'file'}}`` (since 3.0)
   ``-vnc acl`` (since 4.0.0)
   ``-mon ...,control=3Dreadline,pretty=3Don|off`` (since 4.1)
   ``migrate_set_downtime`` and ``migrate_set_speed`` (since 2.8.0)
   ``query-named-block-nodes`` result ``encryption_key_missing`` (since 2.10.0)
   ``query-block`` result ``inserted.encryption_key_missing`` (since 2.10.0)
   ``migrate-set-cache-size`` and ``query-migrate-cache-size`` (since 2.11.0)
   ``query-named-block-nodes`` and ``query-block`` result dirty-bitmaps[i].sta=
tus (ince 4.0)
   ``query-cpus`` (since 2.12.0)
   ``query-cpus-fast`` ``arch`` output member (since 3.0.0)
   ``query-events`` (since 4.0)
   chardev client socket with ``wait`` option (since 4.0)
   ``acl_show``, ``acl_reset``, ``acl_policy``, ``acl_add``, ``acl_remove`` (s=
ince 4.0.0)
   ``ide-drive`` (since 4.2)
   ``scsi-disk`` (since 4.2)

AFAICT, libvirt has ceased to use all of these too.


A quick grep of the libxl code shows it uses -usbdevice, query-cpus, and 
scsi-disk.


There are many more similarly old deprecations not (yet) tackled.


The Xen tools maintainers will need to be more vigilant of the deprecations. I 
don't follow Xen development close enough to know if this topic has already been 
discussed.


Regards,
Jim

Re: [PATCH for-4.15] dmop: Add XEN_DMOP_nr_vcpus

[Andrew Cooper]

On 25/02/2021 17:21, Ian Jackson wrote:
> Andrew Cooper writes ("[PATCH for-4.15] dmop: Add XEN_DMOP_nr_vcpus"):
>> Curiously absent from the stable API/ABIs is an ability to query the number 
>> of
>> vcpus which a domain has.  Emulators need to know this information in
>> particular to know how many stuct ioreq's live in the ioreq server mappings.
>>
>> In practice, this forces all userspace to link against libxenctrl to use
>> xc_domain_getinfo(), which rather defeats the purpose of the stable 
>> libraries.
> Wat

Yeah...  My reaction was similar.

>
>> For 4.15.  This was a surprise discovery in the massive ABI untangling effort
>> I'm currently doing for XenServer's new build system.
> Given that this is a new feature at a late stage I am going to say
> this:
>
> I will R-A it subject to it getting *two* independent Reviewed-by.
>
> I will try to one of them myself :-).
>
> ...
>
>> +/*
>> + * XEN_DMOP_nr_vcpus: Query the number of vCPUs a domain has.
>> + *
>> + * The number of vcpus a domain has is fixed from creation time.  This bound
>> + * is applicable to e.g. the vcpuid parameter of XEN_DMOP_inject_event, or
>> + * number of struct ioreq objects mapped via XENMEM_acquire_resource.
> AIUI from the code, the value is the maximum number of vcpus, in the
> sense that they are not necessarily all online.  In which case I think
> maybe you want to mention that here ?

Yeah - there is no guarantee that they're all online, or running.

Emulators tend to attach before the domain starts executing anyway.  The
important thing they need to do is loop through each struct ioreq in the
ioreq_server mapping to read the domid and bind the per-vcpu event
channel for notification of work to do.

The totally gross way of not needing this API is to scan through the
mapping and identify the first struct ioreq which has 0 listed for an
event channel, which is not a construct I wish to promote.

>
>> diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
>> index 398993d5f4..cbbd20c958 100644
>> --- a/xen/include/xlat.lst
>> +++ b/xen/include/xlat.lst
>> @@ -107,6 +107,7 @@
>>  ?   dm_op_set_pci_intx_levelhvm/dm_op.h
>>  ?   dm_op_set_pci_link_routehvm/dm_op.h
>>  ?   dm_op_track_dirty_vram  hvm/dm_op.h
>> +?   dm_op_nr_vcpus  hvm/dm_op.h
>>  !   hvm_altp2m_set_mem_access_multi hvm/hvm_op.h
>>  ?   vcpu_hvm_contexthvm/hvm_vcpu.h
>>  ?   vcpu_hvm_x86_32 hvm/hvm_vcpu.h
>> -- 
> I have no idea what even.  I read the comment at the top of the file.
>
> So for *everything except the xlat.lst change*
> Reviewed-by: Ian Jackson 

Thanks.

This is the magic to make this hunk:

@@ -641,6 +651,7 @@ CHECK_dm_op_map_mem_type_to_ioreq_server;
 CHECK_dm_op_remote_shutdown;
 CHECK_dm_op_relocate_memory;
 CHECK_dm_op_pin_memory_cacheattr;
+CHECK_dm_op_nr_vcpus;
 
 int compat_dm_op(domid_t domid,
  unsigned int nr_bufs,

work, to do a build time check that the structure is identical between
32bit and 64bit builds.

~Andrew

Re: [PATCH for-4.15] x86/dmop: Properly fail for PV guests

[Ian Jackson]

Andrew Cooper writes ("[PATCH for-4.15] x86/dmop: Properly fail for PV guests"):
> The current code has an early exit for PV guests, but it returns 0 having done
> nothing.

Reviewed-by: Ian Jackson 

> diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
> index 5bc172a5d4..612749442e 100644
> --- a/xen/arch/x86/hvm/dm.c
> +++ b/xen/arch/x86/hvm/dm.c
> @@ -365,6 +365,7 @@ int dm_op(const struct dmop_args *op_args)
>  if ( rc )
>  return rc;
>  
> +rc = -EINVAL;
>  if ( !is_hvm_domain(d) )
>  goto out;

Is this style, of setting rc outside the if, the standard hypervisor
style ?

Ian.

Re: [PATCH for-4.15] dmop: Add XEN_DMOP_nr_vcpus

[Ian Jackson]

Andrew Cooper writes ("[PATCH for-4.15] dmop: Add XEN_DMOP_nr_vcpus"):
> Curiously absent from the stable API/ABIs is an ability to query the number of
> vcpus which a domain has.  Emulators need to know this information in
> particular to know how many stuct ioreq's live in the ioreq server mappings.
> 
> In practice, this forces all userspace to link against libxenctrl to use
> xc_domain_getinfo(), which rather defeats the purpose of the stable libraries.

Wat

> For 4.15.  This was a surprise discovery in the massive ABI untangling effort
> I'm currently doing for XenServer's new build system.

Given that this is a new feature at a late stage I am going to say
this:

I will R-A it subject to it getting *two* independent Reviewed-by.

I will try to one of them myself :-).

...

> +/*
> + * XEN_DMOP_nr_vcpus: Query the number of vCPUs a domain has.
> + *
> + * The number of vcpus a domain has is fixed from creation time.  This bound
> + * is applicable to e.g. the vcpuid parameter of XEN_DMOP_inject_event, or
> + * number of struct ioreq objects mapped via XENMEM_acquire_resource.

AIUI from the code, the value is the maximum number of vcpus, in the
sense that they are not necessarily all online.  In which case I think
maybe you want to mention that here ?

> diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
> index 398993d5f4..cbbd20c958 100644
> --- a/xen/include/xlat.lst
> +++ b/xen/include/xlat.lst
> @@ -107,6 +107,7 @@
>  ?dm_op_set_pci_intx_levelhvm/dm_op.h
>  ?dm_op_set_pci_link_routehvm/dm_op.h
>  ?dm_op_track_dirty_vram  hvm/dm_op.h
> +?dm_op_nr_vcpus  hvm/dm_op.h
>  !hvm_altp2m_set_mem_access_multi hvm/hvm_op.h
>  ?vcpu_hvm_contexthvm/hvm_vcpu.h
>  ?vcpu_hvm_x86_32 hvm/hvm_vcpu.h
> -- 

I have no idea what even.  I read the comment at the top of the file.

So for *everything except the xlat.lst change*
Reviewed-by: Ian Jackson 

Thanks,
Ian.

[PATCH for-4.15] x86/dmop: Properly fail for PV guests

[Andrew Cooper]

The current code has an early exit for PV guests, but it returns 0 having done
nothing.

Fixes: 524a98c2ac5 ("public / x86: introduce __HYPERCALL_dm_op...")
Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Roger Pau Monné 
CC: Wei Liu 
CC: Ian Jackson 
CC: Paul Durrant 

For 4.15.  Found while testing XEN_DMOP_nr_vcpus.  Needs backporting to all
stable releases.
---
 xen/arch/x86/hvm/dm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
index 5bc172a5d4..612749442e 100644
--- a/xen/arch/x86/hvm/dm.c
+++ b/xen/arch/x86/hvm/dm.c
@@ -365,6 +365,7 @@ int dm_op(const struct dmop_args *op_args)
 if ( rc )
 return rc;
 
+rc = -EINVAL;
 if ( !is_hvm_domain(d) )
 goto out;
 
-- 
2.11.0

[PATCH for-4.15] dmop: Add XEN_DMOP_nr_vcpus

[Andrew Cooper]

Curiously absent from the stable API/ABIs is an ability to query the number of
vcpus which a domain has.  Emulators need to know this information in
particular to know how many stuct ioreq's live in the ioreq server mappings.

In practice, this forces all userspace to link against libxenctrl to use
xc_domain_getinfo(), which rather defeats the purpose of the stable libraries.

Introduce a DMOP to retrieve this information and surface it in
libxendevicemodel to help emulators shed their use of unstable interfaces.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Roger Pau Monné 
CC: Wei Liu 
CC: Paul Durrant 
CC: Stefano Stabellini 
CC: Julien Grall 
CC: Volodymyr Babchuk 
CC: Ian Jackson 

For 4.15.  This was a surprise discovery in the massive ABI untangling effort
I'm currently doing for XenServer's new build system.

This is one new read-only op to obtain information which isn't otherwise
available under a stable API/ABI.  As such, its risk for 4.15 is very low,
with a very real quality-of-life improvement for downstreams.

I realise this is technically a new feature and we're long past feature
freeze, but I'm hoping that "really lets some emulators move off the unstable
libraries" is sufficiently convincing argument.

It's not sufficient to let Qemu move off unstable libraries yet - at a
minimum, the add_to_phymap hypercalls need stabilising to support PCI
Passthrough and BAR remapping.

I'd prefer not to duplicate the op handling between ARM and x86, and if this
weren't a release window, I'd submit a prereq patch to dedup the common dmop
handling.  That can wait to 4.16 at this point.  Also, this op ought to work
against x86 PV guests, but fixing that up will also need this rearrangement
into common code, so needs to wait.
---
 tools/include/xendevicemodel.h   | 10 ++
 tools/libs/devicemodel/core.c| 15 +++
 tools/libs/devicemodel/libxendevicemodel.map |  1 +
 xen/arch/arm/dm.c| 10 ++
 xen/arch/x86/hvm/dm.c| 11 +++
 xen/include/public/hvm/dm_op.h   | 15 +++
 xen/include/xlat.lst |  1 +
 7 files changed, 63 insertions(+)

diff --git a/tools/include/xendevicemodel.h b/tools/include/xendevicemodel.h
index c06b3c84b9..33698d67f3 100644
--- a/tools/include/xendevicemodel.h
+++ b/tools/include/xendevicemodel.h
@@ -358,6 +358,16 @@ int xendevicemodel_pin_memory_cacheattr(
 uint32_t type);
 
 /**
+ * Query for the number of vCPUs that a domain has.
+ * @parm dmod a handle to an open devicemodel interface.
+ * @parm domid the domain id to be serviced.
+ * @parm vcpus Number of vcpus.
+ * @return 0 on success and fills @p vcpus, or -1 on failure.
+ */
+int xendevicemodel_nr_vcpus(
+xendevicemodel_handle *dmod, domid_t domid, unsigned int *vcpus);
+
+/**
  * This function restricts the use of this handle to the specified
  * domain.
  *
diff --git a/tools/libs/devicemodel/core.c b/tools/libs/devicemodel/core.c
index 30bd79f8ba..8e619eeb0a 100644
--- a/tools/libs/devicemodel/core.c
+++ b/tools/libs/devicemodel/core.c
@@ -630,6 +630,21 @@ int xendevicemodel_pin_memory_cacheattr(
 return xendevicemodel_op(dmod, domid, 1, &op, sizeof(op));
 }
 
+int xendevicemodel_nr_vcpus(
+xendevicemodel_handle *dmod, domid_t domid, unsigned int *vcpus)
+{
+struct xen_dm_op op = {
+.op = XEN_DMOP_nr_vcpus,
+};
+
+int rc = xendevicemodel_op(dmod, domid, 1, &op, sizeof(op));
+if ( rc )
+return rc;
+
+*vcpus = op.u.nr_vcpus.vcpus;
+return 0;
+}
+
 int xendevicemodel_restrict(xendevicemodel_handle *dmod, domid_t domid)
 {
 return osdep_xendevicemodel_restrict(dmod, domid);
diff --git a/tools/libs/devicemodel/libxendevicemodel.map 
b/tools/libs/devicemodel/libxendevicemodel.map
index 733549327b..f7f9e3d932 100644
--- a/tools/libs/devicemodel/libxendevicemodel.map
+++ b/tools/libs/devicemodel/libxendevicemodel.map
@@ -42,4 +42,5 @@ VERS_1.3 {
 VERS_1.4 {
global:
xendevicemodel_set_irq_level;
+   xendevicemodel_nr_vcpus;
 } VERS_1.3;
diff --git a/xen/arch/arm/dm.c b/xen/arch/arm/dm.c
index 785413372c..d689e336fd 100644
--- a/xen/arch/arm/dm.c
+++ b/xen/arch/arm/dm.c
@@ -38,6 +38,7 @@ int dm_op(const struct dmop_args *op_args)
 [XEN_DMOP_set_ioreq_server_state]   = sizeof(struct 
xen_dm_op_set_ioreq_server_state),
 [XEN_DMOP_destroy_ioreq_server] = sizeof(struct 
xen_dm_op_destroy_ioreq_server),
 [XEN_DMOP_set_irq_level]= sizeof(struct 
xen_dm_op_set_irq_level),
+[XEN_DMOP_nr_vcpus] = sizeof(struct 
xen_dm_op_nr_vcpus),
 };
 
 rc = rcu_lock_remote_domain_by_id(op_args->domid, &d);
@@ -122,6 +123,15 @@ int dm_op(const struct dmop_args *op_args)
 break;
 }
 
+case XEN_DMOP_nr_vcpus:
+{
+struct xen_dm_op_nr_vcpus *data = &op.u.nr_vcpus;
+
+dat

Re: [PATCH] xen-netback: correct success/error reporting for the SKB-with-fraglist case

[Paul Durrant]

On 25/02/2021 14:00, Jan Beulich wrote:

On 25.02.2021 13:11, Paul Durrant wrote:

On 25/02/2021 07:33, Jan Beulich wrote:

On 24.02.2021 17:39, Paul Durrant wrote:

On 23/02/2021 16:29, Jan Beulich wrote:

When re-entering the main loop of xenvif_tx_check_gop() a 2nd time, the
special considerations for the head of the SKB no longer apply. Don't
mistakenly report ERROR to the frontend for the first entry in the list,
even if - from all I can tell - this shouldn't matter much as the overall
transmit will need to be considered failed anyway.

Signed-off-by: Jan Beulich 

--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -499,7 +499,7 @@ check_frags:
 * the header's copy failed, and they are
 * sharing a slot, send an error
 */
-   if (i == 0 && sharedslot)
+   if (i == 0 && !first_shinfo && sharedslot)
xenvif_idx_release(queue, pending_idx,
   XEN_NETIF_RSP_ERROR);
else



I think this will DTRT, but to my mind it would make more sense to clear
'sharedslot' before the 'goto check_frags' at the bottom of the function.


That was my initial idea as well, but
- I think it is for a reason that the variable is "const".
- There is another use of it which would then instead need further
amending (and which I believe is at least part of the reason for
the variable to be "const").



Oh, yes. But now that I look again, don't you want:

if (i == 0 && first_shinfo && sharedslot)

? (i.e no '!')

The comment states that the error should be indicated when the first
frag contains the header in the case that the map succeeded but the
prior copy from the same ref failed. This can only possibly be the case
if this is the 'first_shinfo'


I don't think so, no - there's a difference between "first frag"
(at which point first_shinfo is NULL) and first frag list entry
(at which point first_shinfo is non-NULL).


Yes, I realise I got it backwards. Confusing name but the comment above 
its declaration does explain.





(which is why I still think it is safe to unconst 'sharedslot' and
clear it).


And "no" here as well - this piece of code

/* First error: if the header haven't shared a slot with the
 * first frag, release it as well.
 */
if (!sharedslot)
xenvif_idx_release(queue,
   XENVIF_TX_CB(skb)->pending_idx,
   XEN_NETIF_RSP_OKAY);

specifically requires sharedslot to have the value that was
assigned to it at the start of the function (this property
doesn't go away when switching from fragments to frag list).
Note also how it uses XENVIF_TX_CB(skb)->pending_idx, i.e. the
value the local variable pending_idx was set from at the start
of the function.



True, we do have to deal with freeing up the header if the first map 
error comes on the frag list.


Reviewed-by: Paul Durrant 


Jan

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

[osstest service owner]

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

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  067935804a8e7a33ff7170a2db8ce94bb46d9a63
baseline version:
 xen  60390ccb8b9b2dbf85010f8b47779bb231aa2533

Last test of basis   159644  2021-02-24 19:01:47 Z0 days
Testing same since   159668  2021-02-25 13:01:28 Z0 days1 attempts


People who touched revisions under test:
  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
   60390ccb8b..067935804a  067935804a8e7a33ff7170a2db8ce94bb46d9a63 -> smoke

Re: [PATCH for-next 4/6] xen: Fix build when !CONFIG_GRANT_TABLE

[Jan Beulich]

On 25.02.2021 16:24, Connor Davis wrote:
> --- a/xen/include/xen/grant_table.h
> +++ b/xen/include/xen/grant_table.h
> @@ -66,6 +66,8 @@ int gnttab_acquire_resource(
>  
>  #define opt_max_grant_frames 0
>  
> +struct grant_table {};
> +
>  static inline int grant_table_init(struct domain *d,
> int max_grant_frames,
> int max_maptrack_frames)

You shouldn't actually declare a struct, all you need is to
move the forward decl further up in the file ahead of the
#ifdef.

Jan

Re: [PATCH for-next 3/6] xen/sched: Fix build when NR_CPUS == 1

[Jan Beulich]

On 25.02.2021 16:24, Connor Davis wrote:
> Return from cpu_schedule_up when either cpu is 0 or
> NR_CPUS == 1. This fixes the following:
> 
> core.c: In function 'cpu_schedule_up':
> core.c:2769:19: error: array subscript 1 is above array bounds
> of 'struct vcpu *[1]' [-Werror=array-bounds]
>  2769 | if ( idle_vcpu[cpu] == NULL )
>   |
> 
> Signed-off-by: Connor Davis 
> ---
>  xen/common/sched/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
> index 9745a77eee..f5ec65bf9b 100644
> --- a/xen/common/sched/core.c
> +++ b/xen/common/sched/core.c
> @@ -2763,7 +2763,7 @@ static int cpu_schedule_up(unsigned int cpu)
>  cpumask_set_cpu(cpu, &sched_res_mask);
>  
>  /* Boot CPU is dealt with later in scheduler_init(). */
> -if ( cpu == 0 )
> +if ( cpu == 0 || NR_CPUS == 1 )
>  return 0;
>  
>  if ( idle_vcpu[cpu] == NULL )

I'm not convinced a compiler warning is due here, and in turn
I'm not sure we want/need to work around this the way you do.
First question is whether that's just a specific compiler
version that's flawed. If it's not just a special case (e.g.
some unreleased version) we may want to think of possible
alternatives - the addition looks really odd to me.

Jan

Re: [PATCH for-next 2/6] xen/common: Guard iommu symbols with CONFIG_HAS_PASSTHROUGH

[Jan Beulich]

On 25.02.2021 16:24, Connor Davis wrote:
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -501,7 +501,9 @@ static int sanitise_domain_config(struct 
> xen_domctl_createdomain *config)
>  return -EINVAL;
>  }
>  
> +#ifdef CONFIG_HAS_PASSTHROUGH
>  if ( !iommu_enabled )
> +#endif
>  {
>  dprintk(XENLOG_INFO, "IOMMU requested but not available\n");
>  return -EINVAL;

Where possible - to avoid such #ifdef-ary - the symbol instead should
get #define-d to a sensible value ("false" in this case) in the header.
The other cases here may indeed need to remain as you have them.

Jan

[PATCH] xen-netback: use local var in xenvif_tx_check_gop() instead of re-calculating

[Jan Beulich]

shinfo already holds the result of skb_shinfo(skb) at this point - no
need to re-invoke the construct even twice.

Signed-off-by: Jan Beulich 

--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -557,8 +557,8 @@ check_frags:
}
 
if (skb_has_frag_list(skb) && !first_shinfo) {
-   first_shinfo = skb_shinfo(skb);
-   shinfo = skb_shinfo(skb_shinfo(skb)->frag_list);
+   first_shinfo = shinfo;
+   shinfo = skb_shinfo(shinfo->frag_list);
nr_frags = shinfo->nr_frags;
 
goto check_frags;

[PATCH for-next 0/6] Minimal build for RISCV

[Connor Davis]

Hi all,

This series introduces a minimal build for RISCV. It is based on Bobby's
previous work from last year[0]. I have worked to rebase onto current Xen,
as well as update the various header files borrowed from Linux.

This series provides the patches necessary to get a minimal build
working. The build is "minimal" in the sense that 1) it uses a
minimal config and 2) files, functions, and variables are included if
and only if they are required for a successful build based on the
config. It doesn't run at all, as the functions just have stub
implementations.

My hope is that this can serve as a useful example for future ports as
well as inform the community of exactly what is imposed by common code
onto new architectures.

The first 4 patches are mods to non-RISCV bits that enable building a
config with:

  !CONFIG_HAS_NS16550
  !CONFIG_HAS_PASSTHROUGH
  NR_CPUS == 1
  !CONFIG_GRANT_TABLE

respectively. The fifth patch adds the RISCV files, and the last patch
adds a docker container for doing the build. To build from the docker
container (after creating it locally), you can run the following:

  $ make XEN_TARGET_ARCH=riscv64 SUBSYSTEMS=xen 

The sources taken from Linux are documented in arch/riscv/README.sources.
There were also some files copied from arm:

  asm-arm/softirq.h
  asm-arm/random.h
  asm-arm/nospec.h
  asm-arm/numa.h
  asm-arm/p2m.h
  asm-arm/delay.h
  asm-arm/debugger.h
  asm-arm/desc.h
  asm-arm/guest_access.h
  asm-arm/hardirq.h
  lib/find_next_bit.c

I imagine some of these will want some consolidation, but I put them
under the respective RISCV directories for now.

[0] 
https://lore.kernel.org/xen-devel/cover.1579615303.git.bobbyeshle...@gmail.com/

Connor Davis (6):
  xen/char: Default HAS_NS16550 to y only for X86 and ARM
  xen/common: Guard iommu symbols with CONFIG_HAS_PASSTHROUGH
  xen/sched: Fix build when NR_CPUS == 1
  xen: Fix build when !CONFIG_GRANT_TABLE
  xen: Add files needed for minimal riscv build
  automation: add container for riscv64 builds

 automation/build/archlinux/riscv64.dockerfile |  32 ++
 automation/scripts/containerize   |   1 +
 config/riscv64.mk |   7 +
 xen/Makefile  |   8 +-
 xen/arch/riscv/Kconfig|  54 +++
 xen/arch/riscv/Kconfig.debug  |   0
 xen/arch/riscv/Makefile   |  57 +++
 xen/arch/riscv/README.source  |  19 +
 xen/arch/riscv/Rules.mk   |  13 +
 xen/arch/riscv/arch.mk|   7 +
 xen/arch/riscv/configs/riscv64_defconfig  |  12 +
 xen/arch/riscv/delay.c|  16 +
 xen/arch/riscv/domain.c   | 144 +++
 xen/arch/riscv/domctl.c   |  36 ++
 xen/arch/riscv/guestcopy.c|  57 +++
 xen/arch/riscv/head.S |   6 +
 xen/arch/riscv/irq.c  |  78 
 xen/arch/riscv/lib/Makefile   |   1 +
 xen/arch/riscv/lib/find_next_bit.c| 284 +
 xen/arch/riscv/mm.c   |  93 +
 xen/arch/riscv/p2m.c  | 150 +++
 xen/arch/riscv/percpu.c   |  17 +
 xen/arch/riscv/platforms/Kconfig  |  31 ++
 xen/arch/riscv/riscv64/asm-offsets.c  |  31 ++
 xen/arch/riscv/setup.c|  27 ++
 xen/arch/riscv/shutdown.c |  28 ++
 xen/arch/riscv/smp.c  |  35 ++
 xen/arch/riscv/smpboot.c  |  34 ++
 xen/arch/riscv/sysctl.c   |  33 ++
 xen/arch/riscv/time.c |  35 ++
 xen/arch/riscv/traps.c|  35 ++
 xen/arch/riscv/vm_event.c |  39 ++
 xen/arch/riscv/xen.lds.S  | 113 ++
 xen/common/domain.c   |   2 +
 xen/common/memory.c   |  10 +
 xen/common/sched/core.c   |   2 +-
 xen/common/sysctl.c   |   2 +
 xen/drivers/char/Kconfig  |   2 +-
 xen/drivers/char/serial.c |   1 +
 xen/include/asm-riscv/altp2m.h|  39 ++
 xen/include/asm-riscv/asm.h   |  77 
 xen/include/asm-riscv/asm_defns.h |  24 ++
 xen/include/asm-riscv/atomic.h| 204 ++
 xen/include/asm-riscv/bitops.h| 331 +++
 xen/include/asm-riscv/bug.h   |  54 +++
 xen/include/asm-riscv/byteorder.h |  16 +
 xen/include/asm-riscv/cache.h |  24 ++
 xen/include/asm-riscv/cmpxchg.h   | 382 ++
 xen/include/asm-riscv/compiler_types.h|  32 ++
 xen/include/asm-riscv/config.h| 110 +
 xen/include/asm-riscv/cpufeature.h|  17 +
 xen/include/asm-riscv/csr.h   | 219 ++
 xen

[PATCH for-next 6/6] automation: add container for riscv64 builds

[Connor Davis]

Add a container for cross-compiling xen to riscv64.
This just includes the cross-compiler and necessary packages for
building xen itself (packages for tools, stubdoms, etc., can be
added later).

To build xen in the container run the following:

$ make XEN_TARGET_ARCH=riscv64 SUBSYSTEMS=xen

Signed-off-by: Connor Davis 
---
 automation/build/archlinux/riscv64.dockerfile | 32 +++
 automation/scripts/containerize   |  1 +
 2 files changed, 33 insertions(+)
 create mode 100644 automation/build/archlinux/riscv64.dockerfile

diff --git a/automation/build/archlinux/riscv64.dockerfile 
b/automation/build/archlinux/riscv64.dockerfile
new file mode 100644
index 00..d94048b6c3
--- /dev/null
+++ b/automation/build/archlinux/riscv64.dockerfile
@@ -0,0 +1,32 @@
+FROM archlinux/base
+LABEL maintainer.name="The Xen Project" \
+  maintainer.email="xen-devel@lists.xenproject.org"
+
+# Packages needed for the build
+RUN pacman --noconfirm -Syu \
+base-devel \
+gcc \
+git
+
+# Packages needed for QEMU
+RUN pacman --noconfirm -Syu \
+pixman \
+python \
+sh
+
+# There is a regression in GDB that causes an assertion error
+# when setting breakpoints, use this commit until it is fixed!
+RUN git clone --recursive -j$(nproc) --progress 
https://github.com/riscv/riscv-gnu-toolchain && \
+cd riscv-gnu-toolchain/riscv-gdb && \
+git checkout 1dd588507782591478882a891f64945af9e2b86c && \
+cd  .. && \
+./configure --prefix=/opt/riscv && \
+make linux -j$(nproc) && \
+rm -R /riscv-gnu-toolchain
+
+# Add compiler path
+ENV PATH=/opt/riscv/bin/:${PATH}
+
+RUN useradd --create-home user
+USER user
+WORKDIR /build
diff --git a/automation/scripts/containerize b/automation/scripts/containerize
index da45baed4e..1901e8c0ef 100755
--- a/automation/scripts/containerize
+++ b/automation/scripts/containerize
@@ -25,6 +25,7 @@ die() {
 BASE="registry.gitlab.com/xen-project/xen"
 case "_${CONTAINER}" in
 _archlinux|_arch) CONTAINER="${BASE}/archlinux:current" ;;
+_riscv64) CONTAINER="${BASE}/archlinux:riscv64" ;;
 _centos7) CONTAINER="${BASE}/centos:7" ;;
 _centos72) CONTAINER="${BASE}/centos:7.2" ;;
 _fedora) CONTAINER="${BASE}/fedora:29";;
-- 
2.27.0

[PATCH for-next 4/6] xen: Fix build when !CONFIG_GRANT_TABLE

[Connor Davis]

Declare struct grant_table {}; in grant_table.h when
!CONFIG_GRANT_TABLE. This fixes the following:

/build/xen/include/xen/grant_table.h:84:50: error: 'struct grant_table'
declared inside parameter list will not be visible outside of this
definition or declaration [-Werror]
   84 | static inline int mem_sharing_gref_to_gfn(struct grant_table *gt,
  |

Signed-off-by: Connor Davis 
---
 xen/include/xen/grant_table.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h
index 63b6dc78f4..0e5f6f85c7 100644
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -66,6 +66,8 @@ int gnttab_acquire_resource(
 
 #define opt_max_grant_frames 0
 
+struct grant_table {};
+
 static inline int grant_table_init(struct domain *d,
int max_grant_frames,
int max_maptrack_frames)
-- 
2.27.0

[PATCH for-next 3/6] xen/sched: Fix build when NR_CPUS == 1

[Connor Davis]

Return from cpu_schedule_up when either cpu is 0 or
NR_CPUS == 1. This fixes the following:

core.c: In function 'cpu_schedule_up':
core.c:2769:19: error: array subscript 1 is above array bounds
of 'struct vcpu *[1]' [-Werror=array-bounds]
 2769 | if ( idle_vcpu[cpu] == NULL )
  |

Signed-off-by: Connor Davis 
---
 xen/common/sched/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
index 9745a77eee..f5ec65bf9b 100644
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -2763,7 +2763,7 @@ static int cpu_schedule_up(unsigned int cpu)
 cpumask_set_cpu(cpu, &sched_res_mask);
 
 /* Boot CPU is dealt with later in scheduler_init(). */
-if ( cpu == 0 )
+if ( cpu == 0 || NR_CPUS == 1 )
 return 0;
 
 if ( idle_vcpu[cpu] == NULL )
-- 
2.27.0

[PATCH for-next 1/6] xen/char: Default HAS_NS16550 to y only for X86 and ARM

[Connor Davis]

Defaulting to yes only for X86 and ARM reduces the requirements
for a minimal build when porting new architectures.

Signed-off-by: Connor Davis 
---
 xen/drivers/char/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig
index b572305657..b15b0c8d6a 100644
--- a/xen/drivers/char/Kconfig
+++ b/xen/drivers/char/Kconfig
@@ -1,6 +1,6 @@
 config HAS_NS16550
bool "NS16550 UART driver" if ARM
-   default y
+   default y if (ARM || X86)
help
  This selects the 16550-series UART support. For most systems, say Y.
 
-- 
2.27.0

[PATCH for-next 2/6] xen/common: Guard iommu symbols with CONFIG_HAS_PASSTHROUGH

[Connor Davis]

The variables iommu_enabled and iommu_dont_flush_iotlb are defined in
drivers/passthrough/iommu.c and are referenced in common code, which
causes the link to fail when !CONFIG_HAS_PASSTHROUGH.

Guard references to these variables in common code so that xen
builds when !CONFIG_HAS_PASSTHROUGH.

Signed-off-by: Connor Davis 
---
 xen/common/domain.c |  2 ++
 xen/common/memory.c | 10 ++
 xen/common/sysctl.c |  2 ++
 3 files changed, 14 insertions(+)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index d85984638a..ad66bca325 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -501,7 +501,9 @@ static int sanitise_domain_config(struct 
xen_domctl_createdomain *config)
 return -EINVAL;
 }
 
+#ifdef CONFIG_HAS_PASSTHROUGH
 if ( !iommu_enabled )
+#endif
 {
 dprintk(XENLOG_INFO, "IOMMU requested but not available\n");
 return -EINVAL;
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 76b9f58478..7135324857 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -294,7 +294,9 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
 p2m_type_t p2mt;
 #endif
 mfn_t mfn;
+#ifdef CONFIG_HAS_PASSTHROUGH
 bool *dont_flush_p, dont_flush;
+#endif
 int rc;
 
 #ifdef CONFIG_X86
@@ -385,13 +387,17 @@ int guest_remove_page(struct domain *d, unsigned long 
gmfn)
  * Since we're likely to free the page below, we need to suspend
  * xenmem_add_to_physmap()'s suppressing of IOMMU TLB flushes.
  */
+#ifdef CONFIG_HAS_PASSTHROUGH
 dont_flush_p = &this_cpu(iommu_dont_flush_iotlb);
 dont_flush = *dont_flush_p;
 *dont_flush_p = false;
+#endif
 
 rc = guest_physmap_remove_page(d, _gfn(gmfn), mfn, 0);
 
+#ifdef CONFIG_HAS_PASSTHROUGH
 *dont_flush_p = dont_flush;
+#endif
 
 /*
  * With the lack of an IOMMU on some platforms, domains with DMA-capable
@@ -839,11 +845,13 @@ int xenmem_add_to_physmap(struct domain *d, struct 
xen_add_to_physmap *xatp,
 xatp->gpfn += start;
 xatp->size -= start;
 
+#ifdef CONFIG_HAS_PASSTHROUGH
 if ( is_iommu_enabled(d) )
 {
this_cpu(iommu_dont_flush_iotlb) = 1;
extra.ppage = &pages[0];
 }
+#endif
 
 while ( xatp->size > done )
 {
@@ -868,6 +876,7 @@ int xenmem_add_to_physmap(struct domain *d, struct 
xen_add_to_physmap *xatp,
 }
 }
 
+#ifdef CONFIG_HAS_PASSTHROUGH
 if ( is_iommu_enabled(d) )
 {
 int ret;
@@ -894,6 +903,7 @@ int xenmem_add_to_physmap(struct domain *d, struct 
xen_add_to_physmap *xatp,
 if ( unlikely(ret) && rc >= 0 )
 rc = ret;
 }
+#endif
 
 return rc;
 }
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 3558641cd9..b4dde7bef6 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -271,12 +271,14 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) 
u_sysctl)
 pi->cpu_khz = cpu_khz;
 pi->max_mfn = get_upper_mfn_bound();
 arch_do_physinfo(pi);
+#ifdef CONFIG_HAS_PASSTHROUGH
 if ( iommu_enabled )
 {
 pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio;
 if ( iommu_hap_pt_share )
 pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
 }
+#endif
 if ( vmtrace_available )
 pi->capabilities |= XEN_SYSCTL_PHYSCAP_vmtrace;
 
-- 
2.27.0

Re: [PATCH for-4.15] automation: Fix containerize to understand the Alpine container

[Ian Jackson]

Andrew Cooper writes ("[PATCH for-4.15] automation: Fix containerize to 
understand the Alpine container"):
> This was missing from the work to add the alpine container.
> 
> Fixes: a9afe7768bd ("automation: add alpine linux 3.12 x86 build container")
> Signed-off-by: Andrew Cooper 

Release-Acked-by: Ian Jackson 

[PATCH for-4.15] automation: Fix containerize to understand the Alpine container

[Andrew Cooper]

This was missing from the work to add the alpine container.

Fixes: a9afe7768bd ("automation: add alpine linux 3.12 x86 build container")
Signed-off-by: Andrew Cooper 
---
CC: Wei Liu 
CC: Doug Goldstein 
CC: Ian Jackson 

For 4.15.  This is only developer tooling, with no impact on the build.
---
 automation/scripts/containerize | 1 +
 1 file changed, 1 insertion(+)

diff --git a/automation/scripts/containerize b/automation/scripts/containerize
index da45baed4e..b7c81559fb 100755
--- a/automation/scripts/containerize
+++ b/automation/scripts/containerize
@@ -24,6 +24,7 @@ die() {
 #
 BASE="registry.gitlab.com/xen-project/xen"
 case "_${CONTAINER}" in
+_alpine) CONTAINER="${BASE}/alpine:3.12" ;;
 _archlinux|_arch) CONTAINER="${BASE}/archlinux:current" ;;
 _centos7) CONTAINER="${BASE}/centos:7" ;;
 _centos72) CONTAINER="${BASE}/centos:7.2" ;;
-- 
2.11.0

Re: [PATCH for-4.15] xen/sched: Add missing memory barrier in vcpu_block()

[Julien Grall]

On 23/02/2021 13:24, Ash Wilding wrote:

Hi Julien,


Hi Ash,


Thanks for looking at this,


vcpu_block() is now gaining an smp_mb__after_atomic() to prevent the
CPU to read any information about local events before the flag
_VPF_blocked is set.


Reviewed-by: Ash Wilding 


Thanks!




As an aside,


I couldn't convince myself whether the Arm implementation of
local_events_need_delivery() contains enough barrier to prevent the
re-ordering. However, I don't think we want to play with devil here
as the function may be optimized in the future.


Agreed.

The vgic_vcpu_pending_irq() and vgic_evtchn_irq_pending() in the call
path of local_events_need_delivery() both call spin_lock_irqsave(),
which has an arch_lock_acquire_barrier() in its call path.

That just happens to map to a heavier smp_mb() on Arm right now, but
relying on this behaviour would be shaky; I can imagine a future update
to arch_lock_acquire_barrier() that relaxes it down to just acquire
semantics like its name implies (for example an LSE-based lock_acquire()
using LDUMAXA),in which case any code incorrectly relying on that full
barrier behaviour may break. I'm guessing this is what you meant by the
function may be optimized in future?


That's one of the optimization I had in mind. The other one is we may 
find a way to remove the spinlocks, so the barriers would disappear 
completely.




Do we know whether there is an expectation for previous loads/stores
to have been observed before local_events_need_delivery()? I'm wondering
whether it would make sense to have an smb_mb() at the start of the
*_nomask() variant in local_events_need_delivery()'s call path.


That's a good question :). For Arm, there are 4 users of 
local_events_need_delivery():

  1) do_poll()
  2) vcpu_block()
  3) hypercall_preempt_check()
  4) general_preempt_check()

3 and 4 are used for breaking down long running operations. I guess we 
would want to have an accurate view of the pending events and therefore 
we would need a memory barrier to prevent the loads happening too early.


In this case, I think the smp_mb() would want to be part of the 
hypercall_preempt_check() and general_preempt_check().


Therefore, I think we want to avoid the extra barrier in 
local_events_need_delivery(). Instead, we should require the caller to 
take care of the ordering if needed.


This would have benefits any new architecture as the common code would 
already contain the appropriate barriers.


@Stefano, what do you think?

Cheers,

--
Julien Grall

Re: [PATCH] xen-netback: correct success/error reporting for the SKB-with-fraglist case

[Jan Beulich]

On 25.02.2021 13:11, Paul Durrant wrote:
> On 25/02/2021 07:33, Jan Beulich wrote:
>> On 24.02.2021 17:39, Paul Durrant wrote:
>>> On 23/02/2021 16:29, Jan Beulich wrote:
 When re-entering the main loop of xenvif_tx_check_gop() a 2nd time, the
 special considerations for the head of the SKB no longer apply. Don't
 mistakenly report ERROR to the frontend for the first entry in the list,
 even if - from all I can tell - this shouldn't matter much as the overall
 transmit will need to be considered failed anyway.

 Signed-off-by: Jan Beulich 

 --- a/drivers/net/xen-netback/netback.c
 +++ b/drivers/net/xen-netback/netback.c
 @@ -499,7 +499,7 @@ check_frags:
 * the header's copy failed, and they 
 are
 * sharing a slot, send an error
 */
 -  if (i == 0 && sharedslot)
 +  if (i == 0 && !first_shinfo && sharedslot)
xenvif_idx_release(queue, 
 pending_idx,
   
 XEN_NETIF_RSP_ERROR);
else

>>>
>>> I think this will DTRT, but to my mind it would make more sense to clear
>>> 'sharedslot' before the 'goto check_frags' at the bottom of the function.
>>
>> That was my initial idea as well, but
>> - I think it is for a reason that the variable is "const".
>> - There is another use of it which would then instead need further
>>amending (and which I believe is at least part of the reason for
>>the variable to be "const").
>>
> 
> Oh, yes. But now that I look again, don't you want:
> 
> if (i == 0 && first_shinfo && sharedslot)
> 
> ? (i.e no '!')
> 
> The comment states that the error should be indicated when the first 
> frag contains the header in the case that the map succeeded but the 
> prior copy from the same ref failed. This can only possibly be the case 
> if this is the 'first_shinfo'

I don't think so, no - there's a difference between "first frag"
(at which point first_shinfo is NULL) and first frag list entry
(at which point first_shinfo is non-NULL).

> (which is why I still think it is safe to unconst 'sharedslot' and
> clear it).

And "no" here as well - this piece of code

/* First error: if the header haven't shared a slot with the
 * first frag, release it as well.
 */
if (!sharedslot)
xenvif_idx_release(queue,
   XENVIF_TX_CB(skb)->pending_idx,
   XEN_NETIF_RSP_OKAY);

specifically requires sharedslot to have the value that was
assigned to it at the start of the function (this property
doesn't go away when switching from fragments to frag list).
Note also how it uses XENVIF_TX_CB(skb)->pending_idx, i.e. the
value the local variable pending_idx was set from at the start
of the function.

Jan

[xen-unstable test] 159652: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemut-ws16-amd64  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-xl-qemuu-debianhvm-amd64  8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-shadow 8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-qemut-rhel6hvm-intel  8 xen-boot fail REGR. vs. 159475
 test-xtf-amd64-amd64-3  19 xtf/test-pv32pae-selftest fail REGR. vs. 159475
 test-xtf-amd64-amd64-1  19 xtf/test-pv32pae-selftest fail REGR. vs. 159475
 test-amd64-i386-xl-qemut-debianhvm-amd64  8 xen-boot fail REGR. vs. 159475
 test-xtf-amd64-amd64-4  19 xtf/test-pv32pae-selftest fail REGR. vs. 159475
 test-xtf-amd64-amd64-2  19 xtf/test-pv32pae-selftest fail REGR. vs. 159475
 test-xtf-amd64-amd64-5  19 xtf/test-pv32pae-selftest fail REGR. vs. 159475
 test-amd64-i386-libvirt-xsm   8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-raw8 xen-boot fail REGR. vs. 159475
 test-amd64-coresched-i386-xl  8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-migrupgrade  13 xen-boot/dst_hostfail REGR. vs. 159475
 test-amd64-i386-xl-xsm8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-qemuu-ovmf-amd64  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 8 xen-boot fail REGR. vs. 
159475
 test-amd64-i386-qemut-rhel6hvm-amd  8 xen-boot   fail REGR. vs. 159475
 test-amd64-i386-xl8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-libvirt-pair 12 xen-boot/src_hostfail REGR. vs. 159475
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 8 xen-boot fail REGR. vs. 
159475
 test-amd64-i386-libvirt-pair 13 xen-boot/dst_hostfail REGR. vs. 159475
 test-amd64-i386-qemuu-rhel6hvm-amd  8 xen-boot   fail REGR. vs. 159475
 test-amd64-i386-xl-qemut-win7-amd64  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-freebsd10-amd64  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-freebsd10-i386  8 xen-boot   fail REGR. vs. 159475
 test-amd64-i386-examine   8 reboot   fail REGR. vs. 159475
 test-amd64-i386-libvirt   8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-livepatch 8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 8 xen-boot fail REGR. vs. 
159475
 test-amd64-i386-qemuu-rhel6hvm-intel  8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-qemuu-ws16-amd64  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-pair 12 xen-boot/src_hostfail REGR. vs. 159475
 test-amd64-i386-pair 13 xen-boot/dst_hostfail REGR. vs. 159475
 test-amd64-i386-xl-pvshim 8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 8 xen-boot fail REGR. vs. 
159475
 test-amd64-i386-xl-qemuu-win7-amd64  8 xen-boot  fail REGR. vs. 159475

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 159475
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 159475
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 159475
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 159475
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 159475
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 159475
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 159475
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  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-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
 te

[4.15] Re: [PATCH] x86/EFI: suppress GNU ld 2.36'es creation of base relocs

[Ian Jackson]

Jan Beulich writes ("[4.15] Re: [PATCH] x86/EFI: suppress GNU ld 2.36'es 
creation of base relocs"):
> Since getting Andrew's ack has taken across the firm-freeze boundary,
> may I ask for a release-ack here?

Indeed.

Release-Acked-by: Ian Jackson 

>  As noted this change (alongside
> the earlier one) will want backporting, perhaps even to security-
> support-only branches.

Noted.

Ian.

Re: [PATCH][4.15] x86/shadow: suppress "fast fault path" optimization without reserved bits

[Jan Beulich]

On 25.02.2021 14:17, Ian Jackson wrote:
> Jan Beulich writes ("[PATCH][4.15] x86/shadow: suppress "fast fault path" 
> optimization without reserved bits"):
>> When none of the physical address bits in PTEs are reserved, we can't
>> create any 4k (leaf) PTEs which would trigger reserved bit faults. Hence
>> the present SHOPT_FAST_FAULT_PATH machinery needs to be suppressed in
>> this case, which is most easily achieved by never creating any magic
>> entries.
>>
>> To compensate a little, eliminate sh_write_p2m_entry_post()'s impact on
>> such hardware.
>>
>> While at it, also avoid using an MMIO magic entry when that would
>> truncate the incoming GFN.
> 
> Judging by the description I'm not sure whether this is a bugfix, or
> a change to make it possible to run Xen on hardware where currently it
> doesn't work at all.

It's still a bug fix imo, even if the flawed assumption was harmless
so far.

> I assume "none of the physical address bits in PTEs are reserved" is
> a property of certain hardware, but it wasn't clear to me (i) whether
> such platforms currently exists

If they don't exist yet, they're soon to become available afaict.

> (ii) what the existing Xen code would do in this case.

If memory is populated at the top 4Gb of physical address space,
guests would gain access to that memory through these page table
entries, as those don't cause the expected faults to be raised
anymore (but instead get used for valid - from the hardware's
perspective - translations).

Jan

Re: [PATCH][4.15] x86/shadow: suppress "fast fault path" optimization without reserved bits

[Jan Beulich]

On 25.02.2021 14:20, Ian Jackson wrote:
> Jan Beulich writes ("Re: [PATCH][4.15] x86/shadow: suppress "fast fault path" 
> optimization without reserved bits"):
>> As to 4.15: Without this shadow mode simply won't work on such (new)
>> hardware. Hence something needs to be done anyway. An alternative
>> would be to limit the change to just the guest-no-present entries (to
>> at least allow PV guests to be migrated), and refuse to enable shadow
>> mode for HVM guests on such hardware. (In this case we'd probably
>> better take care of ...
> 
> Thanks for this explanation.
> 
> It sounds like the way you have it in this proposed patch is simpler
> than the alternative.  And that right now it's not a regression, but
> it is needed for running Xen on such newer hardware.

I'm not sure about the "simpler" part.

>> The main risk here is (in particular for the MMIO part of the change
>> I suppose) execution suddenly going a different path, which has been
>> unused / untested (for this specific case) for years.
> 
> That's somewhat concerning.  But I think this only applies to the new
> hardware ?  So it would be risking an XSA but not really risking the
> release very much.

Right - afaict an XSA would also be lurking without us doing anything,
as we'd permit a guest access to pages we didn't mean to hand to it.

> I think therefore:
> 
> Release-Acked-by: Ian Jackson 

Thanks.

Jan

Re: [for-4.15][RESEND PATCH v4 2/2] xen/iommu: x86: Clear the root page-table before freeing the page-tables

[Jan Beulich]

On 25.02.2021 13:01, Julien Grall wrote:
> On 24/02/2021 16:00, Jan Beulich wrote:
>> On 24.02.2021 16:58, Jan Beulich wrote:
>>> On 24.02.2021 10:43, Julien Grall wrote:
 --- a/xen/drivers/passthrough/x86/iommu.c
 +++ b/xen/drivers/passthrough/x86/iommu.c
 @@ -149,6 +149,13 @@ int arch_iommu_domain_init(struct domain *d)
   
   void arch_iommu_domain_destroy(struct domain *d)
   {
 +/*
 + * There should be not page-tables left allocated by the time the
 + * domain is destroyed. Note that arch_iommu_domain_destroy() is
 + * called unconditionally, so pgtables may be unitialized.
 + */
 +ASSERT(dom_iommu(d)->platform_ops == NULL ||
>>>
>>> Since you've used the preferred ! instead of "== 0" /
>>> "== NULL" in the ASSERT()s you add further up, may I ask that
>>> you do so here as well?
>>
>> Oh, and I meant to provide
>> Reviewed-by: Jan Beulich 
>> preferably with that cosmetic adjustment (and ideally also with
>> "uninitialized" in the comment, as I notice only now).
> 
> I don't seem to find your original answer in my inbox and on lore [1]. 
> Could you confirm if the two comments visible in this thread are the 
> only one you made on this patch?

Oh, yes - what I look to have done is to reply to a draft that was
never sent. There indeed was just what's visible above - thanks for
double checking.

Jan

Re: [PATCH][4.15] x86/shadow: suppress "fast fault path" optimization without reserved bits

[Ian Jackson]

Jan Beulich writes ("Re: [PATCH][4.15] x86/shadow: suppress "fast fault path" 
optimization without reserved bits"):
> As to 4.15: Without this shadow mode simply won't work on such (new)
> hardware. Hence something needs to be done anyway. An alternative
> would be to limit the change to just the guest-no-present entries (to
> at least allow PV guests to be migrated), and refuse to enable shadow
> mode for HVM guests on such hardware. (In this case we'd probably
> better take care of ...

Thanks for this explanation.

It sounds like the way you have it in this proposed patch is simpler
than the alternative.  And that right now it's not a regression, but
it is needed for running Xen on such newer hardware.

> The main risk here is (in particular for the MMIO part of the change
> I suppose) execution suddenly going a different path, which has been
> unused / untested (for this specific case) for years.

That's somewhat concerning.  But I think this only applies to the new
hardware ?  So it would be risking an XSA but not really risking the
release very much.

I think therefore:

Release-Acked-by: Ian Jackson 

Ian.

Re: [for-4.15][RESEND PATCH v4 1/2] xen/x86: iommu: Ignore IOMMU mapping requests when a domain is dying

[Jan Beulich]

On 25.02.2021 12:56, Julien Grall wrote:
> On 24/02/2021 14:07, Jan Beulich wrote:
>> On 24.02.2021 10:43, Julien Grall wrote:
>>> --- a/xen/drivers/passthrough/x86/iommu.c
>>> +++ b/xen/drivers/passthrough/x86/iommu.c
>>> @@ -267,6 +267,12 @@ int iommu_free_pgtables(struct domain *d)
>>>   struct page_info *pg;
>>>   unsigned int done = 0;
>>>   
>>> +if ( !is_iommu_enabled(d) )
>>> +return 0;
>>
>> Why is this addition needed? Hitting a not yet initialize spin lock
>> is - afaict - no worse than a not yet initialized list, so it would
>> seem to me that this can't be the reason. No other reason looks to
>> be called out by the description.
> 
> struct domain_iommu will be initially zeroed as it is part of struct domain.
> 
> For the list, we are so far fine because page_list_remove_head() 
> tolerates NULL. If we were using the normal list operations (e.g. 
> list_del), then this code would have segfaulted.

And so we do, in the CONFIG_BIGMEM case. May I suggest then to split
this out as a prereq patch, or add wording to the description
mentioning this additional effect?

Jan

Re: [PATCH][4.15] x86/shadow: suppress "fast fault path" optimization without reserved bits

[Ian Jackson]

Jan Beulich writes ("[PATCH][4.15] x86/shadow: suppress "fast fault path" 
optimization without reserved bits"):
> When none of the physical address bits in PTEs are reserved, we can't
> create any 4k (leaf) PTEs which would trigger reserved bit faults. Hence
> the present SHOPT_FAST_FAULT_PATH machinery needs to be suppressed in
> this case, which is most easily achieved by never creating any magic
> entries.
> 
> To compensate a little, eliminate sh_write_p2m_entry_post()'s impact on
> such hardware.
> 
> While at it, also avoid using an MMIO magic entry when that would
> truncate the incoming GFN.

Judging by the description I'm not sure whether this is a bugfix, or
a change to make it possible to run Xen on hardware where currently it
doesn't work at all.

I assume "none of the physical address bits in PTEs are reserved" is
a property of certain hardware, but it wasn't clear to me (i) whether
such platforms currently exists (ii) what the existing Xen code would
do in this case.

Can you enlighten me ?

Thanks,
Ian.

Re: [PATCH][4.15] x86/shadow: suppress "fast fault path" optimization without reserved bits

[Jan Beulich]

On 25.02.2021 14:03, Jan Beulich wrote:
> When none of the physical address bits in PTEs are reserved, we can't
> create any 4k (leaf) PTEs which would trigger reserved bit faults. Hence
> the present SHOPT_FAST_FAULT_PATH machinery needs to be suppressed in
> this case, which is most easily achieved by never creating any magic
> entries.
> 
> To compensate a little, eliminate sh_write_p2m_entry_post()'s impact on
> such hardware.

As to 4.15: Without this shadow mode simply won't work on such (new)
hardware. Hence something needs to be done anyway. An alternative
would be to limit the change to just the guest-no-present entries (to
at least allow PV guests to be migrated), and refuse to enable shadow
mode for HVM guests on such hardware. (In this case we'd probably
better take care of ...

> While at it, also avoid using an MMIO magic entry when that would
> truncate the incoming GFN.

... this long-standing issue, perhaps as outlined in a post-commit-
message remark.)

The main risk here is (in particular for the MMIO part of the change
I suppose) execution suddenly going a different path, which has been
unused / untested (for this specific case) for years.

Jan

Re: [PATCH v2 0/7] Xen guest loader (to boot Xen+Kernel under TCG)

[Alex Bennée]

Alex Bennée  writes:

> Hi,
>
> These patches have been sitting around as part of a larger series to
> improve the support of Xen on AArch64. The second part of the work is
> currently awaiting other re-factoring and build work to go in to make
> the building of a pure-Xen capable QEMU easier. As that might take
> some time and these patches are essentially ready I thought I'd best
> push for merging them.
>
> There are no fundamental changes since the last revision. I've
> addressed most of the comments although I haven't expanded the use of
> the global *fdt to other device models. I figured that work could be
> done as and when models have support for type-1 hypervisors.
>
> I have added some documentation to describe the feature and added an
> acceptance tests which checks the various versions of Xen can boot.
> The only minor wrinkle is using a custom compiled Linux kernel due to
> missing support in the distro kernels. If anyone can suggest a distro
> which is currently well supported for Xen on AArch64 I can update the
> test.
>
> The following patches still need review:
>
>  - tests/avocado: add boot_xen tests
>  - docs: add some documentation for the guest-loader
>  - docs: move generic-loader documentation into the main manual
>  - hw/core: implement a guest-loader to support static hypervisor guests
>
> Alex Bennée (7):
>   hw/board: promote fdt from ARM VirtMachineState to MachineState
>   hw/riscv: migrate fdt field to generic MachineState
>   device_tree: add qemu_fdt_setprop_string_array helper
>   hw/core: implement a guest-loader to support static hypervisor
> guests

Gentle ping. They all have reviews apart from the core bit of loader
code and I'd like to merge this cycle ;-)

-- 
Alex Bennée

[xen-unstable bisection] complete test-xtf-amd64-amd64-3

[osstest service owner]

branch xen-unstable
xenbranch xen-unstable
job test-xtf-amd64-amd64-3
testid xtf/test-pv32pae-selftest

Tree: linux git://xenbits.xen.org/linux-pvops.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git
Tree: xtf git://xenbits.xen.org/xtf.git

*** Found and reproduced problem changeset ***

  Bug is in tree:  xen git://xenbits.xen.org/xen.git
  Bug introduced:  4dc1815991420b809ce18dddfdf9c0af48944204
  Bug not present: 2d824791504f4119f04f95bafffec2e37d319c25
  Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/159667/


  commit 4dc1815991420b809ce18dddfdf9c0af48944204
  Author: Jan Beulich 
  Date:   Fri Feb 19 17:19:56 2021 +0100
  
  x86/PV: harden guest memory accesses against speculative abuse
  
  Inspired by
  
https://lore.kernel.org/lkml/f12e7d3cecf41b2c29734ea45a393be21d4a8058.1597848273.git.jpoim...@redhat.com/
  and prior work in that area of x86 Linux, suppress speculation with
  guest specified pointer values by suitably masking the addresses to
  non-canonical space in case they fall into Xen's virtual address range.
  
  Introduce a new Kconfig control.
  
  Note that it is necessary in such code to avoid using "m" kind operands:
  If we didn't, there would be no guarantee that the register passed to
  guest_access_mask_ptr is also the (base) one used for the memory access.
  
  As a minor unrelated change in get_unsafe_asm() the unnecessary "itype"
  parameter gets dropped and the XOR on the fixup path gets changed to be
  a 32-bit one in all cases: This way we avoid pointless REX.W or operand
  size overrides, or writes to partial registers.
  
  Requested-by: Andrew Cooper 
  Signed-off-by: Jan Beulich 
  Reviewed-by: Roger Pau Monné 
  Release-Acked-by: Ian Jackson 


For bisection revision-tuple graph see:
   
http://logs.test-lab.xenproject.org/osstest/results/bisect/xen-unstable/test-xtf-amd64-amd64-3.xtf--test-pv32pae-selftest.html
Revision IDs in each graph node refer, respectively, to the Trees above.


Running cs-bisection-step 
--graph-out=/home/logs/results/bisect/xen-unstable/test-xtf-amd64-amd64-3.xtf--test-pv32pae-selftest
 --summary-out=tmp/159667.bisection-summary --basis-template=159475 
--blessings=real,real-bisect,real-retry xen-unstable test-xtf-amd64-amd64-3 
xtf/test-pv32pae-selftest
Searching for failure / basis pass:
 159626 fail [host=chardonnay0] / 159475 [host=godello1] 159453 [host=fiano0] 
159424 [host=albana0] 159396 [host=fiano1] 159362 [host=godello0] 159335 
[host=albana0] 159315 [host=huxelrebe1] 159202 ok.
Failure / basis pass flights: 159626 / 159202
(tree with no url: minios)
(tree with no url: ovmf)
(tree with no url: seabios)
Tree: linux git://xenbits.xen.org/linux-pvops.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git
Tree: xtf git://xenbits.xen.org/xtf.git
Latest c3038e718a19fc596f7b1baba0f83d5146dc7784 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
5d94433a66df29ce314696a13bdd324ec0e342fe 
8ab15139728a8efd3ebbb60beb16a958a6a93fa1
Basis pass c3038e718a19fc596f7b1baba0f83d5146dc7784 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
687121f8a0e7c1ea1c4fa3d056637e5819342f14 
8ab15139728a8efd3ebbb60beb16a958a6a93fa1
Generating revisions with ./adhoc-revtuple-generator  
git://xenbits.xen.org/linux-pvops.git#c3038e718a19fc596f7b1baba0f83d5146dc7784-c3038e718a19fc596f7b1baba0f83d5146dc7784
 
git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860
 
git://xenbits.xen.org/qemu-xen-traditional.git#3d273dd05e51e5a1ffba3d98c7437ee84e8f8764-3d273dd05e51e5a1ffba3d98c7437ee84e8f8764
 git://xenbits.xen.org/qemu-xen.git#7ea428895af2840d85c524f0bd11a38\
 aac308308-7ea428895af2840d85c524f0bd11a38aac308308 
git://xenbits.xen.org/xen.git#687121f8a0e7c1ea1c4fa3d056637e5819342f14-5d94433a66df29ce314696a13bdd324ec0e342fe
 
git://xenbits.xen.org/xtf.git#8ab15139728a8efd3ebbb60beb16a958a6a93fa1-8ab15139728a8efd3ebbb60beb16a958a6a93fa1
Loaded 5001 nodes in revision graph
Searching for test results:
 159202 pass c3038e718a19fc596f7b1baba0f83d5146dc7784 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
687121f8a0e7c1ea1c4fa3d056637e5819342f14 
8ab15139728a8efd3ebbb60beb16a958a6a93fa1
 159315 [host=huxelrebe1]
 159335 [host=albana0]
 159362 [host=godello0]
 159396 [host=fiano1]
 159424 [host=albana0

[PATCH][4.15] x86/shadow: suppress "fast fault path" optimization without reserved bits

[Jan Beulich]

When none of the physical address bits in PTEs are reserved, we can't
create any 4k (leaf) PTEs which would trigger reserved bit faults. Hence
the present SHOPT_FAST_FAULT_PATH machinery needs to be suppressed in
this case, which is most easily achieved by never creating any magic
entries.

To compensate a little, eliminate sh_write_p2m_entry_post()'s impact on
such hardware.

While at it, also avoid using an MMIO magic entry when that would
truncate the incoming GFN.

Requested-by: Andrew Cooper 
Signed-off-by: Jan Beulich 
---
I wonder if subsequently we couldn't arrange for SMEP/SMAP faults to be
utilized instead, on capable hardware (which might well be all having
such large a physical address width).

I further wonder whether SH_L1E_MMIO_GFN_MASK couldn't / shouldn't be
widened. I don't see a reason why it would need confining to the low
32 bits of the PTE - using the full space up to bit 50 ought to be fine
(i.e. just one address bit left set in the magic mask), and we wouldn't
even need that many to encode a 40-bit GFN (i.e. the extra guarding
added here wouldn't then be needed in the first place).

--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -499,7 +499,8 @@ _sh_propagate(struct vcpu *v,
 {
 /* Guest l1e maps emulated MMIO space */
 *sp = sh_l1e_mmio(target_gfn, gflags);
-d->arch.paging.shadow.has_fast_mmio_entries = true;
+if ( sh_l1e_is_magic(*sp) )
+d->arch.paging.shadow.has_fast_mmio_entries = true;
 goto done;
 }
 
--- a/xen/arch/x86/mm/shadow/types.h
+++ b/xen/arch/x86/mm/shadow/types.h
@@ -281,7 +281,8 @@ shadow_put_page_from_l1e(shadow_l1e_t sl
  * pagetables.
  *
  * This is only feasible for PAE and 64bit Xen: 32-bit non-PAE PTEs don't
- * have reserved bits that we can use for this.
+ * have reserved bits that we can use for this.  And even there it can only
+ * be used if the processor doesn't use all 52 address bits.
  */
 
 #define SH_L1E_MAGIC 0x0001ULL
@@ -291,14 +292,24 @@ static inline bool sh_l1e_is_magic(shado
 }
 
 /* Guest not present: a single magic value */
-static inline shadow_l1e_t sh_l1e_gnp(void)
+static inline shadow_l1e_t sh_l1e_gnp_raw(void)
 {
 return (shadow_l1e_t){ -1ULL };
 }
 
+static inline shadow_l1e_t sh_l1e_gnp(void)
+{
+/*
+ * On systems with no reserved physical address bits we can't engage the
+ * fast fault path.
+ */
+return paddr_bits < PADDR_BITS ? sh_l1e_gnp_raw()
+   : shadow_l1e_empty();
+}
+
 static inline bool sh_l1e_is_gnp(shadow_l1e_t sl1e)
 {
-return sl1e.l1 == sh_l1e_gnp().l1;
+return sl1e.l1 == sh_l1e_gnp_raw().l1;
 }
 
 /*
@@ -313,9 +324,14 @@ static inline bool sh_l1e_is_gnp(shadow_
 
 static inline shadow_l1e_t sh_l1e_mmio(gfn_t gfn, u32 gflags)
 {
-return (shadow_l1e_t) { (SH_L1E_MMIO_MAGIC
- | MASK_INSR(gfn_x(gfn), SH_L1E_MMIO_GFN_MASK)
- | (gflags & (_PAGE_USER|_PAGE_RW))) };
+unsigned long gfn_val = MASK_INSR(gfn_x(gfn), SH_L1E_MMIO_GFN_MASK);
+
+if ( paddr_bits >= PADDR_BITS ||
+ gfn_x(gfn) != MASK_EXTR(gfn_val, SH_L1E_MMIO_GFN_MASK) )
+return shadow_l1e_empty();
+
+return (shadow_l1e_t) { (SH_L1E_MMIO_MAGIC | gfn_val |
+ (gflags & (_PAGE_USER | _PAGE_RW))) };
 }
 
 static inline bool sh_l1e_is_mmio(shadow_l1e_t sl1e)

Re: [PATCH v3][4.15] x86: mirror compat argument translation area for 32-bit PV

[Jan Beulich]

On 25.02.2021 13:52, Andrew Cooper wrote:
> On 25/02/2021 09:30, Jan Beulich wrote:
>> --- a/xen/include/asm-x86/config.h
>> +++ b/xen/include/asm-x86/config.h
>> @@ -170,7 +170,11 @@ extern unsigned char boot_edid_info[128]
>>   *Guest-defined use.
>>   *  0xf580 - 0x [168MB, PML4:0]
>>   *Read-only machine-to-phys translation table (GUEST ACCESSIBLE).
>> - *  0x0001 - 0x7fff [128TB-4GB, PML4:0-255]
>> + *  0x0001 - 0x01ff [2TB-4GB,   PML4:0-3]
>> + *Unused / Reserved for future use.
>> + *  0x0200 - 0x027f [512GB, 2^39 bytes, PML4:4]
>> + *Mirror of per-domain mappings (for argument translation area; also 
>> HVM).
>> + *  0x0280 - 0x7fff [125.5TB,   PML4:5-255]
>>   *Unused / Reserved for future use.
>>   */
>>  
>> @@ -207,6 +211,8 @@ extern unsigned char boot_edid_info[128]
>>  #define PERDOMAIN_SLOTS 3
>>  #define PERDOMAIN_VIRT_SLOT(s)  (PERDOMAIN_VIRT_START + (s) * \
>>   (PERDOMAIN_SLOT_MBYTES << 20))
>> +/* Slot 4: mirror of per-domain mappings (for compat xlat area accesses). */
>> +#define PERDOMAIN_ALT_VIRT_START PML4_ADDR(260 % 256)
> 
> 4.
> 
> 260 % 256 is pure obfuscation.

Well, that's why the comment is there. The expression is to
show why 4 isn't entirely arbitrary. But well, if there's no
way to get this in without changing to 4, I will of course do
so. Before I submit v4 - are there any other concerns?

Jan

Re: [PATCH v3][4.15] x86: mirror compat argument translation area for 32-bit PV

[Andrew Cooper]

On 25/02/2021 09:30, Jan Beulich wrote:
> --- a/xen/include/asm-x86/config.h
> +++ b/xen/include/asm-x86/config.h
> @@ -170,7 +170,11 @@ extern unsigned char boot_edid_info[128]
>   *Guest-defined use.
>   *  0xf580 - 0x [168MB, PML4:0]
>   *Read-only machine-to-phys translation table (GUEST ACCESSIBLE).
> - *  0x0001 - 0x7fff [128TB-4GB, PML4:0-255]
> + *  0x0001 - 0x01ff [2TB-4GB,   PML4:0-3]
> + *Unused / Reserved for future use.
> + *  0x0200 - 0x027f [512GB, 2^39 bytes, PML4:4]
> + *Mirror of per-domain mappings (for argument translation area; also 
> HVM).
> + *  0x0280 - 0x7fff [125.5TB,   PML4:5-255]
>   *Unused / Reserved for future use.
>   */
>  
> @@ -207,6 +211,8 @@ extern unsigned char boot_edid_info[128]
>  #define PERDOMAIN_SLOTS 3
>  #define PERDOMAIN_VIRT_SLOT(s)  (PERDOMAIN_VIRT_START + (s) * \
>   (PERDOMAIN_SLOT_MBYTES << 20))
> +/* Slot 4: mirror of per-domain mappings (for compat xlat area accesses). */
> +#define PERDOMAIN_ALT_VIRT_START PML4_ADDR(260 % 256)

4.

260 % 256 is pure obfuscation.

~Andrew

Re: [RFC PATCH 00/10] Preemption in hypervisor (ARM only)

[Volodymyr Babchuk]

Hi Andrew,

Andrew Cooper writes:

> On 24/02/2021 23:58, Volodymyr Babchuk wrote:
>> And I am not mentioning x86 support there...
>
> x86 uses per-pCPU stacks, not per-vCPU stacks.
>
> Transcribing from an old thread which happened in private as part of an
> XSA discussion, concerning the implications of trying to change this.
>
> ~Andrew
>
> -8<-
>
> Here is a partial list off the top of my head of the practical problems
> you're going to have to solve.
>
> Introduction of new SpectreRSB vulnerable gadgets.  I'm really close to
> being able to drop RSB stuffing and recover some performance in Xen.
>
> CPL0 entrypoints need updating across schedule.  SYSCALL entry would
> need to become a stub per vcpu, rather than the current stub per pcpu.
> This requires reintroducing a writeable mapping to the TSS (doable) and
> a shadow stack switch of active stacks (This corner case is so broken it
> looks to be a blocker for CET-SS support in Linux, and is resulting in
> some conversation about tweaking Shstk's in future processors).
>
> All per-cpu variables stop working.  You'd need to rewrite Xen to use
> %gs for TLS which will have churn in the PV logic, and introduce the x86
> architectural corner cases of running with an invalid %gs.  Xen has been
> saved from a large number of privilege escalation vulnerabilities in
> common with Linux and Windows by the fact that we don't use %gs, so
> anyone trying to do this is going to have to come up with some concrete
> way of proving that the corner cases are covered.

Thank you. This is exactly what I needed. I am not a big specialist in
x86, but from what I said, I can see that there is no easy way to switch
contexts while in hypervisor mode.

Then I want to return to a task domain idea, which you mentioned in the
other thread. If I got it right, it would allow to

1. Implement asynchronous hypercalls for cases when there is no reason
to hold calling vCPU in hypervisor for the whole call duration

2. Improve time accounting, as tasklets can be scheduled to run in this
task domain.

I skimmed through ML archives, but didn't found any discussion about it.

As I see it, its implementation would be close to idle domain
implementation, but a little different.

-- 
Volodymyr Babchuk at EPAM

[PATCH 1/2] xen-netback: add module parameter to disable ctrl-ring

[ChiaHao Hsu]

In order to support live migration of guests between kernels
that do and do not support 'feature-ctrl-ring', we add a
module parameter that allows the feature to be disabled
at run time, instead of using hardcode value.
The default value is enable.

Signed-off-by: ChiaHao Hsu 
---
 drivers/net/xen-netback/common.h  |  2 ++
 drivers/net/xen-netback/netback.c |  6 ++
 drivers/net/xen-netback/xenbus.c  | 13 -
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 4a16d6e33c09..bfb7a3054917 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -276,6 +276,7 @@ struct backend_info {
u8 have_hotplug_status_watch:1;
 
const char *hotplug_script;
+   bool ctrl_ring_enabled;
 };
 
 struct xenvif {
@@ -413,6 +414,7 @@ static inline pending_ring_idx_t nr_pending_reqs(struct 
xenvif_queue *queue)
 
 irqreturn_t xenvif_interrupt(int irq, void *dev_id);
 
+extern bool control_ring;
 extern bool separate_tx_rx_irq;
 extern bool provides_xdp_headroom;
 
diff --git a/drivers/net/xen-netback/netback.c 
b/drivers/net/xen-netback/netback.c
index e5c73f819662..20d858f0456a 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -48,6 +48,12 @@
 
 #include 
 
+/* Provide an option to disable control ring which is used to pass
+ * large quantities of data from frontend to backend.
+ */
+bool control_ring = true;
+module_param(control_ring, bool, 0644);
+
 /* Provide an option to disable split event channels at load time as
  * event channels are limited resource. Split event channels are
  * enabled by default.
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index a5439c130130..8a9169cff9c5 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -1123,11 +1123,14 @@ static int netback_probe(struct xenbus_device *dev,
if (err)
pr_debug("Error writing multi-queue-max-queues\n");
 
-   err = xenbus_printf(XBT_NIL, dev->nodename,
-   "feature-ctrl-ring",
-   "%u", true);
-   if (err)
-   pr_debug("Error writing feature-ctrl-ring\n");
+   be->ctrl_ring_enabled = READ_ONCE(control_ring);
+   if (be->ctrl_ring_enabled) {
+   err = xenbus_printf(XBT_NIL, dev->nodename,
+   "feature-ctrl-ring",
+   "%u", true);
+   if (err)
+   pr_debug("Error writing feature-ctrl-ring\n");
+   }
 
backend_switch_state(be, XenbusStateInitWait);
 
-- 
2.23.3

[PATCH 2/2] xen-netback: add module parameter to disable dynamic multicast control

[ChiaHao Hsu]

In order to support live migration of guests between kernels
that do and do not support 'feature-dynamic-multicast-control',
we add a module parameter that allows the feature to be disabled
at run time, instead of using hardcode value.
The default value is enable.

Signed-off-by: ChiaHao Hsu 
---
 drivers/net/xen-netback/common.h  |  1 +
 drivers/net/xen-netback/netback.c |  7 +++
 drivers/net/xen-netback/xenbus.c  | 14 --
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index bfb7a3054917..c166ebb5a81f 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -415,6 +415,7 @@ static inline pending_ring_idx_t nr_pending_reqs(struct 
xenvif_queue *queue)
 irqreturn_t xenvif_interrupt(int irq, void *dev_id);
 
 extern bool control_ring;
+extern bool dynamic_multicast_control;
 extern bool separate_tx_rx_irq;
 extern bool provides_xdp_headroom;
 
diff --git a/drivers/net/xen-netback/netback.c 
b/drivers/net/xen-netback/netback.c
index 20d858f0456a..4c3d92238ae9 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -54,6 +54,13 @@
 bool control_ring = true;
 module_param(control_ring, bool, 0644);
 
+/* Provide an option to extend multicast control protocol. This allows
+ * request-multicast-control to be set by the frontend at any time,
+ * the backend will watch the value and re-sample on watch events.
+ */
+bool dynamic_multicast_control = true;
+module_param(dynamic_multicast_control, bool, 0644);
+
 /* Provide an option to disable split event channels at load time as
  * event channels are limited resource. Split event channels are
  * enabled by default.
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index 8a9169cff9c5..36c699f99da4 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -1092,12 +1092,14 @@ static int netback_probe(struct xenbus_device *dev,
goto abort_transaction;
}
 
-   err = xenbus_printf(xbt, dev->nodename,
-   "feature-dynamic-multicast-control",
-   "%d", 1);
-   if (err) {
-   message = "writing feature-dynamic-multicast-control";
-   goto abort_transaction;
+   if (dynamic_multicast_control) {
+   err = xenbus_printf(xbt, dev->nodename,
+   "feature-dynamic-multicast-control",
+   "%d", 1);
+   if (err) {
+   message = "writing 
feature-dynamic-multicast-control";
+   goto abort_transaction;
+   }
}
 
err = xenbus_transaction_end(xbt, 0);
-- 
2.23.3

Re: [PATCH for-4.15] xen/vgic: Implement write to ISPENDR in vGICv{2, 3}

[Julien Grall]

On 22/02/2021 13:45, Bertrand Marquis wrote:

Hi Julien,


On 20 Feb 2021, at 14:04, Julien Grall  wrote:

From: Julien Grall 

Currently, Xen will send a data abort to a guest trying to write to the
ISPENDR.

Unfortunately, recent version of Linux (at least 5.9+) will start
writing to the register if the interrupt needs to be re-triggered
(see the callback irq_retrigger). This can happen when a driver (such as
the xgbe network driver on AMD Seattle) re-enable an interrupt:

(XEN) d0v0: vGICD: unhandled word write 0x000400 to ISPENDR44
[...]
[   25.635837] Unhandled fault at 0x80001000522c
[...]
[   25.818716]  gic_retrigger+0x2c/0x38
[   25.822361]  irq_startup+0x78/0x138
[   25.825920]  __enable_irq+0x70/0x80
[   25.829478]  enable_irq+0x50/0xa0
[   25.832864]  xgbe_one_poll+0xc8/0xd8
[   25.836509]  net_rx_action+0x110/0x3a8
[   25.840328]  __do_softirq+0x124/0x288
[   25.844061]  irq_exit+0xe0/0xf0
[   25.847272]  __handle_domain_irq+0x68/0xc0
[   25.851442]  gic_handle_irq+0xa8/0xe0
[   25.855171]  el1_irq+0xb0/0x180
[   25.858383]  arch_cpu_idle+0x18/0x28
[   25.862028]  default_idle_call+0x24/0x5c
[   25.866021]  do_idle+0x204/0x278
[   25.869319]  cpu_startup_entry+0x24/0x68
[   25.873313]  rest_init+0xd4/0xe4
[   25.876611]  arch_call_rest_init+0x10/0x1c
[   25.880777]  start_kernel+0x5b8/0x5ec

As a consequence, the OS may become unusable.

Implementing the write part of ISPENDR is somewhat easy. For
virtual interrupt, we only need to inject the interrupt again.

For physical interrupt, we need to be more careful as the de-activation
of the virtual interrupt will be propagated to the physical distributor.
For simplicity, the physical interrupt will be set pending so the
workflow will not differ from a "real" interrupt.

Longer term, we could possible directly activate the physical interrupt
and avoid taking an exception to inject the interrupt to the domain.
(This is the approach taken by the new vGIC based on KVM).

Signed-off-by: Julien Grall 


This is something which will not be done by a guest very often so I think your
implementation actually makes it simpler and reduce possibilities of race 
conditions
so I am not even sure that the XXX comment is needed.


I think the XXX is useful as if someone notice an issue with the code, 
then they know what they could try.


I am open to suggestion how we could keep track of potential improvement.


But i am ok with it being in or not so:

Reviewed-by: Bertrand Marquis 


Thanks!

Cheers,
--
Julien Grall

Re: [PATCH] xen-netback: correct success/error reporting for the SKB-with-fraglist case

[Paul Durrant]

On 25/02/2021 07:33, Jan Beulich wrote:

On 24.02.2021 17:39, Paul Durrant wrote:

On 23/02/2021 16:29, Jan Beulich wrote:

When re-entering the main loop of xenvif_tx_check_gop() a 2nd time, the
special considerations for the head of the SKB no longer apply. Don't
mistakenly report ERROR to the frontend for the first entry in the list,
even if - from all I can tell - this shouldn't matter much as the overall
transmit will need to be considered failed anyway.

Signed-off-by: Jan Beulich 

--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -499,7 +499,7 @@ check_frags:
 * the header's copy failed, and they are
 * sharing a slot, send an error
 */
-   if (i == 0 && sharedslot)
+   if (i == 0 && !first_shinfo && sharedslot)
xenvif_idx_release(queue, pending_idx,
   XEN_NETIF_RSP_ERROR);
else



I think this will DTRT, but to my mind it would make more sense to clear
'sharedslot' before the 'goto check_frags' at the bottom of the function.


That was my initial idea as well, but
- I think it is for a reason that the variable is "const".
- There is another use of it which would then instead need further
   amending (and which I believe is at least part of the reason for
   the variable to be "const").



Oh, yes. But now that I look again, don't you want:

if (i == 0 && first_shinfo && sharedslot)

? (i.e no '!')

The comment states that the error should be indicated when the first 
frag contains the header in the case that the map succeeded but the 
prior copy from the same ref failed. This can only possibly be the case 
if this is the 'first_shinfo' (which is why I still think it is safe to 
unconst 'sharedslot' and clear it).


  Paul



Jan

Re: [for-4.15][RESEND PATCH v4 2/2] xen/iommu: x86: Clear the root page-table before freeing the page-tables

[Julien Grall]

Hi Jan,

On 24/02/2021 16:00, Jan Beulich wrote:

On 24.02.2021 16:58, Jan Beulich wrote:

On 24.02.2021 10:43, Julien Grall wrote:

--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -149,6 +149,13 @@ int arch_iommu_domain_init(struct domain *d)
  
  void arch_iommu_domain_destroy(struct domain *d)

  {
+/*
+ * There should be not page-tables left allocated by the time the
+ * domain is destroyed. Note that arch_iommu_domain_destroy() is
+ * called unconditionally, so pgtables may be unitialized.
+ */
+ASSERT(dom_iommu(d)->platform_ops == NULL ||


Since you've used the preferred ! instead of "== 0" /
"== NULL" in the ASSERT()s you add further up, may I ask that
you do so here as well?


Oh, and I meant to provide
Reviewed-by: Jan Beulich 
preferably with that cosmetic adjustment (and ideally also with
"uninitialized" in the comment, as I notice only now).


I don't seem to find your original answer in my inbox and on lore [1]. 
Could you confirm if the two comments visible in this thread are the 
only one you made on this patch?


Thanks for the review!

Cheers,

[1] https://lore.kernel.org/xen-devel/20210224094356.7606-3-jul...@xen.org/



Jan



--
Julien Grall

Re: [for-4.15][RESEND PATCH v4 1/2] xen/x86: iommu: Ignore IOMMU mapping requests when a domain is dying

[Julien Grall]

Hi Jan,

On 24/02/2021 14:07, Jan Beulich wrote:

On 24.02.2021 10:43, Julien Grall wrote:

From: Julien Grall 

The new x86 IOMMU page-tables allocator will release the pages when
relinquishing the domain resources. However, this is not sufficient
when the domain is dying because nothing prevents page-table to be
allocated.

As the domain is dying, it is not necessary to continue to modify the
IOMMU page-tables as they are going to be destroyed soon.

At the moment, page-table allocates will only happen when iommu_map().
So after this change there will be no more page-table allocation
happening.


While I'm still not happy about this asymmetry, I'm willing to accept
it in the interest of getting the underlying issue addressed. May I
ask though that you add something like "... because we don't use
superpage mappings yet when not sharing page tables"?


Sure.


But there are two more minor things:


--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -267,6 +267,12 @@ int iommu_free_pgtables(struct domain *d)
  struct page_info *pg;
  unsigned int done = 0;
  
+if ( !is_iommu_enabled(d) )

+return 0;


Why is this addition needed? Hitting a not yet initialize spin lock
is - afaict - no worse than a not yet initialized list, so it would
seem to me that this can't be the reason. No other reason looks to
be called out by the description.


struct domain_iommu will be initially zeroed as it is part of struct domain.

For the list, we are so far fine because page_list_remove_head() 
tolerates NULL. If we were using the normal list operations (e.g. 
list_del), then this code would have segfaulted.


Now about the spinlock, at least lock debugging expects a non-zero 
initial value. We are lucky here because this path is not called with 
IRQ disabled. If it were, Xen would crash as it would consider the lock 
was used in a non-IRQ safe environment.


So in the spinlock case, we are really playing with the fire. Hence, the 
check here.



+/* After this barrier, no more IOMMU mapping can happen */
+spin_barrier(&hd->arch.mapping_lock);


On the v3 discussion I thought you did agree to change the wording
of the comment to something like "no new IOMMU mappings can be
inserted"?


Sorry I missed this comment. I will update it in the next version.

Cheers,

--
Julien Grall

Re: [PATCH][4.15] tools: Fix typo in xc_vmtrace_set_option comment

[Ian Jackson]

Hubert Jasudowicz writes ("[PATCH][4.15] tools: Fix typo in 
xc_vmtrace_set_option comment"):
> Signed-off-by: Hubert Jasudowicz 

Release-Acked-by: Ian Jackson 

Re: [PATCH] virtio-gpu: Respect graphics update interval for EDID

[Gerd Hoffmann]

  Hi,

> > Because of the wasted frames I'd like this to be an option you can
> > enable when needed.  For the majority of use cases this seems to be
> > no problem ...
> 
> I see blinks with GNOME on Wayland on Ubuntu 20.04 and virtio-gpu with
> the EDID change included in this patch.

/me looks closely at the patch again.

So you update the edid dynamically, each time the refresh rate changes.
Problem with that approach is software doesn't expect edid to change
dynamically because on physical hardware it is static information about
the connected monitor.

So what the virtio-gpu guest driver does is emulate a monitor hotplug
event to notify userspace.  If you resize the qemu window on the host
it'll look like the monitor with the old window size was unplugged and
a new monitor with the new window size got plugged instead, so gnome
shell goes adapt the display resolution to the new virtual monitor size.

The blink you are seeing probably comes from gnome-shell processing the
monitor hotplug event.

We could try to skip generating a monitor hotplug event in case only the
refresh rate did change.  That would fix the blink, but it would also
have the effect that nobody will notice the update.

Bottom line:  I think making the edid refresh rate configurable might be
useful, but changing it dynamically most likely isn't.

take care,
  Gerd

[PATCH][4.15] tools: Fix typo in xc_vmtrace_set_option comment

[Hubert Jasudowicz]

Signed-off-by: Hubert Jasudowicz 
---
 tools/include/xenctrl.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 0efcdae8b4..318920166c 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -1644,7 +1644,7 @@ int xc_vmtrace_get_option(xc_interface *xch, uint32_t 
domid,
   uint32_t vcpu, uint64_t key, uint64_t *value);
 
 /**
- * Set platform specific vntvmtrace options.
+ * Set platform specific vmtrace options.
  *
  * @parm xch a handle to an open hypervisor interface
  * @parm domid domain identifier
-- 
2.30.1

Re: [PATCH 0/2] hvmloader: drop usage of system headers

[Jan Beulich]

On 25.02.2021 10:50, Ian Jackson wrote:
> Jan Beulich writes ("Re: [PATCH 0/2] hvmloader: drop usage of system 
> headers"):
>> On 24.02.2021 21:08, Andrew Cooper wrote:
>>> After some experimentation in the arch container
> ...
>>> while the freestanding form with `gcc -ffreestanding -m32 -E` is:
> ...
>>> # 34 "/usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0/include/stdint-gcc.h" 3 4
>>> typedef signed char int8_t;
>>> ...
> 
> Um, but what size is size_t ?
> 
> In particular, note that that path contains nothing to do with 32-bit
> so I think it is probably the wrong bitness.

The path doesn't need to include anything bitness specific, when
the headers can deal with both flavors.

>> Why "more subtle"? All we're lacking is -ffreestanding. The
>> question is whether it is an acceptably risky thing to do at
>> this point in the release cycle to add the option.
> 
> If -ffreestanding DTRT then I think it's about as risky as the fix I
> already approved and we have merged...

It would do the right thing, except Roger found Alpine has another
anomaly undermining this (and breaking -ffreestanding itself).

As an aside I'm not sure what you refer to with "we have merged":
So far only patch 1 of this series (plus its fixup) has gone in.

Jan

Re: [PATCH 0/2] hvmloader: drop usage of system headers

[Roger Pau Monné]

On Wed, Feb 24, 2021 at 08:08:25PM +, Andrew Cooper wrote:
> On 24/02/2021 10:26, Roger Pau Monne wrote:
> > Hello,
> >
> > Following two patches aim to make hvmloader standalone, so that it don't
> > try to use system headers. It shouldn't result in any functional
> > change.
> >
> > Thanks, Roger.
> 
> After some experimentation in the arch container

I'm afraid it's the alpine container the one that gives those errors,
not the arch one.

So I've done some testing on alpine and I think there's something
broken. Given the following snipped:

---
#include 

int main(void)
{
_Static_assert(sizeof(uint64_t) == 8, "");
}
---

This is the output of running `gcc -E -m32 -ffreestanding test.c` on
an alpine chroot that has just the 'gcc' package installed:

---
# 1 "test.c"
# 1 ""
# 1 ""
# 1 "test.c"

# 1 "/usr/lib/gcc/x86_64-alpine-linux-musl/10.2.1/include/stdint.h" 1 3
# 11 "/usr/lib/gcc/x86_64-alpine-linux-musl/10.2.1/include/stdint.h" 3
# 1 "/usr/lib/gcc/x86_64-alpine-linux-musl/10.2.1/include/stdint-gcc.h" 1 3
# 34 "/usr/lib/gcc/x86_64-alpine-linux-musl/10.2.1/include/stdint-gcc.h" 3

# 34 "/usr/lib/gcc/x86_64-alpine-linux-musl/10.2.1/include/stdint-gcc.h" 3
typedef signed char int8_t;


typedef short int int16_t;


typedef int int32_t;


typedef long long int int64_t;


typedef unsigned char uint8_t;


typedef short unsigned int uint16_t;


typedef unsigned int uint32_t;


typedef long long unsigned int uint64_t;




typedef signed char int_least8_t;
typedef short int int_least16_t;
typedef int int_least32_t;
typedef long long int int_least64_t;
typedef unsigned char uint_least8_t;
typedef short unsigned int uint_least16_t;
typedef unsigned int uint_least32_t;
typedef long long unsigned int uint_least64_t;



typedef signed char int_fast8_t;
typedef int int_fast16_t;
typedef int int_fast32_t;
typedef long long int int_fast64_t;
typedef unsigned char uint_fast8_t;
typedef unsigned int uint_fast16_t;
typedef unsigned int uint_fast32_t;
typedef long long unsigned int uint_fast64_t;




typedef int intptr_t;


typedef unsigned int uintptr_t;




typedef long long int intmax_t;
typedef long long unsigned int uintmax_t;
# 12 "/usr/lib/gcc/x86_64-alpine-linux-musl/10.2.1/include/stdint.h" 2 3
# 3 "test.c" 2


# 4 "test.c"
int main(void)
{
 _Static_assert(sizeof(uint64_t) == 8, "");
}
---

OTOH, this is the output of the same command run on a chroot that has
the full list of packages required to build Xen (from
automation/build/alpine/3.12.dockerfile):

---
# 1 "test.c"
# 1 ""
# 1 ""
# 1 "test.c"

# 1 "/usr/include/stdint.h" 1 3 4
# 20 "/usr/include/stdint.h" 3 4
# 1 "/usr/include/bits/alltypes.h" 1 3 4
# 55 "/usr/include/bits/alltypes.h" 3 4

# 55 "/usr/include/bits/alltypes.h" 3 4
typedef unsigned long uintptr_t;
# 70 "/usr/include/bits/alltypes.h" 3 4
typedef long intptr_t;
# 96 "/usr/include/bits/alltypes.h" 3 4
typedef signed char int8_t;




typedef signed short int16_t;




typedef signed int int32_t;




typedef signed long int64_t;




typedef signed long intmax_t;




typedef unsigned char uint8_t;




typedef unsigned short uint16_t;




typedef unsigned int uint32_t;




typedef unsigned long uint64_t;
# 146 "/usr/include/bits/alltypes.h" 3 4
typedef unsigned long uintmax_t;
# 21 "/usr/include/stdint.h" 2 3 4

typedef int8_t int_fast8_t;
typedef int64_t int_fast64_t;

typedef int8_t int_least8_t;
typedef int16_t int_least16_t;
typedef int32_t int_least32_t;
typedef int64_t int_least64_t;

typedef uint8_t uint_fast8_t;
typedef uint64_t uint_fast64_t;

typedef uint8_t uint_least8_t;
typedef uint16_t uint_least16_t;
typedef uint32_t uint_least32_t;
typedef uint64_t uint_least64_t;
# 95 "/usr/include/stdint.h" 3 4
# 1 "/usr/include/bits/stdint.h" 1 3 4
typedef int32_t int_fast16_t;
typedef int32_t int_fast32_t;
typedef uint32_t uint_fast16_t;
typedef uint32_t uint_fast32_t;
# 96 "/usr/include/stdint.h" 2 3 4
# 3 "test.c" 2


# 4 "test.c"
int main(void)
{
 _Static_assert(sizeof(uint64_t) == 8, "");
}
---

This is caused by the include path order of gcc on alpine, ie:

---
# cpp -v /dev/null -o /dev/null
Using built-in specs.
COLLECT_GCC=cpp
Target: x86_64-alpine-linux-musl
Configured with: /home/buildozer/aports/main/gcc/src/gcc-10.2.1_pre1/configure 
--prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info 
--build=x86_64-alpine-linux-musl --host=x86_64-alpine-linux-musl 
--target=x86_64-alpine-linux-musl --with-pkgversion='Alpine 10.2.1_pre1' 
--enable-checking=release --disable-fixed-point --disable-libstdcxx-pch 
--disable-multilib --disable-nls --disable-werror --disable-symvers 
--enable-__cxa_atexit --enable-default-pie --enable-default-ssp 
--enable-cloog-backend --enable-languages=c,c++,d,objc,go,fortran,ada 
--disable-libssp --disable-libmpx --disable-libmudflap --disable-libsanitizer 
--enable-shared --enable-threads --enable-tls --with-system-zlib 
--with-linker-hash-style=gnu
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 10.2.1 20201203 (Alpine 10

Re: [PATCH 0/2] hvmloader: drop usage of system headers

[Ian Jackson]

Jan Beulich writes ("Re: [PATCH 0/2] hvmloader: drop usage of system headers"):
> On 24.02.2021 21:08, Andrew Cooper wrote:
> > After some experimentation in the arch container
...
> > while the freestanding form with `gcc -ffreestanding -m32 -E` is:
...
> > # 34 "/usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0/include/stdint-gcc.h" 3 4
> > typedef signed char int8_t;
> > ...

Um, but what size is size_t ?

In particular, note that that path contains nothing to do with 32-bit
so I think it is probably the wrong bitness.

> Why "more subtle"? All we're lacking is -ffreestanding. The
> question is whether it is an acceptably risky thing to do at
> this point in the release cycle to add the option.

If -ffreestanding DTRT then I think it's about as risky as the fix I
already approved and we have merged...

Ian.

Re: [PATCH v3][4.15] x86: mirror compat argument translation area for 32-bit PV

[Jan Beulich]

On 25.02.2021 10:30, Jan Beulich wrote:
> Now that we guard the entire Xen VA space against speculative abuse
> through hypervisor accesses to guest memory, the argument translation
> area's VA also needs to live outside this range, at least for 32-bit PV
> guests. To avoid extra is_hvm_*() conditionals, use the alternative VA
> uniformly.
> 
> While this could be conditionalized upon CONFIG_PV32 &&
> CONFIG_SPECULATIVE_HARDEN_GUEST_ACCESS, omitting such extra conditionals
> keeps the code more legible imo.
> 
> Fixes: 4dc181599142 ("x86/PV: harden guest memory accesses against 
> speculative abuse")
> Signed-off-by: Jan Beulich 
> Acked-by: Roger Pau Monné 

Roger - I would have dropped an R-b, but I've assumed keeping an A-b
would be fine. Please let me know if this was wrong.

Jan

[PATCH v3][4.15] x86: mirror compat argument translation area for 32-bit PV

[Jan Beulich]

Now that we guard the entire Xen VA space against speculative abuse
through hypervisor accesses to guest memory, the argument translation
area's VA also needs to live outside this range, at least for 32-bit PV
guests. To avoid extra is_hvm_*() conditionals, use the alternative VA
uniformly.

While this could be conditionalized upon CONFIG_PV32 &&
CONFIG_SPECULATIVE_HARDEN_GUEST_ACCESS, omitting such extra conditionals
keeps the code more legible imo.

Fixes: 4dc181599142 ("x86/PV: harden guest memory accesses against speculative 
abuse")
Signed-off-by: Jan Beulich 
Acked-by: Roger Pau Monné 
Release-Acked-by: Ian Jackson 
---
v3: Use address range in lower half of address space.
v2: Rename PERDOMAIN2_VIRT_START to PERDOMAIN_ALT_VIRT_START.

--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -1691,6 +1691,13 @@ void init_xen_l4_slots(l4_pgentry_t *l4t
 l4t[l4_table_offset(PERDOMAIN_VIRT_START)] =
 l4e_from_page(d->arch.perdomain_l3_pg, __PAGE_HYPERVISOR_RW);
 
+/* Slot 4: Per-domain mappings mirror. */
+BUILD_BUG_ON(IS_ENABLED(CONFIG_PV32) &&
+ !l4_table_offset(PERDOMAIN_ALT_VIRT_START));
+if ( !is_pv_64bit_domain(d) )
+l4t[l4_table_offset(PERDOMAIN_ALT_VIRT_START)] =
+l4t[l4_table_offset(PERDOMAIN_VIRT_START)];
+
 /* Slot 261-: text/data/bss, RW M2P, vmap, frametable, directmap. */
 #ifndef NDEBUG
 if ( short_directmap &&
--- a/xen/include/asm-x86/config.h
+++ b/xen/include/asm-x86/config.h
@@ -170,7 +170,11 @@ extern unsigned char boot_edid_info[128]
  *Guest-defined use.
  *  0xf580 - 0x [168MB, PML4:0]
  *Read-only machine-to-phys translation table (GUEST ACCESSIBLE).
- *  0x0001 - 0x7fff [128TB-4GB, PML4:0-255]
+ *  0x0001 - 0x01ff [2TB-4GB,   PML4:0-3]
+ *Unused / Reserved for future use.
+ *  0x0200 - 0x027f [512GB, 2^39 bytes, PML4:4]
+ *Mirror of per-domain mappings (for argument translation area; also HVM).
+ *  0x0280 - 0x7fff [125.5TB,   PML4:5-255]
  *Unused / Reserved for future use.
  */
 
@@ -207,6 +211,8 @@ extern unsigned char boot_edid_info[128]
 #define PERDOMAIN_SLOTS 3
 #define PERDOMAIN_VIRT_SLOT(s)  (PERDOMAIN_VIRT_START + (s) * \
  (PERDOMAIN_SLOT_MBYTES << 20))
+/* Slot 4: mirror of per-domain mappings (for compat xlat area accesses). */
+#define PERDOMAIN_ALT_VIRT_START PML4_ADDR(260 % 256)
 /* Slot 261: machine-to-phys conversion table (256GB). */
 #define RDWR_MPT_VIRT_START (PML4_ADDR(261))
 #define RDWR_MPT_VIRT_END   (RDWR_MPT_VIRT_START + MPT_VIRT_SIZE)
--- a/xen/include/asm-x86/x86_64/uaccess.h
+++ b/xen/include/asm-x86/x86_64/uaccess.h
@@ -1,7 +1,17 @@
 #ifndef __X86_64_UACCESS_H
 #define __X86_64_UACCESS_H
 
-#define COMPAT_ARG_XLAT_VIRT_BASE ((void *)ARG_XLAT_START(current))
+/*
+ * With CONFIG_SPECULATIVE_HARDEN_GUEST_ACCESS (apparent) PV guest accesses
+ * are prohibited to touch the Xen private VA range.  The compat argument
+ * translation area, therefore, can't live within this range.  Domains
+ * (potentially) in need of argument translation (32-bit PV, possibly HVM) get
+ * a secondary mapping installed, which needs to be used for such accesses in
+ * the PV case, and will also be used for HVM to avoid extra conditionals.
+ */
+#define COMPAT_ARG_XLAT_VIRT_BASE ((void *)ARG_XLAT_START(current) + \
+   (PERDOMAIN_ALT_VIRT_START - \
+PERDOMAIN_VIRT_START))
 #define COMPAT_ARG_XLAT_SIZE  (2*PAGE_SIZE)
 struct vcpu;
 int setup_compat_arg_xlat(struct vcpu *v);