Re: [PATCH-for-9.0 04/10] hw/xen: Factor xen_arch_align_ioreq_data() out of handle_ioreq()

[Philippe Mathieu-Daudé]

On 13/11/23 18:11, David Woodhouse wrote:

On Mon, 2023-11-13 at 17:09 +0100, Philippe Mathieu-Daudé wrote:

On 13/11/23 16:58, Woodhouse, David wrote:

On 13 Nov 2023 10:22, Philippe Mathieu-Daudé 
wrote:

     Per commit f17068c1c7 ("xen-hvm: reorganize xen-hvm and move
common
     function to xen-hvm-common"), handle_ioreq() is expected to be
     target-agnostic. However it uses 'target_ulong', which is a
target
     specific definition.

     In order to compile this file once for all targets, factor the
     target-specific code out of handle_ioreq() as a per-target
handler
     called xen_arch_align_ioreq_data().

     Signed-off-by: Philippe Mathieu-Daudé 
     ---
     Should we have a 'unsigned qemu_target_long_bits();' helper
     such qemu_target_page_foo() API and target_words_bigendian()?


It can be more fun than that though. What about
qemu_target_alignof_uint64() for example, which differs between
i386 and
x86_64 and causes even structs with *explicitly* sized fields to
differ
because of padding.

I'd *love* to see this series as a step towards my fantasy of being
able
to support Xen under TCG. After all, without that what's the point
in
being target-agnostic?


Another win is we are building all these files once instead of one
for
each i386/x86_64/aarch64 targets, so we save CI time and Amazon
trees.


However, I am mildly concerned that some of these files are
accidentally
using the host ELF ABI, perhaps with explicit management of 32-bit
compatibility, and the target-agnosticity is purely an illusion?

See the "protocol" handling and the three ABIs for the ring in
xen-block, for example.


If so I'd expect build failures or violent runtime assertions.


Heh, mostly the guest just crashes in the cases I've seen so far.

See commit a1c1082908d ("hw/xen: use correct default protocol for xen-
block on x86").


Reviewing quickly hw/block/dataplane/xen-block.c, this code doesn't
seem target specific at all IMHO. Otherwise I'd really expect it to
fail compiling. But I don't know much about Xen, so I'll let block &
xen experts to have a look.


Where it checks dataplane->protocol and does different things for
BLKIF_PROTOCOL_NATIVE/BLKIF_PROTOCOL_X86_32/BLKIF_PROTOCOL_X86_64, the
*structures* it uses are intended to be using the correct ABI. I think
the structs for BLKIF_PROTOCOL_NATIVE may actually be *different*
according to the target, in theory?


OK I see what you mean, blkif_back_rings_t union in hw/block/xen_blkif.h

These structures shouldn't differ between targets, this is the point of
an ABI :) And if they were, they wouldn't compile as target agnostic.


I don't know that they are *correct* right now, if the host is
different from the target. But that's just a bug (that only matters if
we ever want to support Xen-compatible guests using TCG).


Can we be explicit about what's expected to work here and what's
not in scope?


What do you mean? Everything is expected to work like without this
series applied :)


I think that if we ever do support Xen-compatible guests using TCG,
we'll have to fix that bug and use the right target-specific
structures... and then perhaps we'll want the affected files to
actually become target-specfic again?

I think this series makes it look like target-agnostic support *should*
work... but it doesn't really?


For testing we have:

aarch64: tests/avocado/boot_xen.py
x86_64: tests/avocado/kvm_xen_guest.py

No combination with i386 is tested,
Xen within aarch64 KVM is not tested (not sure it works).

Re: [PATCH-for-9.0 08/10] system/physmem: Only include 'hw/xen/xen.h' when Xen is available

[Philippe Mathieu-Daudé]

On 13/11/23 21:03, David Woodhouse wrote:

On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:

"hw/xen/xen.h" contains declarations for Xen hardware. There is
no point including it when Xen is not available.


... if even when Xen *is* available, AFAICT. Can you just remove the
inclusion of hw/xen/xen.h entirely? I think that still builds, at least
for x86.


Yep, also on aarch64, thanks!


  When Xen is not
available, we have enough with declarations of "sysemu/xen.h".


... and system/xen-mapcache.h


Signed-off-by: Philippe Mathieu-Daudé 

Re: [PATCH-for-9.0 04/10] hw/xen: Factor xen_arch_align_ioreq_data() out of handle_ioreq()

[Philippe Mathieu-Daudé]

On 13/11/23 19:16, Richard Henderson wrote:

On 11/13/23 07:21, Philippe Mathieu-Daudé wrote:

diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index c028c1b541..03f9417e7e 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -426,10 +426,7 @@ static void handle_ioreq(XenIOState *state, 
ioreq_t *req)
  trace_handle_ioreq(req, req->type, req->dir, req->df, 
req->data_is_ptr,

 req->addr, req->data, req->count, req->size);
-    if (!req->data_is_ptr && (req->dir == IOREQ_WRITE) &&
-    (req->size < sizeof (target_ulong))) {
-    req->data &= ((target_ulong) 1 << (8 * req->size)) - 1;
-    }



I suspect this should never have been using target_ulong at all: 
req->data is uint64_t.


This could replace it:

-- >8 --
-if (!req->data_is_ptr && (req->dir == IOREQ_WRITE) &&
-(req->size < sizeof (target_ulong))) {
-req->data &= ((target_ulong) 1 << (8 * req->size)) - 1;
+if (!req->data_is_ptr && (req->dir == IOREQ_WRITE)) {
+req->data = extract64(req->data, 0, BITS_PER_BYTE * req->size);
 }
---

Some notes while looking at this.

Per xen/include/public/hvm/ioreq.h header:

#define IOREQ_TYPE_PIO  0 /* pio */
#define IOREQ_TYPE_COPY 1 /* mmio ops */
#define IOREQ_TYPE_PCI_CONFIG   2
#define IOREQ_TYPE_VMWARE_PORT  3
#define IOREQ_TYPE_TIMEOFFSET   7
#define IOREQ_TYPE_INVALIDATE   8 /* mapcache */

  struct ioreq {
uint64_t addr;  /* physical address */
uint64_t data;  /* data (or paddr of data) */
uint32_t count; /* for rep prefixes */
uint32_t size;  /* size in bytes */
uint32_t vp_eport;  /* evtchn for notifications to/from device 
model */

uint16_t _pad0;
uint8_t state:4;
uint8_t data_is_ptr:1;  /* if 1, data above is the guest paddr
 * of the real data to use. */
uint8_t dir:1;  /* 1=read, 0=write */
uint8_t df:1;
uint8_t _pad1:1;
uint8_t type;   /* I/O type */
  };
  typedef struct ioreq ioreq_t;

If 'data' is not a pointer, it is a u64.

- In PIO / VMWARE_PORT modes, only 32-bit are used.

- In MMIO COPY mode, memory is accessed by chunks of 64-bit

- In PCI_CONFIG mode, access is u8 or u16 or u32.

- None of TIMEOFFSET / INVALIDATE use 'req'.

- Fallback is only used in x86 for VMWARE_PORT.

--

Regards,

Phil.

Re: Faking the number of CPUs for dom0 with dom0_max_vcpus

[Jan Beulich]

On 13.11.2023 22:40, Jimmy Lee wrote:
> Hi Jan, thanks for the response! Adding more details and log files here:
> 
> 1. I installed Xen 4.14 on CentOS 7 with yum:
> 
> [root@test-xen ~]# yum list xen kernel
> Loaded plugins: fastestmirror
> Loading mirror speeds from cached hostfile
>  * base: download.cf.centos.org
>  * centos-virt-xen-epel: d2lzkl7pfhq30w.cloudfront.net
>  * extras: download.cf.centos.org
>  * updates: download.cf.centos.org
> Installed Packages
> kernel.x86_64
>  3.10.0-1160.45.1.el7
>  installed
> kernel.x86_64
>  3.10.0-1160.76.1.el7
>  @updates
> kernel.x86_644.9.241-37.el7

I'm afraid neither this ...

>  @centos-virt-xen-414
> xen.x86_64
> 4.14.5.24.g87d90d511c-1.el7
> @centos-virt-xen-414
> [root@test-xen ~]# uname -r
> 4.9.241-37.el7.x86_64

... nor this counts as recent kernel. I did point you at the commit you
(minimally) need.

Also - please don't top-post.

Jan

Re: [XEN PATCH][for-4.19 v4] xen: address violations of Rule 11.9

[Jan Beulich]

On 14.11.2023 00:58, Stefano Stabellini wrote:
> On Mon, 13 Nov 2023, Jan Beulich wrote:
>> On 19.10.2023 09:55, Nicola Vetrini wrote:
>>> The constant 0 is used instead of NULL in '__ACCESS_ONCE' as a
>>> compile-time check to detect non-scalar types; its usage for this
>>> purpose is deviated.
>>>
>>> Furthermore, the 'typeof_field' macro is introduced as a general way
>>> to access the type of a struct member without declaring a variable
>>> of struct type. Both this macro and 'sizeof_field' are moved to
>>> 'xen/macros.h'.
>>>
>>> No functional change intended.
>>>
>>> Signed-off-by: Nicola Vetrini 
>>> Reviewed-by: Jan Beulich 
>>> Reviewed-by: Stefano Stabellini 
>>> ---
>>> Changes in v2:
>>> - added entry in deviations.rst
>>> Changes in v3:
>>> - dropped access_field
>>> - moved macro to macros.h
>>> ---
>>> Changes in v4:
>>> - Amend deviation record.
>>> ---
>>>  automation/eclair_analysis/ECLAIR/deviations.ecl |  9 +
>>>  docs/misra/deviations.rst|  6 ++
>>>  xen/include/xen/compiler.h   |  8 
>>>  xen/include/xen/kernel.h |  2 +-
>>>  xen/include/xen/macros.h | 16 
>>>  5 files changed, 32 insertions(+), 9 deletions(-)
>>
>> I tried to commit this patch, but ...
>>
>>> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl 
>>> b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>> index fa56e5c00a27..28d9c37bedb2 100644
>>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>> @@ -246,6 +246,15 @@ constant expressions are required.\""
>>>"any()"}
>>>  -doc_end
>>>
>>> +#
>>> +# Series 11
>>> +#
>>> +
>>> +-doc_begin="This construct is used to check if the type is scalar, and for 
>>> this purpose the use of 0 as a null pointer constant is deliberate."
>>> +-config=MC3R1.R11.9,reports+={deliberate, 
>>> "any_area(any_loc(any_exp(macro(^__ACCESS_ONCE$"
>>> +}
>>> +-doc_end
>>> +
>>>  #
>>>  # Series 13
>>>  #
>>
>> ... this change doesn't apply, and I also can't see how it would. There were
>> no dependencies specified, so it's also not clear on top of which other
>> (ready to be committed) patch(es) this might have been meant to apply. Please
>> resubmit in a shape applicable to the staging branch.
> 
> The order doesn't matter in that file, you can place # Series 11 just
> after # Series 10

That would have been one of the possible guesses. Yet then ...

> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -274,6 +274,15 @@ still non-negative."
>  -config=MC3R1.R10.1,etypes+={safe, 
> "stmt(operator(logical)||node(conditional_operator||binary_conditional_operator))",
>  "dst_type(ebool||boolean)"}
>  -doc_end
>  
> +#
> +# Series 11
> +#
> +
> +-doc_begin="This construct is used to check if the type is scalar, and for 
> this purpose the use of 0 as a null pointer constant is deliberate."
> +-config=MC3R1.R11.9,reports+={deliberate, 
> "any_area(any_loc(any_exp(macro(^__ACCESS_ONCE$"
> +}
> +-doc_end
> +
>  ### Set 3 ###

... there is this grouping by sets as well.

Jan

[linux-linus test] 183746: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-dom0pvh-xl-amd 22 guest-start/debian.repeat fail pass in 
183749-retest

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

version targeted for testing:
 linux9bacdd8996c77c42ca004440be610692275ff9d0
baseline version:
 linuxb85ea95d086471afb4ad062012a4d73cd328fa86

Last test of basis   183740  2023-11-13 02:38:19 Z1 days
Testing same since   183746  2023-11-13 17:42:24 Z0 days1 attempts


People who touched revisions under test:
  Boris Burkov 
  Dan Carpenter 
  David Sterba 
  Filipe Manana 
  Helen Koike 
  Josef Bacik 
  Linus Torvalds 
  Naohiro Aota 
  Qu Wenruo 
  Shinichiro Kawasaki 

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

Re: [PATCH v2 28/29] tools/xenstored: support complete log capabilities in stubdom

[Juergen Gross]

On 13.11.23 23:40, Julien Grall wrote:

Hi Juergen,

On 10/11/2023 16:08, Juergen Gross wrote:

With 9pfs being fully available in Xenstore-stubdom now, there is no
reason to not fully support all logging capabilities in stubdom.

Open the logfile on stubdom only after the 9pfs file system has been
mounted.

Signed-off-by: Juergen Gross 
Reviewed-by: Jason Andryuk 
---
  tools/hotplug/Linux/launch-xenstore.in |  1 +
  tools/xenstored/control.c  | 30 +-
  tools/xenstored/minios.c   |  3 +++
  3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/tools/hotplug/Linux/launch-xenstore.in 
b/tools/hotplug/Linux/launch-xenstore.in

index e854ca1eb8..da4eeca7c5 100644
--- a/tools/hotplug/Linux/launch-xenstore.in
+++ b/tools/hotplug/Linux/launch-xenstore.in
@@ -98,6 +98,7 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . 
@CONFIG_DIR@/@CONFIG_LEAF

  [ -z "$XENSTORE_DOMAIN_SIZE" ] && XENSTORE_DOMAIN_SIZE=8
  XENSTORE_DOMAIN_ARGS="$XENSTORE_DOMAIN_ARGS --memory 
$XENSTORE_DOMAIN_SIZE"
  [ -z "$XENSTORE_MAX_DOMAIN_SIZE" ] || 
XENSTORE_DOMAIN_ARGS="$XENSTORE_DOMAIN_ARGS --maxmem $XENSTORE_MAX_DOMAIN_SIZE"
+    [ -z "$XENSTORED_TRACE" ] || XENSTORE_DOMAIN_ARGS="$XENSTORE_DOMAIN_ARGS 
-T xenstored-trace.log"


I am probably missing something, but any reason to not use 
@XEN_LOG_DIR@/xenstored-trace.log as we do for xenstored?


Yes. Stubdom has no access to XEN_LOG_DIR.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature.asc
Description: OpenPGP digital signature

Re: [PATCH v2 27/29] tools/xenstored: add helpers for filename handling

[Juergen Gross]

On 13.11.23 23:25, Julien Grall wrote:

Hi Juergen,

On 10/11/2023 16:08, Juergen Gross wrote:

Add some helpers for handling filenames which might need different
implementations between stubdom and daemon environments:

- expansion of relative filenames (those are not really defined today,
   just expand them to be relative to /var/lib/xen/xenstore)
- expansion of xenstore_daemon_rundir() (used e.g. for saving the state
   file in case of live update - needs to be unchanged in the daemon
   case, but should result in /var/lib/xen/xenstore for stubdom)

Signed-off-by: Juergen Gross 
Reviewed-by: Jason Andryuk 
---
  tools/xenstored/core.c  | 9 -
  tools/xenstored/core.h  | 3 +++
  tools/xenstored/lu_daemon.c | 4 ++--
  tools/xenstored/minios.c    | 5 +
  tools/xenstored/posix.c | 5 +
  5 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
index 4a9d874f17..77befd24c9 100644
--- a/tools/xenstored/core.c
+++ b/tools/xenstored/core.c
@@ -158,6 +158,13 @@ void trace_destroy(const void *data, const char *type)
  trace("obj: DESTROY %s %p\n", type, data);
  }
+char *absolute_filename(const void *ctx, const char *filename)


Can the return be const?


I'll have a look.




+{
+    if (filename[0] != '/')
+    return talloc_asprintf(ctx, XENSTORE_LIB_DIR "/%s", filename);


Looking at the rest of patch, you will be using xenstore_rundir(). I wonder 
whether it would not be better to switch to xenstore_rundir() so...



+    return talloc_strdup(ctx, filename);
+}
+
  /**
   * Signal handler for SIGHUP, which requests that the trace log is reopened
   * (in the main loop).  A single byte is written to reopen_log_pipe, to awaken
@@ -2983,7 +2990,7 @@ int main(int argc, char *argv[])
  signal(SIGHUP, trigger_reopen_log);
  if (tracefile)
-    tracefile = talloc_strdup(NULL, tracefile);
+    tracefile = absolute_filename(NULL, tracefile);
  #ifndef NO_LIVE_UPDATE
  /* Read state in case of live update. */
diff --git a/tools/xenstored/core.h b/tools/xenstored/core.h
index a0d3592961..51e1dddb22 100644
--- a/tools/xenstored/core.h
+++ b/tools/xenstored/core.h
@@ -393,6 +393,9 @@ void early_init(void);
  void mount_9pfs(void);
  #endif
+const char *xenstore_rundir(void);
+char *absolute_filename(const void *ctx, const char *filename);
+
  /* Write out the pidfile */
  void write_pidfile(const char *pidfile);
diff --git a/tools/xenstored/lu_daemon.c b/tools/xenstored/lu_daemon.c
index 71bcabadd3..635ab0 100644
--- a/tools/xenstored/lu_daemon.c
+++ b/tools/xenstored/lu_daemon.c
@@ -24,7 +24,7 @@ void lu_get_dump_state(struct lu_dump_state *state)
  state->size = 0;
  state->filename = talloc_asprintf(NULL, "%s/state_dump",
-  xenstore_daemon_rundir());
+  xenstore_rundir());


... call and ...


  if (!state->filename)
  barf("Allocation failure");
@@ -65,7 +65,7 @@ FILE *lu_dump_open(const void *ctx)
  int fd;
  filename = talloc_asprintf(ctx, "%s/state_dump",
-   xenstore_daemon_rundir());
+   xenstore_rundir());


... this one could be replaced with absolute_filename().


No, I don't think this is a good idea.

I don't want the daemon to store trace files specified as relative files
to be stored in /var/run/xen, while I want all files of the stubdom to be
stored under /var/lib/xen.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature.asc
Description: OpenPGP digital signature

Re: [PATCH v2 26/29] tools/xenstored: mount 9pfs device in stubdom

[Juergen Gross]

On 13.11.23 23:09, Julien Grall wrote:

Hi Juergen,

On 10/11/2023 16:08, Juergen Gross wrote:

diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
index 162b87b460..4263c1360f 100644
--- a/tools/xenstored/domain.c
+++ b/tools/xenstored/domain.c
@@ -1236,6 +1236,8 @@ void stubdom_init(void)
  barf_perror("Failed to initialize stubdom");
  xenevtchn_notify(xce_handle, stubdom->port);
+
+    mount_9pfs();
  #endif
  }
diff --git a/tools/xenstored/minios.c b/tools/xenstored/minios.c
index 202d70387a..fddbede869 100644
--- a/tools/xenstored/minios.c
+++ b/tools/xenstored/minios.c
@@ -19,8 +19,16 @@
  #include 
  #include "core.h"
  #include 
+#include 
  #include 
  #include 
+#include 
+#include 
+#include 
+
+#define P9_STATE_PATH    "device/9pfs/0/state"
+
+static void *p9_device;
  void write_pidfile(const char *pidfile)
  {
@@ -62,3 +70,31 @@ void early_init(void)
  if (stub_domid == DOMID_INVALID)
  barf("could not get own domid");
  }
+
+static void mount_thread(void *p)
+{
+    xenbus_event_queue events = NULL;
+    char *err;
+    char *dummy;
+
+    free(xenbus_watch_path_token(XBT_NIL, P9_STATE_PATH, "9pfs", &events));


AFAICT, xenbus_watch_path_token() can fail. I agree this is unlikely, but if it 
fails, then it would be useful to get some logs. Otherwise...



+
+    for (;;) {


... this loop would be infinite.


Okay, will add logging.




+    xenbus_wait_for_watch(&events);
+    err = xenbus_read(XBT_NIL, P9_STATE_PATH, &dummy);


Can you explain why don't care about the value of the node?


I only care about the presence of the "state" node. All real state changes
will be handled in init_9pfront().




+    if (!err)
+    break;
+    free(err);
+    }
+
+    free(dummy);
+
+    free(xenbus_unwatch_path_token(XBT_NIL, P9_STATE_PATH, "9pfs"));


xenbus_unwatch_path_token() could technically fails. It would be helpful to 
print a message.


I can add that, but do we really care? This is a common pattern in Mini-OS.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature.asc
Description: OpenPGP digital signature

Re: [PATCH v2 25/29] tools/xenstored: map stubdom interface

[Juergen Gross]

On 13.11.23 23:04, Julien Grall wrote:

Hi Juergen,

On 10/11/2023 16:08, Juergen Gross wrote:

When running as stubdom, map the stubdom's Xenstore ring page in order
to support using the 9pfs frontend.

Signed-off-by: Juergen Gross 
---
  tools/xenstored/core.c   |  2 ++
  tools/xenstored/domain.c | 27 ++-
  tools/xenstored/domain.h |  1 +
  3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
index b9ec50b34c..4a9d874f17 100644
--- a/tools/xenstored/core.c
+++ b/tools/xenstored/core.c
@@ -2991,6 +2991,8 @@ int main(int argc, char *argv[])
  lu_read_state();
  #endif
+    stubdom_init();
+
  check_store();
  /* Get ready to listen to the tools. */
diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
index fa17f68618..162b87b460 100644
--- a/tools/xenstored/domain.c
+++ b/tools/xenstored/domain.c
@@ -37,6 +37,10 @@
  #include 
  #include 
+#ifdef __MINIOS__
+#include 
+#endif
+
  static xc_interface **xc_handle;
  xengnttab_handle **xgt_handle;
  static evtchn_port_t virq_port;
@@ -500,6 +504,11 @@ static void *map_interface(domid_t domid)
  if (domid == xenbus_master_domid())
  return xenbus_map();
+#ifdef __MINIOS__
+    if (domid == stub_domid)
+    return xenstore_buf;
+#endif
+
  return xengnttab_map_grant_ref(*xgt_handle, domid,
 GNTTAB_RESERVED_XENSTORE,
 PROT_READ|PROT_WRITE);
@@ -509,7 +518,7 @@ static void unmap_interface(domid_t domid, void *interface)
  {
  if (domid == xenbus_master_domid())
  unmap_xenbus(interface);
-    else
+    else if (domid != stub_domid)
  xengnttab_unmap(*xgt_handle, interface, 1);
  }
@@ -1214,6 +1223,22 @@ void dom0_init(void)
  xenevtchn_notify(xce_handle, dom0->port);
  }
+void stubdom_init(void)
+{
+#ifdef __MINIOS__
+    struct domain *stubdom;
+
+    if (stub_domid < 0)
+    return;
+
+    stubdom = introduce_domain(NULL, stub_domid, xenbus_evtchn, false);
+    if (!stubdom)
+    barf_perror("Failed to initialize stubdom");
+
+    xenevtchn_notify(xce_handle, stubdom->port);


If I compare to introduce_domain(), it is not entirely clear to me why the 
notification is done unconditionally here. Can you clarify?


There is no reason to do it conditionally, as we can be sure the event channel
is existing and the ring page is accessible.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature.asc
Description: OpenPGP digital signature

Re: [PATCH v2 02/29] tools: add a new xen logging daemon

[Juergen Gross]

On 13.11.23 18:36, Jason Andryuk wrote:

On Fri, Nov 10, 2023 at 11:08 AM Juergen Gross  wrote:


Add "xen-9pfsd", a new logging daemon meant to support infrastructure
domains (e.g. xenstore-stubdom) to access files in dom0.

For now only add the code needed for starting the daemon and
registering it with Xenstore via a new "libxl/xen-9pfs/state" node by
writing the "running" state to it.

Signed-off-by: Juergen Gross 
---



--- /dev/null
+++ b/tools/xen-9pfsd/xen-9pfsd.c
@@ -0,0 +1,145 @@



+ * The backend device string is "xen_9pfs", the tag used for mounting the
+ * 9pfs device is "Xen".


'_' is much less common in xenstore node names than '-'.  Is there a
particular reason you chose '_'?  I generally prefer '-', but IIRC the
libxl idl can't handle '-'.  Did you hit that?


Yes.



Reviewed-by: Jason Andryuk 


Thanks,

Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature.asc
Description: OpenPGP digital signature

Re: [PATCH v2 22/29] tools/xenstored: get own domid in stubdom case

[Juergen Gross]

On 13.11.23 22:59, Julien Grall wrote:

Hi Juergen,

On 10/11/2023 16:07, Juergen Gross wrote:

Obtain the own domid from the Xenstore special grant entry when running
as stubdom.

Signed-off-by: Juergen Gross 
---
V2:
- replacement of V1 patch (ANdrew Cooper)
---
  tools/xenstored/core.c   | 1 +
  tools/xenstored/core.h   | 1 +
  tools/xenstored/minios.c | 5 +
  3 files changed, 7 insertions(+)

diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
index 0c14823fb0..8e271e31f9 100644
--- a/tools/xenstored/core.c
+++ b/tools/xenstored/core.c
@@ -2738,6 +2738,7 @@ static struct option options[] = {
  int dom0_domid = 0;
  int dom0_event = 0;
  int priv_domid = 0;
+int stub_domid = -1;


After looking at partch #25, I feel that it would make more sense if stub_domid 
is a domid_t and initialized to DOMID_INVALID.


At least this makes clearer that initial value is not supported to be a valid 
domid.


Yes, you are right.


Juergen



OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature.asc
Description: OpenPGP digital signature

RE: [PATCH v12 01/37] x86/cpufeatures: Add the cpu feature bit for WRMSRNS

[Li, Xin3]

> and if you had to be precise, the code should do:
> 
>   if (cpu_feature_enabled(X86_FEATURE_FRED)) {
>   if (cpu_feature_enabled(X86_FEATURE_WRMSRNS))
>   wrmsrns(MSR_IA32_FRED_RSP0, (unsigned
> long)task_stack_page(task) + THREAD_SIZE);
>   else
>   wrmsr(MSR_IA32_FRED_RSP0, (unsigned
> long)task_stack_page(task) + THREAD_SIZE);
>   }

This is exactly what tglx wanted to avoid.

And I love the idea "baseline", especially we have a ton of CPU features.

> 
> > Another patch set should replace WRMSR with WRMSRNS, with SERIALIZE
> > added when needed.
> 
> I sense someone wants to optimize MSR writes ... :-)

:-)

Re: [PATCH v2 0/2] net: Update MemReentrancyGuard for NIC

[Jason Wang]

On Thu, Sep 21, 2023 at 3:16 PM Akihiko Odaki  wrote:
>
> On 2023/06/01 12:18, Akihiko Odaki wrote:
> > Recently MemReentrancyGuard was added to DeviceState to record that the
> > device is engaging in I/O. The network device backend needs to update it
> > when delivering a packet to a device.
> >
> > This implementation follows what bottom half does, but it does not add
> > a tracepoint for the case that the network device backend started
> > delivering a packet to a device which is already engaging in I/O. This
> > is because such reentrancy frequently happens for
> > qemu_flush_queued_packets() and is insignificant.
> >
> > This series consists of two patches. The first patch makes a bulk change to
> > add a new parameter to qemu_new_nic() and does not contain behavioral 
> > changes.
> > The second patch actually implements MemReentrancyGuard update.
> >
> > V1 -> V2: Added the 'Fixes: CVE-2023-3019' tag
> >
> > Akihiko Odaki (2):
> >net: Provide MemReentrancyGuard * to qemu_new_nic()
> >net: Update MemReentrancyGuard for NIC
> >
> >   include/net/net.h |  2 ++
> >   hw/net/allwinner-sun8i-emac.c |  3 ++-
> >   hw/net/allwinner_emac.c   |  3 ++-
> >   hw/net/cadence_gem.c  |  3 ++-
> >   hw/net/dp8393x.c  |  3 ++-
> >   hw/net/e1000.c|  3 ++-
> >   hw/net/e1000e.c   |  2 +-
> >   hw/net/eepro100.c |  4 +++-
> >   hw/net/etraxfs_eth.c  |  3 ++-
> >   hw/net/fsl_etsec/etsec.c  |  3 ++-
> >   hw/net/ftgmac100.c|  3 ++-
> >   hw/net/i82596.c   |  2 +-
> >   hw/net/igb.c  |  2 +-
> >   hw/net/imx_fec.c  |  2 +-
> >   hw/net/lan9118.c  |  3 ++-
> >   hw/net/mcf_fec.c  |  3 ++-
> >   hw/net/mipsnet.c  |  3 ++-
> >   hw/net/msf2-emac.c|  3 ++-
> >   hw/net/mv88w8618_eth.c|  3 ++-
> >   hw/net/ne2000-isa.c   |  3 ++-
> >   hw/net/ne2000-pci.c   |  3 ++-
> >   hw/net/npcm7xx_emc.c  |  3 ++-
> >   hw/net/opencores_eth.c|  3 ++-
> >   hw/net/pcnet.c|  3 ++-
> >   hw/net/rocker/rocker_fp.c |  4 ++--
> >   hw/net/rtl8139.c  |  3 ++-
> >   hw/net/smc91c111.c|  3 ++-
> >   hw/net/spapr_llan.c   |  3 ++-
> >   hw/net/stellaris_enet.c   |  3 ++-
> >   hw/net/sungem.c   |  2 +-
> >   hw/net/sunhme.c   |  3 ++-
> >   hw/net/tulip.c|  3 ++-
> >   hw/net/virtio-net.c   |  6 --
> >   hw/net/vmxnet3.c  |  2 +-
> >   hw/net/xen_nic.c  |  4 ++--
> >   hw/net/xgmac.c|  3 ++-
> >   hw/net/xilinx_axienet.c   |  3 ++-
> >   hw/net/xilinx_ethlite.c   |  3 ++-
> >   hw/usb/dev-network.c  |  3 ++-
> >   net/net.c | 15 +++
> >   40 files changed, 90 insertions(+), 41 deletions(-)
> >
>
> Hi Jason,
>
> Can you review this series?

For some reason it falls through the cracks.

I've queued this for rc1.

Thanks

>
> Regards,
> Akihiko Odaki
>

Re: [PATCH v12 01/37] x86/cpufeatures: Add the cpu feature bit for WRMSRNS

[Borislav Petkov]

On Tue, Nov 14, 2023 at 12:43:38AM +, Li, Xin3 wrote:
> No.  tglx asked for it:
> https://lkml.kernel.org/kvm/87y1h81ht4.ffs@tglx/

Aha

"According to the CPU folks FRED systems are guaranteed to have WRMSRNS -
I asked for that :). It's just not yet documented."

so I'm going to expect that to appear in the next FRED spec revision...

> Because we are doing 
>   wrmsrns(MSR_IA32_FRED_RSP0, ...)
> here, and X86_FEATURE_WRMSRNS doesn't guarantee MSR_IA32_FRED_RSP0 exists.
> 
> Or I missed something?

Well, according to what I'm hearing and reading so far:

FRED means WRMSRNS
FRED means MSR_IA32_FRED_RSP0

and if you had to be precise, the code should do:

if (cpu_feature_enabled(X86_FEATURE_FRED)) {
if (cpu_feature_enabled(X86_FEATURE_WRMSRNS))
wrmsrns(MSR_IA32_FRED_RSP0, (unsigned 
long)task_stack_page(task) + THREAD_SIZE);
else
wrmsr(MSR_IA32_FRED_RSP0, (unsigned 
long)task_stack_page(task) + THREAD_SIZE);
}

but apparently FRED implies WRMSRNS - not documented anywhere currently
- so you can save yourself one check.

But your version checks FRED if it can do WRMSRNS while there's
a separate WRMSRNS flag and that made me wonder...

> Another patch set should replace WRMSR with WRMSRNS, with SERIALIZE added
> when needed.

I sense someone wants to optimize MSR writes ... :-)

Thx.

-- 
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-libvirt-raw 13 guest-start fail pass in 183747-retest

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

version targeted for testing:
 xen  fb41228ececea948c7953c8c16fe28fd65c6536b
baseline version:
 xen  bede1c7e3b790b63f1ff3ea3ee4e476b012fdf2c

Last test of basis   183739  2023-11-13 01:53:4

RE: [PATCH v12 01/37] x86/cpufeatures: Add the cpu feature bit for WRMSRNS

[Li, Xin3]

> Then, further down in the patchset, it says:
> 
> + if (cpu_feature_enabled(X86_FEATURE_FRED)) {
> + /* WRMSRNS is a baseline feature for FRED. */
> 
> but WRMSRNS is not mentioned in the FRED spec "Document Number:
> 346446-005US, Revision: 5.0" which, according to
> 
> https://www.intel.com/content/www/us/en/content-details/780121/flexible-
> return-and-event-delivery-fred-specification.html
> 
> is the latest.
> 
> Am I looking at the wrong one?

No.  tglx asked for it:
https://lkml.kernel.org/kvm/87y1h81ht4.ffs@tglx/

> 
> And now I'm wondering: when you're adding a separate CPUID bit, then the
> above should be
> 
> +   if (cpu_feature_enabled(X86_FEATURE_WRMSRNS)) {
> +   /* WRMSRNS is a baseline feature for FRED. */

Because we are doing 
wrmsrns(MSR_IA32_FRED_RSP0, ...)
here, and X86_FEATURE_WRMSRNS doesn't guarantee MSR_IA32_FRED_RSP0 exists.

Or I missed something?

> 
> I see that you're adding a dependency:
> 
> + { X86_FEATURE_FRED, X86_FEATURE_WRMSRNS   },
> 
> which then means you don't need the X86_FEATURE_WRMSRNS definition at all
> and can use X86_FEATURE_FRED only.
> 
> So, what's up?

FRED just gets the honor to introduce WRMSRNS and its first usage:
https://lkml.kernel.org/kvm/b05e3092-8ba3-f4e1-b5a3-212594493...@zytor.com/

Another patch set should replace WRMSR with WRMSRNS, with SERIALIZE added
when needed.

Sorry for the late response, it was a long weekend in the US.

Thanks!
Xin

Re: [XEN PATCH] CI: Rework RISCV smoke test

[Stefano Stabellini]

On Mon, 13 Nov 2023, Anthony PERARD wrote:
> On Thu, Nov 09, 2023 at 04:52:36PM +, Andrew Cooper wrote:
> > On 09/11/2023 3:49 pm, Anthony PERARD wrote:
> > > Currently, the test rely on QEMU and Xen finishing the boot in under
> > > two seconds. That's both very long and very short. Xen usually managed
> > > to print "All set up" under a second. Unless for some reason we try to
> > > run the test on a machine that's busy doing something else.
> > >
> > > Rework the test to exit as soon as Xen is done.
> > >
> > > There's two `tail -f`, the first one is there simply to monitor test
> > > progress in GitLab console. The second one is used to detect the test
> > > result as soon as QEMU add it to the file. Both `tail` exit as soon as
> > > QEMU exit.
> > >
> > > If QEMU fails at start, and exit early, both `tail` will simply exit,
> > > resulting in a failure.
> > >
> > > If the line we are looking for is never printed, the `timeout` on the
> > > second `tail` will force the test to exit with an error.
> > >
> > > Signed-off-by: Anthony PERARD 
> > 
> > Looks plausible, but all these qemu-smoke scripts are pretty similar,
> > and copied from one-another.
> > 
> > We should make this change consistently to all testing (there's nothing
> > RISC-V specific about why this test is failing on this runner), and it
> > would be really nice if we could try to make it a bit more common than
> > it currently is.
> 
> Yes, it would be nice if a change to a qemu-smoke script was applied to
> every other one. But making those scripts more generic is a lot more
> works, which would be useful to apply a change once for all.
> 
> The problem I'm trying to resolve only appear with this script, because
> of a timeout been too short, a solution could just be to increase the
> timeout (or not allowing runners to do more than one thing at a time).
> 
> BTW, the last time I've been told to apply a change to other things, I
> never managed to finish it and the change wasn't applied at all (would
> have result in some containers been smaller).
> 
> I guess will see if anyone complain about the test randomly failing. :-)

Hi Anthony, I think it is OK if you only fix this script for now. As
part of the tests we are writing for Xen FuSa we want to improve the
basic gitlab testing infrastructure and make the code common like Andrew
asked (we haven't done it yet but soon). We should get to it in the next
few months.

Re: [XEN PATCH] CI: Rework RISCV smoke test

[Stefano Stabellini]

+Artem, Oleksandr

On Mon, 13 Nov 2023, Anthony PERARD wrote:
> On Thu, Nov 09, 2023 at 05:02:08PM -0800, Stefano Stabellini wrote:
> > ### qemu_key.sh is using "expect", see below. I think we should be able
> > ### to achieve the same by using expect to close on the expected string
> > ### (instead of waiting for eof)
> 
> So, `expect` is just a different kind of language than shell is?
> Also, `grep -q` doesn't wait for EOF, and just exit as soon as it found
> the pattern.

Re: [PATCH] automation: set architecture in docker files

[Stefano Stabellini]

On Mon, 13 Nov 2023, Roger Pau Monne wrote:
> Pass the desired architecture of the image in the FROM instruction if the
> image is possibly multi-platform.
> 
> This allows using the x86 Dockerfiles on OS X on arm64 hardware.
> 
> No functional change intended.
> 
> Signed-off-by: Roger Pau Monné 

Although I am not opposed to this change, so far we have been using:
arm64v8/alpine:3.18

for x86 it is not specified but it would be:
amd64/alpine:3.18

Two options:
1) we add amd64/ everywhere and leave the arm containers alone
2) we change all containers, including the arm containers, to use the
--platform option

I don't think is a good idea to have 2 different ways to specify the
architecture for x86 and arm containers



> ---
> I haven't touched the Yocto dockerfile because I'm not sure how it's used.
> ---
>  automation/build/alpine/3.18.dockerfile   | 2 +-
>  automation/build/archlinux/current-riscv64.dockerfile | 2 +-
>  automation/build/archlinux/current.dockerfile | 2 +-
>  automation/build/centos/7.dockerfile  | 2 +-
>  automation/build/debian/bookworm.dockerfile   | 2 +-
>  automation/build/debian/bullseye-ppc64le.dockerfile   | 2 +-
>  automation/build/debian/buster-gcc-ibt.dockerfile | 2 +-
>  automation/build/debian/jessie.dockerfile | 2 +-
>  automation/build/debian/stretch.dockerfile| 2 +-
>  automation/build/fedora/29.dockerfile | 2 +-
>  automation/build/suse/opensuse-leap.dockerfile| 2 +-
>  automation/build/suse/opensuse-tumbleweed.dockerfile  | 2 +-
>  automation/build/ubuntu/bionic.dockerfile | 2 +-
>  automation/build/ubuntu/focal.dockerfile  | 2 +-
>  automation/build/ubuntu/trusty.dockerfile | 2 +-
>  automation/build/ubuntu/xenial-xilinx.dockerfile  | 2 +-
>  automation/build/ubuntu/xenial.dockerfile | 2 +-
>  17 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/automation/build/alpine/3.18.dockerfile 
> b/automation/build/alpine/3.18.dockerfile
> index 5d2a69a06010..4ae9cb5e9e30 100644
> --- a/automation/build/alpine/3.18.dockerfile
> +++ b/automation/build/alpine/3.18.dockerfile
> @@ -1,4 +1,4 @@
> -FROM alpine:3.18
> +FROM --platform=linux/amd64 alpine:3.18
>  LABEL maintainer.name="The Xen Project" \
>maintainer.email="xen-devel@lists.xenproject.org"
>  
> diff --git a/automation/build/archlinux/current-riscv64.dockerfile 
> b/automation/build/archlinux/current-riscv64.dockerfile
> index abf8e7bf0b88..af75b5c720ce 100644
> --- a/automation/build/archlinux/current-riscv64.dockerfile
> +++ b/automation/build/archlinux/current-riscv64.dockerfile
> @@ -1,4 +1,4 @@
> -FROM archlinux
> +FROM --platform=linux/amd64 archlinux
>  LABEL maintainer.name="The Xen Project" \
>maintainer.email="xen-devel@lists.xenproject.org"
>  
> diff --git a/automation/build/archlinux/current.dockerfile 
> b/automation/build/archlinux/current.dockerfile
> index 47e79637a4a6..d974a1434fd5 100644
> --- a/automation/build/archlinux/current.dockerfile
> +++ b/automation/build/archlinux/current.dockerfile
> @@ -1,4 +1,4 @@
> -FROM archlinux:base-devel
> +FROM --platform=linux/amd64 archlinux:base-devel
>  LABEL maintainer.name="The Xen Project" \
>maintainer.email="xen-devel@lists.xenproject.org"
>  
> diff --git a/automation/build/centos/7.dockerfile 
> b/automation/build/centos/7.dockerfile
> index 69dcefb2f011..ab450f0b3a0e 100644
> --- a/automation/build/centos/7.dockerfile
> +++ b/automation/build/centos/7.dockerfile
> @@ -1,4 +1,4 @@
> -FROM centos:7
> +FROM --platform=linux/amd64 centos:7
>  LABEL maintainer.name="The Xen Project" \
>maintainer.email="xen-devel@lists.xenproject.org"
>  
> diff --git a/automation/build/debian/bookworm.dockerfile 
> b/automation/build/debian/bookworm.dockerfile
> index ae008c8d46e5..ac87778b3972 100644
> --- a/automation/build/debian/bookworm.dockerfile
> +++ b/automation/build/debian/bookworm.dockerfile
> @@ -1,4 +1,4 @@
> -FROM debian:bookworm
> +FROM --platform=linux/amd64 debian:bookworm
>  LABEL maintainer.name="The Xen Project" \
>maintainer.email="xen-devel@lists.xenproject.org"
>  
> diff --git a/automation/build/debian/bullseye-ppc64le.dockerfile 
> b/automation/build/debian/bullseye-ppc64le.dockerfile
> index 4de8458445ae..6fdfb6bc2b40 100644
> --- a/automation/build/debian/bullseye-ppc64le.dockerfile
> +++ b/automation/build/debian/bullseye-ppc64le.dockerfile
> @@ -1,4 +1,4 @@
> -FROM debian:bullseye-slim
> +FROM --platform=linux/amd64 debian:bullseye-slim
>  LABEL maintainer.name="The Xen Project" \
>maintainer.email="xen-devel@lists.xenproject.org"
>  
> diff --git a/automation/build/debian/buster-gcc-ibt.dockerfile 
> b/automation/build/debian/buster-gcc-ibt.dockerfile
> index 96ab4fe8a2f1..4328c109b72b 100644
> --- a/automation/build/debian/buster-gcc-ibt.dockerfile
> +++ b/automation/build/debian/buster-gcc-ibt.dockerfile
> @@ -1,4 +1,4 @@
> -FROM d

Re: [XEN PATCH][for-4.19 v4] xen: address violations of Rule 11.9

[Stefano Stabellini]

On Mon, 13 Nov 2023, Jan Beulich wrote:
> On 19.10.2023 09:55, Nicola Vetrini wrote:
> > The constant 0 is used instead of NULL in '__ACCESS_ONCE' as a
> > compile-time check to detect non-scalar types; its usage for this
> > purpose is deviated.
> > 
> > Furthermore, the 'typeof_field' macro is introduced as a general way
> > to access the type of a struct member without declaring a variable
> > of struct type. Both this macro and 'sizeof_field' are moved to
> > 'xen/macros.h'.
> > 
> > No functional change intended.
> > 
> > Signed-off-by: Nicola Vetrini 
> > Reviewed-by: Jan Beulich 
> > Reviewed-by: Stefano Stabellini 
> > ---
> > Changes in v2:
> > - added entry in deviations.rst
> > Changes in v3:
> > - dropped access_field
> > - moved macro to macros.h
> > ---
> > Changes in v4:
> > - Amend deviation record.
> > ---
> >  automation/eclair_analysis/ECLAIR/deviations.ecl |  9 +
> >  docs/misra/deviations.rst|  6 ++
> >  xen/include/xen/compiler.h   |  8 
> >  xen/include/xen/kernel.h |  2 +-
> >  xen/include/xen/macros.h | 16 
> >  5 files changed, 32 insertions(+), 9 deletions(-)
> 
> I tried to commit this patch, but ...
> 
> > diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl 
> > b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > index fa56e5c00a27..28d9c37bedb2 100644
> > --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> > +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > @@ -246,6 +246,15 @@ constant expressions are required.\""
> >"any()"}
> >  -doc_end
> > 
> > +#
> > +# Series 11
> > +#
> > +
> > +-doc_begin="This construct is used to check if the type is scalar, and for 
> > this purpose the use of 0 as a null pointer constant is deliberate."
> > +-config=MC3R1.R11.9,reports+={deliberate, 
> > "any_area(any_loc(any_exp(macro(^__ACCESS_ONCE$"
> > +}
> > +-doc_end
> > +
> >  #
> >  # Series 13
> >  #
> 
> ... this change doesn't apply, and I also can't see how it would. There were
> no dependencies specified, so it's also not clear on top of which other
> (ready to be committed) patch(es) this might have been meant to apply. Please
> resubmit in a shape applicable to the staging branch.

The order doesn't matter in that file, you can place # Series 11 just
after # Series 10


diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl 
b/automation/eclair_analysis/ECLAIR/deviations.ecl
index d8170106b4..14c5134575 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -274,6 +274,15 @@ still non-negative."
 -config=MC3R1.R10.1,etypes+={safe, 
"stmt(operator(logical)||node(conditional_operator||binary_conditional_operator))",
 "dst_type(ebool||boolean)"}
 -doc_end
 
+#
+# Series 11
+#
+
+-doc_begin="This construct is used to check if the type is scalar, and for 
this purpose the use of 0 as a null pointer constant is deliberate."
+-config=MC3R1.R11.9,reports+={deliberate, 
"any_area(any_loc(any_exp(macro(^__ACCESS_ONCE$"
+}
+-doc_end
+
 ### Set 3 ###
 
 #

[PATCH] docs/misra: add R11.1 R11.2 R11.3 R11.6

[Stefano Stabellini]

Add MISRA C Rules 11.1, 11.2, 11.3, 11.6 as discussed.

Explicitly add in the notes that conversions to integer types are
permitted if the destination type has enough bits to hold the entire
value. GCC gives enough guarantees in terms of preserving the bit
content in such situations.

Also allow for bool conversions (e.g. to check if a function point is
valid).

Signed-off-by: Stefano Stabellini 

diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index aa65eb4dd0..8c49b81085 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -383,6 +383,38 @@ maintainers if you want to suggest a change.
 
CFLAGS="-Warith-conversion -Wno-error=arith-conversion" make -C xen
 
+   * - `Rule 11.1 
`_
+ - Required
+ - Conversions shall not be performed between a pointer to a
+   function and any other type
+ - All conversions to integer types are permitted if the destination
+   type has enough bits to hold the entire value. Conversions to
+   bool and void* are permitted.
+
+   * - `Rule 11.2 
`_
+ - Required
+ - Conversions shall not be performed between a pointer to an
+   incomplete type and any other type
+ - All conversions to integer types are permitted if the destination
+   type has enough bits to hold the entire value. Conversions to
+   bool and void* are permitted.
+
+   * - `Rule 11.3 
`_
+ - Required
+ - A cast shall not be performed between a pointer to object type
+   and a pointer to a different object type
+ - All conversions to integer types are permitted if the destination
+   type has enough bits to hold the entire value. Conversions to
+   bool and void* are permitted.
+
+   * - `Rule 11.6 
`_
+ - Required
+ - A cast shall not be performed between pointer to void and an
+   arithmetic type
+ - All conversions to integer types are permitted if the destination
+   type has enough bits to hold the entire value. Conversions to
+   bool and void* are permitted.
+
* - `Rule 11.7 
`_
  - Required
  - A cast shall not be performed between pointer to object and a 
noninteger arithmetic type

[xen-4.18-testing test] 183744: tolerable FAIL - PUSHED

[osstest service owner]

flight 183744 xen-4.18-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/183744/

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  509f737d3e80c75f948eb896a08c324287b00adc
baseline version:
 xen  3e12149eb214fee62fc86ee964d3142d0235330e

Last test of basis   183704  2023-11-07 10:10:25 Z6 days
Testing same since   183744  2023-11-13 15:07:08 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Jan Beulich 
  Stefano Stabellin

Re: [PATCH v2 28/29] tools/xenstored: support complete log capabilities in stubdom

[Julien Grall]

Hi Juergen,

On 10/11/2023 16:08, Juergen Gross wrote:

With 9pfs being fully available in Xenstore-stubdom now, there is no
reason to not fully support all logging capabilities in stubdom.

Open the logfile on stubdom only after the 9pfs file system has been
mounted.

Signed-off-by: Juergen Gross 
Reviewed-by: Jason Andryuk 
---
  tools/hotplug/Linux/launch-xenstore.in |  1 +
  tools/xenstored/control.c  | 30 +-
  tools/xenstored/minios.c   |  3 +++
  3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/tools/hotplug/Linux/launch-xenstore.in 
b/tools/hotplug/Linux/launch-xenstore.in
index e854ca1eb8..da4eeca7c5 100644
--- a/tools/hotplug/Linux/launch-xenstore.in
+++ b/tools/hotplug/Linux/launch-xenstore.in
@@ -98,6 +98,7 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . 
@CONFIG_DIR@/@CONFIG_LEAF
[ -z "$XENSTORE_DOMAIN_SIZE" ] && XENSTORE_DOMAIN_SIZE=8
XENSTORE_DOMAIN_ARGS="$XENSTORE_DOMAIN_ARGS --memory 
$XENSTORE_DOMAIN_SIZE"
[ -z "$XENSTORE_MAX_DOMAIN_SIZE" ] || 
XENSTORE_DOMAIN_ARGS="$XENSTORE_DOMAIN_ARGS --maxmem $XENSTORE_MAX_DOMAIN_SIZE"
+   [ -z "$XENSTORED_TRACE" ] || XENSTORE_DOMAIN_ARGS="$XENSTORE_DOMAIN_ARGS -T 
xenstored-trace.log"


I am probably missing something, but any reason to not use 
@XEN_LOG_DIR@/xenstored-trace.log as we do for xenstored?


Cheers,

--
Julien Grall

Re: [PATCH v2 27/29] tools/xenstored: add helpers for filename handling

[Julien Grall]

Hi Juergen,

On 10/11/2023 16:08, Juergen Gross wrote:

Add some helpers for handling filenames which might need different
implementations between stubdom and daemon environments:

- expansion of relative filenames (those are not really defined today,
   just expand them to be relative to /var/lib/xen/xenstore)
- expansion of xenstore_daemon_rundir() (used e.g. for saving the state
   file in case of live update - needs to be unchanged in the daemon
   case, but should result in /var/lib/xen/xenstore for stubdom)

Signed-off-by: Juergen Gross 
Reviewed-by: Jason Andryuk 
---
  tools/xenstored/core.c  | 9 -
  tools/xenstored/core.h  | 3 +++
  tools/xenstored/lu_daemon.c | 4 ++--
  tools/xenstored/minios.c| 5 +
  tools/xenstored/posix.c | 5 +
  5 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
index 4a9d874f17..77befd24c9 100644
--- a/tools/xenstored/core.c
+++ b/tools/xenstored/core.c
@@ -158,6 +158,13 @@ void trace_destroy(const void *data, const char *type)
trace("obj: DESTROY %s %p\n", type, data);
  }
  
+char *absolute_filename(const void *ctx, const char *filename)


Can the return be const?


+{
+   if (filename[0] != '/')
+   return talloc_asprintf(ctx, XENSTORE_LIB_DIR "/%s", filename);


Looking at the rest of patch, you will be using xenstore_rundir(). I 
wonder whether it would not be better to switch to xenstore_rundir() so...



+   return talloc_strdup(ctx, filename);
+}
+
  /**
   * Signal handler for SIGHUP, which requests that the trace log is reopened
   * (in the main loop).  A single byte is written to reopen_log_pipe, to awaken
@@ -2983,7 +2990,7 @@ int main(int argc, char *argv[])
  
  	signal(SIGHUP, trigger_reopen_log);

if (tracefile)
-   tracefile = talloc_strdup(NULL, tracefile);
+   tracefile = absolute_filename(NULL, tracefile);
  
  #ifndef NO_LIVE_UPDATE

/* Read state in case of live update. */
diff --git a/tools/xenstored/core.h b/tools/xenstored/core.h
index a0d3592961..51e1dddb22 100644
--- a/tools/xenstored/core.h
+++ b/tools/xenstored/core.h
@@ -393,6 +393,9 @@ void early_init(void);
  void mount_9pfs(void);
  #endif
  
+const char *xenstore_rundir(void);

+char *absolute_filename(const void *ctx, const char *filename);
+
  /* Write out the pidfile */
  void write_pidfile(const char *pidfile);
  
diff --git a/tools/xenstored/lu_daemon.c b/tools/xenstored/lu_daemon.c

index 71bcabadd3..635ab0 100644
--- a/tools/xenstored/lu_daemon.c
+++ b/tools/xenstored/lu_daemon.c
@@ -24,7 +24,7 @@ void lu_get_dump_state(struct lu_dump_state *state)
state->size = 0;
  
  	state->filename = talloc_asprintf(NULL, "%s/state_dump",

- xenstore_daemon_rundir());
+ xenstore_rundir());


... call and ...


if (!state->filename)
barf("Allocation failure");
  
@@ -65,7 +65,7 @@ FILE *lu_dump_open(const void *ctx)

int fd;
  
  	filename = talloc_asprintf(ctx, "%s/state_dump",

-  xenstore_daemon_rundir());
+  xenstore_rundir());


... this one could be replaced with absolute_filename().


if (!filename)
return NULL;
  
diff --git a/tools/xenstored/minios.c b/tools/xenstored/minios.c

index fddbede869..cfc2377857 100644
--- a/tools/xenstored/minios.c
+++ b/tools/xenstored/minios.c
@@ -98,3 +98,8 @@ void mount_9pfs(void)
  {
create_thread("mount-9pfs", mount_thread, NULL);
  }
+
+const char *xenstore_rundir(void)
+{
+   return XENSTORE_LIB_DIR;
+}
diff --git a/tools/xenstored/posix.c b/tools/xenstored/posix.c
index fcb7791bd7..76f5c9ab84 100644
--- a/tools/xenstored/posix.c
+++ b/tools/xenstored/posix.c
@@ -168,3 +168,8 @@ void early_init(void)
mkdir(xenstore_daemon_rundir(), 0755);
mkdir(XENSTORE_LIB_DIR, 0755);
  }
+
+const char *xenstore_rundir(void)
+{
+   return xenstore_daemon_rundir();
+}


Cheers,

--
Julien Grall

Re: Informal voting proposal

[Stefano Stabellini]

On Mon, 13 Nov 2023, Jan Beulich wrote:
> On 06.11.2023 17:40, Kelly Choi wrote:
> > Hi all,
> > 
> > As an open-source community, there will always be differences of opinion in
> > approaches and the way we think. It is imperative, however, that we view
> > this diversity as a source of strength rather than a hindrance.
> > 
> > Recent deliberations within our project have led to certain matters being
> > put on hold due to an inability to reach a consensus. While formal voting
> > procedures serve their purpose, they can be time-consuming and may not
> > always lead to meaningful progress.
> > 
> > Having received agreement from a few maintainers already, I would like to
> > propose the following:
> > 
> > *Informal voting method:*
> > 
> >1. Each project should ideally have more than 2 maintainers to
> >facilitate impartial discussions. Projects lacking this configuration 
> > will
> >be addressed at a later stage.
> 
> Terminology question: What is "project" here? Considering how ./MAINTAINERS
> is structured, is it perhaps more "component"?

Yes, I think "component" is the right word


> >2. Anyone in the community is welcome to voice their opinions, ideas,
> >and concerns about any patch or contribution.
> >3. If members cannot agree, the majority informal vote of the
> >maintainers will be the decision that stands. For instance, if, after
> >careful consideration of all suggestions and concerns, 2 out of 3
> >maintainers endorse a solution within the x86 subsystem, it shall be the
> >decision we move forward with.
> 
> In a later reply you make explicit what can only be guessed here: There
> you suggest that out of a range of possible options, up front two are
> picked to then choose between. However, when there is a range options
> available, and when those can be viewed as points on a scale (rather
> than, to take Stefano's earlier example of SAF-* naming, cases where
> it's hard to view choices as being on a linear scale), picking two
> "points" up front may already pose a problem. (See also another reply
> mentioning how to ensure that the various possible options were even
> taken into consideration.)
> 
> Not only in such situations, but in general, to me a prereq to even
> coming to the point of needing an informal vote is the willingness of
> everyone involved to find a compromise. When there's a range of views,
> and when "knowing" what's going to be best for the project would require
> a crystal ball, experience suggests to me that chances for an optimal
> choice are better when picking a "point" not at the far ends of the scale.
> (Such a result then would also much better reflect your named goal of
> seeing diversity as a strength.)
> 
> With such willingness I think even informal votes could be avoided most
> of the time, at which point it becomes questionable whether for the few
> remaining cases informal and formal votes really need specifying
> separately.

The key difference in point of views is that you see as very common to
have options on a linear scale, where finding a middle ground makes
sense, and you see as an exception cases like SAF-* naming that are not
on a scale. In my view cases like SAF-* naming are the common case,
while it would be an exception to have options on a linear scale. If
it turns out there are indeed many cases where multiple options are on a
linear scale, we might be able to come up with another idea on how to
get a quick informal consensus for those issues.


> >4. Naturally, there may be exceptional circumstances, as such, a formal
> >vote may be warranted but should happen only a few times a year for 
> > serious
> >cases only.
> >5. Informal votes can be as easy as 2 out of 3 maintainers providing
> >their Acked-by/Reviewed-by tag. Alternatively, Maintainers can call an
> >informal vote by simply emailing the thread with "informal vote proposed,
> >option 1 and option 2."
> 
> I find this difficult. Both A-b and R-b assert that the person offering
> the tag endorses the presented solution to the indicated degree. It does
> not say anything on possible alternative solutions. As a result taking
> such tags as votes is (once again, and once again in my personal view)
> reasonable only when there's a black-and-white decision to be taken.

>From a practical perspective, A-b and R-b are a quick and easy way to
gauge informal consensus on an issue. Also, exploring alternative
solutions take time. Time for the reviewer, time for the contributor,
time for everyone else involved in the email thread. A-b and R-b have a
very important role: they say "this is good enough". When you have a
majority of people saying "this is good enough", I think we would be
better off spending our timing fixing other deficiencies, reviewing
other patches, rather than trying to further optimize that particular
patch.


> >6. *All maintainers should reply with their vote within 5 working days.*
> > 
> >7. Ple

[PATCH v6 5/5] [FUTURE] tools/arm: enable vPCI for domUs

[Stewart Hildebrand]

Set the vPCI flag in xen_domctl_createdomain to enable vPCI if a pci
device has been specified in the xl domain config file.

Signed-off-by: Stewart Hildebrand 
---
Same story as the patch before this regarding the [FUTURE] tag.

v5->v6:
* no change

v4->v5:
* no change

v3->v4:
* split from ("xen/arm: enable vPCI for domUs")
---
 tools/libs/light/libxl_arm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 15391917748c..6daed958e598 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -222,6 +222,9 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
 config->arch.sve_vl = d_config->b_info.arch_arm.sve_vl / 128U;
 }
 
+if (d_config->num_pcidevs)
+config->flags |= XEN_DOMCTL_CDF_vpci;
+
 return 0;
 }
 
-- 
2.42.0

[PATCH v6 4/5] [FUTURE] xen/arm: enable vPCI for domUs

[Stewart Hildebrand]

Select HAS_VPCI_GUEST_SUPPORT in Kconfig for enabling vPCI support for
domUs.

Add checks to fail guest creation if the configuration is invalid.

Signed-off-by: Stewart Hildebrand 
---
As the tag implies, this patch is not intended to be merged (yet).

Note that CONFIG_HAS_VPCI_GUEST_SUPPORT is not currently used in the
upstream code base. It will be used by the vPCI series [1]. This patch
is intended to be merged as part of the vPCI series. I'll coordinate
with Volodymyr to include this in the vPCI series or resend afterwards.
Meanwhile, I'll include it here until the Kconfig and
xen_domctl_createdomain prerequisites have been committed.

v5->v6:
* drop is_pvh_domain(), simply make arch_needs_vpci() return false on
  x86 for now
* leave has_vpci() alone, instead, add HAS_VPCI_GUEST_SUPPORT check in
  domain_create

v4->v5:
* replace is_system_domain() check with dom_io check
* return an error in XEN_DOMCTL_assign_device (thanks Jan!)
* remove CONFIG_ARM check
* add needs_vpci() and arch_needs_vpci()
* add HAS_VPCI_GUEST_SUPPORT check to has_vpci()

v3->v4:
* refuse to create domain if configuration is invalid
* split toolstack change into separate patch

v2->v3:
* set pci flags in toolstack

v1->v2:
* new patch

[1] https://lists.xenproject.org/archives/html/xen-devel/2023-10/msg00660.html
---
 xen/arch/arm/Kconfig  |  1 +
 xen/arch/arm/include/asm/domain.h |  2 ++
 xen/arch/arm/vpci.c   |  8 
 xen/arch/x86/include/asm/domain.h |  2 ++
 xen/common/domain.c   |  5 +
 xen/drivers/passthrough/pci.c | 20 
 6 files changed, 38 insertions(+)

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 5ff68e5d5979..3845b238a33f 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -195,6 +195,7 @@ config PCI_PASSTHROUGH
depends on ARM_64
select HAS_PCI
select HAS_VPCI
+   select HAS_VPCI_GUEST_SUPPORT
default n
help
  This option enables PCI device passthrough
diff --git a/xen/arch/arm/include/asm/domain.h 
b/xen/arch/arm/include/asm/domain.h
index 3614562eaefe..8e6d5fe9578c 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -31,6 +31,8 @@ enum domain_type {
 
 #define is_domain_direct_mapped(d) ((d)->cdf & CDF_directmap)
 
+#define arch_needs_vpci(d) ({ (void)(d); true; })
+
 /*
  * Is the domain using the host memory layout?
  *
diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c
index 3bc4bb55082a..61e0edcedea9 100644
--- a/xen/arch/arm/vpci.c
+++ b/xen/arch/arm/vpci.c
@@ -2,6 +2,7 @@
 /*
  * xen/arch/arm/vpci.c
  */
+#include 
 #include 
 #include 
 
@@ -90,8 +91,15 @@ int domain_vpci_init(struct domain *d)
 return ret;
 }
 else
+{
+if ( !IS_ENABLED(CONFIG_HAS_VPCI_GUEST_SUPPORT) )
+{
+gdprintk(XENLOG_ERR, "vPCI requested but guest support not 
enabled\n");
+return -EINVAL;
+}
 register_mmio_handler(d, &vpci_mmio_handler,
   GUEST_VPCI_ECAM_BASE, GUEST_VPCI_ECAM_SIZE, 
NULL);
+}
 
 return 0;
 }
diff --git a/xen/arch/x86/include/asm/domain.h 
b/xen/arch/x86/include/asm/domain.h
index cb02a4d1ebb2..d34015391eed 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -503,6 +503,8 @@ struct arch_domain
 #define has_vpit(d)(!!((d)->arch.emulation_flags & X86_EMU_PIT))
 #define has_pirq(d)(!!((d)->arch.emulation_flags & X86_EMU_USE_PIRQ))
 
+#define arch_needs_vpci(d) ({ (void)(d); false; })
+
 #define gdt_ldt_pt_idx(v) \
   ((v)->vcpu_id >> (PAGETABLE_ORDER - GDT_LDT_VCPU_SHIFT))
 #define pv_gdt_ptes(v) \
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 12dc27428972..47d49c57bf83 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -692,6 +692,11 @@ struct domain *domain_create(domid_t domid,
 
 if ( !is_idle_domain(d) )
 {
+err = -EINVAL;
+if ( !is_hardware_domain(d) && (config->flags & XEN_DOMCTL_CDF_vpci) &&
+ !IS_ENABLED(CONFIG_HAS_VPCI_GUEST_SUPPORT) )
+goto fail;
+
 if ( !is_hardware_domain(d) )
 d->nr_pirqs = nr_static_irqs + extra_domU_irqs;
 else
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 04d00c7c37df..2203725a2aa6 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1542,6 +1542,18 @@ void iommu_dev_iotlb_flush_timeout(struct domain *d, 
struct pci_dev *pdev)
 pcidevs_unlock();
 }
 
+static bool needs_vpci(const struct domain *d)
+{
+if ( is_hardware_domain(d) )
+return false;
+
+if ( d == dom_io )
+/* xl pci-assignable-add assigns PCI devices to domIO */
+return false;
+
+return arch_needs_vpci(d);
+}
+
 int iommu_do_pci_domctl(
 struct xen_domctl *domctl, struct domain *d,
 XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
@@ -1618,6 +1630,14 @@ int 

[PATCH v6 3/5] [FUTURE] xen/arm: enable vPCI for dom0

[Stewart Hildebrand]

Set the vPCI flag in xen_domctl_createdomain to enable vPCI for dom0 if
iommu and PCI passthrough are enabled and there exists a PCI host bridge
in the system.

Adjust pci_host_iterate_bridges_and_count() to count the number of host
bridges if no callback is provided.

Signed-off-by: Stewart Hildebrand 
---
Allowing/enabling vPCI for dom0 on ARM should follow or be part of the
PCI passthrough SMMU series [1]. I'm including it here due to
prerequisites in this Kconfig series. Once the prerequisites are
committed I'll move this patch to the PCI passthrough SMMU series.

v5->v6:
* no change

v4->v5:
* add [FUTURE] tag
* move flags_optional change from the previous patch to here

v3->v4:
* depend on iommu_enabled, pci_passthrough_enabled, and whether there
  is a pci host bridge

v2->v3:
* new patch

[1] https://lists.xenproject.org/archives/html/xen-devel/2023-10/msg00210.html
---
 xen/arch/arm/domain.c  |  3 ++-
 xen/arch/arm/domain_build.c|  6 ++
 xen/arch/arm/include/asm/pci.h |  9 +
 xen/arch/arm/pci/pci-host-common.c | 11 ---
 4 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 28e3aaa5e482..1409a4235e13 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -607,7 +607,8 @@ int arch_sanitise_domain_config(struct 
xen_domctl_createdomain *config)
 {
 unsigned int max_vcpus;
 unsigned int flags_required = (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap);
-unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | XEN_DOMCTL_CDF_vpmu);
+unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | XEN_DOMCTL_CDF_vpmu |
+   XEN_DOMCTL_CDF_vpci);
 unsigned int sve_vl_bits = sve_decode_vl(config->arch.sve_vl);
 
 if ( (config->flags & ~flags_optional) != flags_required )
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 2dd2926b4144..512b3c4c76e2 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -3916,6 +3916,12 @@ void __init create_dom0(void)
 panic("SVE vector length error\n");
 }
 
+if ( IS_ENABLED(CONFIG_HAS_VPCI) &&
+ iommu_enabled &&
+ is_pci_passthrough_enabled() &&
+ (pci_host_iterate_bridges_and_count(NULL, NULL) > 0) )
+dom0_cfg.flags |= XEN_DOMCTL_CDF_vpci;
+
 dom0 = domain_create(0, &dom0_cfg, CDF_privileged | CDF_directmap);
 if ( IS_ERR(dom0) )
 panic("Error creating domain 0 (rc = %ld)\n", PTR_ERR(dom0));
diff --git a/xen/arch/arm/include/asm/pci.h b/xen/arch/arm/include/asm/pci.h
index 7f77226c9bbf..74816a687bbb 100644
--- a/xen/arch/arm/include/asm/pci.h
+++ b/xen/arch/arm/include/asm/pci.h
@@ -147,5 +147,14 @@ static inline int pci_get_new_domain_nr(void)
 return -1;
 }
 
+struct pci_host_bridge;
+
+static inline int pci_host_iterate_bridges_and_count(
+struct domain *d,
+int (*cb)(struct domain *d, struct pci_host_bridge *bridge))
+{
+return 0;
+}
+
 #endif  /*!CONFIG_HAS_PCI*/
 #endif /* __ARM_PCI_H__ */
diff --git a/xen/arch/arm/pci/pci-host-common.c 
b/xen/arch/arm/pci/pci-host-common.c
index c0faf0f43675..e6a03ae668f8 100644
--- a/xen/arch/arm/pci/pci-host-common.c
+++ b/xen/arch/arm/pci/pci-host-common.c
@@ -319,9 +319,14 @@ int pci_host_iterate_bridges_and_count(struct domain *d,
 {
 int ret;
 
-ret = cb(d, bridge);
-if ( ret < 0 )
-return ret;
+if ( cb )
+{
+ret = cb(d, bridge);
+if ( ret < 0 )
+return ret;
+}
+else
+ret = 1;
 count += ret;
 }
 return count;
-- 
2.42.0

[PATCH v6 2/5] xen/vpci: move xen_domctl_createdomain vPCI flag to common

[Stewart Hildebrand]

Both x86 and ARM need a way at domain creation time to specify whether
the domain needs vPCI emulation. Move the vPCI flag from x86
xen_domctl_createdomain.arch.emulation_flags to the common
xen_domctl_createdomain.flags.

Move has_vpci() macro to common header.

Bump XEN_DOMCTL_INTERFACE_VERSION since we're modifying flags inside
struct xen_domctl_createdomain and xen_arch_domainconfig.

Signed-off-by: Oleksandr Andrushchenko 
Signed-off-by: Rahul Singh 
Signed-off-by: Stewart Hildebrand 
Acked-by: Christian Lindig 
---
v5->v6:
* don't explicitly clear XEN_DOMCTL_CDF_vpci where unnecessary
* add comment about avoiding re-using bit 10 in emulation_flags
* don't #define _XEN_DOMCTL_CDF_vpci (underscore-prefixed), only
  introduce single new flag identifier
* move ( vpci && !hvm ) check to xen/arch/x86/domain.c
* simplify has_vpci()

v4->v5:
* move flags_optional change in xen/arch/arm/domain.c to next patch
* change latter 2 args to emulation_flags_ok() to unsigned int
* move vpci && !hvm check to common code
* remove stray spaces in emulation_flags_ok(), and a minor style fixup
* add CONFIG_HAS_VPCI check into has_vpci()
* add Christian's A-b (OCaml)

v3->v4:
* renamed, was:
  ("xen/arm: pci: plumb xen_arch_domainconfig with pci info")
* reworked: move x86 vPCI flag to common instead of adding another arch
  specific vPCI flag
* folded ("xen/arm: make has_vpci() depend on d->arch.has_vpci") into
  this patch (retain Signed-off-by's from [1] and [2])

v2->v3:
* new patch

[1] 
https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc/-/commit/27be1729ce8128dbe37275ce7946b2fbd2e5a382
[2] 
https://github.com/xen-troops/xen/commit/bf12185e6fb2e31db0d8e6ea9ccd8a02abadec17
---
 tools/libs/light/libxl_x86.c  |  2 +-
 tools/ocaml/libs/xc/xenctrl.ml|  2 +-
 tools/ocaml/libs/xc/xenctrl.mli   |  2 +-
 tools/python/xen/lowlevel/xc/xc.c |  2 +-
 xen/arch/arm/include/asm/domain.h |  3 ---
 xen/arch/x86/domain.c | 20 +++-
 xen/arch/x86/include/asm/domain.h |  6 +-
 xen/arch/x86/setup.c  |  4 ++--
 xen/common/domain.c   | 10 +-
 xen/include/public/arch-x86/xen.h |  9 +
 xen/include/public/domctl.h   |  6 --
 xen/include/xen/domain.h  |  2 ++
 12 files changed, 42 insertions(+), 26 deletions(-)

diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c
index d16573e72cd4..4a42b7231f8b 100644
--- a/tools/libs/light/libxl_x86.c
+++ b/tools/libs/light/libxl_x86.c
@@ -8,7 +8,7 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
 {
 switch(d_config->c_info.type) {
 case LIBXL_DOMAIN_TYPE_HVM:
-config->arch.emulation_flags = (XEN_X86_EMU_ALL & ~XEN_X86_EMU_VPCI);
+config->arch.emulation_flags = XEN_X86_EMU_ALL;
 break;
 case LIBXL_DOMAIN_TYPE_PVH:
 config->arch.emulation_flags = XEN_X86_EMU_LAPIC;
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index d6c6eb73db44..6f3da9c6e064 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -46,7 +46,6 @@ type x86_arch_emulation_flags =
   | X86_EMU_IOMMU
   | X86_EMU_PIT
   | X86_EMU_USE_PIRQ
-  | X86_EMU_VPCI
 
 type x86_arch_misc_flags =
   | X86_MSR_RELAXED
@@ -70,6 +69,7 @@ type domain_create_flag =
   | CDF_IOMMU
   | CDF_NESTED_VIRT
   | CDF_VPMU
+  | CDF_VPCI
 
 type domain_create_iommu_opts =
   | IOMMU_NO_SHAREPT
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index 3bfc16edba96..e2dd02bec962 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -40,7 +40,6 @@ type x86_arch_emulation_flags =
   | X86_EMU_IOMMU
   | X86_EMU_PIT
   | X86_EMU_USE_PIRQ
-  | X86_EMU_VPCI
 
 type x86_arch_misc_flags =
   | X86_MSR_RELAXED
@@ -63,6 +62,7 @@ type domain_create_flag =
   | CDF_IOMMU
   | CDF_NESTED_VIRT
   | CDF_VPMU
+  | CDF_VPCI
 
 type domain_create_iommu_opts =
   | IOMMU_NO_SHAREPT
diff --git a/tools/python/xen/lowlevel/xc/xc.c 
b/tools/python/xen/lowlevel/xc/xc.c
index d3ea350e07b9..b9f559a8e637 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -159,7 +159,7 @@ static PyObject *pyxc_domain_create(XcObject *self,
 
 #if defined (__i386) || defined(__x86_64__)
 if ( config.flags & XEN_DOMCTL_CDF_hvm )
-config.arch.emulation_flags = (XEN_X86_EMU_ALL & ~XEN_X86_EMU_VPCI);
+config.arch.emulation_flags = XEN_X86_EMU_ALL;
 #elif defined (__arm__) || defined(__aarch64__)
 config.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
 #else
diff --git a/xen/arch/arm/include/asm/domain.h 
b/xen/arch/arm/include/asm/domain.h
index 5fb8cd79c01a..3614562eaefe 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -298,9 +298,6 @@ static inline void arch_vcpu_block(struct vcpu *v) {}
 
 #define arch_vm_assist_valid_mask(d) (1UL << VMASST_TYPE_runstate_update_flag)
 
-/* vPCI is not available on Arm */
-#define has_vpci(d)({ 

[PATCH v6 1/5] xen/arm: pci: introduce PCI_PASSTHROUGH Kconfig option

[Stewart Hildebrand]

From: Rahul Singh 

Setting CONFIG_PCI_PASSTHROUGH=y will enable PCI passthrough on ARM, even though
the feature is not yet complete in the current upstream codebase. The purpose of
this is to make it easier to enable the necessary configs (HAS_PCI, HAS_VPCI) 
for
testing and development of PCI passthrough on ARM.

Since PCI passthrough on ARM is still work in progress at this time, make it
depend on EXPERT.

Signed-off-by: Rahul Singh 
Signed-off-by: Stefano Stabellini 
Signed-off-by: Stewart Hildebrand 
Acked-by: Julien Grall 
---
(cherry picked from commit 9a08f1f7ce28ec619640ba9ce11018bf443e9a0e from the
 downstream branch [1])

v5->v6:
* no change

v4->v5:
* no change

v3->v4:
* no change

v2->v3:
* add Julien's A-b

v1->v2:
* drop "ARM" naming since it is already in an ARM category
* depend on EXPERT instead of UNSUPPORTED

Changes from downstream to v1:
* depends on ARM_64 (Stefano)
* Don't select HAS_VPCI_GUEST_SUPPORT since this config option is not currently
  used in the upstream codebase. This will want to be re-added here once the
  vpci series [2] is merged.
* Don't select ARM_SMMU_V3 since this option can already be selected
  independently. While PCI passthrough on ARM depends on an SMMU, it does not
  depend on a particular version or variant of an SMMU.
* Don't select HAS_ITS since this option can already be selected independently.
  HAS_ITS may want to be added here once the MSI series [1] is merged.
* Don't select LATE_HWDOM since this option is unrelated to PCI passthrough.

[1] 
https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc/-/commits/poc/pci-passthrough
[2] https://lists.xenproject.org/archives/html/xen-devel/2023-10/msg00660.html
---
 xen/arch/arm/Kconfig | 9 +
 1 file changed, 9 insertions(+)

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 2939db429b78..5ff68e5d5979 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -190,6 +190,15 @@ config STATIC_SHM
help
  This option enables statically shared memory on a dom0less system.
 
+config PCI_PASSTHROUGH
+   bool "PCI passthrough" if EXPERT
+   depends on ARM_64
+   select HAS_PCI
+   select HAS_VPCI
+   default n
+   help
+ This option enables PCI device passthrough
+
 endmenu
 
 menu "ARM errata workaround via the alternative framework"
-- 
2.42.0

[PATCH v6 0/5] Kconfig for PCI passthrough on ARM

[Stewart Hildebrand]

There are multiple series in development/review [1], [2] that will benefit from
having a Kconfig option for PCI passthrough on ARM. Hence I have sent this
series independent from any other series.

v5->v6:
* address feedback in individual patches

v4->v5:
* add [FUTURE] tag to ("xen/arm: enable vPCI for dom0")
* address feedback in individual patches

v3->v4:
* rename ("xen/arm: pci: plumb xen_arch_domainconfig with pci info")
  to ("xen/vpci: move xen_domctl_createdomain vPCI flag to common")
* fold ("xen/arm: make has_vpci() depend on d->arch.has_vpci")
  into ("xen/vpci: move xen_domctl_createdomain vPCI flag to common")
* split ("xen/arm: enable vPCI for domUs") into separate hypervisor and
  tools patches

v2->v3:
* add ("xen/arm: pci: plumb xen_arch_domainconfig with pci info")
* rename ("xen/arm: make has_vpci depend on CONFIG_HAS_VPCI")
  to ("xen/arm: make has_vpci() depend on d->arch.has_vpci")
* add ("xen/arm: enable vPCI for dom0")

v1->v2:
* add ("[FUTURE] xen/arm: enable vPCI for domUs")

[1] https://lists.xenproject.org/archives/html/xen-devel/2023-10/msg00660.html
[2] https://lists.xenproject.org/archives/html/xen-devel/2023-10/msg00210.html

Rahul Singh (1):
  xen/arm: pci: introduce PCI_PASSTHROUGH Kconfig option

Stewart Hildebrand (4):
  xen/vpci: move xen_domctl_createdomain vPCI flag to common
  [FUTURE] xen/arm: enable vPCI for dom0
  [FUTURE] xen/arm: enable vPCI for domUs
  [FUTURE] tools/arm: enable vPCI for domUs

 tools/libs/light/libxl_arm.c   |  3 +++
 tools/libs/light/libxl_x86.c   |  2 +-
 tools/ocaml/libs/xc/xenctrl.ml |  2 +-
 tools/ocaml/libs/xc/xenctrl.mli|  2 +-
 tools/python/xen/lowlevel/xc/xc.c  |  2 +-
 xen/arch/arm/Kconfig   | 10 ++
 xen/arch/arm/domain.c  |  3 ++-
 xen/arch/arm/domain_build.c|  6 ++
 xen/arch/arm/include/asm/domain.h  |  5 ++---
 xen/arch/arm/include/asm/pci.h |  9 +
 xen/arch/arm/pci/pci-host-common.c | 11 ---
 xen/arch/arm/vpci.c|  8 
 xen/arch/x86/domain.c  | 20 +++-
 xen/arch/x86/include/asm/domain.h  |  8 +++-
 xen/arch/x86/setup.c   |  4 ++--
 xen/common/domain.c| 15 ++-
 xen/drivers/passthrough/pci.c  | 20 
 xen/include/public/arch-x86/xen.h  |  9 +
 xen/include/public/domctl.h|  6 --
 xen/include/xen/domain.h   |  2 ++
 20 files changed, 117 insertions(+), 30 deletions(-)


base-commit: fb41228ececea948c7953c8c16fe28fd65c6536b
-- 
2.42.0

Re: [PATCH v2 26/29] tools/xenstored: mount 9pfs device in stubdom

[Julien Grall]

Hi Juergen,

On 10/11/2023 16:08, Juergen Gross wrote:

diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
index 162b87b460..4263c1360f 100644
--- a/tools/xenstored/domain.c
+++ b/tools/xenstored/domain.c
@@ -1236,6 +1236,8 @@ void stubdom_init(void)
barf_perror("Failed to initialize stubdom");
  
  	xenevtchn_notify(xce_handle, stubdom->port);

+
+   mount_9pfs();
  #endif
  }
  
diff --git a/tools/xenstored/minios.c b/tools/xenstored/minios.c

index 202d70387a..fddbede869 100644
--- a/tools/xenstored/minios.c
+++ b/tools/xenstored/minios.c
@@ -19,8 +19,16 @@
  #include 
  #include "core.h"
  #include 
+#include 
  #include 
  #include 
+#include 
+#include 
+#include 
+
+#define P9_STATE_PATH  "device/9pfs/0/state"
+
+static void *p9_device;
  
  void write_pidfile(const char *pidfile)

  {
@@ -62,3 +70,31 @@ void early_init(void)
if (stub_domid == DOMID_INVALID)
barf("could not get own domid");
  }
+
+static void mount_thread(void *p)
+{
+   xenbus_event_queue events = NULL;
+   char *err;
+   char *dummy;
+
+   free(xenbus_watch_path_token(XBT_NIL, P9_STATE_PATH, "9pfs", &events));


AFAICT, xenbus_watch_path_token() can fail. I agree this is unlikely, 
but if it fails, then it would be useful to get some logs. Otherwise...



+
+   for (;;) {


... this loop would be infinite.


+   xenbus_wait_for_watch(&events);
+   err = xenbus_read(XBT_NIL, P9_STATE_PATH, &dummy);


Can you explain why don't care about the value of the node?


+   if (!err)
+   break;
+   free(err);
+   }
+
+   free(dummy);
+
+   free(xenbus_unwatch_path_token(XBT_NIL, P9_STATE_PATH, "9pfs"));


xenbus_unwatch_path_token() could technically fails. It would be helpful 
to print a message.



+
+   p9_device = init_9pfront(0, XENSTORE_LIB_DIR);
+}
+
+void mount_9pfs(void)
+{
+   create_thread("mount-9pfs", mount_thread, NULL);
+}


Cheers,

--
Julien Grall

Re: [PATCH v2 25/29] tools/xenstored: map stubdom interface

[Julien Grall]

Hi Juergen,

On 10/11/2023 16:08, Juergen Gross wrote:

When running as stubdom, map the stubdom's Xenstore ring page in order
to support using the 9pfs frontend.

Signed-off-by: Juergen Gross 
---
  tools/xenstored/core.c   |  2 ++
  tools/xenstored/domain.c | 27 ++-
  tools/xenstored/domain.h |  1 +
  3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
index b9ec50b34c..4a9d874f17 100644
--- a/tools/xenstored/core.c
+++ b/tools/xenstored/core.c
@@ -2991,6 +2991,8 @@ int main(int argc, char *argv[])
lu_read_state();
  #endif
  
+	stubdom_init();

+
check_store();
  
  	/* Get ready to listen to the tools. */

diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
index fa17f68618..162b87b460 100644
--- a/tools/xenstored/domain.c
+++ b/tools/xenstored/domain.c
@@ -37,6 +37,10 @@
  #include 
  #include 
  
+#ifdef __MINIOS__

+#include 
+#endif
+
  static xc_interface **xc_handle;
  xengnttab_handle **xgt_handle;
  static evtchn_port_t virq_port;
@@ -500,6 +504,11 @@ static void *map_interface(domid_t domid)
if (domid == xenbus_master_domid())
return xenbus_map();
  
+#ifdef __MINIOS__

+   if (domid == stub_domid)
+   return xenstore_buf;
+#endif
+
return xengnttab_map_grant_ref(*xgt_handle, domid,
   GNTTAB_RESERVED_XENSTORE,
   PROT_READ|PROT_WRITE);
@@ -509,7 +518,7 @@ static void unmap_interface(domid_t domid, void *interface)
  {
if (domid == xenbus_master_domid())
unmap_xenbus(interface);
-   else
+   else if (domid != stub_domid)
xengnttab_unmap(*xgt_handle, interface, 1);
  }
  
@@ -1214,6 +1223,22 @@ void dom0_init(void)

xenevtchn_notify(xce_handle, dom0->port);
  }
  
+void stubdom_init(void)

+{
+#ifdef __MINIOS__
+   struct domain *stubdom;
+
+   if (stub_domid < 0)
+   return;
+
+   stubdom = introduce_domain(NULL, stub_domid, xenbus_evtchn, false);
+   if (!stubdom)
+   barf_perror("Failed to initialize stubdom");
+
+   xenevtchn_notify(xce_handle, stubdom->port);


If I compare to introduce_domain(), it is not entirely clear to me why 
the notification is done unconditionally here. Can you clarify?



+#endif
+}
+
  static unsigned int domhash_fn(const void *k)
  {
return *(const unsigned int *)k;
diff --git a/tools/xenstored/domain.h b/tools/xenstored/domain.h
index 6c00540311..49c60c56bf 100644
--- a/tools/xenstored/domain.h
+++ b/tools/xenstored/domain.h
@@ -85,6 +85,7 @@ int do_reset_watches(const void *ctx, struct connection *conn,
  void domain_static_init(void);
  void domain_init(int evtfd);
  void dom0_init(void);
+void stubdom_init(void);
  void domain_deinit(void);
  void ignore_connection(struct connection *conn, unsigned int err);
  


Cheers,

--
Julien Grall

Re: [PATCH v2 22/29] tools/xenstored: get own domid in stubdom case

[Julien Grall]

Hi Juergen,

On 10/11/2023 16:07, Juergen Gross wrote:

Obtain the own domid from the Xenstore special grant entry when running
as stubdom.

Signed-off-by: Juergen Gross 
---
V2:
- replacement of V1 patch (ANdrew Cooper)
---
  tools/xenstored/core.c   | 1 +
  tools/xenstored/core.h   | 1 +
  tools/xenstored/minios.c | 5 +
  3 files changed, 7 insertions(+)

diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
index 0c14823fb0..8e271e31f9 100644
--- a/tools/xenstored/core.c
+++ b/tools/xenstored/core.c
@@ -2738,6 +2738,7 @@ static struct option options[] = {
  int dom0_domid = 0;
  int dom0_event = 0;
  int priv_domid = 0;
+int stub_domid = -1;


After looking at partch #25, I feel that it would make more sense if 
stub_domid is a domid_t and initialized to DOMID_INVALID.


At least this makes clearer that initial value is not supported to be a 
valid domid.


Cheers,

--
Julien Grall

Re: Faking the number of CPUs for dom0 with dom0_max_vcpus

[Jimmy Lee]

Hi Jan, thanks for the response! Adding more details and log files here:

1. I installed Xen 4.14 on CentOS 7 with yum:

[root@test-xen ~]# yum list xen kernel
Loaded plugins: fastestmirror
Loading mirror speeds from cached hostfile
 * base: download.cf.centos.org
 * centos-virt-xen-epel: d2lzkl7pfhq30w.cloudfront.net
 * extras: download.cf.centos.org
 * updates: download.cf.centos.org
Installed Packages
kernel.x86_64
 3.10.0-1160.45.1.el7
 installed
kernel.x86_64
 3.10.0-1160.76.1.el7
 @updates
kernel.x86_644.9.241-37.el7

 @centos-virt-xen-414
xen.x86_64
4.14.5.24.g87d90d511c-1.el7
@centos-virt-xen-414
[root@test-xen ~]# uname -r
4.9.241-37.el7.x86_64

2. I configured dom0_max_vcpus=16 for Xen, and nr_cpus=16 for kernel:

[root@test-xen ~]# xl info
host   : ip-172-31-52-150.us-west-2.compute.internal
release: 4.9.241-37.el7.x86_64
version: #1 SMP Mon Nov 2 13:55:04 UTC 2020
machine: x86_64
nr_cpus: 4
max_cpu_id : 3
nr_nodes   : 1
cores_per_socket   : 2
threads_per_core   : 2
cpu_mhz: 2999.974
hw_caps:
1f8bfbff:f7fa3203:2c100800:0121:000f:d19f47ab:0008:0100
virt_caps  : pv shadow
total_memory   : 7891
free_memory: 3703
sharing_freed_memory   : 0
sharing_used_memory: 0
outstanding_claims : 0
free_cpus  : 0
xen_major  : 4
xen_minor  : 14
xen_extra  : .5.24.g87d90d51
xen_version: 4.14.5.24.g87d90d51
xen_caps   : xen-3.0-x86_64
xen_scheduler  : credit2
xen_pagesize   : 4096
platform_params: virt_start=0x8000
xen_changeset  :
xen_commandline: placeholder dom0_max_vcpus=16
dom0_mem=4096M,max:4096M cpuinfo com1=115200,8n1 console=com1,tty
loglvl=all guest_loglvl=all
cc_compiler: gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-44)
cc_compile_by  : mockbuild
cc_compile_domain  : centos.org
cc_compile_date: Mon Jul 25 08:40:16 UTC 2022
build_id   : 00d714f3c5983a534f512ca173dbfdcc7ed5a1b5
xend_config_format : 4
[root@test-xen ~]# cat /proc/cmdline
placeholder nr_cpus=16 root=UUID=44a6a613-4e21-478b-a909-ab653c9d39df ro
console=tty0 crashkernel=auto net.ifnames=0 console=ttyS0 console=hvc0
earlyprintk=xen nomodeset
[root@test-xen ~]#

3. With the above config, dom0 only recognized 4 CPUs, and all the other
vCPUs are offline:

[root@test-xen ~]# xl vcpu-list
NameID  VCPU   CPU State   Time(s) Affinity
(Hard / Soft)
Domain-0 0 03   -b-  14.7  all / all
Domain-0 0 13   -b-  11.2  all / all
Domain-0 0 20   r--  12.6  all / all
Domain-0 0 32   -b-  10.9  all / all
Domain-0 0 4-   --p   0.0  all / all
Domain-0 0 5-   --p   0.0  all / all
Domain-0 0 6-   --p   0.0  all / all
Domain-0 0 7-   --p   0.0  all / all
Domain-0 0 8-   --p   0.0  all / all
Domain-0 0 9-   --p   0.0  all / all
Domain-0 010-   --p   0.0  all / all
Domain-0 011-   --p   0.0  all / all
Domain-0 012-   --p   0.0  all / all
Domain-0 013-   --p   0.0  all / all
Domain-0 014-   --p   0.0  all / all
Domain-0 015-   --p   0.0  all / all
[root@test-xen ~]#

There are only 4 CPUs under /sys/devices/system/cpu:

[root@test-xen ~]# ls -l /sys/devices/system/cpu
total 0
drwxr-xr-x. 6 root root0 Nov 13 18:04 cpu0
drwxr-xr-x. 6 root root0 Nov 13 18:04 cpu1
drwxr-xr-x. 6 root root0 Nov 13 18:04 cpu2
drwxr-xr-x. 6 root root0 Nov 13 18:04 cpu3
drwxr-xr-x. 2 root root0 Nov 13 18:43 hotplug
-r--r--r--. 1 root root 4096 Nov 13 18:43 isolated
-r--r--r--. 1 root root 4096 Nov 13 18:43 kernel_max
-r--r--r--. 1 root root 4096 Nov 13 18:43 modalias
-r--r--r--. 1 root root 4096 Nov 13 18:43 nohz_full
-r--r--r--. 1 root root 4096 Nov 13 18:43 offline
-r--r--r--. 1 root root 4096 Nov 13 18:04 online
-r--r--r--. 1 root root 4096 Nov 13 18:43 possible
drwxr-xr-x. 2 root root0 Nov 13 18:43 power
-r--r--r--. 1 root root 4096 Nov 13 18:43 present
drwxr-xr-x. 2 root root0 Nov 13 18:43 smt
-rw-r--r--. 1 root root 4096 Nov 13 18:43 uevent
drwxr-xr-x. 2 root root0 Nov 13 18:43 vulnerabilities

I have attached dmesg for the kernel and xen. Please let me know if you
have any thoughts. Thanks!

Re: [PATCH v5 4/5] [FUTURE] xen/arm: enable vPCI for domUs

[Stewart Hildebrand]

On 11/6/23 04:26, Jan Beulich wrote:
> On 02.11.2023 20:59, Stewart Hildebrand wrote:
>> --- a/xen/arch/x86/include/asm/domain.h
>> +++ b/xen/arch/x86/include/asm/domain.h
>> @@ -503,6 +503,15 @@ struct arch_domain
>>  #define has_vpit(d)(!!((d)->arch.emulation_flags & X86_EMU_PIT))
>>  #define has_pirq(d)(!!((d)->arch.emulation_flags & 
>> X86_EMU_USE_PIRQ))
>>  
>> +#define is_pvh_domain(d) ({  \
>> +unsigned int _emflags = (d)->arch.emulation_flags;   \
>> +IS_ENABLED(CONFIG_HVM) &&\
>> +((_emflags == X86_EMU_LAPIC) ||  \
>> + (_emflags == (X86_EMU_LAPIC | X86_EMU_IOAPIC))); })
> 
> I'm not convinced we want to re-introduce such a predicate; it'll be at
> risk of going stale when the boundary between HVM and PVH becomes more
> "fuzzy".

OK, I'll drop it

> 
>> +/* PCI passthrough may be backed by qemu for non-PVH domains */
>> +#define arch_needs_vpci(d) is_pvh_domain(d)
> 
> Wouldn't we want to check for exactly what the comment alludes to then,
> i.e. whether the domain has any (specific?) device model attached?

This patch is primarily dealing with Arm, so I'm considering simply making it 
return false for now:

#define arch_needs_vpci(d) ({ (void)(d); false; })

If it needs to be changed in the future when we enable vPCI for PVH domUs, we 
can deal with it at that time.

> 
>> --- a/xen/include/xen/domain.h
>> +++ b/xen/include/xen/domain.h
>> @@ -51,8 +51,17 @@ void arch_get_domain_info(const struct domain *d,
>>  
>>  #define is_domain_using_staticmem(d) ((d)->cdf & CDF_staticmem)
>>  
>> -#define has_vpci(d) (((d)->options & XEN_DOMCTL_CDF_vpci) && \
>> - IS_ENABLED(CONFIG_HAS_VPCI))
>> +#define has_vpci(d) ({  
>>   \
>> +const struct domain *_d = (d);  
>>   \
>> +bool _has_vpci = false; 
>>   \
>> +if ( (_d->options & XEN_DOMCTL_CDF_vpci) && IS_ENABLED(CONFIG_HAS_VPCI) 
>> ) \
>> +{   
>>   \
>> +if ( is_hardware_domain(_d) )   
>>   \
>> +_has_vpci = true;   
>>   \
>> +else if ( IS_ENABLED(CONFIG_HAS_VPCI_GUEST_SUPPORT) )   
>>   \
>> +_has_vpci = true;   
>>   \
>> +}   
>>   \
>> +_has_vpci; })
> 
> This is a commonly executed check, and as such wants to remain as simple as
> possible. Wouldn't it be better anyway to prevent XEN_DOMCTL_CDF_vpci getting
> set for a domain which cannot possibly have vPCI?

Yes, agreed, I'll leave has_vpci alone

> 
> Jan

Re: [PATCH-for-9.0 01/10] sysemu/xen: Forbid using Xen headers in user emulation

[David Woodhouse]

On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
> Xen is a system specific accelerator, it makes no sense
> to include its headers in user emulation.
> 
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: David Woodhouse 




smime.p7s
Description: S/MIME cryptographic signature

Re: [PATCH-for-9.0 10/10] hw/xen: Have most of Xen files become target-agnostic

[David Woodhouse]

On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
> Previous commits re-organized the target-specific bits
> from Xen files. We can now build the common files once
> instead of per-target.
> 
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: David Woodhouse 



smime.p7s
Description: S/MIME cryptographic signature

Re: [PATCH-for-9.0 09/10] hw/xen: Extract 'xen_igd.h' from 'xen_pt.h'

[David Woodhouse]

On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
> "hw/xen/xen_pt.h" requires "hw/xen/xen_native.h" which is target
> specific. It also declares IGD methods, which are not target
> specific.
> 
> Target-agnostic code can use IGD methods. To allow that, extract
> these methos into a new "hw/xen/xen_igd.h" header.
> 
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: David Woodhouse 

> ---
> What license for the new "hw/xen/xen_igd.h" header?

The existing xen_pt.h came in with xen_pt.c (GPLv2) in commit
eaab4d60d. I think it has to be GPLv2 (and not later) just like
xen_pt.c?



smime.p7s
Description: S/MIME cryptographic signature

Re: [PATCH-for-9.0 08/10] system/physmem: Only include 'hw/xen/xen.h' when Xen is available

[David Woodhouse]

On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
> "hw/xen/xen.h" contains declarations for Xen hardware. There is
> no point including it when Xen is not available.

... if even when Xen *is* available, AFAICT. Can you just remove the
inclusion of hw/xen/xen.h entirely? I think that still builds, at least
for x86.

>  When Xen is not
> available, we have enough with declarations of "sysemu/xen.h".

... and system/xen-mapcache.h

> Signed-off-by: Philippe Mathieu-Daudé 



smime.p7s
Description: S/MIME cryptographic signature

Re: [PATCH-for-9.0 07/10] sysemu/xen-mapcache: Check Xen availability with CONFIG_XEN_IS_POSSIBLE

[David Woodhouse]

On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
> "sysemu/xen.h" defines CONFIG_XEN_IS_POSSIBLE as a target-agnostic
> version of CONFIG_XEN. Use it in order to use "sysemu/xen-mapcache.h"
> in target-agnostic files.
> 
> Signed-off-by: Philippe Mathieu-Daudé 

Noting that CONFIG_XEN_IS_POSSIBLE is for Xen accelerator support, and
may not be set in all cases when we're hosting Xen-compatible guests,

Reviewed-by: David Woodhouse 


smime.p7s
Description: S/MIME cryptographic signature

Re: [PATCH-for-9.0 06/10] hw/xen: Reduce inclusion of 'cpu.h' to target-specific sources

[David Woodhouse]

On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
> We rarely need to include "cpu.h" in headers. Including it
> 'taint' headers to be target-specific. Here only the i386/arm
> implementations requires "cpu.h", so include it there and
> remove from the "hw/xen/xen-hvm-common.h" *common* header.
> 
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: David Woodhouse 



smime.p7s
Description: S/MIME cryptographic signature

Re: [PATCH-for-9.0 05/10] hw/xen: Use target-agnostic qemu_target_page_bits()

[David Woodhouse]

On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
> Instead of the target-specific TARGET_PAGE_BITS definition,
> use qemu_target_page_bits() which is target agnostic.
> 
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: David Woodhouse 


smime.p7s
Description: S/MIME cryptographic signature

Re: [PATCH v5 2/5] xen/vpci: move xen_domctl_createdomain vPCI flag to common

[Stewart Hildebrand]

On 11/13/23 08:26, Jan Beulich wrote:
> On 02.11.2023 20:59, Stewart Hildebrand wrote:
>> --- a/tools/libs/light/libxl_x86.c
>> +++ b/tools/libs/light/libxl_x86.c
>> @@ -8,13 +8,16 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
>>  {
>>  switch(d_config->c_info.type) {
>>  case LIBXL_DOMAIN_TYPE_HVM:
>> -config->arch.emulation_flags = (XEN_X86_EMU_ALL & 
>> ~XEN_X86_EMU_VPCI);
>> +config->arch.emulation_flags = XEN_X86_EMU_ALL;
>> +config->flags &= ~XEN_DOMCTL_CDF_vpci;
>>  break;
>>  case LIBXL_DOMAIN_TYPE_PVH:
>>  config->arch.emulation_flags = XEN_X86_EMU_LAPIC;
>> +config->flags &= ~XEN_DOMCTL_CDF_vpci;
>>  break;
>>  case LIBXL_DOMAIN_TYPE_PV:
>>  config->arch.emulation_flags = 0;
>> +config->flags &= ~XEN_DOMCTL_CDF_vpci;
>>  break;
> 
> Why all this explicit clearing of XEN_DOMCTL_CDF_vpci? I can't even spot
> where the bit would be set.

You're right, it's not being set currently, so no need to explicitly clear the 
bit here. I'll remove.

> 
>> --- a/tools/python/xen/lowlevel/xc/xc.c
>> +++ b/tools/python/xen/lowlevel/xc/xc.c
>> @@ -159,7 +159,10 @@ static PyObject *pyxc_domain_create(XcObject *self,
>>  
>>  #if defined (__i386) || defined(__x86_64__)
>>  if ( config.flags & XEN_DOMCTL_CDF_hvm )
>> -config.arch.emulation_flags = (XEN_X86_EMU_ALL & ~XEN_X86_EMU_VPCI);
>> +{
>> +config.arch.emulation_flags = XEN_X86_EMU_ALL;
>> +config.flags &= ~XEN_DOMCTL_CDF_vpci;
>> +}
> 
> Same question here then.

Same answer here.

> 
>> @@ -575,6 +577,18 @@ static int sanitise_domain_config(struct 
>> xen_domctl_createdomain *config)
>>  return -EINVAL;
>>  }
>>  
>> +if ( vpci && !hvm )
>> +{
>> +dprintk(XENLOG_INFO, "vPCI requested for non-HVM guest\n");
>> +return -EINVAL;
>> +}
>> +
>> +if ( vpci && !IS_ENABLED(CONFIG_HAS_VPCI) )
>> +{
>> +dprintk(XENLOG_INFO, "vPCI requested but not enabled\n");
>> +return -EINVAL;
>> +}
> 
> Maybe flip the order of these checks? But I'm uncertain about the first
> one anyway: Isn't this something that needs deciding per-arch?

In v4, the equivalent of the ( vpci && !hvm ) check was indeed in 
xen/arch/x86/domain.c:emulation_flags_ok(), but it seemed there was a 
suggestion that it be moved to common code... See discussion at [1]. How about 
putting it back into xen/arch/x86/domain.c, in arch_sanitise_domain_config(), 
not emulation_flags_ok()?

[1] https://lists.xenproject.org/archives/html/xen-devel/2023-10/msg02345.html

> 
>> --- a/xen/include/public/arch-x86/xen.h
>> +++ b/xen/include/public/arch-x86/xen.h
>> @@ -283,15 +283,12 @@ struct xen_arch_domainconfig {
>>  #define XEN_X86_EMU_PIT (1U<<_XEN_X86_EMU_PIT)
>>  #define _XEN_X86_EMU_USE_PIRQ   9
>>  #define XEN_X86_EMU_USE_PIRQ(1U<<_XEN_X86_EMU_USE_PIRQ)
>> -#define _XEN_X86_EMU_VPCI   10
>> -#define XEN_X86_EMU_VPCI(1U<<_XEN_X86_EMU_VPCI)
> 
> This, aiui, being the reason for ...
> 
>> --- a/xen/include/public/domctl.h
>> +++ b/xen/include/public/domctl.h
>> @@ -21,7 +21,7 @@
>>  #include "hvm/save.h"
>>  #include "memory.h"
>>  
>> -#define XEN_DOMCTL_INTERFACE_VERSION 0x0016
>> +#define XEN_DOMCTL_INTERFACE_VERSION 0x0017
> 
> ... this bump, I wonder whether nevertheless we wouldn't better leave a
> comment there to indicate that bit 10 better wouldn't be used again any
> time soon.

I'll add a comment (above, in arch-x86/xen.h)

> 
>> @@ -55,9 +55,12 @@ struct xen_domctl_createdomain {
>>  #define XEN_DOMCTL_CDF_nested_virt(1U << _XEN_DOMCTL_CDF_nested_virt)
>>  /* Should we expose the vPMU to the guest? */
>>  #define XEN_DOMCTL_CDF_vpmu   (1U << 7)
>> +/* Should vPCI be enabled for the guest? */
>> +#define _XEN_DOMCTL_CDF_vpci  8
> 
> What is this needed for besides ...
> 
>> +#define XEN_DOMCTL_CDF_vpci   (1U<<_XEN_DOMCTL_CDF_vpci)
> 
> ... its use here? Imo like was done for vpmu, there should be only one
> new identifier. As an aside, there are blanks missing around <<.

OK, I'll fix this

> 
>> --- a/xen/include/xen/domain.h
>> +++ b/xen/include/xen/domain.h
>> @@ -51,6 +51,9 @@ void arch_get_domain_info(const struct domain *d,
>>  
>>  #define is_domain_using_staticmem(d) ((d)->cdf & CDF_staticmem)
>>  
>> +#define has_vpci(d) (((d)->options & XEN_DOMCTL_CDF_vpci) && \
>> + IS_ENABLED(CONFIG_HAS_VPCI))
> 
> Aiui the IS_ENABLED() is wanted so where suitable code conditional upon
> this predicate can be eliminated by the compiler. Question is whether
> we can't achieve this by, say, overriding XEN_DOMCTL_CDF_vpci to 0 in
> such cases (without touching what you add to the public header, i.e.
> not as easy as what we do in xen/arch/x86/include/asm/domain.h for
> X86_EMU_*).

I had only added the IS_ENABLED() here due to some (unnecessary) related 
changes in the later patch ("xen/arm: enable vPCI for 

Re: [PATCH v12 19/37] x86/fred: Update MSR_IA32_FRED_RSP0 during task switch

[H. Peter Anvin]

On November 13, 2023 1:29:47 PM EST, Borislav Petkov  wrote:
>On Mon, Nov 13, 2023 at 12:36:04PM -0500, H. Peter Anvin wrote:
>> A resource cannot be consumed after the value has been written; this
>> is the only necessary level of serialization, equivalent to, say, RAX.
>
>Lemme see if I understand this correctly using this context as an
>example: after this MSR_IA32_FRED_RSP0 write, any FRED events determined
>to be delivered to level 0 will use this new task stack ptr?
>
>And since the new task is not running yet and the old one isn't running
>either, we're fine here. So the "serialization point" I was talking
>about above is bollocks.
>
>Close? :)
>
>> A serializing instruction stops the entire pipeline until everything
>> has retired and any stores have become globally visible.
>
>Right, we don't need that here.
>
>Thx.
>

Yep!

Re: [PATCH v2 06/15] xen/asm-generic: ifdef inclusion of

[Oleksii]

On Mon, 2023-11-13 at 18:01 +0100, Jan Beulich wrote:
> On 11.11.2023 11:24, Oleksii wrote:
> > This patch should be reworked as it fails Arm builds:
> > https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/1068867920
> 
> Took me a while to actually find the error. Would have been nice if
> you
> had explained what's going wrong, supplying the link only as extra
> info.
Sorry about that. I'll be more precise with links next time! But the
issue is exactly what you wrote below ( with p2m_mem_access_check and
some others from  ) .

> 
> > It looks like it is not possible just to #ifdef CONFIG_MEM_ACCESS.
> > 
> > An empty asm-generic mem_access.h will be better solution.
> 
> I remain unconvinced. The issue looks to be with
> p2m_mem_access_check().
> One solution would be to move that declaration into the common
> header.
> But there's no common code referencing it, so Arm's and x86's version
> might as well diverge at some point. Hence from my pov it again boils
> down to Arm's traps.c including asm/mem_access.h explicitly, to bring
> the declaration in scope. None of the common files really have a need
> to have a dependency on the arch-specific header when MEM_ACCESS=n.
I started to do that in that way you mentioned.

It should be included in 2 files, at least: p2m.c and traps.c.
I am finishind the testing if it is enough.
If it is enough, I will send a separate patch.

~ Oleksii

Re: [PATCH v12 19/37] x86/fred: Update MSR_IA32_FRED_RSP0 during task switch

[Borislav Petkov]

On Mon, Nov 13, 2023 at 12:36:04PM -0500, H. Peter Anvin wrote:
> A resource cannot be consumed after the value has been written; this
> is the only necessary level of serialization, equivalent to, say, RAX.

Lemme see if I understand this correctly using this context as an
example: after this MSR_IA32_FRED_RSP0 write, any FRED events determined
to be delivered to level 0 will use this new task stack ptr?

And since the new task is not running yet and the old one isn't running
either, we're fine here. So the "serialization point" I was talking
about above is bollocks.

Close? :)

> A serializing instruction stops the entire pipeline until everything
> has retired and any stores have become globally visible.

Right, we don't need that here.

Thx.

-- 
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH-for-9.0 06/10] hw/xen: Reduce inclusion of 'cpu.h' to target-specific sources

[Richard Henderson]

On 11/13/23 07:21, Philippe Mathieu-Daudé wrote:

We rarely need to include "cpu.h" in headers. Including it
'taint' headers to be target-specific. Here only the i386/arm
implementations requires "cpu.h", so include it there and
remove from the "hw/xen/xen-hvm-common.h" *common* header.

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/hw/xen/xen-hvm-common.h | 1 -
  hw/arm/xen_arm.c| 1 +
  hw/i386/xen/xen-hvm.c   | 1 +
  3 files changed, 2 insertions(+), 1 deletion(-)


Reviewed-by: Richard Henderson 


r~

Re: [PATCH-for-9.0 05/10] hw/xen: Use target-agnostic qemu_target_page_bits()

[Richard Henderson]

On 11/13/23 07:21, Philippe Mathieu-Daudé wrote:

Instead of the target-specific TARGET_PAGE_BITS definition,
use qemu_target_page_bits() which is target agnostic.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/xen/xen-hvm-common.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [PATCH-for-9.0 04/10] hw/xen: Factor xen_arch_align_ioreq_data() out of handle_ioreq()

[Richard Henderson]

On 11/13/23 07:21, Philippe Mathieu-Daudé wrote:

diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index c028c1b541..03f9417e7e 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -426,10 +426,7 @@ static void handle_ioreq(XenIOState *state, ioreq_t *req)
  trace_handle_ioreq(req, req->type, req->dir, req->df, req->data_is_ptr,
 req->addr, req->data, req->count, req->size);
  
-if (!req->data_is_ptr && (req->dir == IOREQ_WRITE) &&

-(req->size < sizeof (target_ulong))) {
-req->data &= ((target_ulong) 1 << (8 * req->size)) - 1;
-}



I suspect this should never have been using target_ulong at all: req->data is 
uint64_t.


r~

Re: [PATCH-for-9.0 03/10] hw/xen: Merge 'hw/xen/arch_hvm.h' in 'hw/xen/xen-hvm-common.h'

[Richard Henderson]

On 11/13/23 07:21, Philippe Mathieu-Daudé wrote:

We don't need a target-specific header for common target-specific
prototypes. Declare xen_arch_handle_ioreq() and xen_arch_set_memory()
in "hw/xen/xen-hvm-common.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/hw/arm/xen_arch_hvm.h   |  9 -
  include/hw/i386/xen_arch_hvm.h  | 11 ---
  include/hw/xen/arch_hvm.h   |  5 -
  include/hw/xen/xen-hvm-common.h |  6 ++
  hw/arm/xen_arm.c|  1 -
  hw/i386/xen/xen-hvm.c   |  1 -
  hw/xen/xen-hvm-common.c |  1 -
  7 files changed, 6 insertions(+), 28 deletions(-)
  delete mode 100644 include/hw/arm/xen_arch_hvm.h
  delete mode 100644 include/hw/i386/xen_arch_hvm.h
  delete mode 100644 include/hw/xen/arch_hvm.h



Reviewed-by: Richard Henderson 


r~

Re: [PATCH-for-9.0 02/10] hw/xen/xen_arch_hvm: Rename prototypes using 'xen_arch_' prefix

[Richard Henderson]

On 11/13/23 07:21, Philippe Mathieu-Daudé wrote:

Use a common 'xen_arch_' prefix for architecture-specific functions.
Rename xen_arch_set_memory() and xen_arch_handle_ioreq().

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/hw/arm/xen_arch_hvm.h  | 4 ++--
  include/hw/i386/xen_arch_hvm.h | 4 ++--
  hw/arm/xen_arm.c   | 4 ++--
  hw/i386/xen/xen-hvm.c  | 6 +++---
  hw/xen/xen-hvm-common.c| 4 ++--
  5 files changed, 11 insertions(+), 11 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [PATCH-for-9.0 01/10] sysemu/xen: Forbid using Xen headers in user emulation

[Richard Henderson]

On 11/13/23 07:21, Philippe Mathieu-Daudé wrote:

Xen is a system specific accelerator, it makes no sense
to include its headers in user emulation.

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/sysemu/xen.h | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)


Reviewed-by: Richard Henderson 


r~

xen | Successful pipeline for staging-4.18 | 509f737d

[GitLab]

Pipeline #1070443312 has passed!

Project: xen ( https://gitlab.com/xen-project/xen )
Branch: staging-4.18 ( 
https://gitlab.com/xen-project/xen/-/commits/staging-4.18 )

Commit: 509f737d ( 
https://gitlab.com/xen-project/xen/-/commit/509f737d3e80c75f948eb896a08c324287b00adc
 )
Commit Message: docs/sphinx: Fix indexing

sphinx-build reports...
Commit Author: Andrew Cooper ( https://gitlab.com/andyhhp )



Pipeline #1070443312 ( 
https://gitlab.com/xen-project/xen/-/pipelines/1070443312 ) triggered by Ganis 
( https://gitlab.com/ganis )
successfully completed 129 jobs in 3 stages.

-- 
You're receiving this email because of your account on gitlab.com.

Re: [PATCH] xen/x86: On x2APIC mode, derive LDR from APIC_ID

[Roger Pau Monné]

On Mon, Nov 13, 2023 at 04:50:23PM +, Alejandro Vallejo wrote:
> Both Intel and AMD manuals agree that on x2APIC mode, the APIC LDR and ID
> registers are derivable from each other through a fixed formula.
> 
> Xen uses that formula, but applies it to vCPU IDs (which are sequential)
> rather than x2APIC_IDs (which are not, at the moment). As I understand it,
> this is an attempt to tightly pack vCPUs into clusters so each cluster has
> 16 vCPUs rather than 8, but this is problematic for OSs that might read the
> x2APIC_ID and internally derive LDR (or the other way around)

I would replace the underscore from x2APIC ID with a space instead.

Seeing the commit that introduced the bogus LDR value, I'm not sure it
was intentional, as previous Xen code had:

u32 id = vlapic_get_reg(vlapic, APIC_ID);
u32 ldr = ((id & ~0xf) << 16) | (1 << (id & 0xf));
vlapic_set_reg(vlapic, APIC_LDR, ldr);

Which was correct, as the LDR was derived from the APIC ID and not the
vCPU ID.

> This patch fixes the implementation so we follow the rules in the x2APIC
> spec(s).
> 
> While in the neighborhood, replace the u32 type with the standard uint32_t

Likely wants:

Fixes: f9e0cccf7b35 ('x86/HVM: fix ID handling of x2APIC emulation')

> Signed-off-by: Alejandro Vallejo 

I do wonder whether we need to take any precautions with guests being
able to trigger an APIC reset, and thus seeing a changed LDR register
if the guest happens to be migrated from an older hypervisor version
that doesn't have this fix.  IOW: I wonder whether Xen should keep the
previous bogus LDR value across APIC resets for guests that have
already seen it.

Thanks, Roger.

Re: [PATCH v12 19/37] x86/fred: Update MSR_IA32_FRED_RSP0 during task switch

[H. Peter Anvin]

On November 13, 2023 4:37:42 AM EST, Borislav Petkov  wrote:
>On Mon, Oct 02, 2023 at 11:24:40PM -0700, Xin Li wrote:
>> From: "H. Peter Anvin (Intel)" 
>> 
>> MSR_IA32_FRED_RSP0 is used during ring 3 event delivery, and needs to
>> be updated to point to the top of next task stack during task switch.
>> 
>> Signed-off-by: H. Peter Anvin (Intel) 
>> Tested-by: Shan Kang 
>> Signed-off-by: Xin Li 
>> ---
>>  arch/x86/include/asm/switch_to.h | 8 ++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>> 
>> diff --git a/arch/x86/include/asm/switch_to.h 
>> b/arch/x86/include/asm/switch_to.h
>> index f42dbf17f52b..c3bd0c0758c9 100644
>> --- a/arch/x86/include/asm/switch_to.h
>> +++ b/arch/x86/include/asm/switch_to.h
>> @@ -70,9 +70,13 @@ static inline void update_task_stack(struct task_struct 
>> *task)
>>  #ifdef CONFIG_X86_32
>>  this_cpu_write(cpu_tss_rw.x86_tss.sp1, task->thread.sp0);
>>  #else
>> -/* Xen PV enters the kernel on the thread stack. */
>> -if (cpu_feature_enabled(X86_FEATURE_XENPV))
>> +if (cpu_feature_enabled(X86_FEATURE_FRED)) {
>> +/* WRMSRNS is a baseline feature for FRED. */
>> +wrmsrns(MSR_IA32_FRED_RSP0, (unsigned 
>> long)task_stack_page(task) + THREAD_SIZE);
>
>If this non-serializing write happens now and, AFAICT, the CR3 write
>during the task switch has already happened in switch_mm* earlier, what
>is the serialization point that's going to make sure that write is
>committed before the new task starts executing?
>
>Thx.
>

A resource cannot be consumed after the value has been written; this is the 
only necessary level of serialization, equivalent to, say, RAX.

A serializing instruction stops the entire pipeline until everything has 
retired and any stores have become globally visible.

Re: [PATCH v2 02/29] tools: add a new xen logging daemon

[Jason Andryuk]

On Fri, Nov 10, 2023 at 11:08 AM Juergen Gross  wrote:
>
> Add "xen-9pfsd", a new logging daemon meant to support infrastructure
> domains (e.g. xenstore-stubdom) to access files in dom0.
>
> For now only add the code needed for starting the daemon and
> registering it with Xenstore via a new "libxl/xen-9pfs/state" node by
> writing the "running" state to it.
>
> Signed-off-by: Juergen Gross 
> ---

> --- /dev/null
> +++ b/tools/xen-9pfsd/xen-9pfsd.c
> @@ -0,0 +1,145 @@

> + * The backend device string is "xen_9pfs", the tag used for mounting the
> + * 9pfs device is "Xen".

'_' is much less common in xenstore node names than '-'.  Is there a
particular reason you chose '_'?  I generally prefer '-', but IIRC the
libxl idl can't handle '-'.  Did you hit that?

Reviewed-by: Jason Andryuk 

Re: [PATCH-for-9.0 04/10] hw/xen: Factor xen_arch_align_ioreq_data() out of handle_ioreq()

[David Woodhouse]

On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
> Per commit f17068c1c7 ("xen-hvm: reorganize xen-hvm and move common
> function to xen-hvm-common"), handle_ioreq() is expected to be
> target-agnostic. However it uses 'target_ulong', which is a target
> specific definition.
> 
> In order to compile this file once for all targets, factor the
> target-specific code out of handle_ioreq() as a per-target handler
> called xen_arch_align_ioreq_data().
> 
> Signed-off-by: Philippe Mathieu-Daudé 

I prefer commits like this to explicitly state 'No function change
intended', and on that basis:

Reviewed-by: David Woodhouse 

But...



> --- a/hw/i386/xen/xen-hvm.c
> +++ b/hw/i386/xen/xen-hvm.c
> @@ -699,6 +699,14 @@ void xen_arch_set_memory(XenIOState *state, 
> MemoryRegionSection *section,
>  }
>  }
>  
> +void xen_arch_align_ioreq_data(ioreq_t *req)
> +{
> +    if (!req->data_is_ptr && (req->dir == IOREQ_WRITE)
> +    && (req->size < sizeof(target_ulong))) {
> +    req->data &= ((target_ulong) 1 << (8 * req->size)) - 1;
> +    }
> +}
> +

If a 64-bit Xen host is running a 32-bit guest, what is target_ulong,
and what is the actual alignment? I think we are actually communicating
with the 64-bit Xen and it's 64 bits, although the *guest* is 32?

I guess the only time when this would matter is when using
qemu-system-i386 as the device model on 64-bit Xen? And that's not
going to work for various reasons including this?

(I should clarify that I'm not objecting to your patch series, but I
just to understand just what the situation is, before you make it
*look* saner than it is... :)


smime.p7s
Description: S/MIME cryptographic signature

Re: [PATCH 4/7] xen/events: remove some simple helpers from events_base.c

[Oleksandr Tyshchenko]

On 16.10.23 09:28, Juergen Gross wrote:


Hello Juergen.


> The helper functions type_from_irq() and cpu_from_irq() are just one
> line functions used only internally.
> 
> Open code them where needed. At the same time modify and rename
> get_evtchn_to_irq() to return a struct irq_info instead of the IRQ
> number.
> 
> Signed-off-by: Juergen Gross 



[snip]



> 
> @@ -1181,15 +1172,16 @@ static int bind_evtchn_to_irq_chip(evtchn_port_t 
> evtchn, struct irq_chip *chip,
>   {
>   int irq;
>   int ret;
> + struct irq_info *info;
>   
>   if (evtchn >= xen_evtchn_max_channels())
>   return -ENOMEM;


I assume this check is called here (*before* holding a lock) by 
intention, as evtchn_to_info() below contains the same check.

>   
>   mutex_lock(&irq_mapping_update_lock);
>   
> - irq = get_evtchn_to_irq(evtchn);
> + info = evtchn_to_info(evtchn) >
> - if (irq == -1) {
> + if (!info) {
>   irq = xen_allocate_irq_dynamic();
>   if (irq < 0)
>   goto out;
> @@ -1212,8 +1204,8 @@ static int bind_evtchn_to_irq_chip(evtchn_port_t 
> evtchn, struct irq_chip *chip,
>*/
>   bind_evtchn_to_cpu(evtchn, 0, false);
>   } else {
> - struct irq_info *info = info_for_irq(irq);
> - WARN_ON(info == NULL || info->type != IRQT_EVTCHN);
> + WARN_ON(info->type != IRQT_EVTCHN);
> + irq = info->irq;
>   }


This hunk doesn't apply clearly to the latest state, because of 
"9e90e58c11b7 xen: evtchn: Allow shared registration of IRQ handers" 
went in. Please rebase.


With that:
Reviewed-by: Oleksandr Tyshchenko 


Also checkpatch.pl warns about BUG_ON usage in several places, but again 
you didn't introduce them in current patch, just touched their args.


[snip]

Re: [PATCH-for-9.0 03/10] hw/xen: Merge 'hw/xen/arch_hvm.h' in 'hw/xen/xen-hvm-common.h'

[David Woodhouse]

On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
> We don't need a target-specific header for common target-specific
> prototypes. Declare xen_arch_handle_ioreq() and xen_arch_set_memory()
> in "hw/xen/xen-hvm-common.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: David Woodhouse 


smime.p7s
Description: S/MIME cryptographic signature

Re: [PATCH-for-9.0 02/10] hw/xen/xen_arch_hvm: Rename prototypes using 'xen_arch_' prefix

[David Woodhouse]

On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
> Use a common 'xen_arch_' prefix for architecture-specific functions.
> Rename xen_arch_set_memory() and xen_arch_handle_ioreq().
> 
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: David Woodhouse 


smime.p7s
Description: S/MIME cryptographic signature

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

[osstest service owner]

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

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  fb41228ececea948c7953c8c16fe28fd65c6536b
baseline version:
 xen  bede1c7e3b790b63f1ff3ea3ee4e476b012fdf2c

Last test of basis   183709  2023-11-07 21:00:26 Z5 days
Testing same since   183743  2023-11-13 15:02:04 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Federico Serafini 
  George Dunlap 
  Jan Beulich 
  Julien Grall 
  Oleksii Kurochko 
  Roger Pau Monné 
  Stefano Stabellini 

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



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   bede1c7e3b..fb41228ece  fb41228ececea948c7953c8c16fe28fd65c6536b -> smoke

Re: [PATCH v2 01/29] xen/public: add some more 9pfs xenstore paths

[Jason Andryuk]

On Fri, Nov 10, 2023 at 1:18 PM Juergen Gross  wrote:
>
> Add some optional additional backend paths for 9pfs PV devices. Those
> paths will be supported by the new xenlogd 9pfs backend.

xen-9pfsd

>
> Signed-off-by: Juergen Gross 

Reviewed-by: Jason Andryuk 

Re: [PATCH-for-9.0 04/10] hw/xen: Factor xen_arch_align_ioreq_data() out of handle_ioreq()

[David Woodhouse]

On Mon, 2023-11-13 at 17:09 +0100, Philippe Mathieu-Daudé wrote:
> On 13/11/23 16:58, Woodhouse, David wrote:
> > On 13 Nov 2023 10:22, Philippe Mathieu-Daudé 
> > wrote:
> > 
> >     Per commit f17068c1c7 ("xen-hvm: reorganize xen-hvm and move
> > common
> >     function to xen-hvm-common"), handle_ioreq() is expected to be
> >     target-agnostic. However it uses 'target_ulong', which is a
> > target
> >     specific definition.
> > 
> >     In order to compile this file once for all targets, factor the
> >     target-specific code out of handle_ioreq() as a per-target
> > handler
> >     called xen_arch_align_ioreq_data().
> > 
> >     Signed-off-by: Philippe Mathieu-Daudé 
> >     ---
> >     Should we have a 'unsigned qemu_target_long_bits();' helper
> >     such qemu_target_page_foo() API and target_words_bigendian()?
> > 
> > 
> > It can be more fun than that though. What about 
> > qemu_target_alignof_uint64() for example, which differs between
> > i386 and 
> > x86_64 and causes even structs with *explicitly* sized fields to
> > differ 
> > because of padding.
> > 
> > I'd *love* to see this series as a step towards my fantasy of being
> > able 
> > to support Xen under TCG. After all, without that what's the point
> > in 
> > being target-agnostic?
> 
> Another win is we are building all these files once instead of one
> for
> each i386/x86_64/aarch64 targets, so we save CI time and Amazon
> trees.
> 
> > However, I am mildly concerned that some of these files are
> > accidentally 
> > using the host ELF ABI, perhaps with explicit management of 32-bit 
> > compatibility, and the target-agnosticity is purely an illusion?
> > 
> > See the "protocol" handling and the three ABIs for the ring in 
> > xen-block, for example.
> 
> If so I'd expect build failures or violent runtime assertions.

Heh, mostly the guest just crashes in the cases I've seen so far.

See commit a1c1082908d ("hw/xen: use correct default protocol for xen-
block on x86").

> Reviewing quickly hw/block/dataplane/xen-block.c, this code doesn't
> seem target specific at all IMHO. Otherwise I'd really expect it to
> fail compiling. But I don't know much about Xen, so I'll let block &
> xen experts to have a look.

Where it checks dataplane->protocol and does different things for
BLKIF_PROTOCOL_NATIVE/BLKIF_PROTOCOL_X86_32/BLKIF_PROTOCOL_X86_64, the
*structures* it uses are intended to be using the correct ABI. I think
the structs for BLKIF_PROTOCOL_NATIVE may actually be *different*
according to the target, in theory?

I don't know that they are *correct* right now, if the host is
different from the target. But that's just a bug (that only matters if
we ever want to support Xen-compatible guests using TCG).

> > Can we be explicit about what's expected to work here and what's
> > not in scope?
> 
> What do you mean? Everything is expected to work like without this
> series applied :)

I think that if we ever do support Xen-compatible guests using TCG,
we'll have to fix that bug and use the right target-specific
structures... and then perhaps we'll want the affected files to
actually become target-specfic again?

I think this series makes it look like target-agnostic support *should*
work... but it doesn't really?


smime.p7s
Description: S/MIME cryptographic signature

[PATCH] xen/x86: On x2APIC mode, derive LDR from APIC_ID

[Alejandro Vallejo]

Both Intel and AMD manuals agree that on x2APIC mode, the APIC LDR and ID
registers are derivable from each other through a fixed formula.

Xen uses that formula, but applies it to vCPU IDs (which are sequential)
rather than x2APIC_IDs (which are not, at the moment). As I understand it,
this is an attempt to tightly pack vCPUs into clusters so each cluster has
16 vCPUs rather than 8, but this is problematic for OSs that might read the
x2APIC_ID and internally derive LDR (or the other way around)

This patch fixes the implementation so we follow the rules in the x2APIC
spec(s).

While in the neighborhood, replace the u32 type with the standard uint32_t

Signed-off-by: Alejandro Vallejo 
---
 xen/arch/x86/hvm/vlapic.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index c7ce82d064..5a74e84445 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -1063,10 +1063,10 @@ static const struct hvm_mmio_ops vlapic_mmio_ops = {
 
 static void set_x2apic_id(struct vlapic *vlapic)
 {
-u32 id = vlapic_vcpu(vlapic)->vcpu_id;
-u32 ldr = ((id & ~0xf) << 12) | (1 << (id & 0xf));
+uint32_t id = vlapic_vcpu(vlapic)->vcpu_id * 2;
+uint32_t ldr = ((id & ~0xf) << 12) | (1 << (id & 0xf));
 
-vlapic_set_reg(vlapic, APIC_ID, id * 2);
+vlapic_set_reg(vlapic, APIC_ID, id);
 vlapic_set_reg(vlapic, APIC_LDR, ldr);
 }
 
-- 
2.34.1

Re: [PATCH v2 06/15] xen/asm-generic: ifdef inclusion of

[Jan Beulich]

On 11.11.2023 11:24, Oleksii wrote:
> This patch should be reworked as it fails Arm builds:
> https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/1068867920

Took me a while to actually find the error. Would have been nice if you
had explained what's going wrong, supplying the link only as extra info.

> It looks like it is not possible just to #ifdef CONFIG_MEM_ACCESS.
> 
> An empty asm-generic mem_access.h will be better solution.

I remain unconvinced. The issue looks to be with p2m_mem_access_check().
One solution would be to move that declaration into the common header.
But there's no common code referencing it, so Arm's and x86's version
might as well diverge at some point. Hence from my pov it again boils
down to Arm's traps.c including asm/mem_access.h explicitly, to bring
the declaration in scope. None of the common files really have a need
to have a dependency on the arch-specific header when MEM_ACCESS=n.

Jan

Re: [PATCH v2 04/15] xen/asm-generic: introduce generic hypercall.h

[Jan Beulich]

On 13.11.2023 17:45, Jan Beulich wrote:
> On 10.11.2023 17:30, Oleksii Kurochko wrote:
>> Introduce an empty generic hypercall.h for archs which don't
>> implement it.
>>
>> Signed-off-by: Oleksii Kurochko 
> 
> Since - judging from Arm's present header - it looks technically possible
> for an arch to get away with this:
> Acked-by: Jan Beulich 

Actually - why does this patch not drop PPC's now redundant header right
away, along the lines of what you do in patch 1? This is also why I
refrained from offering A-b on patch 5.

Jan

Re: [PATCH v2 05/15] xen/asm-generic: introduce generic header iocap.h

[Jan Beulich]

On 10.11.2023 17:30, Oleksii Kurochko wrote:
> iocap.h is common for Arm, PPC and RISC-V architectures thereby
> it was moved to asm-generic.
> 
> Signed-off-by: Oleksii Kurochko 
> ---
> The same question as with device.h. Should it be in asm-generic?

I think it's okay(ish) here, considering that ...

> --- /dev/null
> +++ b/xen/include/asm-generic/iocap.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef __ASM_GENERIC_IOCAP_H__
> +#define __ASM_GENERIC_IOCAP_H__
> +
> +#define cache_flush_permitted(d)\
> +(!rangeset_is_empty((d)->iomem_caps))

... the only thing referenced (iomem_caps) is a common entity. And
again Arm demonstrates that an arch can get away with just this.

Jan

Re: [PATCH v2 04/15] xen/asm-generic: introduce generic hypercall.h

[Jan Beulich]

On 10.11.2023 17:30, Oleksii Kurochko wrote:
> Introduce an empty generic hypercall.h for archs which don't
> implement it.
> 
> Signed-off-by: Oleksii Kurochko 

Since - judging from Arm's present header - it looks technically possible
for an arch to get away with this:
Acked-by: Jan Beulich 

Jan

Re: [PATCH v2 02/15] xen/asm-generic: introduce generic device.h

[Jan Beulich]

On 10.11.2023 17:30, Oleksii Kurochko wrote:
> --- /dev/null
> +++ b/xen/include/asm-generic/device.h
> @@ -0,0 +1,140 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef __ASM_GENERIC_DEVICE_H__
> +#define __ASM_GENERIC_DEVICE_H__
> +
> +enum device_type
> +{
> +DEV_DT,
> +#ifdef HAS_PCI
> +DEV_PCI,
> +#endif
> +};
> +
> +struct dev_archdata {
> +void *iommu;/* IOMMU private data */
> +};
> +
> +/* struct device - The basic device structure */
> +struct device
> +{
> +enum device_type type;
> +#ifdef CONFIG_HAS_DEVICE_TREE
> +struct dt_device_node *of_node; /* Used by drivers imported from Linux */
> +#endif

There's just this instance where HAS_DEVICE_TREE is checked. Why not elsewhere?
Imo, if you really want this header in asm-generic/, then it wants to be truly
generic (i.e. not implying DT just like you're not implying PCI or ACPI).

Jan

Re: [PATCH v2 01/15] xen/asm-generic: introduce stub header paging.h

[Jan Beulich]

On 10.11.2023 17:30, Oleksii Kurochko wrote:
> The patch introduces generic paging.h header for Arm, PPC and
> RISC-V.
> 
> All mentioned above architectures use hardware virt extensions
> and hardware pagetable extensions thereby it makes sense to set
> paging_mode_translate and paging_mode_external by default.
> 
> Also in this patch Arm and PPC architectures are switched to
> generic paging.h header.
> 
> Signed-off-by: Oleksii Kurochko 

Reviewed-by: Jan Beulich 
with one nit (applicable twice):

> --- a/xen/arch/arm/include/asm/Makefile
> +++ b/xen/arch/arm/include/asm/Makefile
> @@ -1,2 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  generic-y += vm_event.h
> +generic-y += paging.h

Like in obj-y lists it would be nice for the items here and ...

> --- a/xen/arch/ppc/include/asm/Makefile
> +++ b/xen/arch/ppc/include/asm/Makefile
> @@ -1,2 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  generic-y += vm_event.h
> +generic-y += paging.h

... here to be sorted alphabetically from the beginning. Right here I'd
be happy to make the change while committing, but I'm not going to promise
making the same offer on subsequent changes (should the same issue exist
there).

Jan

xen | Successful pipeline for staging | fb41228e

[GitLab]

Pipeline #1070425841 has passed!

Project: xen ( https://gitlab.com/xen-project/xen )
Branch: staging ( https://gitlab.com/xen-project/xen/-/commits/staging )

Commit: fb41228e ( 
https://gitlab.com/xen-project/xen/-/commit/fb41228ececea948c7953c8c16fe28fd65c6536b
 )
Commit Message: docs/sphinx: Fix indexing

sphinx-build reports...
Commit Author: Andrew Cooper ( https://gitlab.com/andyhhp )



Pipeline #1070425841 ( 
https://gitlab.com/xen-project/xen/-/pipelines/1070425841 ) triggered by Ganis 
( https://gitlab.com/ganis )
successfully completed 129 jobs in 3 stages.

-- 
You're receiving this email because of your account on gitlab.com.

Re: Clang-format configuration discussion - pt 1

[Jan Beulich]

On 13.11.2023 16:20, Luca Fancellu wrote:
>> On 13 Nov 2023, at 11:31, Jan Beulich  wrote:
>> On 08.11.2023 10:53, Luca Fancellu wrote:
>> --
>>>
>>> Standard: C++03
>>>
>>> ---
>>> From the documentation: Parse and format C++ constructs compatible with 
>>> this standard.
>>
>> Since I continue to be puzzled - iirc you said this is because of lack
>> of availability of "C99" as a value here. What's entirely unclear to
>> me is: How does this matter to a tool checking coding style (which is
>> largely about formatting, not any lexical or syntactical aspects)?
>>
>>> This value is used also in Linux.
>>
>> Considering how different the two styles are, I don't think this is
>> overly relevant.
> 
> Ok, maybe I understand your point, you are looking for a reason to declare 
> this configurable instead
> of not specifying it at all?

Not really, no. Here I was merely saying that with the styles being
sufficiently different, what Linux uses is probably not very significant
for our own decision.

> If it’s that, from what I understand clang-format will use the default value 
> if we don’t specify anything
> for this one, so it will take ‘Latest’. I think we should put a value for 
> this one to fix it and don’t have
> surprises if that behaviour changes and seeing that also in Linux that value 
> is fixed increased my
> confidence.
> 
> However, if you feel that we should not specify it, I’ve done a test and not 
> specifying it is not changing
> the current output. I can’t say that for a different clang-format version 
> though or if changes happen in the
> future.

It's fine to set values. All I'm saying is that at least I would prefer
if it was also clear what exact effect the setting of a value has,
especially when that does not really match the language we use in the
project.

>> --
>>>
>>> AttributeMacros:
>>>  - '__init'
>>>  - '__exit'
>>>  - '__initdata'
>>>  - '__initconst'
>>>  - '__initconstrel'
>>>  - '__initdata_cf_clobber'
>>>  - '__initconst_cf_clobber'
>>>  - '__hwdom_init'
>>>  - '__hwdom_initdata'
>>>  - '__maybe_unused'
>>>  - '__packed'
>>>  - '__stdcall'
>>>  - '__vfp_aligned'
>>>  - '__alt_call_maybe_initdata'
>>>  - '__cacheline_aligned'
>>>  - '__ro_after_init'
>>>  - 'always_inline'
>>>  - 'noinline'
>>>  - 'noreturn'
>>>  - '__weak'
>>>  - '__inline__'
>>>  - '__attribute_const__'
>>>  - '__transparent__'
>>>  - '__used'
>>>  - '__must_check'
>>>  - '__kprobes'
>>>
>>> ---
>>> A vector of strings that should be interpreted as attributes/qualifiers 
>>> instead of identifiers.
>>> I’ve tried to list all the attributes I’ve found.
>>
>> Like above, the significance of having this list and needing to keep it
>> up-to-date is unclear to me. I guess the same also applies to a few
>> further settings down from here.
> 
> From what I understand, we should give to the formatter tool all the hint 
> possible to make it do a good
> job, for example here we should maintain a list of our attributes so that 
> clang-format doesn’t think the function
> below is called __init instead of device_tree_node_matches.
> 
> static bool __init device_tree_node_matches(const void *fdt, int node, const 
> char *match)
> { ... }

Well, perhaps they indeed need to do some level of syntax analysis, in
which case knowing which identifiers act as attributes is likely going
to help. Which is where the "needs keeping up-to-date" aspect comes into
play. The example above is simple enough that I wouldn't think a parser
needs to guess what this represents, but presumably there are cases
where ambiguities might arise. (I also wouldn't exclude that the more
involved syntax in C++ increases the desire to have apriori knowledge on
the purpose of certain identifiers. In the end, as per above, the tool
is being told to expect C++ code.)

Jan

Re: Clang-format configuration discussion - pt 1

[Alejandro Vallejo]

On Mon, Nov 13, 2023 at 03:20:53PM +, Luca Fancellu wrote:
> 
> 
> > On 13 Nov 2023, at 11:31, Jan Beulich  wrote:
> > 
> > On 08.11.2023 10:53, Luca Fancellu wrote:
> > --
> >> 
> >> Standard: C++03
> >> 
> >> ---
> >> From the documentation: Parse and format C++ constructs compatible with 
> >> this standard.
> > 
> > Since I continue to be puzzled - iirc you said this is because of lack
> > of availability of "C99" as a value here. What's entirely unclear to
> > me is: How does this matter to a tool checking coding style (which is
> > largely about formatting, not any lexical or syntactical aspects)?
> > 
> >> This value is used also in Linux.
> > 
> > Considering how different the two styles are, I don't think this is
> > overly relevant.
On C it _shouldn't_ matter because it's meant to affect C++ constructs
only. That said, clang-format doesn't understand (or care) whether the code
is C or C++, because C's syntax is strictly contained in that of C++ as far
as formatting cares.

While I agree it feels wrong to apply a C++ policy to a C project, it's
largely irrelevant. Setting a value here gives more deterministic output
because it fixes several options' default settings. One would hope none of
those settings affect C, but the world is complex and it's better to be
safe than sorry. Particularly when it's an inocuous one-liner.

There aren't strict C values. And Latest or Auto are simply shorthands for
one of the C++ options.

  https://clang.llvm.org/docs/ClangFormatStyleOptions.html#standard

> 
> Ok, maybe I understand your point, you are looking for a reason to declare 
> this configurable instead
> of not specifying it at all?
> 
> If it’s that, from what I understand clang-format will use the default value 
> if we don’t specify anything
> for this one, so it will take ‘Latest’. I think we should put a value for 
> this one to fix it and don’t have
> surprises if that behaviour changes and seeing that also in Linux that value 
> is fixed increased my
> confidence.
> 
> However, if you feel that we should not specify it, I’ve done a test and not 
> specifying it is not changing
> the current output. I can’t say that for a different clang-format version 
> though or if changes happen in the
> future.
> 
IMO, C++03 is as good as any other. As long as it's a fixed one.

Cheers,
Alejandro

[PATCH 2/2] livepatch-tools: fix isnumber() function clash

[Roger Pau Monne]

isnumber() is already defined for some libcs [0] but the interface is not the
same, the isnumber() helper just checks if a single character is a digit.

Rename isnumber() to is_number() in order to avoid the clash.

[0] https://man.freebsd.org/cgi/man.cgi?query=isnumber&sektion=3

Signed-off-by: Roger Pau Monné 
---
 create-diff-object.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/create-diff-object.c b/create-diff-object.c
index 67784642bcd7..d0e14e3a62bb 100644
--- a/create-diff-object.c
+++ b/create-diff-object.c
@@ -1352,7 +1352,7 @@ static void kpatch_process_special_sections(struct 
kpatch_elf *kelf)
 }
 
 /* Returns true if s is a string of only numbers with length > 0. */
-static bool isnumber(const char *s)
+static bool is_number(const char *s)
 {
do {
if (!*s || !isdigit(*s))
@@ -1380,13 +1380,13 @@ static bool is_rodata_str_section(const char *name)
 
/* Check if name matches ".rodata.str1.[0-9]+" */
if (!strncmp(name, GCC_5_SECTION_NAME, strlen(GCC_5_SECTION_NAME)))
-   return isnumber(name + strlen(GCC_5_SECTION_NAME));
+   return is_number(name + strlen(GCC_5_SECTION_NAME));
 
/* Check if name matches ".rodata..str1.[0-9]+" */
s = strstr(name, GCC_6_SECTION_NAME);
if (!s)
return false;
-   return isnumber(s + strlen(GCC_6_SECTION_NAME));
+   return is_number(s + strlen(GCC_6_SECTION_NAME));
 #undef GCC_5_SECTION_NAME
 #undef GCC_6_SECTION_NAME
 }
-- 
2.42.0

[PATCH 0/2] livepatch-tools: fixes for building on non-glibc

[Roger Pau Monne]

Hello,

A couple of fixes to allow building the tools on non-glibc systems (BSD
and musl tested).

No functional changes intended for either of the two fixes.

Thanks, Roger.

Roger Pau Monne (2):
  livepatch-tools: add -largp option when required
  livepatch-tools: fix isnumber() function clash

 Makefile | 4 
 create-diff-object.c | 6 +++---
 2 files changed, 7 insertions(+), 3 deletions(-)

-- 
2.42.0

Re: [PATCH-for-9.0 04/10] hw/xen: Factor xen_arch_align_ioreq_data() out of handle_ioreq()

[Philippe Mathieu-Daudé]

On 13/11/23 16:58, Woodhouse, David wrote:

On 13 Nov 2023 10:22, Philippe Mathieu-Daudé  wrote:

Per commit f17068c1c7 ("xen-hvm: reorganize xen-hvm and move common
function to xen-hvm-common"), handle_ioreq() is expected to be
target-agnostic. However it uses 'target_ulong', which is a target
specific definition.

In order to compile this file once for all targets, factor the
target-specific code out of handle_ioreq() as a per-target handler
called xen_arch_align_ioreq_data().

Signed-off-by: Philippe Mathieu-Daudé 
---
Should we have a 'unsigned qemu_target_long_bits();' helper
such qemu_target_page_foo() API and target_words_bigendian()?


It can be more fun than that though. What about 
qemu_target_alignof_uint64() for example, which differs between i386 and 
x86_64 and causes even structs with *explicitly* sized fields to differ 
because of padding.


I'd *love* to see this series as a step towards my fantasy of being able 
to support Xen under TCG. After all, without that what's the point in 
being target-agnostic?


Another win is we are building all these files once instead of one for
each i386/x86_64/aarch64 targets, so we save CI time and Amazon trees.

However, I am mildly concerned that some of these files are accidentally 
using the host ELF ABI, perhaps with explicit management of 32-bit 
compatibility, and the target-agnosticity is purely an illusion?


See the "protocol" handling and the three ABIs for the ring in 
xen-block, for example.


If so I'd expect build failures or violent runtime assertions.

Reviewing quickly hw/block/dataplane/xen-block.c, this code doesn't
seem target specific at all IMHO. Otherwise I'd really expect it to
fail compiling. But I don't know much about Xen, so I'll let block &
xen experts to have a look.

Can we be explicit about what's expected to work here and what's not in 
scope?


What do you mean? Everything is expected to work like without this
series applied :)

Regards,

Phil.

[PATCH 1/2] livepatch-tools: add -largp option when required

[Roger Pau Monne]

crate-diff-object makes use of argp library, and depending on the libc
used by the system (ie: musl or BSD libc) argp is a separate library
and requires the addition of -largp to the build rune.

Introduce some shell logic to detect whether -largp is required for
linking create-diff-object.

I haven't done this as a reusable macro because I'm not sure there's
much point in doing so, the only library we need to test for is argp,
anything else is likely to be a mandatory library flag that doesn't
require such testing (like libelf for example).

Signed-off-by: Roger Pau Monné 
---
 Makefile | 4 
 1 file changed, 4 insertions(+)

diff --git a/Makefile b/Makefile
index f96b150539d2..2f74c346 100644
--- a/Makefile
+++ b/Makefile
@@ -11,6 +11,10 @@ LIBEXECDIR = 
$(DESTDIR)$(PREFIX)/libexec/livepatch-build-tools
 CFLAGS  += -Iinsn -Wall -g
 LDFLAGS = -lelf
 
+# Non GNU libc systems usually have argp as a standalone library.
+LDFLAGS += $(shell if test -z "`$(CC) -largp 2>&1 | grep '\-largp'`"; \
+  then echo "-largp"; fi;)
+
 TARGETS = create-diff-object prelink
 CREATE_DIFF_OBJECT_OBJS = create-diff-object.o lookup.o insn/insn.o 
insn/inat.o common.o
 PRELINK_OBJS = prelink.o lookup.o insn/insn.o insn/inat.o common.o
-- 
2.42.0

Re: [PATCH-for-9.0 04/10] hw/xen: Factor xen_arch_align_ioreq_data() out of handle_ioreq()

[Woodhouse, David]

On 13 Nov 2023 10:22, Philippe Mathieu-Daudé  wrote:
Per commit f17068c1c7 ("xen-hvm: reorganize xen-hvm and move common
function to xen-hvm-common"), handle_ioreq() is expected to be
target-agnostic. However it uses 'target_ulong', which is a target
specific definition.

In order to compile this file once for all targets, factor the
target-specific code out of handle_ioreq() as a per-target handler
called xen_arch_align_ioreq_data().

Signed-off-by: Philippe Mathieu-Daudé 
---
Should we have a 'unsigned qemu_target_long_bits();' helper
such qemu_target_page_foo() API and target_words_bigendian()?

It can be more fun than that though. What about qemu_target_alignof_uint64() 
for example, which differs between i386 and x86_64 and causes even structs with 
*explicitly* sized fields to differ because of padding.

I'd *love* to see this series as a step towards my fantasy of being able to 
support Xen under TCG. After all, without that what's the point in being 
target-agnostic?

However, I am mildly concerned that some of these files are accidentally using 
the host ELF ABI, perhaps with explicit management of 32-bit compatibility, and 
the target-agnosticity is purely an illusion?

See the "protocol" handling and the three ABIs for the ring in xen-block, for 
example.

Can we be explicit about what's expected to work here and what's not in scope?




Amazon Development Centre (London) Ltd. Registered in England and Wales with 
registration number 04543232 with its registered office at 1 Principal Place, 
Worship Street, London EC2A 2FA, United Kingdom.

[PATCH] automation: set architecture in docker files

[Roger Pau Monne]

Pass the desired architecture of the image in the FROM instruction if the
image is possibly multi-platform.

This allows using the x86 Dockerfiles on OS X on arm64 hardware.

No functional change intended.

Signed-off-by: Roger Pau Monné 
---
I haven't touched the Yocto dockerfile because I'm not sure how it's used.
---
 automation/build/alpine/3.18.dockerfile   | 2 +-
 automation/build/archlinux/current-riscv64.dockerfile | 2 +-
 automation/build/archlinux/current.dockerfile | 2 +-
 automation/build/centos/7.dockerfile  | 2 +-
 automation/build/debian/bookworm.dockerfile   | 2 +-
 automation/build/debian/bullseye-ppc64le.dockerfile   | 2 +-
 automation/build/debian/buster-gcc-ibt.dockerfile | 2 +-
 automation/build/debian/jessie.dockerfile | 2 +-
 automation/build/debian/stretch.dockerfile| 2 +-
 automation/build/fedora/29.dockerfile | 2 +-
 automation/build/suse/opensuse-leap.dockerfile| 2 +-
 automation/build/suse/opensuse-tumbleweed.dockerfile  | 2 +-
 automation/build/ubuntu/bionic.dockerfile | 2 +-
 automation/build/ubuntu/focal.dockerfile  | 2 +-
 automation/build/ubuntu/trusty.dockerfile | 2 +-
 automation/build/ubuntu/xenial-xilinx.dockerfile  | 2 +-
 automation/build/ubuntu/xenial.dockerfile | 2 +-
 17 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/automation/build/alpine/3.18.dockerfile 
b/automation/build/alpine/3.18.dockerfile
index 5d2a69a06010..4ae9cb5e9e30 100644
--- a/automation/build/alpine/3.18.dockerfile
+++ b/automation/build/alpine/3.18.dockerfile
@@ -1,4 +1,4 @@
-FROM alpine:3.18
+FROM --platform=linux/amd64 alpine:3.18
 LABEL maintainer.name="The Xen Project" \
   maintainer.email="xen-devel@lists.xenproject.org"
 
diff --git a/automation/build/archlinux/current-riscv64.dockerfile 
b/automation/build/archlinux/current-riscv64.dockerfile
index abf8e7bf0b88..af75b5c720ce 100644
--- a/automation/build/archlinux/current-riscv64.dockerfile
+++ b/automation/build/archlinux/current-riscv64.dockerfile
@@ -1,4 +1,4 @@
-FROM archlinux
+FROM --platform=linux/amd64 archlinux
 LABEL maintainer.name="The Xen Project" \
   maintainer.email="xen-devel@lists.xenproject.org"
 
diff --git a/automation/build/archlinux/current.dockerfile 
b/automation/build/archlinux/current.dockerfile
index 47e79637a4a6..d974a1434fd5 100644
--- a/automation/build/archlinux/current.dockerfile
+++ b/automation/build/archlinux/current.dockerfile
@@ -1,4 +1,4 @@
-FROM archlinux:base-devel
+FROM --platform=linux/amd64 archlinux:base-devel
 LABEL maintainer.name="The Xen Project" \
   maintainer.email="xen-devel@lists.xenproject.org"
 
diff --git a/automation/build/centos/7.dockerfile 
b/automation/build/centos/7.dockerfile
index 69dcefb2f011..ab450f0b3a0e 100644
--- a/automation/build/centos/7.dockerfile
+++ b/automation/build/centos/7.dockerfile
@@ -1,4 +1,4 @@
-FROM centos:7
+FROM --platform=linux/amd64 centos:7
 LABEL maintainer.name="The Xen Project" \
   maintainer.email="xen-devel@lists.xenproject.org"
 
diff --git a/automation/build/debian/bookworm.dockerfile 
b/automation/build/debian/bookworm.dockerfile
index ae008c8d46e5..ac87778b3972 100644
--- a/automation/build/debian/bookworm.dockerfile
+++ b/automation/build/debian/bookworm.dockerfile
@@ -1,4 +1,4 @@
-FROM debian:bookworm
+FROM --platform=linux/amd64 debian:bookworm
 LABEL maintainer.name="The Xen Project" \
   maintainer.email="xen-devel@lists.xenproject.org"
 
diff --git a/automation/build/debian/bullseye-ppc64le.dockerfile 
b/automation/build/debian/bullseye-ppc64le.dockerfile
index 4de8458445ae..6fdfb6bc2b40 100644
--- a/automation/build/debian/bullseye-ppc64le.dockerfile
+++ b/automation/build/debian/bullseye-ppc64le.dockerfile
@@ -1,4 +1,4 @@
-FROM debian:bullseye-slim
+FROM --platform=linux/amd64 debian:bullseye-slim
 LABEL maintainer.name="The Xen Project" \
   maintainer.email="xen-devel@lists.xenproject.org"
 
diff --git a/automation/build/debian/buster-gcc-ibt.dockerfile 
b/automation/build/debian/buster-gcc-ibt.dockerfile
index 96ab4fe8a2f1..4328c109b72b 100644
--- a/automation/build/debian/buster-gcc-ibt.dockerfile
+++ b/automation/build/debian/buster-gcc-ibt.dockerfile
@@ -1,4 +1,4 @@
-FROM debian:buster-slim AS builder
+FROM --platform=linux/amd64 debian:buster-slim AS builder
 
 ENV DEBIAN_FRONTEND=noninteractive
 ENV USER root
diff --git a/automation/build/debian/jessie.dockerfile 
b/automation/build/debian/jessie.dockerfile
index 63b2c1e5b771..db0962953c9a 100644
--- a/automation/build/debian/jessie.dockerfile
+++ b/automation/build/debian/jessie.dockerfile
@@ -1,4 +1,4 @@
-FROM debian/eol:jessie
+FROM --platform=linux/amd64 debian/eol:jessie
 LABEL maintainer.name="The Xen Project" \
   maintainer.email="xen-devel@lists.xenproject.org"
 
diff --git a/automation/build/debian/stretch.dockerfile 
b/automation/build/debian/stretch.dockerfile
index 1af6c691

Re: [PATCH 3/7] xen/events: reduce externally visible helper functions

[Oleksandr Tyshchenko]

On 16.10.23 09:28, Juergen Gross wrote:


Hello Juergen

> get_evtchn_to_irq() has only one external user while irq_from_evtchn()
> provides the same functionality and is exported for a wider user base.
> Modify the only external user of get_evtchn_to_irq() to use
> irq_from_evtchn() instead and make get_evtchn_to_irq() static.
> 
> evtchn_from_irq() and irq_from_virq() have a single external user and
> can easily be combined to a new helper irq_evtchn_from_virq() allowing
> to drop irq_from_virq() and to make evtchn_from_irq() static.
> 
> Signed-off-by: Juergen Gross 


Reviewed-by: Oleksandr Tyshchenko 

Two NITs *NOT* directly related to current patch below.


> ---
>   drivers/xen/events/events_2l.c   |  8 
>   drivers/xen/events/events_base.c | 13 +
>   drivers/xen/events/events_internal.h |  1 -
>   include/xen/events.h |  4 ++--
>   4 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/xen/events/events_2l.c b/drivers/xen/events/events_2l.c
> index b8f2f971c2f0..e3585330cf98 100644
> --- a/drivers/xen/events/events_2l.c
> +++ b/drivers/xen/events/events_2l.c
> @@ -171,11 +171,11 @@ static void evtchn_2l_handle_events(unsigned cpu, 
> struct evtchn_loop_ctrl *ctrl)
>   int i;
>   struct shared_info *s = HYPERVISOR_shared_info;
>   struct vcpu_info *vcpu_info = __this_cpu_read(xen_vcpu);
> + evtchn_port_t evtchn;
>   
>   /* Timer interrupt has highest priority. */
> - irq = irq_from_virq(cpu, VIRQ_TIMER);
> + irq = irq_evtchn_from_virq(cpu, VIRQ_TIMER, &evtchn);
>   if (irq != -1) {
> - evtchn_port_t evtchn = evtchn_from_irq(irq);

Most users of evtchn_from_irq() check returned evtchn via VALID_EVTCHN() 
as it might be 0. But this user doesn't.


>   word_idx = evtchn / BITS_PER_LONG;
>   bit_idx = evtchn % BITS_PER_LONG;
>   if (active_evtchns(cpu, s, word_idx) & (1ULL << bit_idx))
> @@ -328,9 +328,9 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
>   for (i = 0; i < EVTCHN_2L_NR_CHANNELS; i++) {
>   if (sync_test_bit(i, BM(sh->evtchn_pending))) {
>   int word_idx = i / BITS_PER_EVTCHN_WORD;
> - printk("  %d: event %d -> irq %d%s%s%s\n",
> + printk("  %d: event %d -> irq %u%s%s%s\n",

checkpatch.pl says:

WARNING: printk() should include KERN_ facility level
#37: FILE: drivers/xen/events/events_2l.c:331:
+   printk("  %d: event %d -> irq %u%s%s%s\n",


>  cpu_from_evtchn(i), i,
> -get_evtchn_to_irq(i),
> +irq_from_evtchn(i),
>  sync_test_bit(word_idx, 
> BM(&v->evtchn_pending_sel))
>  ? "" : " l2-clear",
>  !sync_test_bit(i, BM(sh->evtchn_mask))
> 

[snip]

Re: [PATCH v4 3/5] arm/dom0less: put dom0less feature code in a separate module

[Michal Orzel]

Hi Luca,

On 13/11/2023 15:17, Luca Fancellu wrote:
> 
> 
>> On 13 Nov 2023, at 11:58, Michal Orzel  wrote:
>>
>> Hi Luca,
>>
>> Apart from pending question on static event channels code movement, a few 
>> NITs.
> 
> Related to that, it seems to me that this part can be handled by a separate 
> patch/serie from
> this one, moving code from domain_build.c to a new module, so I’m wondering, 
> is this point
> a blocker for this serie?
My recommendation was to move it to dom0less-build for now if we all agree that 
it should be guarded
by dom0less. That said, it is not a blocker from my POV. I can handle it later 
on after your series.

~Michal

[PATCH-for-9.0 10/10] hw/xen: Have most of Xen files become target-agnostic

[Philippe Mathieu-Daudé]

Previous commits re-organized the target-specific bits
from Xen files. We can now build the common files once
instead of per-target.

Signed-off-by: Philippe Mathieu-Daudé 
---
 accel/xen/meson.build  |  2 +-
 hw/block/dataplane/meson.build |  2 +-
 hw/xen/meson.build | 13 -
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/accel/xen/meson.build b/accel/xen/meson.build
index 002bdb03c6..455ad5d6be 100644
--- a/accel/xen/meson.build
+++ b/accel/xen/meson.build
@@ -1 +1 @@
-specific_ss.add(when: 'CONFIG_XEN', if_true: files('xen-all.c'))
+system_ss.add(when: 'CONFIG_XEN', if_true: files('xen-all.c'))
diff --git a/hw/block/dataplane/meson.build b/hw/block/dataplane/meson.build
index 025b3b061b..4d8bcb0bb9 100644
--- a/hw/block/dataplane/meson.build
+++ b/hw/block/dataplane/meson.build
@@ -1,2 +1,2 @@
 system_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c'))
-specific_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xen-block.c'))
+system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xen-block.c'))
diff --git a/hw/xen/meson.build b/hw/xen/meson.build
index d887fa9ba4..29adfadd1c 100644
--- a/hw/xen/meson.build
+++ b/hw/xen/meson.build
@@ -9,15 +9,12 @@ system_ss.add(when: ['CONFIG_XEN_BUS'], if_true: files(
 
 system_ss.add(when: ['CONFIG_XEN', xen], if_true: files(
   'xen-operations.c',
+  'xen-hvm-common.c',
+  'xen-mapcache.c',
 ))
 
-xen_specific_ss = ss.source_set()
-xen_specific_ss.add(files(
-  'xen-mapcache.c',
-  'xen-hvm-common.c',
-))
 if have_xen_pci_passthrough
-  xen_specific_ss.add(files(
+  system_ss.add(files(
 'xen-host-pci-device.c',
 'xen_pt.c',
 'xen_pt_config_init.c',
@@ -26,7 +23,5 @@ if have_xen_pci_passthrough
 'xen_pt_msi.c',
   ))
 else
-  xen_specific_ss.add(files('xen_pt_stub.c'))
+  system_ss.add(files('xen_pt_stub.c'))
 endif
-
-specific_ss.add_all(when: ['CONFIG_XEN', xen], if_true: xen_specific_ss)
-- 
2.41.0

[PATCH-for-9.0 09/10] hw/xen: Extract 'xen_igd.h' from 'xen_pt.h'

[Philippe Mathieu-Daudé]

"hw/xen/xen_pt.h" requires "hw/xen/xen_native.h" which is target
specific. It also declares IGD methods, which are not target
specific.

Target-agnostic code can use IGD methods. To allow that, extract
these methos into a new "hw/xen/xen_igd.h" header.

Signed-off-by: Philippe Mathieu-Daudé 
---
What license for the new "hw/xen/xen_igd.h" header?
---
 hw/xen/xen_pt.h | 14 --
 include/hw/xen/xen_igd.h| 23 +++
 accel/xen/xen-all.c |  1 +
 hw/i386/pc_piix.c   |  1 +
 hw/xen/xen_pt.c |  3 ++-
 hw/xen/xen_pt_config_init.c |  3 ++-
 hw/xen/xen_pt_graphics.c|  3 ++-
 hw/xen/xen_pt_stub.c|  2 +-
 8 files changed, 32 insertions(+), 18 deletions(-)
 create mode 100644 include/hw/xen/xen_igd.h

diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 31bcfdf705..da5af67638 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -5,9 +5,6 @@
 #include "xen-host-pci-device.h"
 #include "qom/object.h"
 
-bool xen_igd_gfx_pt_enabled(void);
-void xen_igd_gfx_pt_set(bool value, Error **errp);
-
 void xen_pt_log(const PCIDevice *d, const char *f, ...) G_GNUC_PRINTF(2, 3);
 
 #define XEN_PT_ERR(d, _f, _a...) xen_pt_log(d, "%s: Error: "_f, __func__, ##_a)
@@ -52,12 +49,6 @@ typedef struct XenPTDeviceClass {
 XenPTQdevRealize pci_qdev_realize;
 } XenPTDeviceClass;
 
-uint32_t igd_read_opregion(XenPCIPassthroughState *s);
-void xen_igd_reserve_slot(PCIBus *pci_bus);
-void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val);
-void xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
-   XenHostPCIDevice *dev);
-
 /* function type for config reg */
 typedef int (*xen_pt_conf_reg_init)
 (XenPCIPassthroughState *, XenPTRegInfo *, uint32_t real_offset,
@@ -343,11 +334,6 @@ static inline bool 
xen_pt_has_msix_mapping(XenPCIPassthroughState *s, int bar)
 void *pci_assign_dev_load_option_rom(PCIDevice *dev, int *size,
  unsigned int domain, unsigned int bus,
  unsigned int slot, unsigned int function);
-static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev)
-{
-return (xen_igd_gfx_pt_enabled()
-&& ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA));
-}
 int xen_pt_register_vga_regions(XenHostPCIDevice *dev);
 int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev);
 void xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev,
diff --git a/include/hw/xen/xen_igd.h b/include/hw/xen/xen_igd.h
new file mode 100644
index 00..52f1f8244c
--- /dev/null
+++ b/include/hw/xen/xen_igd.h
@@ -0,0 +1,23 @@
+#ifndef XEN_IGD_H
+#define XEN_IGD_H
+
+#include "hw/xen/xen-host-pci-device.h"
+
+typedef struct XenPCIPassthroughState XenPCIPassthroughState;
+
+bool xen_igd_gfx_pt_enabled(void);
+void xen_igd_gfx_pt_set(bool value, Error **errp);
+
+uint32_t igd_read_opregion(XenPCIPassthroughState *s);
+void xen_igd_reserve_slot(PCIBus *pci_bus);
+void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val);
+void xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
+   XenHostPCIDevice *dev);
+
+static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev)
+{
+return (xen_igd_gfx_pt_enabled()
+&& ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA));
+}
+
+#endif
diff --git a/accel/xen/xen-all.c b/accel/xen/xen-all.c
index 5ff0cb8bd9..0bdefce537 100644
--- a/accel/xen/xen-all.c
+++ b/accel/xen/xen-all.c
@@ -15,6 +15,7 @@
 #include "hw/xen/xen_native.h"
 #include "hw/xen/xen-legacy-backend.h"
 #include "hw/xen/xen_pt.h"
+#include "hw/xen/xen_igd.h"
 #include "chardev/char.h"
 #include "qemu/accel.h"
 #include "sysemu/cpus.h"
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index eace854335..a607dcb56c 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -56,6 +56,7 @@
 #ifdef CONFIG_XEN
 #include 
 #include "hw/xen/xen_pt.h"
+#include "hw/xen/xen_igd.h"
 #endif
 #include "hw/xen/xen-x86.h"
 #include "hw/xen/xen.h"
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index 36e6f93c37..a8edabdabc 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -59,7 +59,8 @@
 #include "hw/pci/pci.h"
 #include "hw/qdev-properties.h"
 #include "hw/qdev-properties-system.h"
-#include "xen_pt.h"
+#include "hw/xen/xen_pt.h"
+#include "hw/xen/xen_igd.h"
 #include "hw/xen/xen.h"
 #include "hw/xen/xen-legacy-backend.h"
 #include "qemu/range.h"
diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index 2b8680b112..ba4cd78238 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -15,7 +15,8 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/timer.h"
-#include "xen_pt.h"
+#include "hw/xen/xen_pt.h"
+#include "hw/xen/xen_igd.h"
 #include "hw/xen/xen-legacy-backend.h"
 
 #define XEN_PT_MERGE_VALUE(value, data, val_mask) \
diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
i

Re: Clang-format configuration discussion - pt 1

[Luca Fancellu]

Hi George,

Thanks a lot for taking the time to have a look on that.

> 
> Luca,
> 
> Thank you so much for the work that you've done here.
> 
> The approach in your v2 series looks plausible, as does a brief
> overview of the items in this list.
> 
> One problem I have is how to really evaluate the proposed changes.  I
> spent a few minutes skimming through the "megadiff" [1] output from
> the v2 series, and while everything looked fine, that is a HUGE patch
> to skim through.  I don't really have any way to know if there's some
> rule introduced that I don't really agree with.
> 
> On the other hand, I want to avoid busy make-work and the invitation
> to interminable bike-shedding discussions.
> 
> Is it possible, for instance, to start with a diff which will enforce
> *just these settings* (column width, indentation, and so on)?  And
> then add on new coding style changes one (or a few) at a time, in a
> way that would make it easier to understand what effect each change is
> having?  If so, do you think that's a reasonable approach?
> 
> If not, how do you propose to proceed?

Yes they are a lot of modifications, the issue is that when we don’t specify
a configurable, the default option will take place, so it’s not really feasible
to produce an output where only the specified configurable will affect the
format.

The easiest, but difficult at the same time, way I thought we can proceed is
discussing a set of rule at the time where we all (well the maintainers) agree
in principle, so that we apply them to the codebase until the list is completed
and also CODING_STYLE can reflect them.

Anyway if someone came up with a better idea, I’m open to suggestions.

Cheers,
Luca

[PATCH-for-9.0 08/10] system/physmem: Only include 'hw/xen/xen.h' when Xen is available

[Philippe Mathieu-Daudé]

"hw/xen/xen.h" contains declarations for Xen hardware. There is
no point including it when Xen is not available. When Xen is not
available, we have enough with declarations of "sysemu/xen.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
 system/physmem.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/system/physmem.c b/system/physmem.c
index fc2b0fee01..fa667437da 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -35,9 +35,9 @@
 #include "hw/qdev-core.h"
 #include "hw/qdev-properties.h"
 #include "hw/boards.h"
-#include "hw/xen/xen.h"
 #include "sysemu/kvm.h"
 #include "sysemu/tcg.h"
+#include "sysemu/xen.h"
 #include "sysemu/qtest.h"
 #include "qemu/timer.h"
 #include "qemu/config-file.h"
@@ -51,6 +51,9 @@
 #include "sysemu/hostmem.h"
 #include "sysemu/hw_accel.h"
 #include "sysemu/xen-mapcache.h"
+#ifdef CONFIG_XEN
+#include "hw/xen/xen.h"
+#endif
 #include "trace/trace-root.h"
 
 #ifdef CONFIG_FALLOCATE_PUNCH_HOLE
-- 
2.41.0

[PATCH-for-9.0 07/10] sysemu/xen-mapcache: Check Xen availability with CONFIG_XEN_IS_POSSIBLE

[Philippe Mathieu-Daudé]

"sysemu/xen.h" defines CONFIG_XEN_IS_POSSIBLE as a target-agnostic
version of CONFIG_XEN. Use it in order to use "sysemu/xen-mapcache.h"
in target-agnostic files.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/sysemu/xen-mapcache.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/sysemu/xen-mapcache.h b/include/sysemu/xen-mapcache.h
index c8e7c2f6cf..10c2e3082a 100644
--- a/include/sysemu/xen-mapcache.h
+++ b/include/sysemu/xen-mapcache.h
@@ -10,10 +10,11 @@
 #define XEN_MAPCACHE_H
 
 #include "exec/cpu-common.h"
+#include "sysemu/xen.h"
 
 typedef hwaddr (*phys_offset_to_gaddr_t)(hwaddr phys_offset,
  ram_addr_t size);
-#ifdef CONFIG_XEN
+#ifdef CONFIG_XEN_IS_POSSIBLE
 
 void xen_map_cache_init(phys_offset_to_gaddr_t f,
 void *opaque);
-- 
2.41.0

[PATCH-for-9.0 05/10] hw/xen: Use target-agnostic qemu_target_page_bits()

[Philippe Mathieu-Daudé]

Instead of the target-specific TARGET_PAGE_BITS definition,
use qemu_target_page_bits() which is target agnostic.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/xen/xen-hvm-common.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index 03f9417e7e..35b3b5407d 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -1,6 +1,7 @@
 #include "qemu/osdep.h"
 #include "qemu/units.h"
 #include "qapi/error.h"
+#include "exec/target_page.h"
 #include "trace.h"
 
 #include "hw/pci/pci_host.h"
@@ -13,6 +14,7 @@ MemoryRegion ram_memory;
 void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr,
Error **errp)
 {
+unsigned target_page_bits = qemu_target_page_bits();
 unsigned long nr_pfn;
 xen_pfn_t *pfn_list;
 int i;
@@ -31,11 +33,11 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, 
MemoryRegion *mr,
 
 trace_xen_ram_alloc(ram_addr, size);
 
-nr_pfn = size >> TARGET_PAGE_BITS;
+nr_pfn = size >> target_page_bits;
 pfn_list = g_new(xen_pfn_t, nr_pfn);
 
 for (i = 0; i < nr_pfn; i++) {
-pfn_list[i] = (ram_addr >> TARGET_PAGE_BITS) + i;
+pfn_list[i] = (ram_addr >> target_page_bits) + i;
 }
 
 if (xc_domain_populate_physmap_exact(xen_xc, xen_domid, nr_pfn, 0, 0, 
pfn_list)) {
-- 
2.41.0

[PATCH-for-9.0 06/10] hw/xen: Reduce inclusion of 'cpu.h' to target-specific sources

[Philippe Mathieu-Daudé]

We rarely need to include "cpu.h" in headers. Including it
'taint' headers to be target-specific. Here only the i386/arm
implementations requires "cpu.h", so include it there and
remove from the "hw/xen/xen-hvm-common.h" *common* header.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/xen/xen-hvm-common.h | 1 -
 hw/arm/xen_arm.c| 1 +
 hw/i386/xen/xen-hvm.c   | 1 +
 3 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/hw/xen/xen-hvm-common.h b/include/hw/xen/xen-hvm-common.h
index 734bfa3183..ca941fd3eb 100644
--- a/include/hw/xen/xen-hvm-common.h
+++ b/include/hw/xen/xen-hvm-common.h
@@ -4,7 +4,6 @@
 #include "qemu/osdep.h"
 #include "qemu/units.h"
 
-#include "cpu.h"
 #include "hw/pci/pci.h"
 #include "hw/hw.h"
 #include "hw/xen/xen_native.h"
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index c646fd70d0..2c97d6adc8 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -33,6 +33,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/xen/xen-hvm-common.h"
 #include "sysemu/tpm.h"
+#include "cpu.h"
 
 #define TYPE_XEN_ARM  MACHINE_TYPE_NAME("xenpvh")
 OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index aff5c5b81d..369d738b50 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -22,6 +22,7 @@
 
 #include "hw/xen/xen-hvm-common.h"
 #include 
+#include "cpu.h"
 
 static MemoryRegion ram_640k, ram_lo, ram_hi;
 static MemoryRegion *framebuffer;
-- 
2.41.0

[PATCH-for-9.0 03/10] hw/xen: Merge 'hw/xen/arch_hvm.h' in 'hw/xen/xen-hvm-common.h'

[Philippe Mathieu-Daudé]

We don't need a target-specific header for common target-specific
prototypes. Declare xen_arch_handle_ioreq() and xen_arch_set_memory()
in "hw/xen/xen-hvm-common.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/arm/xen_arch_hvm.h   |  9 -
 include/hw/i386/xen_arch_hvm.h  | 11 ---
 include/hw/xen/arch_hvm.h   |  5 -
 include/hw/xen/xen-hvm-common.h |  6 ++
 hw/arm/xen_arm.c|  1 -
 hw/i386/xen/xen-hvm.c   |  1 -
 hw/xen/xen-hvm-common.c |  1 -
 7 files changed, 6 insertions(+), 28 deletions(-)
 delete mode 100644 include/hw/arm/xen_arch_hvm.h
 delete mode 100644 include/hw/i386/xen_arch_hvm.h
 delete mode 100644 include/hw/xen/arch_hvm.h

diff --git a/include/hw/arm/xen_arch_hvm.h b/include/hw/arm/xen_arch_hvm.h
deleted file mode 100644
index 6a974f2020..00
--- a/include/hw/arm/xen_arch_hvm.h
+++ /dev/null
@@ -1,9 +0,0 @@
-#ifndef HW_XEN_ARCH_ARM_HVM_H
-#define HW_XEN_ARCH_ARM_HVM_H
-
-#include 
-void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req);
-void xen_arch_set_memory(XenIOState *state,
- MemoryRegionSection *section,
- bool add);
-#endif
diff --git a/include/hw/i386/xen_arch_hvm.h b/include/hw/i386/xen_arch_hvm.h
deleted file mode 100644
index 2822304955..00
--- a/include/hw/i386/xen_arch_hvm.h
+++ /dev/null
@@ -1,11 +0,0 @@
-#ifndef HW_XEN_ARCH_I386_HVM_H
-#define HW_XEN_ARCH_I386_HVM_H
-
-#include 
-#include "hw/xen/xen-hvm-common.h"
-
-void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req);
-void xen_arch_set_memory(XenIOState *state,
- MemoryRegionSection *section,
- bool add);
-#endif
diff --git a/include/hw/xen/arch_hvm.h b/include/hw/xen/arch_hvm.h
deleted file mode 100644
index c7c515220d..00
--- a/include/hw/xen/arch_hvm.h
+++ /dev/null
@@ -1,5 +0,0 @@
-#if defined(TARGET_I386) || defined(TARGET_X86_64)
-#include "hw/i386/xen_arch_hvm.h"
-#elif defined(TARGET_ARM) || defined(TARGET_ARM_64)
-#include "hw/arm/xen_arch_hvm.h"
-#endif
diff --git a/include/hw/xen/xen-hvm-common.h b/include/hw/xen/xen-hvm-common.h
index 4e9904f1a6..27e938d268 100644
--- a/include/hw/xen/xen-hvm-common.h
+++ b/include/hw/xen/xen-hvm-common.h
@@ -96,4 +96,10 @@ void xen_register_ioreq(XenIOState *state, unsigned int 
max_cpus,
 const MemoryListener *xen_memory_listener);
 
 void cpu_ioreq_pio(ioreq_t *req);
+
+void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req);
+void xen_arch_set_memory(XenIOState *state,
+ MemoryRegionSection *section,
+ bool add);
+
 #endif /* HW_XEN_HVM_COMMON_H */
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index 28d790f4ce..6a1d7719e9 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -33,7 +33,6 @@
 #include "sysemu/sysemu.h"
 #include "hw/xen/xen-hvm-common.h"
 #include "sysemu/tpm.h"
-#include "hw/xen/arch_hvm.h"
 
 #define TYPE_XEN_ARM  MACHINE_TYPE_NAME("xenpvh")
 OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index ffa95e3c3d..f8a195270a 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -21,7 +21,6 @@
 #include "qemu/range.h"
 
 #include "hw/xen/xen-hvm-common.h"
-#include "hw/xen/arch_hvm.h"
 #include 
 
 static MemoryRegion ram_640k, ram_lo, ram_hi;
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index 1d8bd9aea7..c028c1b541 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -7,7 +7,6 @@
 #include "hw/xen/xen-hvm-common.h"
 #include "hw/xen/xen-bus.h"
 #include "hw/boards.h"
-#include "hw/xen/arch_hvm.h"
 
 MemoryRegion ram_memory;
 
-- 
2.41.0

[PATCH-for-9.0 04/10] hw/xen: Factor xen_arch_align_ioreq_data() out of handle_ioreq()

[Philippe Mathieu-Daudé]

Per commit f17068c1c7 ("xen-hvm: reorganize xen-hvm and move common
function to xen-hvm-common"), handle_ioreq() is expected to be
target-agnostic. However it uses 'target_ulong', which is a target
specific definition.

In order to compile this file once for all targets, factor the
target-specific code out of handle_ioreq() as a per-target handler
called xen_arch_align_ioreq_data().

Signed-off-by: Philippe Mathieu-Daudé 
---
Should we have a 'unsigned qemu_target_long_bits();' helper
such qemu_target_page_foo() API and target_words_bigendian()?
---
 include/hw/xen/xen-hvm-common.h | 1 +
 hw/arm/xen_arm.c| 8 
 hw/i386/xen/xen-hvm.c   | 8 
 hw/xen/xen-hvm-common.c | 5 +
 4 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/include/hw/xen/xen-hvm-common.h b/include/hw/xen/xen-hvm-common.h
index 27e938d268..734bfa3183 100644
--- a/include/hw/xen/xen-hvm-common.h
+++ b/include/hw/xen/xen-hvm-common.h
@@ -97,6 +97,7 @@ void xen_register_ioreq(XenIOState *state, unsigned int 
max_cpus,
 
 void cpu_ioreq_pio(ioreq_t *req);
 
+void xen_arch_align_ioreq_data(ioreq_t *req);
 void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req);
 void xen_arch_set_memory(XenIOState *state,
  MemoryRegionSection *section,
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index 6a1d7719e9..c646fd70d0 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -128,6 +128,14 @@ static void xen_init_ram(MachineState *machine)
 }
 }
 
+void xen_arch_align_ioreq_data(ioreq_t *req)
+{
+if (!req->data_is_ptr && (req->dir == IOREQ_WRITE)
+&& (req->size < sizeof(target_ulong))) {
+req->data &= ((target_ulong) 1 << (8 * req->size)) - 1;
+}
+}
+
 void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req)
 {
 hw_error("Invalid ioreq type 0x%x\n", req->type);
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index f8a195270a..aff5c5b81d 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -699,6 +699,14 @@ void xen_arch_set_memory(XenIOState *state, 
MemoryRegionSection *section,
 }
 }
 
+void xen_arch_align_ioreq_data(ioreq_t *req)
+{
+if (!req->data_is_ptr && (req->dir == IOREQ_WRITE)
+&& (req->size < sizeof(target_ulong))) {
+req->data &= ((target_ulong) 1 << (8 * req->size)) - 1;
+}
+}
+
 void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req)
 {
 switch (req->type) {
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index c028c1b541..03f9417e7e 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -426,10 +426,7 @@ static void handle_ioreq(XenIOState *state, ioreq_t *req)
 trace_handle_ioreq(req, req->type, req->dir, req->df, req->data_is_ptr,
req->addr, req->data, req->count, req->size);
 
-if (!req->data_is_ptr && (req->dir == IOREQ_WRITE) &&
-(req->size < sizeof (target_ulong))) {
-req->data &= ((target_ulong) 1 << (8 * req->size)) - 1;
-}
+xen_arch_align_ioreq_data(req);
 
 if (req->dir == IOREQ_WRITE)
 trace_handle_ioreq_write(req, req->type, req->df, req->data_is_ptr,
-- 
2.41.0

[PATCH-for-9.0 00/10] hw/xen: Have most of Xen files become target-agnostic

[Philippe Mathieu-Daudé]

Hi,

After discussing with Alex Bennée I realized most Xen code
should be target-agnostic. David Woodhouse confirmed that
last week, so I had a quick look and here is the result.

More work is required to be able to instanciate Xen HW in
an heterogeneous machine, but this doesn't make sense yet
until we can run multiple accelerators concurrently.

Only build-tested.

Regards,

Phil.

Philippe Mathieu-Daudé (10):
  sysemu/xen: Forbid using Xen headers in user emulation
  hw/xen/xen_arch_hvm: Rename prototypes using 'xen_arch_' prefix
  hw/xen: Merge 'hw/xen/arch_hvm.h' in 'hw/xen/xen-hvm-common.h'
  hw/xen: Factor xen_arch_align_ioreq_data() out of handle_ioreq()
  hw/xen: Use target-agnostic qemu_target_page_bits()
  hw/xen: Reduce inclusion of 'cpu.h' to target-specific sources
  sysemu/xen-mapcache: Check Xen availability with
CONFIG_XEN_IS_POSSIBLE
  system/physmem: Only include 'hw/xen/xen.h' when Xen is available
  hw/xen: Extract 'xen_igd.h' from 'xen_pt.h'
  hw/xen: Have most of Xen files become target-agnostic

 hw/xen/xen_pt.h | 14 --
 include/hw/arm/xen_arch_hvm.h   |  9 -
 include/hw/i386/xen_arch_hvm.h  | 11 ---
 include/hw/xen/arch_hvm.h   |  5 -
 include/hw/xen/xen-hvm-common.h |  8 +++-
 include/hw/xen/xen_igd.h| 23 +++
 include/sysemu/xen-mapcache.h   |  3 ++-
 include/sysemu/xen.h|  8 
 accel/xen/xen-all.c |  1 +
 hw/arm/xen_arm.c| 14 +++---
 hw/i386/pc_piix.c   |  1 +
 hw/i386/xen/xen-hvm.c   | 16 
 hw/xen/xen-hvm-common.c | 16 +++-
 hw/xen/xen_pt.c |  3 ++-
 hw/xen/xen_pt_config_init.c |  3 ++-
 hw/xen/xen_pt_graphics.c|  3 ++-
 hw/xen/xen_pt_stub.c|  2 +-
 system/physmem.c|  5 -
 accel/xen/meson.build   |  2 +-
 hw/block/dataplane/meson.build  |  2 +-
 hw/xen/meson.build  | 13 -
 21 files changed, 85 insertions(+), 77 deletions(-)
 delete mode 100644 include/hw/arm/xen_arch_hvm.h
 delete mode 100644 include/hw/i386/xen_arch_hvm.h
 delete mode 100644 include/hw/xen/arch_hvm.h
 create mode 100644 include/hw/xen/xen_igd.h

-- 
2.41.0

[PATCH-for-9.0 02/10] hw/xen/xen_arch_hvm: Rename prototypes using 'xen_arch_' prefix

[Philippe Mathieu-Daudé]

Use a common 'xen_arch_' prefix for architecture-specific functions.
Rename xen_arch_set_memory() and xen_arch_handle_ioreq().

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/arm/xen_arch_hvm.h  | 4 ++--
 include/hw/i386/xen_arch_hvm.h | 4 ++--
 hw/arm/xen_arm.c   | 4 ++--
 hw/i386/xen/xen-hvm.c  | 6 +++---
 hw/xen/xen-hvm-common.c| 4 ++--
 5 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/hw/arm/xen_arch_hvm.h b/include/hw/arm/xen_arch_hvm.h
index 8fd645e723..6a974f2020 100644
--- a/include/hw/arm/xen_arch_hvm.h
+++ b/include/hw/arm/xen_arch_hvm.h
@@ -2,8 +2,8 @@
 #define HW_XEN_ARCH_ARM_HVM_H
 
 #include 
-void arch_handle_ioreq(XenIOState *state, ioreq_t *req);
-void arch_xen_set_memory(XenIOState *state,
+void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req);
+void xen_arch_set_memory(XenIOState *state,
  MemoryRegionSection *section,
  bool add);
 #endif
diff --git a/include/hw/i386/xen_arch_hvm.h b/include/hw/i386/xen_arch_hvm.h
index 1000f8f543..2822304955 100644
--- a/include/hw/i386/xen_arch_hvm.h
+++ b/include/hw/i386/xen_arch_hvm.h
@@ -4,8 +4,8 @@
 #include 
 #include "hw/xen/xen-hvm-common.h"
 
-void arch_handle_ioreq(XenIOState *state, ioreq_t *req);
-void arch_xen_set_memory(XenIOState *state,
+void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req);
+void xen_arch_set_memory(XenIOState *state,
  MemoryRegionSection *section,
  bool add);
 #endif
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index a5631529d0..28d790f4ce 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -129,14 +129,14 @@ static void xen_init_ram(MachineState *machine)
 }
 }
 
-void arch_handle_ioreq(XenIOState *state, ioreq_t *req)
+void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req)
 {
 hw_error("Invalid ioreq type 0x%x\n", req->type);
 
 return;
 }
 
-void arch_xen_set_memory(XenIOState *state, MemoryRegionSection *section,
+void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
  bool add)
 {
 }
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index f42621e674..ffa95e3c3d 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -659,8 +659,8 @@ void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
 }
 }
 
-void arch_xen_set_memory(XenIOState *state, MemoryRegionSection *section,
-bool add)
+void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
+ bool add)
 {
 hwaddr start_addr = section->offset_within_address_space;
 ram_addr_t size = int128_get64(section->size);
@@ -700,7 +700,7 @@ void arch_xen_set_memory(XenIOState *state, 
MemoryRegionSection *section,
 }
 }
 
-void arch_handle_ioreq(XenIOState *state, ioreq_t *req)
+void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req)
 {
 switch (req->type) {
 case IOREQ_TYPE_VMWARE_PORT:
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index 565dc39c8f..1d8bd9aea7 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -65,7 +65,7 @@ static void xen_set_memory(struct MemoryListener *listener,
 }
 }
 
-arch_xen_set_memory(state, section, add);
+xen_arch_set_memory(state, section, add);
 }
 
 void xen_region_add(MemoryListener *listener,
@@ -452,7 +452,7 @@ static void handle_ioreq(XenIOState *state, ioreq_t *req)
 cpu_ioreq_config(state, req);
 break;
 default:
-arch_handle_ioreq(state, req);
+xen_arch_handle_ioreq(state, req);
 }
 if (req->dir == IOREQ_READ) {
 trace_handle_ioreq_read(req, req->type, req->df, req->data_is_ptr,
-- 
2.41.0

[PATCH-for-9.0 01/10] sysemu/xen: Forbid using Xen headers in user emulation

[Philippe Mathieu-Daudé]

Xen is a system specific accelerator, it makes no sense
to include its headers in user emulation.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/sysemu/xen.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/sysemu/xen.h b/include/sysemu/xen.h
index bc13ad5692..a9f591f26d 100644
--- a/include/sysemu/xen.h
+++ b/include/sysemu/xen.h
@@ -10,6 +10,10 @@
 #ifndef SYSEMU_XEN_H
 #define SYSEMU_XEN_H
 
+#ifdef CONFIG_USER_ONLY
+#error Cannot include sysemu/xen.h from user emulation
+#endif
+
 #include "exec/cpu-common.h"
 
 #ifdef NEED_CPU_H
@@ -26,16 +30,13 @@ extern bool xen_allowed;
 
 #define xen_enabled()   (xen_allowed)
 
-#ifndef CONFIG_USER_ONLY
 void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length);
 void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
struct MemoryRegion *mr, Error **errp);
-#endif
 
 #else /* !CONFIG_XEN_IS_POSSIBLE */
 
 #define xen_enabled() 0
-#ifndef CONFIG_USER_ONLY
 static inline void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
 {
 /* nothing */
@@ -45,7 +46,6 @@ static inline void xen_ram_alloc(ram_addr_t ram_addr, 
ram_addr_t size,
 {
 g_assert_not_reached();
 }
-#endif
 
 #endif /* CONFIG_XEN_IS_POSSIBLE */
 
-- 
2.41.0

Re: Clang-format configuration discussion - pt 1

[Luca Fancellu]

> On 13 Nov 2023, at 11:31, Jan Beulich  wrote:
> 
> On 08.11.2023 10:53, Luca Fancellu wrote:
> --
>> 
>> Standard: C++03
>> 
>> ---
>> From the documentation: Parse and format C++ constructs compatible with this 
>> standard.
> 
> Since I continue to be puzzled - iirc you said this is because of lack
> of availability of "C99" as a value here. What's entirely unclear to
> me is: How does this matter to a tool checking coding style (which is
> largely about formatting, not any lexical or syntactical aspects)?
> 
>> This value is used also in Linux.
> 
> Considering how different the two styles are, I don't think this is
> overly relevant.

Ok, maybe I understand your point, you are looking for a reason to declare this 
configurable instead
of not specifying it at all?

If it’s that, from what I understand clang-format will use the default value if 
we don’t specify anything
for this one, so it will take ‘Latest’. I think we should put a value for this 
one to fix it and don’t have
surprises if that behaviour changes and seeing that also in Linux that value is 
fixed increased my
confidence.

However, if you feel that we should not specify it, I’ve done a test and not 
specifying it is not changing
the current output. I can’t say that for a different clang-format version 
though or if changes happen in the
future.


> 
> --
>> 
>> AttributeMacros:
>>  - '__init'
>>  - '__exit'
>>  - '__initdata'
>>  - '__initconst'
>>  - '__initconstrel'
>>  - '__initdata_cf_clobber'
>>  - '__initconst_cf_clobber'
>>  - '__hwdom_init'
>>  - '__hwdom_initdata'
>>  - '__maybe_unused'
>>  - '__packed'
>>  - '__stdcall'
>>  - '__vfp_aligned'
>>  - '__alt_call_maybe_initdata'
>>  - '__cacheline_aligned'
>>  - '__ro_after_init'
>>  - 'always_inline'
>>  - 'noinline'
>>  - 'noreturn'
>>  - '__weak'
>>  - '__inline__'
>>  - '__attribute_const__'
>>  - '__transparent__'
>>  - '__used'
>>  - '__must_check'
>>  - '__kprobes'
>> 
>> ---
>> A vector of strings that should be interpreted as attributes/qualifiers 
>> instead of identifiers.
>> I’ve tried to list all the attributes I’ve found.
> 
> Like above, the significance of having this list and needing to keep it
> up-to-date is unclear to me. I guess the same also applies to a few
> further settings down from here.

From what I understand, we should give to the formatter tool all the hint 
possible to make it do a good
job, for example here we should maintain a list of our attributes so that 
clang-format doesn’t think the function
below is called __init instead of device_tree_node_matches.

static bool __init device_tree_node_matches(const void *fdt, int node, const 
char *match)
{ ... }

> 
> --
>> 
>> StatementMacros:
>>  - 'PROGRESS'
>>  - 'PROGRESS_VCPU'
>>  - 'bitop'
>>  - 'guest_bitop'
>>  - 'testop'
>>  - 'guest_testop'
>>  - 'DEFINE_XEN_GUEST_HANDLE'
>>  - '__DEFINE_XEN_GUEST_HANDLE'
>>  - '___DEFINE_XEN_GUEST_HANDLE'
>>  - 'presmp_initcall'
>>  - '__initcall'
>>  - '__exitcall'
>> 
>> ---
>> A vector of macros that should be interpreted as complete statements.
>> Typical macros are expressions, and require a semi-colon to be added; 
>> sometimes this is not the case, and this allows
>> to make clang-format aware of such cases.
>> 
>> While I was writing this, I’ve found that from ‘DEFINE_XEN_GUEST_HANDLE’ 
>> until the end of the list, probably I
>> shouldn’t list these entries because all of them end with semi-colon.
> 
> Ah, just wanted to ask. I agree that we'd better have here only items
> which truly require an exception.

Yes my mistake, I’ll fix the list.

> 
> Jan

Re: [XEN PATCH] CI: Rework RISCV smoke test

[Anthony PERARD]

On Thu, Nov 09, 2023 at 04:52:36PM +, Andrew Cooper wrote:
> On 09/11/2023 3:49 pm, Anthony PERARD wrote:
> > Currently, the test rely on QEMU and Xen finishing the boot in under
> > two seconds. That's both very long and very short. Xen usually managed
> > to print "All set up" under a second. Unless for some reason we try to
> > run the test on a machine that's busy doing something else.
> >
> > Rework the test to exit as soon as Xen is done.
> >
> > There's two `tail -f`, the first one is there simply to monitor test
> > progress in GitLab console. The second one is used to detect the test
> > result as soon as QEMU add it to the file. Both `tail` exit as soon as
> > QEMU exit.
> >
> > If QEMU fails at start, and exit early, both `tail` will simply exit,
> > resulting in a failure.
> >
> > If the line we are looking for is never printed, the `timeout` on the
> > second `tail` will force the test to exit with an error.
> >
> > Signed-off-by: Anthony PERARD 
> 
> Looks plausible, but all these qemu-smoke scripts are pretty similar,
> and copied from one-another.
> 
> We should make this change consistently to all testing (there's nothing
> RISC-V specific about why this test is failing on this runner), and it
> would be really nice if we could try to make it a bit more common than
> it currently is.

Yes, it would be nice if a change to a qemu-smoke script was applied to
every other one. But making those scripts more generic is a lot more
works, which would be useful to apply a change once for all.

The problem I'm trying to resolve only appear with this script, because
of a timeout been too short, a solution could just be to increase the
timeout (or not allowing runners to do more than one thing at a time).

BTW, the last time I've been told to apply a change to other things, I
never managed to finish it and the change wasn't applied at all (would
have result in some containers been smaller).

I guess will see if anyone complain about the test randomly failing. :-)

Cheers,

-- 
Anthony PERARD

xen | Successful pipeline for staging | 162a1589

[GitLab]

Pipeline #1070357357 has passed!

Project: xen ( https://gitlab.com/xen-project/xen )
Branch: staging ( https://gitlab.com/xen-project/xen/-/commits/staging )

Commit: 162a1589 ( 
https://gitlab.com/xen-project/xen/-/commit/162a1589e362ec47671b69a05464048360df5002
 )
Commit Message: xen/set_{c,p}x_pminfo: address violations od MI...
Commit Author: Federico Serafini
Committed by: Jan Beulich ( https://gitlab.com/jbeulich )



Pipeline #1070357357 ( 
https://gitlab.com/xen-project/xen/-/pipelines/1070357357 ) triggered by Ganis 
( https://gitlab.com/ganis )
successfully completed 129 jobs in 3 stages.

-- 
You're receiving this email because of your account on gitlab.com.

Re: [PATCH v2 03/15] xen: ifdef inclusion of in

[Oleksii]

On Mon, 2023-11-13 at 14:29 +0100, Jan Beulich wrote:
> On 13.11.2023 14:13, Oleksii wrote:
> > On Sat, 2023-11-11 at 12:25 +0200, Oleksii wrote:
> > > I missed to check the patch properly.
> > > 
> > > The patch fails for Arm randconfigs:
> > > https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/1068865674
> > > 
> > > I need to do an additional investigation.
> > So the only one macro cause compile issue if move it to
> > xen/grant_table.h compilation will pass:
> > 
> > --- a/xen/include/xen/grant_table.h
> > +++ b/xen/include/xen/grant_table.h
> > @@ -23,10 +23,14 @@
> >  #ifndef __XEN_GRANT_TABLE_H__
> >  #define __XEN_GRANT_TABLE_H__
> >  
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > +
> > +#ifdef CONFIG_GRANT_TABLE
> >  #include 
> > +#endif
> >  
> >  struct grant_table;
> >  
> > @@ -112,6 +116,16 @@ static inline int gnttab_acquire_resource(
> >  return -EINVAL;
> >  }
> >  
> > +/*
> > + * The region used by Xen on the memory will never be mapped in
> > DOM0
> > + * memory layout. Therefore it can be used for the grant table.
> > + *
> > + * Only use the text section as it's always present and will
> > contain
> > + * enough space for a large grant table
> > + */
> > +#define
> > gnttab_dom0_frames() 
> > \
> > +    min_t(unsigned int, opt_max_grant_frames, PFN_DOWN(_etext -
> > _stext))
> > +
> >  #endif /* CONFIG_GRANT_TABLE */
> >  
> >  #endif /* __XEN_GRANT_TABLE_H__ */
> > 
> > 
> > But gnttab_dom0_frames() is used only for ARM, so probably moving
> > it to
> >  is not a good idea.
> 
> Indeed. But wouldn't dealing with this again be a matter of having
> Arm's domain_build.c simply include asm/grant_table.h explicitly, if
> need
> be alongside xen/grant_table.h?
It can be a solution. Then I'll send a separate patch.

Thanks.

~ Oleksii

Re: [XEN PATCH] CI: Rework RISCV smoke test

[Anthony PERARD]

On Thu, Nov 09, 2023 at 05:02:08PM -0800, Stefano Stabellini wrote:
> ### qemu_key.sh is using "expect", see below. I think we should be able
> ### to achieve the same by using expect to close on the expected string
> ### (instead of waiting for eof)

So, `expect` is just a different kind of language than shell is?
Also, `grep -q` doesn't wait for EOF, and just exit as soon as it found
the pattern.

Cheers,

-- 
Anthony PERARD