Re: [PATCH v2 05/15] x86: amend cpu_has_xen_[ibt,shstk}

[Jan Beulich]

On 24.11.2023 14:52, Andrew Cooper wrote:
> In the subject, [ -> {
> 
> ?

Right, noticed (sadly) just after sending, and already corrected locally.

> On 24/11/2023 8:38 am, Jan Beulich wrote:
>> ... to evaluate to false at compile-time when the respective Kconfig
>> control is off, thus allowing the compiler to eliminate then-dead code.
>>
>> Signed-off-by: Jan Beulich 
> 
> Acked-by: Andrew Cooper 

Thanks.

Jan

Re: [PATCH v2 06/15] x86/HVM: improve CET-IBT pruning of ENDBR

[Jan Beulich]

On 24.11.2023 23:28, Andrew Cooper wrote:
> On 24/11/2023 8:39 am, Jan Beulich wrote:
>> __init{const,data}_cf_clobber can have an effect only for pointers
>> actually populated in the respective tables. While not the case for SVM
>> right now, VMX installs a number of pointers only under certain
>> conditions. Hence the respective functions would have their ENDBR purged
>> only when those conditions are met. Invoke "pruning" functions after
>> having copied the respective tables, for them to install any "missing"
>> pointers.
>>
>> Signed-off-by: Jan Beulich 
> 
> In theory Acked-by: Andrew Cooper , but see
> later.

Thanks. See below as well.

>> ---
>> This is largely cosmetic for present hardware, which when supporting
>> CET-IBT likely also supports all of the advanced VMX features for which
>> hook pointers are installed conditionally. The only case this would make
>> a difference there is when use of respective features was suppressed via
>> command line option (where available). For future hooks it may end up
>> relevant even by default, and it also would be if AMD started supporting
>> CET-IBT; right now it matters only for .pi_update_irte, as iommu_intpost
>> continues to default to off.
>>
>> Originally I had meant to put the SVM and VMX functions in presmp-
>> initcalls, but hvm/{svm,vmx}/built_in.o are linked into hvm/built_in.o
>> before hvm/hvm.o. And I don't think I want to fiddle with link order
>> here.
> 
> An alternative is the form I used for microcode, where start_{vmx,svm}()
> fills in fns, and doesn't have to fill in all hooks.
> 
> That will be more amenable to Kconfig-ing generally, and will probably
> be less fragile to getting forgotten.

You mean specifically "x86/ucode: Move vendor specifics back out of
early_microcode_init()", which looks to not have gone in yet? That's going
the opposite route (NULLing out hooks after copying), yet this looks to
still go against what you're asking for below. In how far this would reduce
the chance of being forgotten is not entirely clear to me right away, but I
think I see your point.

One (further) benefit of copying before editing would of course be that the
static struct instances then can be const.

>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -3033,6 +3033,30 @@ const struct hvm_function_table * __init
>>  return &vmx_function_table;
>>  }
>>  
>> +void __init prune_vmx(void)
>> +{
>> +/*
>> + * Now that vmx_function_table was copied, populate all function 
>> pointers
>> + * which may have been left at NULL, for __initdata_cf_clobber to have 
>> as
>> + * much of an effect as possible.
>> + */
>> +if ( !cpu_has_xen_ibt )
>> +return;
>> +
>> +vmx_function_table.set_descriptor_access_exiting =
>> +vmx_set_descriptor_access_exiting;
>> +
>> +vmx_function_table.update_eoi_exit_bitmap = vmx_update_eoi_exit_bitmap;
>> +vmx_function_table.process_isr= vmx_process_isr;
>> +vmx_function_table.handle_eoi = vmx_handle_eoi;
>> +
>> +vmx_function_table.pi_update_irte = vmx_pi_update_irte;
>> +
>> +vmx_function_table.deliver_posted_intr = vmx_deliver_posted_intr;
>> +vmx_function_table.sync_pir_to_irr = vmx_sync_pir_to_irr;
>> +vmx_function_table.test_pir= vmx_test_pir;
> 
> That said...
> 
> This (the hooks being conditional in the first place) is bogus to begin
> with.  Posted interrupts (or not) are a per-VM property even if we don't
> wire this up properly yet.  It will be forced to be done properly in
> order to support nested virt, as L0 Xen *must* comply with the settings
> chosen by the L1 hypervisor.
> 
> So the choice to use the hooks will have to come from per-vCPU state,
> and not from the conditional-ness of them.
> 
> Any chance I can talk you into instead making the hooks unconditional? 
> If not, someone (George was volunteering) is going to have to undo this
> in fairly short order.

As you can see from "x86/HVM: drop tsc_scaling.setup() hook" I actually
did consider removing the conditional parts, where sufficiently
straightforward. I'll take another close look for the other hooks, but
I'm not going to promise anything towards actually taking the alternative
route.

Jan

Re: [PATCH] x86/x2apic: introduce a mixed physical/cluster mode

[Roger Pau Monné]

On Fri, Nov 24, 2023 at 05:15:34PM -0800, Elliott Mitchell wrote:
> On Thu, Nov 23, 2023 at 10:39:37AM +0100, Roger Pau Monné wrote:
> > On Tue, Nov 21, 2023 at 04:56:47PM -0800, Elliott Mitchell wrote:
> > > It was insisted that full logs be sent to xen-devel.  Perhaps I am
> > > paranoid, but I doubt I would have been successful at scrubbing all
> > > hardware serial numbers.  As such, I was willing to post substantial
> > > sections, but not everything.
> > 
> > Can you please point out which hardware serial numbers are you
> > referring to, and where are they printed in Xen dmesg?
> 
> I didn't confirm the presence of hardware serial numbers getting into
> Xen's dmesg, but there was a lot of data and many hexadecimal numbers.
> With 4000 lines of output, checking for concerning data is troublesome.

4000 lines of output from Xen dmesg seems quite suspicious.  Xen dmesg
on a fully booted box is ~400 lines on my local box.  That might get
quite longer if you have a box with a lot of memory region, or a lot
of CPUs.

> There was enough for alarms to trigger.

That's fine, but without pointing out which information is sensitive,
it's very unlikely such concerns will ever get fixed.

Could you at least go over the log and point out the first instance of
such line that you consider contains sensitive information?

The primary way we have to diagnose and solve issues reported by users
is based on Xen dmesg, so if it's impossible to provide it for privacy
reasons that's an issue that we need to address.

Regards, Roger.

Re: [EXTERNAL] [PATCH v3 2/5] xen: backends: don't overwrite XenStore nodes created by toolstack

[David Woodhouse]

On Fri, 2023-11-24 at 23:24 +, Volodymyr Babchuk wrote:
> Xen PV devices in QEMU can be created in two ways: either by QEMU
> itself, if they were passed via command line, or by Xen toolstack. In
> the latter case, QEMU scans XenStore entries and configures devices
> accordingly.
> 
> In the second case we don't want QEMU to write/delete front-end
> entries for two reasons: it might have no access to those entries if
> it is running in un-privileged domain and it is just incorrect to
> overwrite entries already provided by Xen toolstack, because
> toolstack
> manages those nodes. For example, it might read backend- or frontend-
> state to be sure that they are both disconnected and it is safe to
> destroy a domain.
> 
> This patch checks presence of xendev->backend to check if Xen PV
> device was configured by Xen toolstack to decide if it should touch
> frontend entries in XenStore. Also, when we need to remove XenStore
> entries during device teardown only if they weren't created by Xen
> toolstack. If they were created by toolstack, then it is toolstack's
> job to do proper clean-up.
> 
> Suggested-by: Paul Durrant 
> Suggested-by: David Woodhouse 
> Co-Authored-by: Oleksandr Tyshchenko 
> Signed-off-by: Volodymyr Babchuk 

Reviewed-by: David Woodhouse 

... albeit with a couple of suggestions... 

> diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
> index bef8a3a621..b52abf 100644
> --- a/hw/char/xen_console.c
> +++ b/hw/char/xen_console.c
> @@ -450,7 +450,7 @@ static void xen_console_realize(XenDevice *xendev, Error 
> **errp)
> 
>  trace_xen_console_realize(con->dev, object_get_typename(OBJECT(cs)));
> 
> -    if (CHARDEV_IS_PTY(cs)) {
> +    if (CHARDEV_IS_PTY(cs) && !xendev->backend) {
>  /* Strip the leading 'pty:' */
>  xen_device_frontend_printf(xendev, "tty", "%s", cs->filename + 4);
>  }


It's kind of weird that that one is a frontend node at all; surely it
should have been a backend node? But it is known only to QEMU once it
actually opens /dev/ptmx and creates a new pty. It can't be populated
by the toolstack in advance.

So shouldn't the toolstack have made it writable by the driver domain? 

I think we should attempt to write this and just gracefully handle the
failure if we can't. (In fact, xen_device_frontend_printf() will just
use error_report_err() which is probably OK unless you feel strongly
about silencing it).

> diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
> index afa10c96e8..27442bef38 100644
> --- a/hw/net/xen_nic.c
> +++ b/hw/net/xen_nic.c
> @@ -315,14 +315,16 @@ static void xen_netdev_realize(XenDevice *xendev, Error 
> **errp)
> 
>  qemu_macaddr_default_if_unset(&netdev->conf.macaddr);
> 
> -    xen_device_frontend_printf(xendev, "mac", 
> "%02x:%02x:%02x:%02x:%02x:%02x",
> -   netdev->conf.macaddr.a[0],
> -   netdev->conf.macaddr.a[1],
> -   netdev->conf.macaddr.a[2],
> -   netdev->conf.macaddr.a[3],
> -   netdev->conf.macaddr.a[4],
> -   netdev->conf.macaddr.a[5]);
> -
> +    if (!xendev->backend) {
> +    xen_device_frontend_printf(xendev, "mac",
> +   "%02x:%02x:%02x:%02x:%02x:%02x",
> +   netdev->conf.macaddr.a[0],
> +   netdev->conf.macaddr.a[1],
> +   netdev->conf.macaddr.a[2],
> +   netdev->conf.macaddr.a[3],
> +   netdev->conf.macaddr.a[4],
> +   netdev->conf.macaddr.a[5]);
> +    }
>  netdev->nic = qemu_new_nic(&net_xen_info, &netdev->conf,
>     object_get_typename(OBJECT(xendev)),
>     DEVICE(xendev)->id, netdev);


Perhaps here you should create the "mac" node if it doesn't exist (or
is that "if it doesn't match netdev->conf.macaddr"?) and just
gracefully accept failure too?





smime.p7s
Description: S/MIME cryptographic signature

Re: [PATCH v4] xen/x86: On x2APIC mode, derive LDR from APIC ID

[Jan Beulich]

On 23.11.2023 18:30, Alejandro Vallejo wrote:
> @@ -1498,27 +1511,36 @@ static int cf_check lapic_save_regs(struct vcpu *v, 
> hvm_domain_context_t *h)
>   */
>  static void lapic_load_fixup(struct vlapic *vlapic)
>  {
> -uint32_t id = vlapic->loaded.id;
> +uint32_t good_ldr = x2apic_ldr_from_id(vlapic->loaded.id);
>  
> -if ( vlapic_x2apic_mode(vlapic) && id && vlapic->loaded.ldr == 1 )
> -{
> +/* Skip fixups on xAPIC mode, or if the x2APIC LDR is already correct */
> +if ( !vlapic_x2apic_mode(vlapic) ||
> + (vlapic->loaded.ldr == good_ldr) )
> +return;
> +
> +if ( vlapic->loaded.ldr == 1 )
> +   /*
> +* Xen <= 4.4 may have a bug by which all the APICs configured in
> +* x2APIC mode got LDR = 1. We can't leave it as-is because it
> +* assigned the same LDR to every CPU.  We'll fix fix the bug now
> +* and assign an LDR value consistent with the APIC ID.
> +*/

Just one comment on top of Andrew's: Is the double "fix" really intended
here? (I could see it might be, but then "fix the bug fix" would read
slightly more smoothly to me as a non-native speaker.)

Another aspect here is what exactly the comment states (and does not
state). Original comments made explicit that LDR == 1 contradicts ID == 0.
In the new comment you only emphasize that all CPUs cannot have that same
LDR. But the value of 1 being bogus in the first place doesn't become clear
anymore.

Jan

Re: [RFC PATCH v3 3/5] xen: add option to disable legacy backends

[Woodhouse, David]

On Fri, 2023-11-24 at 23:24 +, Volodymyr Babchuk wrote:
> This patch makes legacy backends optional. As was discussed at [1]
> this is a solution to a problem when we can't run QEMU as a device
> model in a non-privileged domain. This is because legacy backends
> assume that they are always running in domain with ID = 0. Actually,
> this may prevent running QEMU in a privileged domain with ID not equal
> to zero.
> 
> To be able to disable legacy backends we need to alter couple of
> source files that unintentionally depend on them. For example
> xen-all.c used xen_pv_printf to report errors, while not providing any
> additional like xendev pointer. Also, we need to move xenstore
> structure from xen-legacy-backend.c, because it is apparently used in
> xen-all.c.
> 
> With this patch it is possible to provide
> "--disable-xen-legacy-backends" configure option to get QEMU binary
> that can run in a driver domain. With price of not be able to use
> legacy backends of course.
> 
> [1]
> https://lists.gnu.org/archive/html/qemu-devel/2023-11/msg05022.html
> 
> Signed-off-by: Volodymyr Babchuk 
> 
> ---
> 
> I am not sure if I made correct changes to the build system, thus this
> patch is tagged as RFC.

Hm, I was imagining a new CONFIG_LEGACY_XEN_BACKENDS option which would
look a lot like CONFIG_XEN_BUS (which would now be only for the new
XenBus code).

This looks weird to me:

> --- a/hw/display/meson.build
> +++ b/hw/display/meson.build
> @@ -14,7 +14,9 @@ system_ss.add(when: 'CONFIG_PL110', if_true:
> files('pl110.c'))
>  system_ss.add(when: 'CONFIG_SII9022', if_true: files('sii9022.c'))
>  system_ss.add(when: 'CONFIG_SSD0303', if_true: files('ssd0303.c'))
>  system_ss.add(when: 'CONFIG_SSD0323', if_true: files('ssd0323.c'))
> -system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xenfb.c'))
> +if have_xen_legacy_backends
> +  system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xenfb.c'))
> +endif
> 
>  system_ss.add(when: 'CONFIG_VGA_PCI', if_true: files('vga-pci.c'))
>  system_ss.add(when: 'CONFIG_VGA_ISA', if_true: files('vga-isa.c'))

I'd prefer to see just:

-system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xenfb.c'))
+system_ss.add(when: 'CONFIG_XEN_LEGACY_BACKENDS', if_true: files('xenfb.c'))


Probably also better to split out the bits in accel/xen/xen-all.c and
hw/xen/xen-legacy-backend.c to a separate preparatory commit.


smime.p7s
Description: S/MIME cryptographic signature



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.

Re: [RFC PATCH v3 3/5] xen: add option to disable legacy backends

[Volodymyr Babchuk]

Hi David,

"Woodhouse, David"  writes:

> [[S/MIME Signed Part:Undecided]]
> On Fri, 2023-11-24 at 23:24 +, Volodymyr Babchuk wrote:
>> This patch makes legacy backends optional. As was discussed at [1]
>> this is a solution to a problem when we can't run QEMU as a device
>> model in a non-privileged domain. This is because legacy backends
>> assume that they are always running in domain with ID = 0. Actually,
>> this may prevent running QEMU in a privileged domain with ID not equal
>> to zero.
>> 
>> To be able to disable legacy backends we need to alter couple of
>> source files that unintentionally depend on them. For example
>> xen-all.c used xen_pv_printf to report errors, while not providing any
>> additional like xendev pointer. Also, we need to move xenstore
>> structure from xen-legacy-backend.c, because it is apparently used in
>> xen-all.c.
>> 
>> With this patch it is possible to provide
>> "--disable-xen-legacy-backends" configure option to get QEMU binary
>> that can run in a driver domain. With price of not be able to use
>> legacy backends of course.
>> 
>> [1]
>> https://lists.gnu.org/archive/html/qemu-devel/2023-11/msg05022.html
>> 
>> Signed-off-by: Volodymyr Babchuk 
>> 
>> ---
>> 
>> I am not sure if I made correct changes to the build system, thus this
>> patch is tagged as RFC.
>
> Hm, I was imagining a new CONFIG_LEGACY_XEN_BACKENDS option which would
> look a lot like CONFIG_XEN_BUS (which would now be only for the new
> XenBus code).
>

It was my original intention too. But it appears that it is not possible
to add Kconfig value and then make it configurable via ./config
script. As I understood it can be set only via defconfig file.

> This looks weird to me:
>
>> --- a/hw/display/meson.build
>> +++ b/hw/display/meson.build
>> @@ -14,7 +14,9 @@ system_ss.add(when: 'CONFIG_PL110', if_true:
>> files('pl110.c'))
>>  system_ss.add(when: 'CONFIG_SII9022', if_true: files('sii9022.c'))
>>  system_ss.add(when: 'CONFIG_SSD0303', if_true: files('ssd0303.c'))
>>  system_ss.add(when: 'CONFIG_SSD0323', if_true: files('ssd0323.c'))
>> -system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xenfb.c'))
>> +if have_xen_legacy_backends
>> +  system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xenfb.c'))
>> +endif
>> 
>>  system_ss.add(when: 'CONFIG_VGA_PCI', if_true: files('vga-pci.c'))
>>  system_ss.add(when: 'CONFIG_VGA_ISA', if_true: files('vga-isa.c'))
>
> I'd prefer to see just:
>
> -system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xenfb.c'))
> +system_ss.add(when: 'CONFIG_XEN_LEGACY_BACKENDS', if_true: files('xenfb.c'))

I tried, but it does not work this way. I need to create Kconfig
variable to do this, but then other problems appear.

>
> Probably also better to split out the bits in accel/xen/xen-all.c and
> hw/xen/xen-legacy-backend.c to a separate preparatory commit.

Okay, will do.

-- 
WBR, Volodymyr

[PATCH v3 00/32] Mini-OS: hide mini-os internal symbols

[Juergen Gross]

In order to avoid conflicts due to symbols with the same name when
linking Mini-OS with an application, hide all Mini9-OS internal symbols
from the application by linking the Mini-OS kernel individually and
then removing all symbols which should be used internally only.

Changes in V2:
- added more symbols in patch 2

Changes in V3:
- Instead of a global file with symbol names use EXPORT_SYMBOL()
  instances for each symbol. Former patch 2 is replaced with new
  patches 2-32.

Juergen Gross (32):
  Mini-OS: link kernel separately
  Mini-OS: explicitly mark symbols to be visible for apps
  Mini-OS: add EXPORT_SYMBOL() instances to kernel.c
  Mini-OS: add EXPORT_SYMBOL() instances to hypervisor.c
  Mini-OS: add EXPORT_SYMBOL() instances to sched.c
  Mini-OS: add EXPORT_SYMBOL() instances to mm.c
  Mini-OS: add EXPORT_SYMBOL() instances to lib/sys.c
  Mini-OS: add EXPORT_SYMBOL() instances to lib/string.c
  Mini-OS: add EXPORT_SYMBOL() instances to lib/math.c
  Mini-OS: add EXPORT_SYMBOL() instances to lib/printf.c
  Mini-OS: add EXPORT_SYMBOL() instances to lib/ctype.c
  Mini-OS: add EXPORT_SYMBOL() instances to lib/xmalloc.c
  Mini-OS: add EXPORT_SYMBOL() instances to lib/xs.c
  Mini-OS: add EXPORT_SYMBOL() instances to main.c
  Mini-OS: add EXPORT_SYMBOL() instances to console.c
  Mini-OS: add EXPORT_SYMBOL() instances to events.c
  Mini-OS: add EXPORT_SYMBOL() instances to gntmap.c
  Mini-OS: add EXPORT_SYMBOL() instances to gnttab.c
  Mini-OS: add EXPORT_SYMBOL() instances to ioremap.c
  Mini-OS: add EXPORT_SYMBOL() instances to lock.c
  Mini-OS: add EXPORT_SYMBOL() instances to time.c
  Mini-OS: add EXPORT_SYMBOL() instances to lwip-*.c
  Mini-OS: add EXPORT_SYMBOL() instances to 9pfront.c
  Mini-OS: add EXPORT_SYMBOL() instances to blkfront.c
  Mini-OS: add EXPORT_SYMBOL() instances to fbfront.c
  Mini-OS: add EXPORT_SYMBOL() instances to netfront.c
  Mini-OS: add EXPORT_SYMBOL() instances to pcifront.c
  Mini-OS: add EXPORT_SYMBOL() instances to tpmback.c
  Mini-OS: add EXPORT_SYMBOL() instances to tpmfront.c
  Mini-OS: add EXPORT_SYMBOL() instances to tpm_tis.c
  Mini-OS: add EXPORT_SYMBOL() instances to xenbus.c
  Mini-OS: hide all symbols not exported via EXPORT_SYMBOLS()

 .gitignore |  1 +
 9pfront.c  |  2 +
 Makefile   | 11 --
 arch/arm/mm.c  |  2 +
 arch/arm/time.c|  1 +
 arch/x86/ioremap.c |  5 ++-
 arch/x86/mm.c  |  5 +++
 arch/x86/sched.c   |  5 +++
 arch/x86/time.c|  2 +-
 blkfront.c | 12 --
 console.c  |  5 +++
 events.c   |  7 
 fbfront.c  | 11 +-
 gntmap.c   |  5 +++
 gnttab.c   |  6 +++
 hypervisor.c   |  5 +++
 include/export.h   | 10 +
 include/lib.h  |  1 +
 kernel.c   |  5 +++
 lib/ctype.c|  2 +
 lib/math.c |  6 +++
 lib/printf.c   | 10 -
 lib/string.c   | 18 +
 lib/sys.c  | 92 +++---
 lib/xmalloc.c  |  3 ++
 lib/xs.c   | 10 +
 lock.c |  9 -
 lwip-arch.c|  9 +
 lwip-net.c |  2 +-
 main.c |  3 ++
 mm.c   |  4 ++
 netfront.c |  7 
 pcifront.c | 11 +-
 sched.c|  7 
 tpm_tis.c  |  5 +++
 tpmback.c  | 11 ++
 tpmfront.c |  6 +++
 xenbus.c   | 23 
 38 files changed, 320 insertions(+), 19 deletions(-)
 create mode 100644 include/export.h

-- 
2.35.3

[PATCH v3 02/32] Mini-OS: explicitly mark symbols to be visible for apps

[Juergen Gross]

Add an EXPORT_SYMBOL() macro to explicitly mark a symbol to be visible
for an app or library linked with Mini-OS. This prepares hiding all
other symbols from external components, avoiding any problems with
duplicate symbol names.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 .gitignore   |  1 +
 Makefile |  3 ++-
 include/export.h | 10 ++
 include/lib.h|  1 +
 4 files changed, 14 insertions(+), 1 deletion(-)
 create mode 100644 include/export.h

diff --git a/.gitignore b/.gitignore
index abef46b2..bacf787e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -16,3 +16,4 @@ mini-os.gz
 minios-config.mk
 mini-os-debug
 mini-os-debug.gz
+syms
diff --git a/Makefile b/Makefile
index 85c6db75..588496cb 100644
--- a/Makefile
+++ b/Makefile
@@ -166,10 +166,11 @@ $(OBJ_DIR)/arch/x86/minios-x86%.lds:  
arch/x86/minios-x86.lds.S
 
 $(OBJ_DIR)/$(TARGET)-kern.o: $(OBJS) arch_lib 
$(OBJ_DIR)/$(TARGET_ARCH_DIR)/minios-$(MINIOS_TARGET_ARCH).lds
$(LD) -r $(LDFLAGS) $(HEAD_OBJ) $(OBJS) $(LDARCHLIB) -o $@
+   $(OBJCOPY) --dump-section .export_symbol=$(OBJ_DIR)/syms $@
 
 $(OBJ_DIR)/$(TARGET): $(OBJ_DIR)/$(TARGET)-kern.o $(APP_O)
$(LD) -r $(LDFLAGS) $(OBJ_DIR)/$(TARGET)-kern.o $(APP_O) $(LDLIBS) -o 
$@.o
-   $(OBJCOPY) -w -G $(GLOBAL_PREFIX)* -G _start $@.o $@.o
+   $(OBJCOPY) -w -G $(GLOBAL_PREFIX)* -G _start 
--remove-section=.export_symbol $@.o $@.o
$(LD) $(LDFLAGS) $(LDFLAGS_FINAL) $@.o $(EXTRA_OBJS) -o $@-debug
strip -s $@-debug -o $@
gzip -n -f -9 -c $@-debug >$@-debug.gz
diff --git a/include/export.h b/include/export.h
new file mode 100644
index ..7140ac68
--- /dev/null
+++ b/include/export.h
@@ -0,0 +1,10 @@
+#ifndef _EXPORT_H_
+#define _EXPORT_H_
+
+/* Mark a symbol to be visible for apps and libs. */
+#define EXPORT_SYMBOL(sym)  \
+asm(".section .export_symbol\n" \
+".ascii \""#sym"\\n\"\n"\
+".previous\n")
+
+#endif /* _EXPORT_H_ */
diff --git a/include/lib.h b/include/lib.h
index dd68985a..abd4e9ab 100644
--- a/include/lib.h
+++ b/include/lib.h
@@ -53,6 +53,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "gntmap.h"
 
 #if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
-- 
2.35.3

[PATCH v3 04/32] Mini-OS: add EXPORT_SYMBOL() instances to hypervisor.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to hypervisor.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 hypervisor.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hypervisor.c b/hypervisor.c
index d3857e70..f2cbbc1c 100644
--- a/hypervisor.c
+++ b/hypervisor.c
@@ -31,6 +31,8 @@
 #include 
 #include 
 
+EXPORT_SYMBOL(hypercall_page);
+
 #define active_evtchns(cpu,sh,idx)  \
 ((sh)->evtchn_pending[idx] &\
  ~(sh)->evtchn_mask[idx])
@@ -157,6 +159,7 @@ inline void mask_evtchn(uint32_t port)
 shared_info_t *s = HYPERVISOR_shared_info;
 synch_set_bit(port, &s->evtchn_mask[0]);
 }
+EXPORT_SYMBOL(mask_evtchn);
 
 inline void unmask_evtchn(uint32_t port)
 {
@@ -180,9 +183,11 @@ inline void unmask_evtchn(uint32_t port)
 force_evtchn_callback();
 }
 }
+EXPORT_SYMBOL(unmask_evtchn);
 
 inline void clear_evtchn(uint32_t port)
 {
 shared_info_t *s = HYPERVISOR_shared_info;
 synch_clear_bit(port, &s->evtchn_pending[0]);
 }
+EXPORT_SYMBOL(clear_evtchn);
-- 
2.35.3

[PATCH v3 01/32] Mini-OS: link kernel separately

[Juergen Gross]

Add an additional link step with linking all Mini-OS kernel binaries
into a single object file.

This is done in preparation of hiding Mini-OS internal symbols before
linking the kernel with libraries and an application.

Signed-off-by: Juergen Gross 
Reviewed-by: Samuel Thibault 
---
 Makefile | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 7ee181a2..85c6db75 100644
--- a/Makefile
+++ b/Makefile
@@ -164,8 +164,11 @@ endif
 $(OBJ_DIR)/arch/x86/minios-x86%.lds:  arch/x86/minios-x86.lds.S
$(CPP) $(ASFLAGS) -P $< -o $@
 
-$(OBJ_DIR)/$(TARGET): $(OBJS) $(APP_O) arch_lib 
$(OBJ_DIR)/$(TARGET_ARCH_DIR)/minios-$(MINIOS_TARGET_ARCH).lds
-   $(LD) -r $(LDFLAGS) $(HEAD_OBJ) $(APP_O) $(OBJS) $(LDARCHLIB) $(LDLIBS) 
-o $@.o
+$(OBJ_DIR)/$(TARGET)-kern.o: $(OBJS) arch_lib 
$(OBJ_DIR)/$(TARGET_ARCH_DIR)/minios-$(MINIOS_TARGET_ARCH).lds
+   $(LD) -r $(LDFLAGS) $(HEAD_OBJ) $(OBJS) $(LDARCHLIB) -o $@
+
+$(OBJ_DIR)/$(TARGET): $(OBJ_DIR)/$(TARGET)-kern.o $(APP_O)
+   $(LD) -r $(LDFLAGS) $(OBJ_DIR)/$(TARGET)-kern.o $(APP_O) $(LDLIBS) -o 
$@.o
$(OBJCOPY) -w -G $(GLOBAL_PREFIX)* -G _start $@.o $@.o
$(LD) $(LDFLAGS) $(LDFLAGS_FINAL) $@.o $(EXTRA_OBJS) -o $@-debug
strip -s $@-debug -o $@
-- 
2.35.3

[PATCH v3 03/32] Mini-OS: add EXPORT_SYMBOL() instances to kernel.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to kernel.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 kernel.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/kernel.c b/kernel.c
index 1f97d8dd..0b444940 100644
--- a/kernel.c
+++ b/kernel.c
@@ -50,6 +50,9 @@
 #include 
 #include 
 
+void _start(void);
+EXPORT_SYMBOL(_start);
+
 uint8_t xen_features[XENFEAT_NR_SUBMAPS * 32];
 char cmdline[MAX_CMDLINE_SIZE];
 
@@ -181,6 +184,7 @@ void stop_kernel(void)
 /* Reset arch details */
 arch_fini();
 }
+EXPORT_SYMBOL(stop_kernel);
 
 /*
  * do_exit: This is called whenever an IRET fails in entry.S.
@@ -199,3 +203,4 @@ void do_exit(void)
 HYPERVISOR_sched_op(SCHEDOP_shutdown, &sched_shutdown);
 }
 }
+EXPORT_SYMBOL(do_exit);
-- 
2.35.3

[PATCH v3 05/32] Mini-OS: add EXPORT_SYMBOL() instances to sched.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to sched.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 arch/x86/sched.c | 5 +
 sched.c  | 7 +++
 2 files changed, 12 insertions(+)

diff --git a/arch/x86/sched.c b/arch/x86/sched.c
index e7b6954e..dabe6fd6 100644
--- a/arch/x86/sched.c
+++ b/arch/x86/sched.c
@@ -142,11 +142,13 @@ unsigned long __local_irq_save(void)
 local_irq_save(flags);
 return flags;
 }
+EXPORT_SYMBOL(__local_irq_save);
 
 void __local_irq_restore(unsigned long flags)
 {
 local_irq_restore(flags);
 }
+EXPORT_SYMBOL(__local_irq_restore);
 
 unsigned long __local_save_flags(void)
 {
@@ -155,13 +157,16 @@ unsigned long __local_save_flags(void)
 local_save_flags(flags);
 return flags;
 }
+EXPORT_SYMBOL(__local_save_flags);
 
 void __local_irq_disable(void)
 {
 local_irq_disable();
 }
+EXPORT_SYMBOL(__local_irq_disable);
 
 void __local_irq_enable(void)
 {
 local_irq_enable();
 }
+EXPORT_SYMBOL(__local_irq_enable);
diff --git a/sched.c b/sched.c
index 6f89ea4d..e162cb60 100644
--- a/sched.c
+++ b/sched.c
@@ -128,6 +128,7 @@ void schedule(void)
 }
 }
 }
+EXPORT_SYMBOL(schedule);
 
 struct thread* create_thread(char *name, void (*function)(void *), void *data)
 {
@@ -147,6 +148,7 @@ struct thread* create_thread(char *name, void 
(*function)(void *), void *data)
 local_irq_restore(flags);
 return thread;
 }
+EXPORT_SYMBOL(create_thread);
 
 #ifdef HAVE_LIBC
 static struct _reent callback_reent;
@@ -184,6 +186,7 @@ struct _reent *__getreent(void)
 #endif
 return _reent;
 }
+EXPORT_SYMBOL(__getreent);
 #endif
 
 void exit_thread(void)
@@ -205,12 +208,14 @@ void exit_thread(void)
 printk("schedule() returned!  Trying again\n");
 }
 }
+EXPORT_SYMBOL(exit_thread);
 
 void block(struct thread *thread)
 {
 thread->wakeup_time = 0LL;
 clear_runnable(thread);
 }
+EXPORT_SYMBOL(block);
 
 void msleep(uint32_t millisecs)
 {
@@ -219,12 +224,14 @@ void msleep(uint32_t millisecs)
 clear_runnable(thread);
 schedule();
 }
+EXPORT_SYMBOL(msleep);
 
 void wake(struct thread *thread)
 {
 thread->wakeup_time = 0LL;
 set_runnable(thread);
 }
+EXPORT_SYMBOL(wake);
 
 void idle_thread_fn(void *unused)
 {
-- 
2.35.3

[PATCH v3 20/32] Mini-OS: add EXPORT_SYMBOL() instances to lock.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to lock.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 lock.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/lock.c b/lock.c
index 61194e5d..8f58344f 100644
--- a/lock.c
+++ b/lock.c
@@ -17,6 +17,7 @@ int ___lock_init(_LOCK_T *lock)
 init_waitqueue_head(&lock->wait);
 return 0;
 }
+EXPORT_SYMBOL(___lock_init);
 
 int ___lock_acquire(_LOCK_T *lock)
 {
@@ -32,6 +33,7 @@ int ___lock_acquire(_LOCK_T *lock)
 local_irq_restore(flags);
 return 0;
 }
+EXPORT_SYMBOL(___lock_acquire);
 
 int ___lock_try_acquire(_LOCK_T *lock)
 {
@@ -45,6 +47,7 @@ int ___lock_try_acquire(_LOCK_T *lock)
 local_irq_restore(flags);
 return ret;
 }
+EXPORT_SYMBOL(___lock_try_acquire);
 
 int ___lock_release(_LOCK_T *lock)
 {
@@ -55,7 +58,7 @@ int ___lock_release(_LOCK_T *lock)
 local_irq_restore(flags);
 return 0;
 }
-
+EXPORT_SYMBOL(___lock_release);
 
 int ___lock_init_recursive(_LOCK_RECURSIVE_T *lock)
 {
@@ -63,6 +66,7 @@ int ___lock_init_recursive(_LOCK_RECURSIVE_T *lock)
 init_waitqueue_head(&lock->wait);
 return 0;
 }
+EXPORT_SYMBOL(___lock_init_recursive);
 
 int ___lock_acquire_recursive(_LOCK_RECURSIVE_T *lock)
 {
@@ -81,6 +85,7 @@ int ___lock_acquire_recursive(_LOCK_RECURSIVE_T *lock)
 lock->count++;
 return 0;
 }
+EXPORT_SYMBOL(___lock_acquire_recursive);
 
 int ___lock_try_acquire_recursive(_LOCK_RECURSIVE_T *lock)
 {
@@ -95,6 +100,7 @@ int ___lock_try_acquire_recursive(_LOCK_RECURSIVE_T *lock)
 local_irq_restore(flags);
 return ret;
 }
+EXPORT_SYMBOL(___lock_try_acquire_recursive);
 
 int ___lock_release_recursive(_LOCK_RECURSIVE_T *lock)
 {
@@ -108,5 +114,6 @@ int ___lock_release_recursive(_LOCK_RECURSIVE_T *lock)
 local_irq_restore(flags);
 return 0;
 }
+EXPORT_SYMBOL(___lock_release_recursive);
 
 #endif
-- 
2.35.3

[PATCH v3 19/32] Mini-OS: add EXPORT_SYMBOL() instances to ioremap.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to ioremap.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 arch/x86/ioremap.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/ioremap.c b/arch/x86/ioremap.c
index 4384b1c0..fda74d5d 100644
--- a/arch/x86/ioremap.c
+++ b/arch/x86/ioremap.c
@@ -51,11 +51,13 @@ void *ioremap(unsigned long phys_addr, unsigned long size)
 {
 return __do_ioremap(phys_addr, size, IO_PROT);
 }
+EXPORT_SYMBOL(ioremap);
 
 void *ioremap_nocache(unsigned long phys_addr, unsigned long size)
 {
 return __do_ioremap(phys_addr, size, IO_PROT_NOCACHE);
 }
+EXPORT_SYMBOL(ioremap_nocache);
 
 /* Un-map the io-remapped region. Currently no list of existing mappings is
  * maintained, so the caller has to supply the size */
@@ -69,7 +71,6 @@ void iounmap(void *virt_addr, unsigned long size)
 
 unmap_frames(va & PAGE_MASK, num_pages);
 }
-
-
+EXPORT_SYMBOL(iounmap);
 
 /* -*-  Mode:C; c-basic-offset:4; tab-width:4 indent-tabs-mode:nil -*- */
-- 
2.35.3

[PATCH v3 21/32] Mini-OS: add EXPORT_SYMBOL() instances to time.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to time.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 arch/arm/time.c | 1 +
 arch/x86/time.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/time.c b/arch/arm/time.c
index a088981e..3483511a 100644
--- a/arch/arm/time.c
+++ b/arch/arm/time.c
@@ -83,6 +83,7 @@ int gettimeofday(struct timeval *tv, void *tz)
 
 return 0;
 }
+EXPORT_SYMBOL(gettimeofday);
 
 /* Set the timer and mask. */
 void write_timer_ctl(uint32_t value) {
diff --git a/arch/x86/time.c b/arch/x86/time.c
index 8077c80c..332c0260 100644
--- a/arch/x86/time.c
+++ b/arch/x86/time.c
@@ -204,7 +204,7 @@ int gettimeofday(struct timeval *tv, void *tz)
 
 return 0;
 }
-
+EXPORT_SYMBOL(gettimeofday);
 
 void block_domain(s_time_t until)
 {
-- 
2.35.3

[PATCH v3 24/32] Mini-OS: add EXPORT_SYMBOL() instances to blkfront.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to blkfront.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 blkfront.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/blkfront.c b/blkfront.c
index 8f140896..b0fc80d1 100644
--- a/blkfront.c
+++ b/blkfront.c
@@ -23,9 +23,7 @@
 /* Note: we really suppose non-preemptive threads.  */
 
 DECLARE_WAIT_QUEUE_HEAD(blkfront_queue);
-
-
-
+EXPORT_SYMBOL(blkfront_queue);
 
 #define BLK_RING_SIZE __RING_SIZE((struct blkif_sring *)0, PAGE_SIZE)
 #define GRANT_INVALID_REF 0
@@ -243,6 +241,7 @@ error:
 free_blkfront(dev);
 return NULL;
 }
+EXPORT_SYMBOL(init_blkfront);
 
 void shutdown_blkfront(struct blkfront_dev *dev)
 {
@@ -304,6 +303,7 @@ close:
 if (!err)
 free_blkfront(dev);
 }
+EXPORT_SYMBOL(shutdown_blkfront);
 
 static void blkfront_wait_slot(struct blkfront_dev *dev)
 {
@@ -386,6 +386,7 @@ void blkfront_aio(struct blkfront_aiocb *aiocbp, int write)
 
 if(notify) notify_remote_via_evtchn(dev->evtchn);
 }
+EXPORT_SYMBOL(blkfront_aio);
 
 static void blkfront_aio_cb(struct blkfront_aiocb *aiocbp, int ret)
 {
@@ -417,6 +418,7 @@ void blkfront_io(struct blkfront_aiocb *aiocbp, int write)
 remove_waiter(w, blkfront_queue);
 local_irq_restore(flags);
 }
+EXPORT_SYMBOL(blkfront_io);
 
 static void blkfront_push_operation(struct blkfront_dev *dev, uint8_t op, 
uint64_t id)
 {
@@ -444,6 +446,7 @@ void blkfront_aio_push_operation(struct blkfront_aiocb 
*aiocbp, uint8_t op)
 struct blkfront_dev *dev = aiocbp->aio_dev;
 blkfront_push_operation(dev, op, (uintptr_t) aiocbp);
 }
+EXPORT_SYMBOL(blkfront_aio_push_operation);
 
 void blkfront_sync(struct blkfront_dev *dev)
 {
@@ -473,6 +476,7 @@ void blkfront_sync(struct blkfront_dev *dev)
 remove_waiter(w, blkfront_queue);
 local_irq_restore(flags);
 }
+EXPORT_SYMBOL(blkfront_sync);
 
 int blkfront_aio_poll(struct blkfront_dev *dev)
 {
@@ -557,6 +561,7 @@ moretodo:
 
 return nr_consumed;
 }
+EXPORT_SYMBOL(blkfront_aio_poll);
 
 #ifdef HAVE_LIBC
 static int blkfront_posix_rwop(struct file *file, uint8_t *buf, size_t count,
@@ -773,4 +778,5 @@ int blkfront_open(struct blkfront_dev *dev)
 
 return dev->fd;
 }
+EXPORT_SYMBOL(blkfront_open);
 #endif
-- 
2.35.3

[PATCH v3 30/32] Mini-OS: add EXPORT_SYMBOL() instances to tpm_tis.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to tpm_tis.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 tpm_tis.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/tpm_tis.c b/tpm_tis.c
index c3998f2d..ad95e289 100644
--- a/tpm_tis.c
+++ b/tpm_tis.c
@@ -673,6 +673,7 @@ int tpm_tis_request_locality(struct tpm_chip* tpm, int l) {
printk("REQ LOCALITY FAILURE\n");
return -1;
 }
+EXPORT_SYMBOL(tpm_tis_request_locality);
 
 static uint8_t tpm_tis_status(struct tpm_chip* tpm) {
return ioread8(TPM_STS(tpm, tpm->locality));
@@ -1264,6 +1265,7 @@ abort_egress:
}
return NULL;
 }
+EXPORT_SYMBOL(init_tpm_tis);
 
 int tpm_tis_cmd(struct tpm_chip* tpm, uint8_t* req, size_t reqlen, uint8_t** 
resp, size_t* resplen)
 {
@@ -1281,6 +1283,7 @@ int tpm_tis_cmd(struct tpm_chip* tpm, uint8_t* req, 
size_t reqlen, uint8_t** res
memcpy(*resp, tpm->data_buffer, *resplen);
return 0;
 }
+EXPORT_SYMBOL(tpm_tis_cmd);
 
 #ifdef HAVE_LIBC
 #include 
@@ -1392,6 +1395,7 @@ int tpm_tis_open(struct tpm_chip *tpm)
 
 return tpm->fd;
 }
+EXPORT_SYMBOL(tpm_tis_open);
 
 /* TPM 2.0 */
 
@@ -1550,4 +1554,5 @@ abort_egress:
 }
 return NULL;
 }
+EXPORT_SYMBOL(init_tpm2_tis);
 #endif
-- 
2.35.3

[PATCH v3 11/32] Mini-OS: add EXPORT_SYMBOL() instances to lib/ctype.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to lib/ctype.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 lib/ctype.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/ctype.c b/lib/ctype.c
index 3f3bdb00..309ebbc9 100644
--- a/lib/ctype.c
+++ b/lib/ctype.c
@@ -1,5 +1,6 @@
 #ifndef HAVE_LIBC
 #include 
+#include 
 
 unsigned char _ctype[] = {
 _C,_C,_C,_C,_C,_C,_C,_C,/* 0-7 */
@@ -26,4 +27,5 @@ _U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,   /* 
192-207 */
 _U,_U,_U,_U,_U,_U,_U,_P,_U,_U,_U,_U,_U,_U,_U,_L,   /* 208-223 */
 _L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,   /* 224-239 */
 _L,_L,_L,_L,_L,_L,_L,_P,_L,_L,_L,_L,_L,_L,_L,_L};  /* 240-255 */
+EXPORT_SYMBOL(_ctype);
 #endif
-- 
2.35.3

Re: We are not able to virtualize FreeBSD using xen 4.17 on Arm 32 bit

[Henry Wang]

+(xen-devel and Arm maintainers, including Julien)

> On Nov 27, 2023, at 18:03, Mario Marietto  wrote:
> 
> Hello.
> We have just virtualized Debian 12 on our arm (32 bit) Chromebook model 
> xe303c12 . As host / dom0 we have chosen Devuan 5,and for guest / domU,Debian 
> 12. It works great. But our goal is different. We want to virtualize FreeBSD 
> as domU. Can we have a working Xen PV network driver for a FreeBSD arm guest 
> ?. I found that Julien Grall has ported the Xen drivers to FreeBSD on arm. I 
> would like to know if Julien's work was accepted upstream by FreeBSD, in 
> which case FreeBSD as a Xen guest on arm should work if we enable the Xen PV 
> drivers in the FreeBSD on arm kernel. If Julien's work was not accepted 
> upstream by FreeBSD, we will have to find his patches and apply them 
> ourselves to the FreeBSD on arm kernel.
> We found these slides :
> 
> https://events.static.linuxfound.org/sites/events/files/slides/Porting%20FreeBSD%20on%20Xen%20on%20ARM%20.pdf
> 
> Slide 13 refers to a XENHVM FreeBSD on arm kernel config - that is what we 
> want to find.
> It looks like when that slide presentation was written, there were some 
> limitations on FreeBSD Xen guests. For example, for our debian bookworm 
> guest, I am using vcpus = '2' to match the number of real cpus on our 
> Chromebook, but slide 13 mentions support for only 1 VCPU with a FreeBSD 
> guest, so I will need to change that vcpus = '1' in the FreeBSD guest config 
> unless support for 2 or more vcpus was added later, which is possible because 
> that slide presentation is 9 years old.
> Here is where I would expect to find the XENHVM FreeBSD on arm kernel config 
> file:
> 
> https://cgit.freebsd.org/src/tree/sys/arm/conf
> 
> But it is not there unless I am not understanding something correctly. For 
> now, unfortunately conclude that the support for Xen on arm that Julien Grall 
> mentioned in that slide presentation 9 years ago was never added to the 
> official FreeBSD source code. I am searching the web now to see if the 
> patches that Julien Grall wrote are still posted somewhere online. If we 
> cannot find them, we can ask here and on the xen-users mailing list. Julien 
> regularly reads that list and responds to questions about Xen on arm, so I 
> think he will tell us how to find the patches if we cannot find them online.
> According to this page from the FreeBSD wiki:
> 
> https://wiki.freebsd.org/Xen
> 
> I think FreeBSD only supports Xen on x86, not arm. So this is going to be a 
> bit of a challenge to get a Xen FreeBSD guest on arm working. We know Julien 
> Grall has some patches that made it work in the past !
> I found a slightly newer slide presentation by Julien here:
> 
> https://www.slideshare.net/xen_com_mgr/bsdcan-2015-how-to-port-your-bsd
> 
> It is about the same, but it mentions the GENERIC FreeBSD kernel supports Xen 
> on arm64, but still says we need the XENHVM FreeBSD config for Xen on arm 32 
> bit, which I haven't found online yet.
> Please,take a look at this output of the linux kernel that can boot on Xen, 
> and the FreeBSD kernel that cannot :
> 
> % file zImage-6.1.59-stb-xen-cbe+
> zImage-6.1.59-stb-xen-cbe+: Linux kernel ARM boot executable zImage 
> (little-endian)
> 
> % file FREEBSD-XENVIRT 
> FREEBSD-XENVIRT: ELF 32-bit LSB executable, ARM, EABI5 version 1 (SYSV), 
> dynamically linked, interpreter /red/herring, for FreeBSD 11.0 (1100048), not 
> stripped
> 
> 
> The FreeBSD kernel that won't boot is in ELF format but the Linux kernel that 
> does boot is in zImage format. This is the error we get :
> # xl create freebsd.cfg 
> Parsing config from freebsd.cfg xc: error: panic: xg_dom_elfloader.c:63: 
> xc_dom_guest_type: image not capable of booting inside a HV
> M container: Invalid kernel 
> libxl: error: libxl_dom.c:571:libxl__build_dom: xc_dom_parse_image failed 
> libxl: error: libxl_create.c:1640:domcreate_rebuild_done: Domain 4:cannot 
> (re-)build domain: -3 
> libxl: error: libxl_domain.c:1183:libxl__destroy_domid: Domain 4:Non-existant 
> domain 
> libxl: error: libxl_domain.c:1137:domain_destroy_callback: Domain 4:Unable to 
> destroy guest 
> libxl: error: libxl_domain.c:1064:domain_destroy_cb: Domain 4:Destruction of 
> domain failed
> 
> We are spending time reading the docs on xenbits.xenproject.org, and 
> according to those docs Xen on arm only knows how to boot a kernel in the 
> zImage format, so the FreeBSD kernel is in a format that modern Xen 
> incorrectly detects as an x86 kernel.
> I also watched Julien Grall's 30 minute video presentation of his work to 
> boot FreeBSD/arm on Xen at FOSDEM 2014 here :
> 
> https://archive.fosdem.org/2014/schedule/event/freebsd_xen_arm/
> 
> In that video, and in other places, Julien mentions that the boot ABI for 
> FreeBSD/arm on Xen was not yet developed and he was getting occasional 
> crashes and needed to investigate the problem. He mentioned the zImage ABI 
> that Linux uses, but pointed out FreeBSD does not use

[PATCH v3 13/32] Mini-OS: add EXPORT_SYMBOL() instances to lib/xs.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to lib/xs.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 lib/xs.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/lib/xs.c b/lib/xs.c
index 7fe9b03e..ff0768c1 100644
--- a/lib/xs.c
+++ b/lib/xs.c
@@ -64,6 +64,7 @@ struct xs_handle *xs_daemon_open()
 printk("xs_daemon_open -> %d, %p\n", fd, &file->dev);
 return (void*)(intptr_t) fd;
 }
+EXPORT_SYMBOL(xs_daemon_open);
 
 void xs_daemon_close(struct xs_handle *h)
 {
@@ -74,6 +75,7 @@ int xs_fileno(struct xs_handle *h)
 {
 return _xs_fileno(h);
 }
+EXPORT_SYMBOL(xs_fileno);
 
 void *xs_read(struct xs_handle *h, xs_transaction_t t,
 const char *path, unsigned int *len)
@@ -92,6 +94,7 @@ void *xs_read(struct xs_handle *h, xs_transaction_t t,
*len = strlen(value);
 return value;
 }
+EXPORT_SYMBOL(xs_read);
 
 bool xs_write(struct xs_handle *h, xs_transaction_t t,
  const char *path, const void *data, unsigned int len)
@@ -110,6 +113,7 @@ bool xs_write(struct xs_handle *h, xs_transaction_t t,
 }
 return true;
 }
+EXPORT_SYMBOL(xs_write);
 
 static bool xs_bool(char *reply)
 {
@@ -123,6 +127,7 @@ bool xs_rm(struct xs_handle *h, xs_transaction_t t, const 
char *path)
 {
 return xs_bool(xenbus_rm(t, path));
 }
+EXPORT_SYMBOL(xs_rm);
 
 static void *xs_talkv(struct xs_handle *h, xs_transaction_t t,
enum xsd_sockmsg_type type,
@@ -163,6 +168,7 @@ char *xs_get_domain_path(struct xs_handle *h, unsigned int 
domid)
 
 return xs_single(h, XBT_NULL, XS_GET_DOMAIN_PATH, domid_str, NULL);
 }
+EXPORT_SYMBOL(xs_get_domain_path);
 
 char **xs_directory(struct xs_handle *h, xs_transaction_t t,
const char *path, unsigned int *num)
@@ -199,6 +205,7 @@ char **xs_directory(struct xs_handle *h, xs_transaction_t t,
 free(res);
 return entries;
 }
+EXPORT_SYMBOL(xs_directory);
 
 bool xs_watch(struct xs_handle *h, const char *path, const char *token)
 {
@@ -208,6 +215,7 @@ bool xs_watch(struct xs_handle *h, const char *path, const 
char *token)
 return xs_bool(xenbus_watch_path_token(XBT_NULL, path, token,
(xenbus_event_queue *)&file->dev));
 }
+EXPORT_SYMBOL(xs_watch);
 
 char **xs_read_watch(struct xs_handle *h, unsigned int *num)
 {
@@ -220,10 +228,12 @@ char **xs_read_watch(struct xs_handle *h, unsigned int 
*num)
 *num = 2;
 return (char **) &event->path;
 }
+EXPORT_SYMBOL(xs_read_watch);
 
 bool xs_unwatch(struct xs_handle *h, const char *path, const char *token)
 {
 printk("xs_unwatch(%s, %s)\n", path, token);
 return xs_bool(xenbus_unwatch_path_token(XBT_NULL, path, token));
 }
+EXPORT_SYMBOL(xs_unwatch);
 #endif
-- 
2.35.3

[PATCH v3 25/32] Mini-OS: add EXPORT_SYMBOL() instances to fbfront.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to fbfront.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 fbfront.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/fbfront.c b/fbfront.c
index 1b67f527..d4e28d0c 100644
--- a/fbfront.c
+++ b/fbfront.c
@@ -201,6 +201,7 @@ error:
 free_kbdfront(dev);
 return NULL;
 }
+EXPORT_SYMBOL(init_kbdfront);
 
 int kbdfront_receive(struct kbdfront_dev *dev, union xenkbd_in_event *buf, int 
n)
 {
@@ -239,7 +240,7 @@ int kbdfront_receive(struct kbdfront_dev *dev, union 
xenkbd_in_event *buf, int n
 
 return i;
 }
-
+EXPORT_SYMBOL(kbdfront_receive);
 
 void shutdown_kbdfront(struct kbdfront_dev *dev)
 {
@@ -301,6 +302,7 @@ close_kbdfront:
 if (!err)
 free_kbdfront(dev);
 }
+EXPORT_SYMBOL(shutdown_kbdfront);
 
 #ifdef HAVE_LIBC
 static int kbd_read(struct file *file, void *buf, size_t nbytes)
@@ -351,6 +353,7 @@ int kbdfront_open(struct kbdfront_dev *dev)
 
 return dev->fd;
 }
+EXPORT_SYMBOL(kbdfront_open);
 #endif
 
 
@@ -451,6 +454,7 @@ int fbfront_receive(struct fbfront_dev *dev, union 
xenfb_in_event *buf, int n)
 
 return i;
 }
+EXPORT_SYMBOL(fbfront_receive);
 
 struct fbfront_dev *init_fbfront(char *_nodename, unsigned long *mfns, int 
width, int height, int depth, int stride, int n)
 {
@@ -617,6 +621,7 @@ error:
 free_fbfront(dev);
 return NULL;
 }
+EXPORT_SYMBOL(init_fbfront);
 
 static void fbfront_out_event(struct fbfront_dev *dev, union xenfb_out_event 
*event)
 {
@@ -668,6 +673,7 @@ void fbfront_update(struct fbfront_dev *dev, int x, int y, 
int width, int height
 update.height = height;
 fbfront_out_event(dev, (union xenfb_out_event *) &update);
 }
+EXPORT_SYMBOL(fbfront_update);
 
 void fbfront_resize(struct fbfront_dev *dev, int width, int height, int 
stride, int depth, int offset)
 {
@@ -681,6 +687,7 @@ void fbfront_resize(struct fbfront_dev *dev, int width, int 
height, int stride,
 dev->offset = resize.offset = offset;
 fbfront_out_event(dev, (union xenfb_out_event *) &resize);
 }
+EXPORT_SYMBOL(fbfront_resize);
 
 void shutdown_fbfront(struct fbfront_dev *dev)
 {
@@ -745,6 +752,7 @@ close_fbfront:
 if (!err)
 free_fbfront(dev);
 }
+EXPORT_SYMBOL(shutdown_fbfront);
 
 #ifdef HAVE_LIBC
 static int fbfront_read(struct file *file, void *buf, size_t nbytes)
@@ -794,5 +802,6 @@ int fbfront_open(struct fbfront_dev *dev)
 file->dev = dev;
 return dev->fd;
 }
+EXPORT_SYMBOL(fbfront_open);
 #endif
 
-- 
2.35.3

[PATCH v3 07/32] Mini-OS: add EXPORT_SYMBOL() instances to lib/sys.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to lib/sys.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 lib/sys.c | 92 ---
 1 file changed, 87 insertions(+), 5 deletions(-)

diff --git a/lib/sys.c b/lib/sys.c
index 118fc441..481067f7 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -20,6 +20,7 @@
 
 #ifdef HAVE_LIBC
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -63,7 +64,8 @@
 { \
print_unsupported(#function); \
do_exit(); \
-}
+} \
+EXPORT_SYMBOL(function)
 
 /* Log and err out on function call */
 #define unsupported_function_log(type, function, ret) \
@@ -73,7 +75,8 @@
print_unsupported(#function); \
errno = ENOSYS; \
return ret; \
-}
+} \
+EXPORT_SYMBOL(function)
 
 /* Err out on function call */
 #define unsupported_function(type, function, ret) \
@@ -82,7 +85,8 @@
 { \
errno = ENOSYS; \
return ret; \
-}
+} \
+EXPORT_SYMBOL(function)
 
 #define NOFILE 32
 #define N_MOUNTS  16
@@ -187,6 +191,7 @@ unsigned int alloc_file_type(const struct file_ops *ops)
 
 return ret;
 }
+EXPORT_SYMBOL(alloc_file_type);
 
 static const struct file_ops *get_file_ops(unsigned int type)
 {
@@ -203,8 +208,10 @@ struct file *get_file_from_fd(int fd)
 
 return (files[fd].type == FTYPE_NONE) ? NULL : files + fd;
 }
+EXPORT_SYMBOL(get_file_from_fd);
 
 DECLARE_WAIT_QUEUE_HEAD(event_queue);
+EXPORT_SYMBOL(event_queue);
 
 int alloc_fd(unsigned int type)
 {
@@ -222,6 +229,7 @@ int alloc_fd(unsigned int type)
 printk("Too many opened files\n");
 do_exit();
 }
+EXPORT_SYMBOL(alloc_fd);
 
 void close_all_files(void)
 {
@@ -232,6 +240,7 @@ void close_all_files(void)
 close(i);
 pthread_mutex_unlock(&fd_lock);
 }
+EXPORT_SYMBOL(close_all_files);
 
 int dup2(int oldfd, int newfd)
 {
@@ -243,33 +252,39 @@ int dup2(int oldfd, int newfd)
 pthread_mutex_unlock(&fd_lock);
 return 0;
 }
+EXPORT_SYMBOL(dup2);
 
 pid_t getpid(void)
 {
 return 1;
 }
+EXPORT_SYMBOL(getpid);
 
 pid_t getppid(void)
 {
 return 1;
 }
+EXPORT_SYMBOL(getppid);
 
 pid_t setsid(void)
 {
 return 1;
 }
+EXPORT_SYMBOL(setsid);
 
 char *getcwd(char *buf, size_t size)
 {
 snprintf(buf, size, "/");
 return buf;
 }
+EXPORT_SYMBOL(getcwd);
 
 int mkdir(const char *pathname, mode_t mode)
 {
 errno = EIO;
 return -1;
 }
+EXPORT_SYMBOL(mkdir);
 
 #ifdef CONFIG_CONSFRONT
 int posix_openpt(int flags)
@@ -282,6 +297,7 @@ int posix_openpt(int flags)
 
 return fd;
 }
+EXPORT_SYMBOL(posix_openpt);
 
 static int open_pt(struct mount_point *mnt, const char *pathname, int flags,
mode_t mode)
@@ -313,6 +329,8 @@ int posix_openpt(int flags)
errno = EIO;
return -1;
 }
+EXPORT_SYMBOL(posix_openpt);
+
 int open_savefile(const char *path, int save)
 {
errno = EIO;
@@ -378,6 +396,8 @@ int open(const char *pathname, int flags, ...)
 errno = EIO;
 return -1;
 }
+EXPORT_SYMBOL(open);
+EXPORT_SYMBOL(open64);
 
 int mount(const char *path, void *dev,
   int (*open)(struct mount_point *, const char *, int, mode_t))
@@ -422,6 +442,7 @@ int isatty(int fd)
 {
 return files[fd].type == FTYPE_CONSOLE;
 }
+EXPORT_SYMBOL(isatty);
 
 int read(int fd, void *buf, size_t nbytes)
 {
@@ -440,6 +461,7 @@ int read(int fd, void *buf, size_t nbytes)
 errno = EBADF;
 return -1;
 }
+EXPORT_SYMBOL(read);
 
 int write(int fd, const void *buf, size_t nbytes)
 {
@@ -458,6 +480,7 @@ int write(int fd, const void *buf, size_t nbytes)
 errno = EBADF;
 return -1;
 }
+EXPORT_SYMBOL(write);
 
 off_t lseek_default(struct file *file, off_t offset, int whence)
 {
@@ -510,11 +533,14 @@ off_t lseek(int fd, off_t offset, int whence)
 errno = ESPIPE;
 return (off_t) -1;
 }
+EXPORT_SYMBOL(lseek);
+EXPORT_SYMBOL(lseek64);
 
 int fsync(int fd) {
 errno = EBADF;
 return -1;
 }
+EXPORT_SYMBOL(fsync);
 
 int close(int fd)
 {
@@ -542,6 +568,7 @@ int close(int fd)
 errno = EBADF;
 return -1;
 }
+EXPORT_SYMBOL(close);
 
 static void init_stat(struct stat *buf)
 {
@@ -559,6 +586,7 @@ int stat(const char *path, struct stat *buf)
 errno = EIO;
 return -1;
 }
+EXPORT_SYMBOL(stat);
 
 int fstat(int fd, struct stat *buf)
 {
@@ -579,28 +607,34 @@ int fstat(int fd, struct stat *buf)
 errno = EBADF;
 return -1;
 }
+EXPORT_SYMBOL(fstat);
+EXPORT_SYMBOL(fstat64);
 
 int ftruncate(int fd, off_t length)
 {
 errno = EBADF;
 return -1;
 }
+EXPORT_SYMBOL(ftruncate);
 
 int remove(const char *pathname)
 {
 errno = EIO;
 return -1;
 }
+EXPORT_SYMBOL(remove);
 
 int unlink(const char *pathname)
 {
 return remove(pathname);
 }
+EXPORT_SYMBOL(unlink);
 
 int rmdir(const char *pathname)
 {
 return remove(pathname);
 }
+EXPORT_SYMBOL(rmdir);
 
 int fcntl(int fd, int cmd, ...)
 {
@@ -635,6 +669,7 @@ int fcntl(int fd, int cmd, ...)
 errno = ENOSYS;
 return -1;
 }
+EXPORT_SYMBOL(fcntl);
 

[PATCH v3 12/32] Mini-OS: add EXPORT_SYMBOL() instances to lib/xmalloc.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to lib/xmalloc.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 lib/xmalloc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/xmalloc.c b/lib/xmalloc.c
index e16f161a..cc5db11e 100644
--- a/lib/xmalloc.c
+++ b/lib/xmalloc.c
@@ -271,6 +271,7 @@ void *malloc(size_t size)
 {
 return _xmalloc(size, DEFAULT_ALIGN);
 }
+EXPORT_SYMBOL(malloc);
 
 void *realloc(void *ptr, size_t size)
 {
@@ -301,11 +302,13 @@ void *realloc(void *ptr, size_t size)
 
 return new;
 }
+EXPORT_SYMBOL(realloc);
 
 void free(void *ptr)
 {
 xfree(ptr);
 }
+EXPORT_SYMBOL(free);
 #endif
 
 /*
-- 
2.35.3

[PATCH v3 14/32] Mini-OS: add EXPORT_SYMBOL() instances to main.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to main.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/main.c b/main.c
index a93bc9d6..c5c1c420 100644
--- a/main.c
+++ b/main.c
@@ -38,10 +38,12 @@ int main(int argc, char *argv[], char *envp[])
 void _init(void)
 {
 }
+EXPORT_SYMBOL(_init);
 
 void _fini(void)
 {
 }
+EXPORT_SYMBOL(_fini);
 
 extern char __app_bss_start, __app_bss_end;
 static void call_main(void *p)
@@ -189,6 +191,7 @@ void _exit(int ret)
 }
 do_exit();
 }
+EXPORT_SYMBOL(_exit);
 
 int app_main(void *p)
 {
-- 
2.35.3

Re: [PATCH v2] xen: remove

[Oleksii]

Hello Shawn,

Could you kindly review the patch when you have a moment?
It can help with merging other patch series.

Thanks in advance.

~ Oleksii

On Tue, 2023-10-31 at 16:28 +0200, Oleksii Kurochko wrote:
>  only declares udelay() function so udelay()
> declaration was moved to xen/delay.h.
> 
> For x86, __udelay() was renamed to udelay() and removed
> inclusion of  in x86 code.
> 
> For ppc, udelay() stub definition was moved to ppc/stubs.c.
> 
> Suggested-by: Jan Beulich 
> Signed-off-by: Oleksii Kurochko 
> Reviewed-by: Jan Beulich 
> ---
> Changes in v2:
>  - rebase on top of the latest staging.
>  - add Suggested-by:/Reviewed-by: Jan Beulich .
>  - remove  for PPC.
>  - remove changes related to RISC-V's  as they've not
>    introduced in staging branch yet.
> ---
>  xen/arch/arm/include/asm/delay.h  | 14 --
>  xen/arch/ppc/include/asm/delay.h  | 12 
>  xen/arch/ppc/stubs.c  |  7 +++
>  xen/arch/x86/cpu/microcode/core.c |  2 +-
>  xen/arch/x86/delay.c  |  2 +-
>  xen/arch/x86/include/asm/delay.h  | 13 -
>  xen/include/xen/delay.h   |  2 +-
>  7 files changed, 10 insertions(+), 42 deletions(-)
>  delete mode 100644 xen/arch/arm/include/asm/delay.h
>  delete mode 100644 xen/arch/ppc/include/asm/delay.h
>  delete mode 100644 xen/arch/x86/include/asm/delay.h
> 
> diff --git a/xen/arch/arm/include/asm/delay.h
> b/xen/arch/arm/include/asm/delay.h
> deleted file mode 100644
> index 042907d9d5..00
> --- a/xen/arch/arm/include/asm/delay.h
> +++ /dev/null
> @@ -1,14 +0,0 @@
> -#ifndef _ARM_DELAY_H
> -#define _ARM_DELAY_H
> -
> -extern void udelay(unsigned long usecs);
> -
> -#endif /* defined(_ARM_DELAY_H) */
> -/*
> - * Local variables:
> - * mode: C
> - * c-file-style: "BSD"
> - * c-basic-offset: 4
> - * indent-tabs-mode: nil
> - * End:
> - */
> diff --git a/xen/arch/ppc/include/asm/delay.h
> b/xen/arch/ppc/include/asm/delay.h
> deleted file mode 100644
> index da6635888b..00
> --- a/xen/arch/ppc/include/asm/delay.h
> +++ /dev/null
> @@ -1,12 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -#ifndef __ASM_PPC_DELAY_H__
> -#define __ASM_PPC_DELAY_H__
> -
> -#include 
> -
> -static inline void udelay(unsigned long usecs)
> -{
> -    BUG_ON("unimplemented");
> -}
> -
> -#endif /* __ASM_PPC_DELAY_H__ */
> diff --git a/xen/arch/ppc/stubs.c b/xen/arch/ppc/stubs.c
> index 4c276b0e39..a96e45626d 100644
> --- a/xen/arch/ppc/stubs.c
> +++ b/xen/arch/ppc/stubs.c
> @@ -337,3 +337,10 @@ int __init parse_arch_dom0_param(const char *s,
> const char *e)
>  {
>  BUG_ON("unimplemented");
>  }
> +
> +/* delay.c */
> +
> +void udelay(unsigned long usecs)
> +{
> +    BUG_ON("unimplemented");
> +}
> diff --git a/xen/arch/x86/cpu/microcode/core.c
> b/xen/arch/x86/cpu/microcode/core.c
> index 65ebeb50de..22d5e04552 100644
> --- a/xen/arch/x86/cpu/microcode/core.c
> +++ b/xen/arch/x86/cpu/microcode/core.c
> @@ -23,6 +23,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -35,7 +36,6 @@
>  
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/xen/arch/x86/delay.c b/xen/arch/x86/delay.c
> index 2662c26272..b3a41881a1 100644
> --- a/xen/arch/x86/delay.c
> +++ b/xen/arch/x86/delay.c
> @@ -15,7 +15,7 @@
>  #include 
>  #include 
>  
> -void __udelay(unsigned long usecs)
> +void udelay(unsigned long usecs)
>  {
>  unsigned long ticks = usecs * (cpu_khz / 1000);
>  unsigned long s, e;
> diff --git a/xen/arch/x86/include/asm/delay.h
> b/xen/arch/x86/include/asm/delay.h
> deleted file mode 100644
> index 9be2f46590..00
> --- a/xen/arch/x86/include/asm/delay.h
> +++ /dev/null
> @@ -1,13 +0,0 @@
> -#ifndef _X86_DELAY_H
> -#define _X86_DELAY_H
> -
> -/*
> - * Copyright (C) 1993 Linus Torvalds
> - *
> - * Delay routines calling functions in arch/i386/lib/delay.c
> - */
> -
> -extern void __udelay(unsigned long usecs);
> -#define udelay(n) __udelay(n)
> -
> -#endif /* defined(_X86_DELAY_H) */
> diff --git a/xen/include/xen/delay.h b/xen/include/xen/delay.h
> index 9150226271..8fd3b8f99f 100644
> --- a/xen/include/xen/delay.h
> +++ b/xen/include/xen/delay.h
> @@ -3,7 +3,7 @@
>  
>  /* Copyright (C) 1993 Linus Torvalds */
>  
> -#include 
> +void udelay(unsigned long usecs);
>  
>  static inline void mdelay(unsigned long msec)
>  {

[PATCH v3 06/32] Mini-OS: add EXPORT_SYMBOL() instances to mm.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to mm.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 arch/arm/mm.c | 2 ++
 arch/x86/mm.c | 5 +
 mm.c  | 4 
 3 files changed, 11 insertions(+)

diff --git a/arch/arm/mm.c b/arch/arm/mm.c
index 11962f8b..c269ab14 100644
--- a/arch/arm/mm.c
+++ b/arch/arm/mm.c
@@ -85,6 +85,7 @@ int do_map_frames(unsigned long addr,
 {
 return -ENOSYS;
 }
+EXPORT_SYMBOL(do_map_frames);
 
 /* Get Xen's suggested physical page assignments for the grant table. */
 static paddr_t get_gnttab_base(void)
@@ -149,3 +150,4 @@ unsigned long map_frame_virt(unsigned long mfn)
 {
 return mfn_to_virt(mfn);
 }
+EXPORT_SYMBOL(map_frame_virt);
diff --git a/arch/x86/mm.c b/arch/x86/mm.c
index cfc978f6..767a5f07 100644
--- a/arch/x86/mm.c
+++ b/arch/x86/mm.c
@@ -493,6 +493,7 @@ pgentry_t *need_pgt(unsigned long va)
 offset = l1_table_offset(va);
 return &tab[offset];
 }
+EXPORT_SYMBOL(need_pgt);
 
 /*
  * Reserve an area of virtual address space for mappings and Heap
@@ -634,6 +635,7 @@ int do_map_frames(unsigned long va,
 
 return 0;
 }
+EXPORT_SYMBOL(do_map_frames);
 
 /*
  * Map an array of MFNs contiguous into virtual address space. Virtual
@@ -654,6 +656,7 @@ void *map_frames_ex(const unsigned long *mfns, unsigned 
long n,
 
 return (void *)va;
 }
+EXPORT_SYMBOL(map_frames_ex);
 
 /*
  * Unmap nun_frames frames mapped at virtual address va.
@@ -724,6 +727,7 @@ int unmap_frames(unsigned long va, unsigned long num_frames)
 }
 return 0;
 }
+EXPORT_SYMBOL(unmap_frames);
 
 /*
  * Clear some of the bootstrap memory
@@ -859,3 +863,4 @@ unsigned long map_frame_virt(unsigned long mfn)
 
 return addr;
 }
+EXPORT_SYMBOL(map_frame_virt);
diff --git a/mm.c b/mm.c
index 6493bdd4..eb0e34de 100644
--- a/mm.c
+++ b/mm.c
@@ -280,6 +280,7 @@ unsigned long alloc_pages(int order)
 
 return 0;
 }
+EXPORT_SYMBOL(alloc_pages);
 
 void free_pages(void *pointer, int order)
 {
@@ -336,6 +337,7 @@ void free_pages(void *pointer, int order)
 free_head[order] = freed_ch;   

 }
+EXPORT_SYMBOL(free_pages);
 
 int free_physical_pages(xen_pfn_t *mfns, int n)
 {
@@ -352,6 +354,7 @@ int map_frame_rw(unsigned long addr, unsigned long mfn)
 {
 return do_map_frames(addr, &mfn, 1, 1, 1, DOMID_SELF, NULL, L1_PROT);
 }
+EXPORT_SYMBOL(map_frame_rw);
 
 #ifdef HAVE_LIBC
 void *sbrk(ptrdiff_t increment)
@@ -385,6 +388,7 @@ void *sbrk(ptrdiff_t increment)
 
 return (void *) old_brk;
 }
+EXPORT_SYMBOL(sbrk);
 #endif
 
 
-- 
2.35.3

[PATCH v3 09/32] Mini-OS: add EXPORT_SYMBOL() instances to lib/math.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to lib/math.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 lib/math.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/lib/math.c b/lib/math.c
index b98cc1d2..9ee9fcdb 100644
--- a/lib/math.c
+++ b/lib/math.c
@@ -338,6 +338,7 @@ __qdivrem(u_quad_t uq, u_quad_t vq, u_quad_t *arq)
tmp.ul[L] = COMBINE(qspace[3], qspace[4]);
return (tmp.q);
 }
+EXPORT_SYMBOL(__qdivrem);
 
 /*
  * From
@@ -365,6 +366,7 @@ __divdi3(quad_t a, quad_t b)
uq = __qdivrem(ua, ub, (u_quad_t *)0);
return (neg ? -uq : uq);
 }
+EXPORT_SYMBOL(__divdi3);
 
 /*
  * From
@@ -379,6 +381,7 @@ __udivdi3(u_quad_t a, u_quad_t b)
 {
return (__qdivrem(a, b, (u_quad_t *)0));
 }
+EXPORT_SYMBOL(__udivdi3);
 
 /*
  * From
@@ -396,6 +399,7 @@ __umoddi3(u_quad_t a, u_quad_t b)
(void)__qdivrem(a, b, &r);
return (r);
 }
+EXPORT_SYMBOL(__umoddi3);
 
 /*
  * Returns the quotient and places remainder in r
@@ -405,6 +409,7 @@ __udivmoddi4(u_quad_t a, u_quad_t b, u_quad_t *r)
 {
return __qdivrem(a, b, r);
 }
+EXPORT_SYMBOL(__udivmoddi4);
 
 /*
  * From
@@ -434,3 +439,4 @@ __moddi3(quad_t a, quad_t b)
(void)__qdivrem(ua, ub, &ur);
return (neg ? -ur : ur);
 }
+EXPORT_SYMBOL(__moddi3);
-- 
2.35.3

[PATCH v3 26/32] Mini-OS: add EXPORT_SYMBOL() instances to netfront.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to netfront.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 netfront.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/netfront.c b/netfront.c
index 164fdc66..7de2498a 100644
--- a/netfront.c
+++ b/netfront.c
@@ -370,16 +370,19 @@ out:
 err:
 return dev;
 }
+EXPORT_SYMBOL(init_netfront);
 
 char *netfront_get_netmask(struct netfront_dev *dev)
 {
 return dev->mask ? strdup(dev->mask) : NULL;
 }
+EXPORT_SYMBOL(netfront_get_netmask);
 
 char *netfront_get_gateway(struct netfront_dev *dev)
 {
 return dev->gw ? strdup(dev->gw) : NULL;
 }
+EXPORT_SYMBOL(netfront_get_gateway);
 
 static struct netfront_dev *_init_netfront(struct netfront_dev *dev)
 {
@@ -625,6 +628,7 @@ int netfront_tap_open(char *nodename) {
 file->dev = dev;
 return dev->fd;
 }
+EXPORT_SYMBOL(netfront_tap_open);
 #endif
 
 void shutdown_netfront(struct netfront_dev *dev)
@@ -654,6 +658,7 @@ void shutdown_netfront(struct netfront_dev *dev)
 free_netfront(dev);
 }
 }
+EXPORT_SYMBOL(shutdown_netfront);
 
 static int _shutdown_netfront(struct netfront_dev *dev)
 {
@@ -813,6 +818,7 @@ void netfront_xmit(struct netfront_dev *dev, const unsigned 
char *data, int len)
 network_tx_buf_gc(dev);
 local_irq_restore(flags);
 }
+EXPORT_SYMBOL(netfront_xmit);
 
 #ifdef HAVE_LIBC
 ssize_t netfront_receive(struct netfront_dev *dev, unsigned char *data, size_t 
len)
@@ -840,6 +846,7 @@ ssize_t netfront_receive(struct netfront_dev *dev, unsigned 
char *data, size_t l
 
 return dev->rlen;
 }
+EXPORT_SYMBOL(netfront_receive);
 #endif
 
 void netfront_set_rx_handler(struct netfront_dev *dev,
-- 
2.35.3

[PATCH v3 18/32] Mini-OS: add EXPORT_SYMBOL() instances to gnttab.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to gnttab.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 gnttab.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/gnttab.c b/gnttab.c
index 6978a9bc..8168ed5d 100644
--- a/gnttab.c
+++ b/gnttab.c
@@ -80,6 +80,7 @@ gnttab_grant_access(domid_t domid, unsigned long frame, int 
readonly)
 
 return ref;
 }
+EXPORT_SYMBOL(gnttab_grant_access);
 
 grant_ref_t
 gnttab_grant_transfer(domid_t domid, unsigned long pfn)
@@ -94,6 +95,7 @@ gnttab_grant_transfer(domid_t domid, unsigned long pfn)
 
 return ref;
 }
+EXPORT_SYMBOL(gnttab_grant_transfer);
 
 int
 gnttab_end_access(grant_ref_t ref)
@@ -114,6 +116,7 @@ gnttab_end_access(grant_ref_t ref)
 put_free_entry(ref);
 return 1;
 }
+EXPORT_SYMBOL(gnttab_end_access);
 
 unsigned long
 gnttab_end_transfer(grant_ref_t ref)
@@ -144,6 +147,7 @@ gnttab_end_transfer(grant_ref_t ref)
 
 return frame;
 }
+EXPORT_SYMBOL(gnttab_end_transfer);
 
 grant_ref_t
 gnttab_alloc_and_grant(void **map)
@@ -156,6 +160,7 @@ gnttab_alloc_and_grant(void **map)
 gref = gnttab_grant_access(0, mfn, 0);
 return gref;
 }
+EXPORT_SYMBOL(gnttab_alloc_and_grant);
 
 static const char * const gnttabop_error_msgs[] = GNTTABOP_error_msgs;
 
@@ -168,6 +173,7 @@ gnttabop_error(int16_t status)
 else
 return gnttabop_error_msgs[status];
 }
+EXPORT_SYMBOL(gnttabop_error);
 
 void
 init_gnttab(void)
-- 
2.35.3

[PATCH v3 15/32] Mini-OS: add EXPORT_SYMBOL() instances to console.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to console.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 console.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/console.c b/console.c
index 5d205c7d..0107b685 100644
--- a/console.c
+++ b/console.c
@@ -125,6 +125,7 @@ void console_print(struct consfront_dev *dev, const char 
*data, int length)
 
 ring_send_fn(dev, copied_ptr, length);
 }
+EXPORT_SYMBOL(console_print);
 
 void print(int direct, const char *fmt, va_list args)
 {
@@ -155,6 +156,7 @@ void printk(const char *fmt, ...)
 print(0, fmt, args);
 va_end(args);
 }
+EXPORT_SYMBOL(printk);
 
 void xprintk(const char *fmt, ...)
 {
@@ -164,6 +166,8 @@ void xprintk(const char *fmt, ...)
 print(1, fmt, args);
 va_end(args);
 }
+EXPORT_SYMBOL(xprintk);
+
 void init_console(void)
 {
 printk("Initialising console ... ");
@@ -320,6 +324,7 @@ int xencons_ring_avail(struct consfront_dev *dev)
 
 return prod - cons;
 }
+EXPORT_SYMBOL(xencons_ring_avail);
 
 int xencons_ring_recv(struct consfront_dev *dev, char *data, unsigned int len)
 {
-- 
2.35.3

[PATCH v3 10/32] Mini-OS: add EXPORT_SYMBOL() instances to lib/printf.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to lib/printf.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 lib/printf.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/lib/printf.c b/lib/printf.c
index f9e9d682..0e054957 100644
--- a/lib/printf.c
+++ b/lib/printf.c
@@ -150,6 +150,7 @@ strtoul(const char *nptr, char **endptr, int base)
 *endptr = __DECONST(char *, any ? s - 1 : nptr);
 return (acc);
 }
+EXPORT_SYMBOL(strtoul);
 
 /*
  * Convert a string to a quad integer.
@@ -241,6 +242,7 @@ strtoq(const char *nptr, char **endptr, int base)
 *endptr = __DECONST(char *, any ? s - 1 : nptr);
 return (acc);
 }
+EXPORT_SYMBOL(strtoq);
 
 /*
  * Convert a string to an unsigned quad integer.
@@ -309,6 +311,7 @@ strtouq(const char *nptr, char **endptr, int base)
 *endptr = __DECONST(char *, any ? s - 1 : nptr);
 return (acc);
 }
+EXPORT_SYMBOL(strtouq);
 
 /*
  * Scaled down version of printf(3).
@@ -585,6 +588,7 @@ number:
 }
 #undef PCHAR
 }
+EXPORT_SYMBOL(vsnprintf);
 
 /**
  * snprintf - Format a string and place it in a buffer
@@ -603,6 +607,7 @@ int snprintf(char * buf, size_t size, const char *fmt, ...)
 va_end(args);
 return i;
 }
+EXPORT_SYMBOL(snprintf);
 
 /**
  * vsprintf - Format a string and place it in a buffer
@@ -617,7 +622,7 @@ int vsprintf(char *buf, const char *fmt, va_list args)
 {
 return vsnprintf(buf, 0xUL, fmt, args);
 }
-
+EXPORT_SYMBOL(vsprintf);
 
 /**
  * sprintf - Format a string and place it in a buffer
@@ -635,6 +640,7 @@ int sprintf(char * buf, const char *fmt, ...)
 va_end(args);
 return i;
 }
+EXPORT_SYMBOL(sprintf);
 
 /*
  * Fill in the given table from the scanset at the given format
@@ -1214,6 +1220,7 @@ input_failure:
 match_failure:
 return (nassigned);
 }
+EXPORT_SYMBOL(vsscanf);
 
 /**
  * sscanf - Unformat a buffer into a list of arguments
@@ -1231,5 +1238,6 @@ int sscanf(const char * buf, const char * fmt, ...)
va_end(args);
return i;
 }
+EXPORT_SYMBOL(sscanf);
 
 #endif
-- 
2.35.3

[PATCH v3 29/32] Mini-OS: add EXPORT_SYMBOL() instances to tpmfront.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to tpmfront.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 tpmfront.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/tpmfront.c b/tpmfront.c
index f4864d61..83768d07 100644
--- a/tpmfront.c
+++ b/tpmfront.c
@@ -338,6 +338,8 @@ error:
shutdown_tpmfront(dev);
return NULL;
 }
+EXPORT_SYMBOL(init_tpmfront);
+
 void shutdown_tpmfront(struct tpmfront_dev* dev)
 {
char* err;
@@ -402,6 +404,7 @@ void shutdown_tpmfront(struct tpmfront_dev* dev)
}
free(dev);
 }
+EXPORT_SYMBOL(shutdown_tpmfront);
 
 int tpmfront_send(struct tpmfront_dev* dev, const uint8_t* msg, size_t length)
 {
@@ -528,6 +531,7 @@ int tpmfront_cmd(struct tpmfront_dev* dev, uint8_t* req, 
size_t reqlen, uint8_t*
 
return 0;
 }
+EXPORT_SYMBOL(tpmfront_cmd);
 
 int tpmfront_set_locality(struct tpmfront_dev* dev, int locality)
 {
@@ -536,6 +540,7 @@ int tpmfront_set_locality(struct tpmfront_dev* dev, int 
locality)
dev->page->locality = locality;
return 0;
 }
+EXPORT_SYMBOL(tpmfront_set_locality);
 
 #ifdef HAVE_LIBC
 #include 
@@ -663,5 +668,6 @@ int tpmfront_open(struct tpmfront_dev *dev)
 
 return dev->fd;
 }
+EXPORT_SYMBOL(tpmfront_open);
 
 #endif
-- 
2.35.3

[PATCH v3 32/32] Mini-OS: hide all symbols not exported via EXPORT_SYMBOLS()

[Juergen Gross]

When doing the final linking of the Mini-OS kernel only keep the
symbols visible which have been marked via EXPORT_SYMBOL().

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile b/Makefile
index 588496cb..456aed0b 100644
--- a/Makefile
+++ b/Makefile
@@ -167,6 +167,7 @@ $(OBJ_DIR)/arch/x86/minios-x86%.lds:  
arch/x86/minios-x86.lds.S
 $(OBJ_DIR)/$(TARGET)-kern.o: $(OBJS) arch_lib 
$(OBJ_DIR)/$(TARGET_ARCH_DIR)/minios-$(MINIOS_TARGET_ARCH).lds
$(LD) -r $(LDFLAGS) $(HEAD_OBJ) $(OBJS) $(LDARCHLIB) -o $@
$(OBJCOPY) --dump-section .export_symbol=$(OBJ_DIR)/syms $@
+   $(OBJCOPY) -w -G $(GLOBAL_PREFIX)* 
--keep-global-symbols=$(OBJ_DIR)/syms --remove-section=.export_symbol $@ $@
 
 $(OBJ_DIR)/$(TARGET): $(OBJ_DIR)/$(TARGET)-kern.o $(APP_O)
$(LD) -r $(LDFLAGS) $(OBJ_DIR)/$(TARGET)-kern.o $(APP_O) $(LDLIBS) -o 
$@.o
-- 
2.35.3

[PATCH v3 16/32] Mini-OS: add EXPORT_SYMBOL() instances to events.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to events.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 events.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/events.c b/events.c
index 4683e8e1..9f3dbf05 100644
--- a/events.c
+++ b/events.c
@@ -119,6 +119,7 @@ void unbind_evtchn(evtchn_port_t port )
 if ( rc )
 printk("WARN: close_port %d failed rc=%d. ignored\n", port, rc);
 }
+EXPORT_SYMBOL(unbind_evtchn);
 
 evtchn_port_t bind_virq(uint32_t virq, evtchn_handler_t handler, void *data)
 {
@@ -138,6 +139,7 @@ evtchn_port_t bind_virq(uint32_t virq, evtchn_handler_t 
handler, void *data)
 bind_evtchn(op.port, handler, data);
return op.port;
 }
+EXPORT_SYMBOL(bind_virq);
 
 evtchn_port_t bind_pirq(uint32_t pirq, int will_share,
 evtchn_handler_t handler, void *data)
@@ -157,6 +159,7 @@ evtchn_port_t bind_pirq(uint32_t pirq, int will_share,
bind_evtchn(op.port, handler, data);
return op.port;
 }
+EXPORT_SYMBOL(bind_pirq);
 
 /*
  * Initially all events are without a handler and disabled
@@ -217,6 +220,7 @@ int evtchn_alloc_unbound(domid_t pal, evtchn_handler_t 
handler,
 *port = bind_evtchn(op.port, handler, data);
 return rc;
 }
+EXPORT_SYMBOL(evtchn_alloc_unbound);
 
 /* Connect to a port so as to allow the exchange of notifications with
the pal. Returns the result of the hypervisor call. */
@@ -240,6 +244,7 @@ int evtchn_bind_interdomain(domid_t pal, evtchn_port_t 
remote_port,
 *local_port = bind_evtchn(port, handler, data);
 return rc;
 }
+EXPORT_SYMBOL(evtchn_bind_interdomain);
 
 int evtchn_get_peercontext(evtchn_port_t local_port, char *ctx, int size)
 {
@@ -260,6 +265,7 @@ int evtchn_get_peercontext(evtchn_port_t local_port, char 
*ctx, int size)
 rc = HYPERVISOR_xsm_op(&op);
 return rc;
 }
+EXPORT_SYMBOL(evtchn_get_peercontext);
 
 /* Replace below when a hypercall is available to get the domid. */
 domid_t get_domid(void)
@@ -294,6 +300,7 @@ domid_t get_domid(void)
 
 return domid;
 }
+EXPORT_SYMBOL(get_domid);
 
 /*
  * Local variables:
-- 
2.35.3

[PATCH v3 31/32] Mini-OS: add EXPORT_SYMBOL() instances to xenbus.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to xenbus.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 xenbus.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/xenbus.c b/xenbus.c
index 8bfd5bd4..eb9af055 100644
--- a/xenbus.c
+++ b/xenbus.c
@@ -123,6 +123,7 @@ char **xenbus_wait_for_watch_return(xenbus_event_queue 
*queue)
 
 return &event->path;
 }
+EXPORT_SYMBOL(xenbus_wait_for_watch_return);
 
 void xenbus_wait_for_watch(xenbus_event_queue *queue)
 {
@@ -136,6 +137,7 @@ void xenbus_wait_for_watch(xenbus_event_queue *queue)
 else
 printk("unexpected path returned by watch\n");
 }
+EXPORT_SYMBOL(xenbus_wait_for_watch);
 
 void xenbus_release_wait_for_watch(xenbus_event_queue *queue)
 {
@@ -145,6 +147,7 @@ void xenbus_release_wait_for_watch(xenbus_event_queue 
*queue)
 *queue = event;
 wake_up(&xenbus_watch_queue);
 }
+EXPORT_SYMBOL(xenbus_release_wait_for_watch);
 
 char *xenbus_wait_for_value(const char *path, const char *value,
 xenbus_event_queue *queue)
@@ -170,6 +173,7 @@ char *xenbus_wait_for_value(const char *path, const char 
*value,
 xenbus_wait_for_watch(queue);
 }
 }
+EXPORT_SYMBOL(xenbus_wait_for_value);
 
 char *xenbus_switch_state(xenbus_transaction_t xbt, const char *path,
   XenbusState state)
@@ -220,6 +224,7 @@ exit:
 
 return msg;
 }
+EXPORT_SYMBOL(xenbus_switch_state);
 
 char *xenbus_wait_for_state_change(const char *path, XenbusState *state,
xenbus_event_queue *queue)
@@ -249,6 +254,7 @@ char *xenbus_wait_for_state_change(const char *path, 
XenbusState *state,
 }
 return NULL;
 }
+EXPORT_SYMBOL(xenbus_wait_for_state_change);
 
 static void xenbus_read_data(char *buf, unsigned int len)
 {
@@ -602,6 +608,7 @@ struct xsd_sockmsg *xenbus_msg_reply(int type, 
xenbus_transaction_t trans,
 
 return rep;
 }
+EXPORT_SYMBOL(xenbus_msg_reply);
 
 static char *errmsg(struct xsd_sockmsg *rep)
 {
@@ -664,6 +671,7 @@ char *xenbus_ls(xenbus_transaction_t xbt, const char *pre, 
char ***contents)
 
 return NULL;
 }
+EXPORT_SYMBOL(xenbus_ls);
 
 char *xenbus_read(xenbus_transaction_t xbt, const char *path, char **value)
 {
@@ -687,6 +695,7 @@ char *xenbus_read(xenbus_transaction_t xbt, const char 
*path, char **value)
 
 return NULL;
 }
+EXPORT_SYMBOL(xenbus_read);
 
 char *xenbus_write(xenbus_transaction_t xbt, const char *path,
const char *value)
@@ -707,6 +716,7 @@ char *xenbus_write(xenbus_transaction_t xbt, const char 
*path,
 
 return NULL;
 }
+EXPORT_SYMBOL(xenbus_write);
 
 char* xenbus_watch_path_token(xenbus_transaction_t xbt, const char *path,
   const char *token, xenbus_event_queue *events)
@@ -738,6 +748,7 @@ char* xenbus_watch_path_token(xenbus_transaction_t xbt, 
const char *path,
 
 return NULL;
 }
+EXPORT_SYMBOL(xenbus_watch_path_token);
 
 char* xenbus_unwatch_path_token(xenbus_transaction_t xbt, const char *path,
 const char *token)
@@ -771,6 +782,7 @@ char* xenbus_unwatch_path_token(xenbus_transaction_t xbt, 
const char *path,
 
 return NULL;
 }
+EXPORT_SYMBOL(xenbus_unwatch_path_token);
 
 char *xenbus_rm(xenbus_transaction_t xbt, const char *path)
 {
@@ -787,6 +799,7 @@ char *xenbus_rm(xenbus_transaction_t xbt, const char *path)
 
 return NULL;
 }
+EXPORT_SYMBOL(xenbus_rm);
 
 char *xenbus_get_perms(xenbus_transaction_t xbt, const char *path, char 
**value)
 {
@@ -810,6 +823,7 @@ char *xenbus_get_perms(xenbus_transaction_t xbt, const char 
*path, char **value)
 
 return NULL;
 }
+EXPORT_SYMBOL(xenbus_get_perms);
 
 #define PERM_MAX_SIZE 32
 char *xenbus_set_perms(xenbus_transaction_t xbt, const char *path, domid_t dom,
@@ -834,6 +848,7 @@ char *xenbus_set_perms(xenbus_transaction_t xbt, const char 
*path, domid_t dom,
 
 return NULL;
 }
+EXPORT_SYMBOL(xenbus_set_perms);
 
 char *xenbus_transaction_start(xenbus_transaction_t *xbt)
 {
@@ -855,6 +870,7 @@ char *xenbus_transaction_start(xenbus_transaction_t *xbt)
 
 return NULL;
 }
+EXPORT_SYMBOL(xenbus_transaction_start);
 
 char *xenbus_transaction_end(xenbus_transaction_t t, int abort, int *retry)
 {
@@ -883,6 +899,7 @@ char *xenbus_transaction_end(xenbus_transaction_t t, int 
abort, int *retry)
 
 return NULL;
 }
+EXPORT_SYMBOL(xenbus_transaction_end);
 
 int xenbus_read_integer(const char *path)
 {
@@ -902,6 +919,7 @@ int xenbus_read_integer(const char *path)
 
 return t;
 }
+EXPORT_SYMBOL(xenbus_read_integer);
 
 int xenbus_read_uuid(const char *path, unsigned char uuid[16])
 {
@@ -935,6 +953,7 @@ int xenbus_read_uuid(const char *path, unsigned char 
uuid[16])
 
 return 1;
 }
+EXPORT_SYMBOL(xenbus_read_uuid);
 
 #define BUFFER_SIZE 256
 static void xenbus_build_path(const char *dir, const char *node, char *res)
@@ -957,6 +976,7 @@ char *xenbus_printf(xenbus_transaction_t xbt, const char* 
node,
 
 return xenbus_write(xbt,

[PATCH v3 23/32] Mini-OS: add EXPORT_SYMBOL() instances to 9pfront.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to 9pfront.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 9pfront.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/9pfront.c b/9pfront.c
index 35c5552b..315089bc 100644
--- a/9pfront.c
+++ b/9pfront.c
@@ -1243,6 +1243,7 @@ void *init_9pfront(unsigned int id, const char *mnt)
 free(msg);
 return NULL;
 }
+EXPORT_SYMBOL(init_9pfront);
 
 void shutdown_9pfront(void *dev)
 {
@@ -1286,6 +1287,7 @@ void shutdown_9pfront(void *dev)
 printk("9pfsfront shutdown %u failed, %s\n", dev9p->id, reason);
 free(msg);
 }
+EXPORT_SYMBOL(shutdown_9pfront);
 
 static const struct file_ops ops_9pfs = {
 .name = "9pfs",
-- 
2.35.3

[PATCH v3 08/32] Mini-OS: add EXPORT_SYMBOL() instances to lib/string.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to lib/string.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 lib/string.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/lib/string.c b/lib/string.c
index 8b241464..b0b628c6 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -19,6 +19,7 @@
  */
 
 #include 
+#include 
 
 /* newlib defines ffs but not ffsll or ffsl */
 int __ffsti2 (long long int lli)
@@ -49,11 +50,13 @@ int ffsl (long int li)
 {
 return __ffsti2 ((long long int) li);
 }
+EXPORT_SYMBOL(ffsl);
 
 int ffsll (long long int lli)
 {
 return __ffsti2 (lli);
 }
+EXPORT_SYMBOL(ffsll);
 
 #if !defined HAVE_LIBC
 
@@ -72,6 +75,7 @@ int memcmp(const void * cs,const void * ct,size_t count)
break;
return res;
 }
+EXPORT_SYMBOL(memcmp);
 
 void * memcpy(void * dest,const void *src,size_t count)
 {
@@ -83,6 +87,7 @@ void * memcpy(void * dest,const void *src,size_t count)
 
return dest;
 }
+EXPORT_SYMBOL(memcpy);
 
 int strncmp(const char * cs,const char * ct,size_t count)
 {
@@ -96,6 +101,7 @@ int strncmp(const char * cs,const char * ct,size_t count)
 
return __res;
 }
+EXPORT_SYMBOL(strncmp);
 
 int strcmp(const char * cs,const char * ct)
 {
@@ -108,6 +114,7 @@ int strcmp(const char * cs,const char * ct)
 
 return __res;
 }
+EXPORT_SYMBOL(strcmp);
 
 char * strcpy(char * dest,const char *src)
 {
@@ -117,6 +124,7 @@ char * strcpy(char * dest,const char *src)
 /* nothing */;
 return tmp;
 }
+EXPORT_SYMBOL(strcpy);
 
 char * strncpy(char * dest,const char *src,size_t count)
 {
@@ -127,6 +135,7 @@ char * strncpy(char * dest,const char *src,size_t count)
 
 return tmp;
 }
+EXPORT_SYMBOL(strncpy);
 
 void * memset(void * s,int c,size_t count)
 {
@@ -137,6 +146,7 @@ void * memset(void * s,int c,size_t count)
 
 return s;
 }
+EXPORT_SYMBOL(memset);
 
 size_t strnlen(const char * s, size_t count)
 {
@@ -146,6 +156,7 @@ size_t strnlen(const char * s, size_t count)
 /* nothing */;
 return sc - s;
 }
+EXPORT_SYMBOL(strnlen);
 
 
 char * strcat(char * dest, const char * src)
@@ -159,6 +170,7 @@ char * strcat(char * dest, const char * src)
 
 return tmp;
 }
+EXPORT_SYMBOL(strcat);
 
 size_t strlen(const char * s)
 {
@@ -168,6 +180,7 @@ size_t strlen(const char * s)
/* nothing */;
return sc - s;
 }
+EXPORT_SYMBOL(strlen);
 
 char * strchr(const char * s, int c)
 {
@@ -176,6 +189,7 @@ char * strchr(const char * s, int c)
 return NULL;
 return (char *)s;
 }
+EXPORT_SYMBOL(strchr);
 
 char * strrchr(const char * s, int c)
 {
@@ -185,6 +199,7 @@ char * strrchr(const char * s, int c)
 res = s;
 return (char *)res;
 }
+EXPORT_SYMBOL(strrchr);
 
 char * strstr(const char * s1,const char * s2)
 {
@@ -202,6 +217,7 @@ char * strstr(const char * s1,const char * s2)
 }
 return NULL;
 }
+EXPORT_SYMBOL(strstr);
 
 char *strdup(const char *x)
 {
@@ -211,6 +227,7 @@ char *strdup(const char *x)
 memcpy(res, x, l + 1);
 return res;
 }
+EXPORT_SYMBOL(strdup);
 
 int ffs(int i)
 {
@@ -224,5 +241,6 @@ int ffs(int i)
} while (i);
return 0;
 }
+EXPORT_SYMBOL(ffs);
 
 #endif
-- 
2.35.3

[PATCH v3 22/32] Mini-OS: add EXPORT_SYMBOL() instances to lwip-*.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to lwip-*.c.

Most added symbols are in LWIP, but we can't add the EXPORT_SYMBOL()
instances easily there.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 lwip-arch.c | 9 +
 lwip-net.c  | 2 +-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/lwip-arch.c b/lwip-arch.c
index 21e941e9..7907f446 100644
--- a/lwip-arch.c
+++ b/lwip-arch.c
@@ -12,7 +12,9 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+#include 
 
 /* Is called to initialize the sys_arch layer */
 void sys_init(void)
@@ -293,3 +295,10 @@ void lwip_die(char *fmt, ...)
 printk("\n");
 BUG();
 }
+
+EXPORT_SYMBOL(htonl);
+EXPORT_SYMBOL(htons);
+EXPORT_SYMBOL(inet_aton);
+EXPORT_SYMBOL(inet_ntoa);
+EXPORT_SYMBOL(ntohl);
+EXPORT_SYMBOL(ntohs);
diff --git a/lwip-net.c b/lwip-net.c
index 16950d32..e71a11c7 100644
--- a/lwip-net.c
+++ b/lwip-net.c
@@ -250,7 +250,7 @@ void networking_set_addr(struct ip_addr *ipaddr, struct 
ip_addr *netmask, struct
   netif_set_netmask(the_interface, netmask);
   netif_set_gw(the_interface, gw);
 }
-
+EXPORT_SYMBOL(networking_set_addr);
 
 static void
 arp_timer(void *arg)
-- 
2.35.3

[PATCH v3 17/32] Mini-OS: add EXPORT_SYMBOL() instances to gntmap.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to gntmap.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 gntmap.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/gntmap.c b/gntmap.c
index 3422ab23..5f57d6b3 100644
--- a/gntmap.c
+++ b/gntmap.c
@@ -106,6 +106,7 @@ gntmap_set_max_grants(struct gntmap *map, int count)
 map->nentries = count;
 return 0;
 }
+EXPORT_SYMBOL(gntmap_set_max_grants);
 
 static int
 _gntmap_unmap_grant_ref(struct gntmap *map, int idx)
@@ -209,6 +210,7 @@ gntmap_munmap(struct gntmap *map, unsigned long 
start_address, int count)
 
 return 0;
 }
+EXPORT_SYMBOL(gntmap_munmap);
 
 void*
 gntmap_map_grant_refs(struct gntmap *map, 
@@ -251,6 +253,7 @@ gntmap_map_grant_refs(struct gntmap *map,
 
 return (void*) addr;
 }
+EXPORT_SYMBOL(gntmap_map_grant_refs);
 
 void
 gntmap_init(struct gntmap *map)
@@ -259,6 +262,7 @@ gntmap_init(struct gntmap *map)
 map->nentries = 0;
 map->entries = NULL;
 }
+EXPORT_SYMBOL(gntmap_init);
 
 void
 gntmap_fini(struct gntmap *map)
@@ -281,3 +285,4 @@ gntmap_fini(struct gntmap *map)
 map->entries = NULL;
 map->nentries = 0;
 }
+EXPORT_SYMBOL(gntmap_fini);
-- 
2.35.3

[PATCH v3 27/32] Mini-OS: add EXPORT_SYMBOL() instances to pcifront.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to pcifront.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 pcifront.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/pcifront.c b/pcifront.c
index 56423562..f1d69396 100644
--- a/pcifront.c
+++ b/pcifront.c
@@ -278,6 +278,7 @@ error:
 free_pcifront(dev);
 return NULL;
 }
+EXPORT_SYMBOL(init_pcifront);
 
 void pcifront_scan(struct pcifront_dev *dev, void (*func)(unsigned int domain, 
unsigned int bus, unsigned slot, unsigned int fun))
 {
@@ -319,6 +320,7 @@ void pcifront_scan(struct pcifront_dev *dev, void 
(*func)(unsigned int domain, u
 }
 free(path);
 }
+EXPORT_SYMBOL(pcifront_scan);
 
 void shutdown_pcifront(struct pcifront_dev *dev)
 {
@@ -377,6 +379,7 @@ close_pcifront:
 if (!err)
 free_pcifront(dev);
 }
+EXPORT_SYMBOL(shutdown_pcifront);
 
 int pcifront_physical_to_virtual (struct pcifront_dev *dev,
   unsigned int *dom,
@@ -451,6 +454,7 @@ void pcifront_op(struct pcifront_dev *dev, struct 
xen_pci_op *op)
 rmb();
 *op = dev->info->op;
 }
+EXPORT_SYMBOL(pcifront_op);
 
 int pcifront_conf_read(struct pcifront_dev *dev,
unsigned int dom,
@@ -481,6 +485,7 @@ int pcifront_conf_read(struct pcifront_dev *dev,
 
 return 0;
 }
+EXPORT_SYMBOL(pcifront_conf_read);
 
 int pcifront_conf_write(struct pcifront_dev *dev,
 unsigned int dom,
@@ -508,6 +513,7 @@ int pcifront_conf_write(struct pcifront_dev *dev,
 
 return op.err;
 }
+EXPORT_SYMBOL(pcifront_conf_write);
 
 int pcifront_enable_msi(struct pcifront_dev *dev,
 unsigned int dom,
@@ -533,6 +539,7 @@ int pcifront_enable_msi(struct pcifront_dev *dev,
 else
 return op.value;
 }
+EXPORT_SYMBOL(pcifront_enable_msi);
 
 int pcifront_disable_msi(struct pcifront_dev *dev,
  unsigned int dom,
@@ -555,6 +562,7 @@ int pcifront_disable_msi(struct pcifront_dev *dev,
 
 return op.err;
 }
+EXPORT_SYMBOL(pcifront_disable_msi);
 
 int pcifront_enable_msix(struct pcifront_dev *dev,
  unsigned int dom,
@@ -589,7 +597,7 @@ int pcifront_enable_msix(struct pcifront_dev *dev,
 
 return 0;
 }
-
+EXPORT_SYMBOL(pcifront_enable_msix);
 
 int pcifront_disable_msix(struct pcifront_dev *dev,
   unsigned int dom,
@@ -612,3 +620,4 @@ int pcifront_disable_msix(struct pcifront_dev *dev,
 
 return op.err;
 }
+EXPORT_SYMBOL(pcifront_disable_msix);
-- 
2.35.3

[PATCH v3 28/32] Mini-OS: add EXPORT_SYMBOL() instances to tpmback.c

[Juergen Gross]

Add the needed instances of EXPORT_SYMBOL() to tpmback.c.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
 tpmback.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/tpmback.c b/tpmback.c
index 22adbd32..ab0df127 100644
--- a/tpmback.c
+++ b/tpmback.c
@@ -804,6 +804,7 @@ void* tpmback_get_opaque(domid_t domid, unsigned int handle)
 
return tpmif->opaque;
 }
+EXPORT_SYMBOL(tpmback_get_opaque);
 
 int tpmback_set_opaque(domid_t domid, unsigned int handle, void *opaque)
 {
@@ -816,6 +817,7 @@ int tpmback_set_opaque(domid_t domid, unsigned int handle, 
void *opaque)
tpmif->opaque = opaque;
return 0;
 }
+EXPORT_SYMBOL(tpmback_set_opaque);
 
 unsigned char* tpmback_get_uuid(domid_t domid, unsigned int handle)
 {
@@ -827,6 +829,7 @@ unsigned char* tpmback_get_uuid(domid_t domid, unsigned int 
handle)
 
return tpmif->uuid;
 }
+EXPORT_SYMBOL(tpmback_get_uuid);
 
 int tpmback_get_peercontext(domid_t domid, unsigned int handle, void* buffer, 
int buflen)
 {
@@ -838,6 +841,7 @@ int tpmback_get_peercontext(domid_t domid, unsigned int 
handle, void* buffer, in
 
return evtchn_get_peercontext(tpmif->evtchn, buffer, buflen);
 }
+EXPORT_SYMBOL(tpmback_get_peercontext);
 
 static void event_listener(void)
 {
@@ -903,6 +907,7 @@ void init_tpmback(void (*open_cb)(domid_t, unsigned int), 
void (*close_cb)(domid
eventthread = create_thread("tpmback-listener", event_thread, NULL);
 
 }
+EXPORT_SYMBOL(init_tpmback);
 
 void shutdown_tpmback(void)
 {
@@ -923,6 +928,7 @@ void shutdown_tpmback(void)
wake_up(&waitq);
schedule();
 }
+EXPORT_SYMBOL(shutdown_tpmback);
 
 static void init_tpmcmd(tpmcmd_t* tpmcmd, domid_t domid, unsigned int handle, 
void *opaque)
 {
@@ -1061,6 +1067,7 @@ tpmcmd_t* tpmback_req_any(void)
TPMBACK_ERR("backend request ready flag was set but no interfaces were 
actually ready\n");
return NULL;
 }
+EXPORT_SYMBOL(tpmback_req_any);
 
 tpmcmd_t* tpmback_req(domid_t domid, unsigned int handle)
 {
@@ -1081,6 +1088,7 @@ tpmcmd_t* tpmback_req(domid_t domid, unsigned int handle)
 
return get_request(tpmif);
 }
+EXPORT_SYMBOL(tpmback_req);
 
 void tpmback_resp(tpmcmd_t* tpmcmd)
 {
@@ -1108,6 +1116,7 @@ end:
free(tpmcmd);
return;
 }
+EXPORT_SYMBOL(tpmback_resp);
 
 int tpmback_wait_for_frontend_connect(domid_t *domid, unsigned int *handle)
 {
@@ -1125,8 +1134,10 @@ int tpmback_wait_for_frontend_connect(domid_t *domid, 
unsigned int *handle)
 
return 0;
 }
+EXPORT_SYMBOL(tpmback_wait_for_frontend_connect);
 
 int tpmback_num_frontends(void)
 {
return gtpmdev.num_tpms;
 }
+EXPORT_SYMBOL(tpmback_num_frontends);
-- 
2.35.3

Re: [XEN PATCH v2 2/3] xen/arm: add SAF deviation for debugging and logging effects

[Jan Beulich]

On 24.11.2023 18:29, Simone Ballarin wrote:
> Rule 13.1: Initializer lists shall not contain persistent side effects
> 
> Effects caused by debug/logging macros and functions (like ASSERT, 
> __bad_atomic_size,
> LOG, etc ...) that crash execution or produce logs are not dangerous in 
> initializer
> lists. The evaluation order in abnormal conditions is not relevant. 
> Evaluation order
> of logging effects is always safe.
> 
> This patch deviates violations using SAF commits caused by debug/logging 
> macros and
> functions.
> 
> Asm volatile statements in initializer lists that do not perform any 
> persistent side
> effect are safe: this patch deviates violations caused by uses of the current 
> macro
> (that contains an asm volatile) in initializer lists.
> 
> No functional changes.
> 
> Signed-off-by: Simone Ballarin 
> 
> ---
> Changes in v2:
> New patch based on the discussion for "xen/arm: address violations of MISRA 
> C:2012 Rule 13.1".
> ---
>  docs/misra/safe.json | 16 
>  xen/arch/arm/device.c|  1 +
>  xen/arch/arm/guestcopy.c |  4 
>  xen/arch/x86/hvm/hvm.c   |  1 +
>  xen/common/sched/core.c  |  3 +++

The latter two don't really fit the title prefix.

> --- a/docs/misra/safe.json
> +++ b/docs/misra/safe.json
> @@ -28,6 +28,22 @@
>  },
>  {
>  "id": "SAF-3-safe",
> +"analyser": {
> +"eclair": "MC3R1.R13.1"
> +},
> +"name": "MC3R1.R13.1: effects for debugging and logging",
> +"text": "Effects for debugging and loggings reasons that crash 
> execution or produce logs are allowed in initializer lists. The evaluation 
> order in abnormal conditions is not relevant."
> +},

I'm wary of this statement. Order may not matter much anymore _after_ an
abnormal condition was encountered, but in the course of determining whether
an abnormal condition might have been reached it may very well still matter.

> +{
> +"id": "SAF-4-safe",
> +"analyser": {
> +"eclair": "MC3R1.R13.1"
> +},
> +"name": "MC3R1.R13.1: volatile asm statements that do not 
> perform any persistent side effect",
> +"text": "Volatile asm statements in an initializer list if do 
> not perform persistent side effects are safe."

Since each respective comment ought to affect just a single asm(), I think
this wants writing in singular. I further don't think it is useful for
"text" to largely restate what "name" already says.

> --- a/xen/arch/arm/device.c
> +++ b/xen/arch/arm/device.c
> @@ -331,6 +331,7 @@ int handle_device(struct domain *d, struct dt_device_node 
> *dev, p2m_type_t p2mt,
>  .p2mt = p2mt,
>  .skip_mapping = !own_device ||
>  (is_pci_passthrough_enabled() &&
> +/* SAF-3-safe effects for debugging/logging reasons 
> are safe */
>  (device_get_class(dev) == DEVICE_PCI_HOSTBRIDGE)),

What's the debugging / logging reason on the commented line?

> --- a/xen/arch/arm/guestcopy.c
> +++ b/xen/arch/arm/guestcopy.c
> @@ -110,18 +110,21 @@ static unsigned long copy_guest(void *buf, uint64_t 
> addr, unsigned int len,
>  unsigned long raw_copy_to_guest(void *to, const void *from, unsigned int len)
>  {
>  return copy_guest((void *)from, (vaddr_t)to, len,
> +  /* SAF-4-safe No persistent side effects */
>GVA_INFO(current), COPY_to_guest | COPY_linear);
>  }
>  
>  unsigned long raw_copy_to_guest_flush_dcache(void *to, const void *from,
>   unsigned int len)
>  {
> +/* SAF-4-safe No persistent side effects */
>  return copy_guest((void *)from, (vaddr_t)to, len, GVA_INFO(current),
>COPY_to_guest | COPY_flush_dcache | COPY_linear);
>  }
>  
>  unsigned long raw_clear_guest(void *to, unsigned int len)
>  {
> +/* SAF-4-safe No persistent side effects */
>  return copy_guest(NULL, (vaddr_t)to, len, GVA_INFO(current),
>COPY_to_guest | COPY_linear);
>  }
> @@ -129,6 +132,7 @@ unsigned long raw_clear_guest(void *to, unsigned int len)
>  unsigned long raw_copy_from_guest(void *to, const void __user *from,
>unsigned int len)
>  {
> +/* SAF-4-safe No persistent side effects */
>  return copy_guest(to, (vaddr_t)from, len, GVA_INFO(current),
>COPY_from_guest | COPY_linear);
>  }

I can only guess that in all four of these it's the use of "current" which
requires the comment. Yet imo that either needs making explicit, or such a
comment shouldn't go on use sites of "current", but on its definition site.

> --- a/xen/common/sched/core.c
> +++ b/xen/common/sched/core.c
> @@ -1517,6 +1517,7 @@ long vcpu_yield(void)
>  
>  SCHED_STAT_CRANK(vcpu_yield);
>  
> +/* SAF-4-safe No persistent side effects */
>  TRACE_2D(TRC_SCHED_YIELD,

Re: [XEN PATCH v2 3/3] xen: address violations of MISRA C:2012 Rule 13.1

[Jan Beulich]

On 24.11.2023 18:29, Simone Ballarin wrote:
> Rule 13.1: Initializer lists shall not contain persistent side effects
> 
> The assignment operation in:
> 
> .irq = rc = uart->irq,
> 
> is a persistent side effect in a struct initializer list.
> 
> This patch avoids rc assignment and directly uses uart->irq
> in the following if statement.
> 
> No functional changes.
> 
> Signed-off-by: Maria Celeste Cesario  
> Signed-off-by: Simone Ballarin 

Who's the author of this patch? (Either the order of the SoB is wrong, or
there's a From: tag missing.)

> ---
> Changes in v2:
> - avoid assignment of rc;
> - drop changes in vcpu_yield(void).
> ---
>  xen/drivers/char/ns16550.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)

This warrants a more specific subject prefix. Also there's only a single
violation being dealt with here.

> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -445,11 +445,13 @@ static void __init cf_check ns16550_init_postirq(struct 
> serial_port *port)
>  struct msi_info msi = {
>  .sbdf = PCI_SBDF(0, uart->ps_bdf[0], uart->ps_bdf[1],
>   uart->ps_bdf[2]),
> -.irq = rc = uart->irq,
> +.irq = uart->irq,
>  .entry_nr = 1
>  };
>  
> -if ( rc > 0 )
> +rc = 0;
> +
> +if ( uart->irq > 0 )
>  {
>  struct msi_desc *msi_desc = NULL;

The fact that there's no functional change here isn't really obvious.
Imo you want to prove that to a reasonable degree in the description.

Jan

Re: We are not able to virtualize FreeBSD using xen 4.17 on Arm 32 bit

[Roger Pau Monné]

On Mon, Nov 27, 2023 at 10:28:13AM +, Henry Wang wrote:
> +(xen-devel and Arm maintainers, including Julien)
> 
> > On Nov 27, 2023, at 18:03, Mario Marietto 
> > wrote:
> > 
> > Hello.  We have just virtualized Debian 12 on our arm (32 bit)
> > Chromebook model xe303c12 . As host / dom0 we have chosen Devuan
> > 5,and for guest / domU,Debian 12. It works great. But our goal is
> > different. We want to virtualize FreeBSD as domU. Can we have a
> > working Xen PV network driver for a FreeBSD arm guest ?. I found
> > that Julien Grall has ported the Xen drivers to FreeBSD on arm. I
> > would like to know if Julien's work was accepted upstream by
> > FreeBSD, in which case FreeBSD as a Xen guest on arm should work
> > if we enable the Xen PV drivers in the FreeBSD on arm kernel. If
> > Julien's work was not accepted upstream by FreeBSD, we will have
> > to find his patches and apply them ourselves to the FreeBSD on arm
> > kernel.

I've added Elliot on Cc as he is working on upstreaming the patches to
FreeBSD.  He will be able to provide a better update than myself.

Regards, Roger.

Re: [PATCH v3 11/14] xen/asm-generic: introduce stub header numa.h

[Oleksii]

Hello Michal,

On Fri, 2023-11-24 at 14:03 +0100, Michal Orzel wrote:
> 
> On 17/11/2023 13:24, Oleksii Kurochko wrote:
> > 
> > 
> >  is common through some archs so it is moved
> > to asm-generic.
> > 
> > Signed-off-by: Oleksii Kurochko 
> > ---
> > Changes in V3:
> >  - Remove old header inclusion in asm-generic numa.h and include
> >     and 
> >  - Drop Arm and PPC's numa.h and use asm-generic version instead.
> > ---
> > Changes in V2:
> >     - update the commit message.
> >     - change u8 to uint8_t.
> >     - add ifnded CONFIG_NUMA.
> > ---
> >  xen/arch/arm/include/asm/Makefile |  1 +
> >  xen/arch/ppc/include/asm/Makefile |  1 +
> >  xen/arch/ppc/include/asm/numa.h   | 26 ---
> > 
> >  .../asm => include/asm-generic}/numa.h    | 13 ++
> >  4 files changed, 10 insertions(+), 31 deletions(-)
> >  delete mode 100644 xen/arch/ppc/include/asm/numa.h
> >  rename xen/{arch/arm/include/asm => include/asm-generic}/numa.h
> > (75%)
> > 
> > diff --git a/xen/arch/arm/include/asm/Makefile
> > b/xen/arch/arm/include/asm/Makefile
> > index 8221429c2c..0c855a798a 100644
> > --- a/xen/arch/arm/include/asm/Makefile
> > +++ b/xen/arch/arm/include/asm/Makefile
> > @@ -2,6 +2,7 @@
> >  generic-y += altp2m.h
> >  generic-y += hardirq.h
> >  generic-y += iocap.h
> > +generic-y += numa.h
> >  generic-y += paging.h
> >  generic-y += percpu.h
> >  generic-y += random.h
> > diff --git a/xen/arch/ppc/include/asm/Makefile
> > b/xen/arch/ppc/include/asm/Makefile
> > index 9bbae4cec8..d5a94bc718 100644
> > --- a/xen/arch/ppc/include/asm/Makefile
> > +++ b/xen/arch/ppc/include/asm/Makefile
> > @@ -5,6 +5,7 @@ generic-y += hardirq.h
> >  generic-y += hypercall.h
> >  generic-y += iocap.h
> >  generic-y += monitor.h
> > +generic-y += numa.h
> >  generic-y += paging.h
> >  generic-y += percpu.h
> >  generic-y += random.h
> > diff --git a/xen/arch/ppc/include/asm/numa.h
> > b/xen/arch/ppc/include/asm/numa.h
> > deleted file mode 100644
> > index 7fdf66c3da..00
> > --- a/xen/arch/ppc/include/asm/numa.h
> > +++ /dev/null
> > @@ -1,26 +0,0 @@
> > -#ifndef __ASM_PPC_NUMA_H__
> > -#define __ASM_PPC_NUMA_H__
> > -
> > -#include 
> > -#include 
> > -
> > -typedef uint8_t nodeid_t;
> > -
> > -/* Fake one node for now. See also node_online_map. */
> > -#define cpu_to_node(cpu) 0
> > -#define node_to_cpumask(node)   (cpu_online_map)
> > -
> > -/*
> > - * TODO: make first_valid_mfn static when NUMA is supported on
> > PPC, this
> > - * is required because the dummy helpers are using it.
> > - */
> > -extern mfn_t first_valid_mfn;
> > -
> > -/* XXX: implement NUMA support */
> > -#define node_spanned_pages(nid) (max_page -
> > mfn_x(first_valid_mfn))
> > -#define node_start_pfn(nid) (mfn_x(first_valid_mfn))
> > -#define __node_distance(a, b) (20)
> > -
> > -#define arch_want_default_dmazone() (false)
> > -
> > -#endif /* __ASM_PPC_NUMA_H__ */
> > diff --git a/xen/arch/arm/include/asm/numa.h b/xen/include/asm-
> > generic/numa.h
> > similarity index 75%
> > rename from xen/arch/arm/include/asm/numa.h
> > rename to xen/include/asm-generic/numa.h
> > index e2bee2bd82..c5b522dee8 100644
> > --- a/xen/arch/arm/include/asm/numa.h
> > +++ b/xen/include/asm-generic/numa.h
> > @@ -1,9 +1,11 @@
> > -#ifndef __ARCH_ARM_NUMA_H
> > -#define __ARCH_ARM_NUMA_H
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +#ifndef __ARCH_GENERIC_NUMA_H
> > +#define __ARCH_GENERIC_NUMA_H
> ASM_GENERIC ?
Thanks. I'll rename the guard.
> 
> > 
> > -#include 
> > +#include 
> If this is for mfn_t, shouldn't this inclusion be moved under #ifndef
> ?
It would be better. I'll update that in the next patch series.

> 
> Apart from that:
> Reviewed-by: Michal Orzel 


~ Oleksii

Re: [Xen-devel] PV guest with PCI passthrough crash on Xen 4.8.3 inside KVM when booted through OVMF

[Marek Marczykowski-Górecki]

On Mon, Nov 27, 2023 at 11:20:36AM +, Frediano Ziglio wrote:
> On Sun, Nov 26, 2023 at 2:51 PM Marek Marczykowski-Górecki
>  wrote:
> >
> > On Mon, Feb 19, 2018 at 06:30:14PM +0100, Juergen Gross wrote:
> > > On 16/02/18 20:02, Andrew Cooper wrote:
> > > > On 16/02/18 18:51, Marek Marczykowski-Górecki wrote:
> > > >> On Fri, Feb 16, 2018 at 05:52:50PM +, Andrew Cooper wrote:
> > > >>> On 16/02/18 17:48, Marek Marczykowski-Górecki wrote:
> > >  Hi,
> > > 
> > >  As in the subject, the guest crashes on boot, before kernel output
> > >  anything. I've isolated this to the conditions below:
> > >   - PV guest have PCI device assigned (e1000e emulated by QEMU in 
> > >  this case),
> > > without PCI device it works
> > >   - Xen (in KVM) is started through OVMF; with seabios it works
> > >   - nested HVM is disabled in KVM
> > >   - AMD IOMMU emulation is disabled in KVM; when enabled qemu crashes 
> > >  on
> > > boot (looks like qemu bug, unrelated to this one)
> > > 
> > >  Version info:
> > >   - KVM host: OpenSUSE 42.3, qemu 2.9.1, 
> > >  ovmf-2017+git1492060560.b6d11d7c46-4.1, AMD
> > >   - Xen host: Xen 4.8.3, dom0: Linux 4.14.13
> > >   - Xen domU: Linux 4.14.13, direct boot
> > > 
> > >  Not sure if relevant, but initially I've tried booting xen.efi /mapbs
> > >  /noexitboot and then dom0 kernel crashed saying something about 
> > >  conflict
> > >  between e820 and kernel mapping. But now those options are disabled.
> > > 
> > >  The crash message:
> > >  (XEN) d1v0 Unhandled invalid opcode fault/trap [#6, ec=]
> > >  (XEN) domain_crash_sync called from entry.S: fault at 
> > >  82d080218720 entry.o#create_bounce_frame+0x137/0x146
> > >  (XEN) Domain 1 (vcpu#0) crashed on cpu#1:
> > >  (XEN) [ Xen-4.8.3  x86_64  debug=n   Not tainted ]
> > >  (XEN) CPU:1
> > >  (XEN) RIP:e033:[]
> > > >>> This is #UD, which is most probably hitting a BUG().  addr2line this ^
> > > >>> to find some code to look at.
> > > >> addr2line failed me
> > > >
> > > > By default, vmlinux is stripped and compressed.  Ideally you want to
> > > > addr2line the vmlinux artefact in the root of your kernel build, which
> > > > is the plain elf with debugging symbols.
> > > >
> > > > Alternatively, use scripts/extract-vmlinux on the binary you actually
> > > > booted, which might get you somewhere.
> > > >
> > > >> , but System.map says its xen_memory_setup. And it
> > > >> looks like the BUG() is the same as I had in dom0 before:
> > > >> "Xen hypervisor allocated kernel memory conflicts with E820 map".
> > > >
> > > > Juergen: Is there anything we can do to try and insert some dummy
> > > > exception handlers right at PV start, so we could at least print out a
> > > > oneliner to the host console which is a little more helpful than Xen
> > > > saying "something unknown went wrong" ?
> > >
> > > You mean something like commit 42b3a4cb5609de757f5445fcad18945ba9239a07
> > > added to kernel 4.15?
> > >
> > > >
> > > >>
> > > >> Disabling e820_host in guest config solved the problem. Thanks!
> > > >>
> > > >> Is this some bug in Xen or OVMF, or is it expected behavior and 
> > > >> e820_host
> > > >> should be avoided?
> > > >
> > > > I don't really know.  e820_host is a gross hack which shouldn't really
> > > > be present.  The actually problem is that Linux can't cope with the
> > > > memory layout it was given (and I can't recall if there is anything
> > > > Linux could potentially to do cope).  OTOH, the toolstack, which knew
> > > > about e820_host and chose to lay the guest out in an overlapping way is
> > > > probably also at fault.
> > >
> > > The kernel can cope with lots of E820 scenarios (e.g. by relocating
> > > initrd or the p2m map), but moving itself out of the way is not
> > > possible.
> >
> > I'm afraid I need to resurrect this thread...
> >
> > With recent kernel (6.6+), the host_e820=0 workaround is not an option
> > anymore. It makes Linux not initialize xen-swiotlb (due to
> > f9a38ea5172a3365f4594335ed5d63e15af2fd18), so PCI passthrough doesn't
> > work at all. While I can add yet another layer of workaround (force
> > xen-swiotlb with iommu=soft), that's getting unwieldy.
> >
> > Furthermore, I don't get the crash message anymore, even with debug
> > hypervisor and guest_loglvl=all. Not even "Domain X crashed" in `xl
> > dmesg`. It looks like the "crash" shutdown reason doesn't reach Xen, and
> > it's considered clean shutdown (I can confirm it by changing various
> > `on_*` settings (via libvirt) and observing which gets applied).
> >
> > Most tests I've done with 6.7-rc1, but the issue I observed on 6.6.1
> > already.
> >
> > This is on Xen 4.17.2. And the L0 is running Linux 6.6.1, and then uses
> > QEMU 8.1.2 + OVMF 202308 to run Xen as L1.
> >
> 
> So basically you start the domain and it looks like it's shutting down
> cleanly from logs.
> Ca

Re: [PATCH v3 09/14] xen/asm-generic: introduce generic header altp2m.h

[Oleksii]

On Fri, 2023-11-24 at 14:18 +0100, Jan Beulich wrote:
> On 24.11.2023 14:00, Michal Orzel wrote:
> > On 17/11/2023 13:24, Oleksii Kurochko wrote:
> > > --- a/xen/arch/arm/include/asm/altp2m.h
> > > +++ /dev/null
> > > @@ -1,39 +0,0 @@
> > > -/*
> > > - * Alternate p2m
> > > - *
> > > - * Copyright (c) 2014, Intel Corporation.
> > Shouldn't this copyright be moved to generic header as well?
> 
> In earlier communication I raised the question whether such trivial
> stubs are actually copyright-able.
Could you please give a link? I couldn't remember what was the decision
and why.

Thanks in advance.

~ Oleksii

Re: [PATCH] x86/x2apic: introduce a mixed physical/cluster mode

[Andrew Cooper]

On 24/11/2023 7:54 pm, Neowutran wrote:
> Hi, 
> I did some more tests and research, indeed this patch improved/solved my 
> specific case. 
>
> Starting point: 
>
> I am using Xen version 4.17.2 (exactly this source 
> https://github.com/QubesOS/qubes-vmm-xen).
> In the bios (a Asus motherboard), I configured the "local apic" parameter to 
> "X2APIC".
> For Xen, I did not set the parameter "x2apic-mode" nor the parameter 
> "x2apic_phys". 
>
> Case 1:
> I tryied to boot just like that, result: system is unusuably slow
>
> Case 2:
> Then, I applied a backport of the patch  
> https://lore.kernel.org/xen-devel/20231106142739.19650-1-roger@citrix.com/raw
>  
> to the original Xen version of QubesOS and I recompiled. 
> (https://github.com/neowutran/qubes-vmm-xen/blob/x2apic3/X2APIC.patch)
> Result: it work, the system is usable. 
>
> Case 3:
> Then, I applied the patch 
> https://github.com/xen-project/xen/commit/26a449ce32cef33f2cb50602be19fcc0c4223ba9
> to the original Xen version of QubesOS and I recompiled.
> (https://github.com/neowutran/qubes-vmm-xen/blob/x2apic4/X2APIC.patch)
> Result: system is  
> unusuably slow. 
>
>
> In "Case 2", the value returned by the function "apic_x2apic_probe" is 
> "&apic_x2apic_mixed". 
> In "Case 3", the value returned by the function "apic_x2apic_probe" is 
> "&apic_x2apic_cluster". 
>
>
> ---
> If you want / need, details for the function "apic_x2apic_probe":
>
> Known "input" value:
>
> "CONFIG_X2APIC_PHYSICAL" is not defined
> "iommu_intremap == iommu_intremap_off" = false
> "acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL" -> 0
> "acpi_gbl_FADT.flags" = 247205 (in decimal)
> "CONFIG_X2APIC_PHYSICAL" is not defined
> "CONFIG_X2APIC_MIXED" is defined, because it is the default choice
> "x2apic_mode" = 0
> "x2apic_phys" = -1
>
>
>
> Trace log (I did some call "printk" to trace what was going on)
> Case 2:
> (XEN) NEOWUTRAN: X2APIC_MODE: 0 
> (XEN) NEOWUTRAN: X2APIC_PHYS: -1 
> (XEN) NEOWUTRAN: acpi_gbl_FADT.flags: 247205 
> (XEN) NEOWUTRAN IOMMU_INTREMAP: different 
> (XEN) Neowutran: PASSE 2 
> (XEN) Neowutran: PASSE 4 
> (XEN) NEOWUTRAN: X2APIC_MODE: 3 
> (XEN) Neowutran: PASSE 7 
> (XEN) NEOWUTRAN: X2APIC_MODE: 3 
>  
> (XEN) NEOWUTRAN: X2APIC_PHYS: -1 
> (XEN) NEOWUTRAN: acpi_gbl_FADT.flags: 247205 
> (XEN) NEOWUTRAN IOMMU_INTREMAP: different 
>
> Case 3:
> (XEN) NEOWUTRAN2: X2APIC_PHYS: -1 
> (XEN) NEOWUTRAN2: acpi_gbl_FADT.flags: 247205 
> (XEN) NEOWUTRAN2 IOMMU_INTREMAP: different 
> (XEN) Neowutran2: Passe 1 
> (XEN) NEOWUTRAN2: X2APIC_PHYS: 0 
> (XEN) Neowutran2: Passe 6 
> (XEN) Neowutran2: Passe 7 
> (XEN) NEOWUTRAN2: X2APIC_PHYS: 0 
> (XEN) NEOWUTRAN2: acpi_gbl_FADT.flags: 247205 
> (XEN) NEOWUTRAN2 IOMMU_INTREMAP: different 
> (XEN) Neowutran2: Passe 2 
> (XEN) Neowutran2: Passe 4 
> (XEN) Neowutran2: Passe 7
>
>
>
> If you require the full logs, I could publish the full logs somewhere.
> --
>
> ( However I do not understand if the root issue is a buggy motherboard, a
> bug in xen, or if the parameter "X2APIC_PHYSICAL" should have been set
> by the QubesOS project, or something else)

Hello,

Thankyou for the analysis.

For your base version of QubeOS Xen, was that 4.13.2-5 ?   I can't see
any APIC changes in the patchqueue, and I believe all relevant bugfixes
are in 4.17.2, but I'd just like to confirm.

First, by "unusable slow", other than the speed, did everything else
appear to operate adequately?  Any chance you could guess the slowdown. 
i.e. was it half the speed, or "seconds per log console line during
boot" levels of slow?


Having re-reviewed 26a449ce32, the patch is correct but the reasoning is
wrong.

ACPI_FADT_APIC_CLUSTER predates x2APIC by almost a decade (it appeared
in ACPI 3.0), and is not relevant outside of xAPIC mode.  xAPIC has 2
different logical destination modes, cluster and flat, and their
applicability is dependent on whether you have fewer or more than 8
local APICs, hence that property being called out in the ACPI spec.

x2APIC does not have this property.  DFR was removed from the
architecture, and logical mode is strictly cluster.  So the bit should
never have been interpreted on an x2APIC code path.

Not that it matters in your case - the bit isn't set in your FADT, hence
why case 1 and 3 have the same behaviour.


This brings us to case 2, where mixed mode does seem to resolve the perf
problem.

Since that patch was written, I've learnt how cluster delivery mode
works for external interrupts, and Xen should never ever have been using
it (Xen appears to be alone in OS software here).  For an external
interrupt in Logical cluster mode, it always sends to the lowest ID in
the cluster.  If that APIC decides that the local processor is too busy
to handle the interrupt now, it forwards the interrupt to the next APIC
in the cluster, and this cycle continues until one APIC accepts the message.

You get most interrupts hitting the lowest APIC in the cluster, but the
interrupt can be forwarded 

[xen-unstable test] 183866: trouble: blocked/broken

[osstest service owner]

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64  broken
 build-amd64-prev broken
 build-amd64-pvopsbroken
 build-amd64-xsm  broken
 build-amd64-xtf  broken
 build-arm64  broken
 build-arm64-pvopsbroken
 build-arm64-xsm  broken
 build-armhf  broken
 build-armhf-pvopsbroken
 build-i386   broken
 build-i386-prev  broken
 build-i386-pvops broken
 build-i386-xsm   broken
 build-i386-pvops  4 host-install(4)broken REGR. vs. 183860
 build-i386-prev   4 host-install(4)broken REGR. vs. 183860
 build-i386-xsm4 host-install(4)broken REGR. vs. 183860
 build-i3864 host-install(4)broken REGR. vs. 183860
 build-arm64-xsm   4 host-install(4)broken REGR. vs. 183860
 build-arm64   4 host-install(4)broken REGR. vs. 183860
 build-arm64-pvops 4 host-install(4)broken REGR. vs. 183860
 build-amd64-pvops 4 host-install(4)broken REGR. vs. 183860
 build-amd64-prev  4 host-install(4)broken REGR. vs. 183860
 build-amd64-xsm   4 host-install(4)broken REGR. vs. 183860
 build-amd64-xtf   4 host-install(4)broken REGR. vs. 183860
 build-armhf-pvops 4 host-install(4)broken REGR. vs. 183860
 build-amd64   4 host-install(4)broken REGR. vs. 183860
 build-armhf   4 host-install(4)broken REGR. vs. 183860

Tests which did not succeed, but are not blocking:
 test-amd64-coresched-amd64-xl  1 build-check(1)   blocked  n/a
 test-amd64-coresched-i386-xl  1 build-check(1)   blocked  n/a
 test-amd64-i386-examine   1 build-check(1)   blocked  n/a
 test-amd64-i386-examine-bios  1 build-check(1)   blocked  n/a
 test-amd64-i386-examine-uefi  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-i386-libvirt-raw   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-amd64-i386-livepatch 1 build-check(1)   blocked  n/a
 test-amd64-i386-migrupgrade   1 build-check(1)   blocked  n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-qemut-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-qemut-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-qemuu-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-amd64-i386-xl1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-pvshim 1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-debianhvm-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked 
n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 test-amd64-amd64-xl-rtds  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ws16-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm  1 build-check(1) blocke

Re: [PATCH v2 2/2] Mini-OS: keep a positive list of externally visible symbols

[Andrew Cooper]

On 23/11/2023 11:45 am, Juergen Gross wrote:
> diff --git a/mini-os.map b/mini-os.map
> new file mode 100644
> index ..58a3a0ee
> --- /dev/null
> +++ b/mini-os.map
> @@ -0,0 +1,295 @@
> +# Mini-OS symbols being externally visible

\n

> +# entry point
> +_start

\n

etc.  A few blank lines go a long way in terms of clarity.

~Andrew

Re: [PATCH v4] xen/x86: On x2APIC mode, derive LDR from APIC ID

[Alejandro Vallejo]

On 24/11/2023 22:05, Andrew Cooper wrote:

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 5cb87f8649..cd4701c5a2 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -1061,13 +1061,26 @@ static const struct hvm_mmio_ops vlapic_mmio_ops = {
  .write = vlapic_mmio_write,
  };
  
+static uint32_t x2apic_ldr_from_id(uint32_t id)

+{
+return ((id & ~0xf) << 12) | (1 << (id & 0xf));
+}
+
  static void set_x2apic_id(struct vlapic *vlapic)
  {


const struct vcpu *v = vlapic_vcpu(vlapic);

seeing as you've got the expression used twice already.

With that, the following logic is shorter too; you can get away with
dropping the vcpu_id variable as v->vcpu_id is the more common form to
use in Xen.


Twice? I can see a vague raison-d'etre in lapic_load_fixup(), but
there's a single occurence here.




  We must preserve LDRs so new vCPUs use consistent
+ * derivations and existing guests, which may have already read the
+ * LDR at the source host, aren't surprised when interrupts stop
+ * working the way they did at the other end.
   */
-if ( GET_xAPIC_ID(id) != vlapic_vcpu(vlapic)->vcpu_id * 2 ||
- id != SET_xAPIC_ID(GET_xAPIC_ID(id)) )
-printk(XENLOG_G_WARNING "%pv: bogus APIC ID %#x loaded\n",
-   vlapic_vcpu(vlapic), id);
-set_x2apic_id(vlapic);
-}
-else /* Undo an eventual earlier fixup. */
-{
-vlapic_set_reg(vlapic, APIC_ID, id);
-vlapic_set_reg(vlapic, APIC_LDR, vlapic->loaded.ldr);
-}
+vlapic_domain(vlapic)->arch.hvm.bug_x2apic_ldr_vcpu_id = true;
+else
+printk(XENLOG_G_WARNING
+   "%pv: bogus x2APIC loaded id=%#x ldr=%#x (expected ldr=%#x)\n",


%pv: bogus loaded x2APIC ID %#x, LDR %#x, expected LDR %#x\n

If you properly capitalise x2APIC, you should capitalise ID and LDR.
The other changes are matter of taste, but make for a less cluttered log
message IMO.



"bogus x2APIC loaded" is meant to be a sentence followed by key-value 
pairs. Uppercasing the keys is fine (albeit unnecessary, IMO), but you 
choice of word order feels backwards.




This is a long list of minor niggles, but they're all style/comment
issues, and nothing to do with the logic itself.  I'm happy to fix them
all up on commit, and here is the result with them merged:

https://xenbits.xen.org/gitweb/?p=people/andrewcoop/xen.git;a=commitdiff;h=953dcb0317d20959ffee14e404595cfbb66c607a

for you to glance over.

~Andrew


Except for the 2 bits pointed out, the others are cosmetic changes
I don't particularly mind. Jan spotted a typo in the second comment 
block in lapic_load_fixup() that should be corrected too.

(s/fix fix/fix/)

Cheers,
Alejandro

Re: [PATCH v3 09/14] xen/asm-generic: introduce generic header altp2m.h

[Jan Beulich]

On 27.11.2023 12:48, Oleksii wrote:
> On Fri, 2023-11-24 at 14:18 +0100, Jan Beulich wrote:
>> On 24.11.2023 14:00, Michal Orzel wrote:
>>> On 17/11/2023 13:24, Oleksii Kurochko wrote:
 --- a/xen/arch/arm/include/asm/altp2m.h
 +++ /dev/null
 @@ -1,39 +0,0 @@
 -/*
 - * Alternate p2m
 - *
 - * Copyright (c) 2014, Intel Corporation.
>>> Shouldn't this copyright be moved to generic header as well?
>>
>> In earlier communication I raised the question whether such trivial
>> stubs are actually copyright-able.
> Could you please give a link?

I'm sorry, but no, I don't recall in the context of which patch this was.

Jan

> I couldn't remember what was the decision
> and why.
> 
> Thanks in advance.
> 
> ~ Oleksii

Re: [PATCH v2 2/2] Mini-OS: keep a positive list of externally visible symbols

[Juergen Gross]

On 27.11.23 13:07, Andrew Cooper wrote:

On 23/11/2023 11:45 am, Juergen Gross wrote:

diff --git a/mini-os.map b/mini-os.map
new file mode 100644
index ..58a3a0ee
--- /dev/null
+++ b/mini-os.map
@@ -0,0 +1,295 @@
+# Mini-OS symbols being externally visible


\n


+# entry point
+_start


\n

etc.  A few blank lines go a long way in terms of clarity.


V3 has gone out today, replacing the global list with EXPORT_SYMBOL()
directives sprinkled over the code.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature.asc
Description: OpenPGP digital signature

Re: [PATCH v4] xen/x86: On x2APIC mode, derive LDR from APIC ID

[Alejandro Vallejo]

On 27/11/2023 08:40, Jan Beulich wrote:

On 23.11.2023 18:30, Alejandro Vallejo wrote:

@@ -1498,27 +1511,36 @@ static int cf_check lapic_save_regs(struct vcpu *v, 
hvm_domain_context_t *h)
   */
  static void lapic_load_fixup(struct vlapic *vlapic)
  {
-uint32_t id = vlapic->loaded.id;
+uint32_t good_ldr = x2apic_ldr_from_id(vlapic->loaded.id);
  
-if ( vlapic_x2apic_mode(vlapic) && id && vlapic->loaded.ldr == 1 )

-{
+/* Skip fixups on xAPIC mode, or if the x2APIC LDR is already correct */
+if ( !vlapic_x2apic_mode(vlapic) ||
+ (vlapic->loaded.ldr == good_ldr) )
+return;
+
+if ( vlapic->loaded.ldr == 1 )
+   /*
+* Xen <= 4.4 may have a bug by which all the APICs configured in
+* x2APIC mode got LDR = 1. We can't leave it as-is because it
+* assigned the same LDR to every CPU.  We'll fix fix the bug now
+* and assign an LDR value consistent with the APIC ID.
+*/


Just one comment on top of Andrew's: Is the double "fix" really intended
here? (I could see it might be, but then "fix the bug fix" would read
slightly more smoothly to me as a non-native speaker.)


It's not intended indeed. s/fix fix/fix/



Another aspect here is what exactly the comment states (and does not
state). Original comments made explicit that LDR == 1 contradicts ID == 0.
In the new comment you only emphasize that all CPUs cannot have that same
LDR. But the value of 1 being bogus in the first place doesn't become clear
anymore.

Jan


1 is bogus for id!=0, but so would be 3, 7 or 42. In particular we have
ID==2 contradicting LDR=2, and we're allowing it. The reason why we must
fix this other case is because all LDRs are equal, otherwise it would
get the same treatment as the other bug.

Cheers,
Alejandro

Re: Clang-format configuration discussion - pt 2

[Jan Beulich]

On 24.11.2023 15:52, Luca Fancellu wrote:
>> On 24 Nov 2023, at 12:47, Jan Beulich  wrote:
>> On 23.11.2023 15:47, Luca Fancellu wrote:
>>> Let’s continue the discussion about clang-format configuration, this is 
>>> part 2, previous discussions are:
>>>
>>> - https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg00498.html
>>>
>>> You can find the serie introducing clang-format here:
>>> https://patchwork.kernel.org/project/xen-devel/cover/20231031132304.2573924-1-luca.fance...@arm.com/
>>> and there is also a patch linked to my gitlab account where you can find 
>>> the output for the hypervisor code.
>>>
>>> For a full list of configurables and to find the possible values for them, 
>>> please refer to this page:
>>> https://clang.llvm.org/docs/ClangFormatStyleOptions.html
>>>
>>> --
>>>
>>> Our coding style doesn’t mention anything about alignment, shall we add a 
>>> new section?
>>> I can send patches when we reach agreement on each of these rules.
>>>
>>>
>>> QualifierAlignment: Custom
>>> QualifierOrder: ['static', 'inline', 'const', 'volatile', 'type']
>>>
>>> ---
>>> For “QualifierAlignment” I chose Custom in order to apply in 
>>> “QualifierOrder” an order for the
>>> qualifiers that match the current codebase, we could specify also “Leave” 
>>> in order to keep
>>> them as they are.
>>
>> Where do attributes go in this sequence?
> 
> I think function declaration/definition and variables.

How does this relate to my question? I asked about the sequence of elements
listed for QualifierOrder:, where attributes don't appear at all right now.

>>> --
>>>
>>> AlignAfterOpenBracket: Align
>>>
>>> ---
>>> This one is to align function parameters that overflows the line length, I 
>>> chose to align them
>>> to the open bracket to match the current codebase (hopefully)
>>>
>>> e.g.:
>>> someLongFunction(argument1,
>>>argument2);
>>
>> The above matches neither of the two generally permitted styles:
>>
>>someLongFunction(argument1,
>> argument2);
>>
>>someLongFunction(
>>argument1,
>>argument2);
>>
>> Then again from its name I would infer this isn't just about function
>> arguments?
> 
> I think it applies to parameters and arguments of functions and macro, given 
> the description in the docs.
> 
> I see your two snippets above but I’ve always found at least on arm a 
> predominance of
> the style above for functions, so arguments aligned after the opening 
> bracket, for macros
> there is a mix.

The latter "above" refers to which form exactly? The one you originally
spelled out, or the former of what my reply had?

> I might be wrong though and so another opinion from another maintainer would 
> help.
> 
> In any case we can choose among many value: 
> https://clang.llvm.org/docs/ClangFormatStyleOptions.html#alignafteropenbracket,
> but when we do so, we need to stick to one permitted style only, the tool 
> don’t allow to specify more than one.

On top of my earlier reply yet another reason perhaps that this tool then
won't really fit our intended use.

>>> --
>>>
>>> AlignArrayOfStructures: Left
>>>
>>> ---
>>> “When using initialization for an array of structs aligns the fields into 
>>> columns."
>>> It’s important to say that even if we specify “None”, it is going to format 
>>> the data structure anyway,
>>> I choose left, but clearly I’m open to suggestions.
>>
>> You don't say in which way it re-formats such constructs.
> 
> Sure, taking as example an array of structure, xen/drivers/video/modelines.h,
> 
> With AlignArrayOfStructures: None we have this below.
> 
> diff --git a/xen/drivers/video/modelines.h b/xen/drivers/video/modelines.h
> index 9cb7cdde055f..3ff23ef1f8a7 100644
> --- a/xen/drivers/video/modelines.h
> +++ b/xen/drivers/video/modelines.h
> @@ -42,36 +42,36 @@ struct modeline {
>  };
>  
>  struct modeline __initdata videomodes[] = {
> -{ "640x480@60",   25175,  640,  16,   96,   48,   480,  11,   2,31 },
> -{ "640x480@72",   31500,  640,  24,   40,   128,  480,  9,3,28 },
> -{ "640x480@75",   31500,  640,  16,   96,   48,   480,  11,   2,32 },
> -{ "640x480@85",   36000,  640,  32,   48,   112,  480,  1,3,25 },
> -{ "800x600@56",   38100,  800,  32,   128,  128,  600,  1,4,14 },
> -{ "800x600@60",   4,  800,  40,   128,  88 ,  600,  1,4,23 },
> -{ "800x600@72",   5,  800,  56,   120,  64 ,  600,  37,   6,23 },
> -{ "800x600@75",   49500,  800,  16,   80,   160,  600,  1,2,21 },

Re: [PATCH v4] xen/x86: On x2APIC mode, derive LDR from APIC ID

[Andrew Cooper]

On 27/11/2023 12:08 pm, Alejandro Vallejo wrote:
> On 24/11/2023 22:05, Andrew Cooper wrote:
>>> diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
>>> index 5cb87f8649..cd4701c5a2 100644
>>> --- a/xen/arch/x86/hvm/vlapic.c
>>> +++ b/xen/arch/x86/hvm/vlapic.c
>>> @@ -1061,13 +1061,26 @@ static const struct hvm_mmio_ops
>>> vlapic_mmio_ops = {
>>>   .write = vlapic_mmio_write,
>>>   };
>>>   +static uint32_t x2apic_ldr_from_id(uint32_t id)
>>> +{
>>> +    return ((id & ~0xf) << 12) | (1 << (id & 0xf));
>>> +}
>>> +
>>>   static void set_x2apic_id(struct vlapic *vlapic)
>>>   {
>>
>> const struct vcpu *v = vlapic_vcpu(vlapic);
>>
>> seeing as you've got the expression used twice already.
>>
>> With that, the following logic is shorter too; you can get away with
>> dropping the vcpu_id variable as v->vcpu_id is the more common form to
>> use in Xen.
>
> Twice? I can see a vague raison-d'etre in lapic_load_fixup(), but
> there's a single occurence here.

It's hidden in the vlapic_domain(), which is vlacpi_vcpu()->domain.

>
>>
>>>   We must preserve LDRs so new vCPUs use consistent
>>> + * derivations and existing guests, which may have already
>>> read the
>>> + * LDR at the source host, aren't surprised when interrupts
>>> stop
>>> + * working the way they did at the other end.
>>>    */
>>> -    if ( GET_xAPIC_ID(id) != vlapic_vcpu(vlapic)->vcpu_id * 2 ||
>>> - id != SET_xAPIC_ID(GET_xAPIC_ID(id)) )
>>> -    printk(XENLOG_G_WARNING "%pv: bogus APIC ID %#x loaded\n",
>>> -   vlapic_vcpu(vlapic), id);
>>> -    set_x2apic_id(vlapic);
>>> -    }
>>> -    else /* Undo an eventual earlier fixup. */
>>> -    {
>>> -    vlapic_set_reg(vlapic, APIC_ID, id);
>>> -    vlapic_set_reg(vlapic, APIC_LDR, vlapic->loaded.ldr);
>>> -    }
>>> +    vlapic_domain(vlapic)->arch.hvm.bug_x2apic_ldr_vcpu_id = true;
>>> +    else
>>> +    printk(XENLOG_G_WARNING
>>> +   "%pv: bogus x2APIC loaded id=%#x ldr=%#x (expected
>>> ldr=%#x)\n",
>>
>> %pv: bogus loaded x2APIC ID %#x, LDR %#x, expected LDR %#x\n
>>
>> If you properly capitalise x2APIC, you should capitalise ID and LDR.
>> The other changes are matter of taste, but make for a less cluttered log
>> message IMO.
>>
>
> "bogus x2APIC loaded" is meant to be a sentence followed by key-value
> pairs. Uppercasing the keys is fine (albeit unnecessary, IMO), but you
> choice of word order feels backwards.

The grammar is awkward either way.

How about "bogus x2APIC record: "

?

That is much clearer I think.

~Andrew

Re: [PATCH v4] xen/x86: On x2APIC mode, derive LDR from APIC ID

[Jan Beulich]

On 27.11.2023 13:17, Alejandro Vallejo wrote:
> On 27/11/2023 08:40, Jan Beulich wrote:
>> On 23.11.2023 18:30, Alejandro Vallejo wrote:
>>> @@ -1498,27 +1511,36 @@ static int cf_check lapic_save_regs(struct vcpu *v, 
>>> hvm_domain_context_t *h)
>>>*/
>>>   static void lapic_load_fixup(struct vlapic *vlapic)
>>>   {
>>> -uint32_t id = vlapic->loaded.id;
>>> +uint32_t good_ldr = x2apic_ldr_from_id(vlapic->loaded.id);
>>>   
>>> -if ( vlapic_x2apic_mode(vlapic) && id && vlapic->loaded.ldr == 1 )
>>> -{
>>> +/* Skip fixups on xAPIC mode, or if the x2APIC LDR is already correct 
>>> */
>>> +if ( !vlapic_x2apic_mode(vlapic) ||
>>> + (vlapic->loaded.ldr == good_ldr) )
>>> +return;
>>> +
>>> +if ( vlapic->loaded.ldr == 1 )
>>> +   /*
>>> +* Xen <= 4.4 may have a bug by which all the APICs configured in
>>> +* x2APIC mode got LDR = 1. We can't leave it as-is because it
>>> +* assigned the same LDR to every CPU.  We'll fix fix the bug now
>>> +* and assign an LDR value consistent with the APIC ID.
>>> +*/
>>
>> Just one comment on top of Andrew's: Is the double "fix" really intended
>> here? (I could see it might be, but then "fix the bug fix" would read
>> slightly more smoothly to me as a non-native speaker.)
> 
> It's not intended indeed. s/fix fix/fix/
> 
>>
>> Another aspect here is what exactly the comment states (and does not
>> state). Original comments made explicit that LDR == 1 contradicts ID == 0.
>> In the new comment you only emphasize that all CPUs cannot have that same
>> LDR. But the value of 1 being bogus in the first place doesn't become clear
>> anymore.
> 
> 1 is bogus for id!=0, but so would be 3, 7 or 42.

Yet 3, 7, and 42 aren't interesting in the context of that older bug.

> In particular we have
> ID==2 contradicting LDR=2, and we're allowing it. The reason why we must
> fix this other case is because all LDRs are equal, otherwise it would
> get the same treatment as the other bug.

I understand all that; still there's loss of information in the comments,
from my perspective.

Jan

Re: [PATCH v2 08/15] VMX: convert vmx_basic_msr

[Jan Beulich]

On 24.11.2023 23:41, Andrew Cooper wrote:
> On 24/11/2023 8:41 am, Jan Beulich wrote:
>> ... to a struct field, which is then going to be accompanied by other
>> capability/control data presently living in individual variables. As
>> this structure isn't supposed to be altered post-boot, put it in
>> .data.ro_after_init right away.
>>
>> Suggested-by: Roger Pau Monné 
>> Signed-off-by: Jan Beulich 
> 
> For (usable) nested virt, we're going to need the VMX MSRs, in their
> architectural form, in struct cpu_policy.  And just like CPUID features,
> I want it to end up with nice bitfields to use.
> 
> Looking through the rest of this series, vmx_caps ends up almost in
> architectural form.
> 
> Could I talk you into having a "struct vmx_msrs" (or similar - 'caps'
> doesn't feel quite right here) in the policy object, and also
> instantiating one instance of it for this purpose here?

I was actually wondering while doing the conversion. The main reason I
didn't go that route right away was that I wasn't really certain whether
what I'd put there would the really be the (largely) final shape it
wants to take there. (One thing you've likely noticed I didn't convert
is _vmx_misc_cap, which right now only exists as a local variable in
vmx_init_vmcs_config().)

> AFAICT, it would only be a minor deviation to the latter half of this
> series, but it would be an excellent start to fixing nested virt - and
> getting this data in the policy really is the first task in getting the
> ball rolling on nested virt.

How much of a further change it would end up being (or where that change
would occur) depends on another aspect: When put in cpu-policy.h (and I
take it you mean the lib/ instance, not the asm/ one), it would seem
natural and perhaps even necessary to properly introduce bitfields for
each of the MSRs right away. That'll lead to a "raw" field as well. In
VMX code (mostly its cpu_has_* #define-s), I'd then either need to use
.raw (perhaps a little ugly here and there) or go with using the
individual bitfields right away (likely eliminating the need for many of
the constant #define-s), which increases the risk of inadvertent mistakes
(and their overlooking during review).

> I don't mind about serialising/de-serialsing it - that still has a bit
> of userspace complexity to work out, and depends on some of the cleanup
> still needing a repost.
> 
> If you don't want to take the added space in cpu_policy yet, how about
> having the declaration there and just forgo instantiating the subobject
> in the short term?

There's quite a bit of effectively dead space in the struct already; I
think I wouldn't mind instantiating the struct there right away. So long
as you're convinced it's going to be used there in not too distant a
future.

But: If I go as far, why would I introduce a global instance of the new
struct? Wouldn't it then make more sense to use host_cpu_policy right
away? I probably would keep populating it in vmx_init_vmcs_config() to
limit churn for now, but consumers of the flags could then right away
use the host policy.

Jan

Re: We are not able to virtualize FreeBSD using xen 4.17 on Arm 32 bit

[Mario Marietto]

@Chuck Zmudzinski  : Stay tuned. They want to help
us. The xen developers are great. Very good support for us. I'm sure that
you can give a good contribution to understand what's our problem and how
to implement a fix with the help of all those good guys.

On Mon, Nov 27, 2023 at 11:56 AM Roger Pau Monné 
wrote:

> On Mon, Nov 27, 2023 at 10:28:13AM +, Henry Wang wrote:
> > +(xen-devel and Arm maintainers, including Julien)
> >
> > > On Nov 27, 2023, at 18:03, Mario Marietto 
> > > wrote:
> > >
> > > Hello.  We have just virtualized Debian 12 on our arm (32 bit)
> > > Chromebook model xe303c12 . As host / dom0 we have chosen Devuan
> > > 5,and for guest / domU,Debian 12. It works great. But our goal is
> > > different. We want to virtualize FreeBSD as domU. Can we have a
> > > working Xen PV network driver for a FreeBSD arm guest ?. I found
> > > that Julien Grall has ported the Xen drivers to FreeBSD on arm. I
> > > would like to know if Julien's work was accepted upstream by
> > > FreeBSD, in which case FreeBSD as a Xen guest on arm should work
> > > if we enable the Xen PV drivers in the FreeBSD on arm kernel. If
> > > Julien's work was not accepted upstream by FreeBSD, we will have
> > > to find his patches and apply them ourselves to the FreeBSD on arm
> > > kernel.
>
> I've added Elliot on Cc as he is working on upstreaming the patches to
> FreeBSD.  He will be able to provide a better update than myself.
>
> Regards, Roger.
>


-- 
Mario.

[PATCH] xen/events: fix error codes in xen_bind_pirq_msi_to_irq()

[Dan Carpenter]

The error code needs to be set on these error paths.

Fixes: 5dd9ad32d775 ("xen/events: drop xen_allocate_irqs_dynamic()")
Fixes: d2ba3166f23b ("xen/events: move drivers/xen/events.c into 
drivers/xen/events/")
Signed-off-by: Dan Carpenter 
---
Are we going to backport these to stable?  Should I split this into two
patches?

 drivers/xen/events/events_base.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index f5edb9e27e3c..aae62603b461 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -1105,13 +1105,17 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, 
struct msi_desc *msidesc,
mutex_lock(&irq_mapping_update_lock);
 
irq = irq_alloc_descs(-1, 0, nvec, -1);
-   if (irq < 0)
+   if (irq < 0) {
+   ret = irq;
goto out;
+   }
 
for (i = 0; i < nvec; i++) {
info = xen_irq_init(irq + i);
-   if (!info)
+   if (!info) {
+   ret = -ENOMEM;
goto error_irq;
+   }
 
irq_set_chip_and_handler_name(irq + i, &xen_pirq_chip, 
handle_edge_irq, name);
 
-- 
2.42.0

Re: [PATCH block/for-next v2 01/16] block: add a new helper to get inode from block_device

[Yu Kuai]

Hi,

在 2023/11/27 15:21, Christoph Hellwig 写道:

On Mon, Nov 27, 2023 at 02:21:01PM +0800, Yu Kuai wrote:

From: Yu Kuai 

block_devcie is allocated from bdev_alloc() by bdev_alloc_inode(), and
currently block_device contains a pointer that point to the address of
inode, while such inode is allocated together:


This is going the wrong way.  Nothing outside of core block layer code
should ever directly use the bdev inode.  We've been rather sloppy
and added a lot of direct reference to it, but they really need to
go away and be replaced with well defined high level operation on
struct block_device.  Once that is done we can remove the bd_inode
pointer, but replacing it with something that pokes even more deeply
into bdev internals is a bad idea.


Thanks for the advice, however, after collecting how other modules are
using bdev inode, I got two main questions:

1) Is't okay to add a new helper to pass in bdev for following apis?
If so, then almost all the fs and driver can avoid to access bd_inode
dirctly.

errseq_check(&bdev->bd_inode->i_mapping->wb_err, wb_err);
errseq_check_and_advance(&bdev->bd_inode->i_mapping->wb_err, &wb_err);
mapping_gfp_constraint(bdev->bd_inode->i_mapping, gfp);
i_size_read(bdev->bd_inode)
find_get_page(bdev->bd_inode->i_mapping, offset);
find_or_create_page(bdev->bd_inode->i_mapping, index, gfp);
read_cache_page_gfp(bdev->bd_inode->i_mapping, index, gfp);
invalidate_inode_pages2(bdev->bd_inode->i_mapping);
invalidate_inode_pages2_range(bdev->bd_inode->i_mapping, start, end);
read_mapping_folio(bdev->bd_inode->i_mapping, index, file);
read_mapping_page(bdev->bd_inode->i_mapping, index, file);
balance_dirty_pages_ratelimited(bdev->bd_inode->i_mapping)
file_ra_state_init(ra, bdev->bd_inode->i_mapping);
page_cache_sync_readahead(bdev->bd_inode->i_mapping, ra, file, index, 
req_count);

inode_to_bdi(bdev->bd_inode)

2) For the file fs/buffer.c, there are some special usage like
following that I don't think it's good to add a helper:

spin_lock(&bd_inode->i_mapping->private_lock);

Is't okay to move following apis from fs/buffer.c directly to
block/bdev.c?

__find_get_block
bdev_getblk

Thanks,
Kuai


.

Re: [PATCH] xen/events: fix error codes in xen_bind_pirq_msi_to_irq()

[Juergen Gross]

On 27.11.23 13:57, Dan Carpenter wrote:

The error code needs to be set on these error paths.

Fixes: 5dd9ad32d775 ("xen/events: drop xen_allocate_irqs_dynamic()")
Fixes: d2ba3166f23b ("xen/events: move drivers/xen/events.c into 
drivers/xen/events/")


Please drop the last Fixes: tag. Said patch didn't introduce any new problem.


Signed-off-by: Dan Carpenter 
---
Are we going to backport these to stable?  Should I split this into two
patches?

  drivers/xen/events/events_base.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index f5edb9e27e3c..aae62603b461 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -1105,13 +1105,17 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, 
struct msi_desc *msidesc,
mutex_lock(&irq_mapping_update_lock);
  
  	irq = irq_alloc_descs(-1, 0, nvec, -1);

-   if (irq < 0)
+   if (irq < 0) {
+   ret = irq;
goto out;
+   }


Why? The return value for the out: label is in irq.

  
  	for (i = 0; i < nvec; i++) {

info = xen_irq_init(irq + i);
-   if (!info)
+   if (!info) {
+   ret = -ENOMEM;
goto error_irq;
+   }


It would be easier to just preset ret with -ENOMEM when defining it.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature.asc
Description: OpenPGP digital signature

Re: [XEN PATCH v4 1/2] x86/mm: preparation work to uniform modify_xen_mappings* interfaces

[Jan Beulich]

On 22.11.2023 23:20, Stefano Stabellini wrote:
> On Wed, 22 Nov 2023, Federico Serafini wrote:
>> The objective is to use parameter name "nf" to denote "new flags"
>> in all the modify_xen_mappings* functions.
>> Since modify_xen_mappings_lite() is currently using "nf" as identifier
>> for a local variable, bad things could happen if new uses of such
>> variable are committed while a renaming patch is waiting for the
>> approval.
>> To avoid such danger, as first thing rename the local variable from
>> "nf" to "flags".
>>
>> No functional change.
>>
>> Suggested-by: Jan Beulich 
>> Signed-off-by: Federico Serafini 
> 
> Reviewed-by: Stefano Stabellini 

Acked-by: Jan Beulich 

Re: [XEN PATCH v4 2/2] xen/mm: address violations of MISRA C:2012 Rules 8.2 and 8.3

[Jan Beulich]

On 22.11.2023 23:22, Stefano Stabellini wrote:
> On Wed, 22 Nov 2023, Federico Serafini wrote:
>> Add missing parameter names and uniform the interfaces of
>> modify_xen_mappings() and modify_xen_mappings_lite().
>>
>> No functional change.
>>
>> Signed-off-by: Federico Serafini 
> 
> Reviewed-by: Stefano Stabellini 

Acked-by: Jan Beulich 

Re: [PATCH v4] xen/x86: On x2APIC mode, derive LDR from APIC ID

[Alejandro Vallejo]

On 27/11/2023 12:20, Andrew Cooper wrote:
> On 27/11/2023 12:08 pm, Alejandro Vallejo wrote:
>> On 24/11/2023 22:05, Andrew Cooper wrote:
 diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
 index 5cb87f8649..cd4701c5a2 100644
 --- a/xen/arch/x86/hvm/vlapic.c
 +++ b/xen/arch/x86/hvm/vlapic.c
 @@ -1061,13 +1061,26 @@ static const struct hvm_mmio_ops
 vlapic_mmio_ops = {
    .write = vlapic_mmio_write,
    };
    +static uint32_t x2apic_ldr_from_id(uint32_t id)
 +{
 +    return ((id & ~0xf) << 12) | (1 << (id & 0xf));
 +}
 +
    static void set_x2apic_id(struct vlapic *vlapic)
    {
>>>
>>> const struct vcpu *v = vlapic_vcpu(vlapic);
>>>
>>> seeing as you've got the expression used twice already.
>>>
>>> With that, the following logic is shorter too; you can get away with
>>> dropping the vcpu_id variable as v->vcpu_id is the more common form to
>>> use in Xen.
>>
>> Twice? I can see a vague raison-d'etre in lapic_load_fixup(), but
>> there's a single occurence here.
> 
> It's hidden in the vlapic_domain(), which is vlacpi_vcpu()->domain.
> 
>>
>>>
    We must preserve LDRs so new vCPUs use consistent
 + * derivations and existing guests, which may have already
 read the
 + * LDR at the source host, aren't surprised when interrupts
 stop
 + * working the way they did at the other end.
     */
 -    if ( GET_xAPIC_ID(id) != vlapic_vcpu(vlapic)->vcpu_id * 2 ||
 - id != SET_xAPIC_ID(GET_xAPIC_ID(id)) )
 -    printk(XENLOG_G_WARNING "%pv: bogus APIC ID %#x loaded\n",
 -   vlapic_vcpu(vlapic), id);
 -    set_x2apic_id(vlapic);
 -    }
 -    else /* Undo an eventual earlier fixup. */
 -    {
 -    vlapic_set_reg(vlapic, APIC_ID, id);
 -    vlapic_set_reg(vlapic, APIC_LDR, vlapic->loaded.ldr);
 -    }
 +    vlapic_domain(vlapic)->arch.hvm.bug_x2apic_ldr_vcpu_id = true;
 +    else
 +    printk(XENLOG_G_WARNING
 +   "%pv: bogus x2APIC loaded id=%#x ldr=%#x (expected
 ldr=%#x)\n",
>>>
>>> %pv: bogus loaded x2APIC ID %#x, LDR %#x, expected LDR %#x\n
>>>
>>> If you properly capitalise x2APIC, you should capitalise ID and LDR.
>>> The other changes are matter of taste, but make for a less cluttered log
>>> message IMO.
>>>
>>
>> "bogus x2APIC loaded" is meant to be a sentence followed by key-value
>> pairs. Uppercasing the keys is fine (albeit unnecessary, IMO), but you
>> choice of word order feels backwards.
> 
> The grammar is awkward either way.
> 
> How about "bogus x2APIC record: "
> 
> ?
> 
> That is much clearer I think.
> 
> ~Andrew

LGTM.

Do you want me to send a v5 with it all?

Cheers,
Alejandro

Cheers,
Alejandro

Re: [PATCH v4] xen/x86: On x2APIC mode, derive LDR from APIC ID

[Alejandro Vallejo]

On 27/11/2023 12:24, Jan Beulich wrote:
> On 27.11.2023 13:17, Alejandro Vallejo wrote:
>> On 27/11/2023 08:40, Jan Beulich wrote:
>>> On 23.11.2023 18:30, Alejandro Vallejo wrote:
 @@ -1498,27 +1511,36 @@ static int cf_check lapic_save_regs(struct vcpu 
 *v, hvm_domain_context_t *h)
*/
   static void lapic_load_fixup(struct vlapic *vlapic)
   {
 -uint32_t id = vlapic->loaded.id;
 +uint32_t good_ldr = x2apic_ldr_from_id(vlapic->loaded.id);
   
 -if ( vlapic_x2apic_mode(vlapic) && id && vlapic->loaded.ldr == 1 )
 -{
 +/* Skip fixups on xAPIC mode, or if the x2APIC LDR is already correct 
 */
 +if ( !vlapic_x2apic_mode(vlapic) ||
 + (vlapic->loaded.ldr == good_ldr) )
 +return;
 +
 +if ( vlapic->loaded.ldr == 1 )
 +   /*
 +* Xen <= 4.4 may have a bug by which all the APICs configured in
 +* x2APIC mode got LDR = 1. We can't leave it as-is because it
 +* assigned the same LDR to every CPU.  We'll fix fix the bug now
 +* and assign an LDR value consistent with the APIC ID.
 +*/
>>>
>>> Just one comment on top of Andrew's: Is the double "fix" really intended
>>> here? (I could see it might be, but then "fix the bug fix" would read
>>> slightly more smoothly to me as a non-native speaker.)
>>
>> It's not intended indeed. s/fix fix/fix/
>>
>>>
>>> Another aspect here is what exactly the comment states (and does not
>>> state). Original comments made explicit that LDR == 1 contradicts ID == 0.
>>> In the new comment you only emphasize that all CPUs cannot have that same
>>> LDR. But the value of 1 being bogus in the first place doesn't become clear
>>> anymore.
>>
>> 1 is bogus for id!=0, but so would be 3, 7 or 42.
> 
> Yet 3, 7, and 42 aren't interesting in the context of that older bug.
> 
>> In particular we have
>> ID==2 contradicting LDR=2, and we're allowing it. The reason why we must
>> fix this other case is because all LDRs are equal, otherwise it would
>> get the same treatment as the other bug.
> 
> I understand all that; still there's loss of information in the comments,
> from my perspective.
> 
> Jan

v2 did have a "Note that "x2apic_id == 0" has always been correct and
can't be used to discriminate these cases." and another in front of the
early exit "No need to perform fixups in non-x2apic mode, and x2apic_id
== 0 has always been correct.". They were trimmed as versions went on.

As mentioned before this is all cosmetic, so I'm happy either way. I'll
reinstate something to this effect in a v5.

Cheers,
Alejandro

[ovmf test] 183867: trouble: blocked/broken

[osstest service owner]

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64  broken
 build-amd64-pvopsbroken
 build-amd64-xsm  broken
 build-i386   broken
 build-i386-pvops broken
 build-i386-xsm   broken
 build-i3864 host-install(4)broken REGR. vs. 183825
 build-i386-xsm4 host-install(4)broken REGR. vs. 183825
 build-i386-pvops  4 host-install(4)broken REGR. vs. 183825
 build-amd64-pvops 4 host-install(4)broken REGR. vs. 183825
 build-amd64   4 host-install(4)broken REGR. vs. 183825
 build-amd64-xsm   4 host-install(4)broken REGR. vs. 183825

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf 33e31c289cc7b417f110d6da70dc8224443d32d3
baseline version:
 ovmf 8736b8fdca85e02933cdb0a13309de14c9799ece

Last test of basis   183825  2023-11-22 13:41:06 Z4 days
Testing same since   183867  2023-11-27 05:41:07 Z0 days1 attempts


People who touched revisions under test:
  xieyuanh 
  Yuanhao Xie 

jobs:
 build-amd64-xsm  broken  
 build-i386-xsm   broken  
 build-amd64  broken  
 build-i386   broken  
 build-amd64-libvirt  blocked 
 build-i386-libvirt   blocked 
 build-amd64-pvopsbroken  
 build-i386-pvops broken  
 test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked 
 test-amd64-i386-xl-qemuu-ovmf-amd64  blocked 



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

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

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

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

broken-job build-amd64 broken
broken-job build-amd64-pvops broken
broken-job build-amd64-xsm broken
broken-job build-i386 broken
broken-job build-i386-pvops broken
broken-job build-i386-xsm broken
broken-step build-i386 host-install(4)
broken-step build-i386-xsm host-install(4)
broken-step build-i386-pvops host-install(4)
broken-step build-amd64-pvops host-install(4)
broken-step build-amd64 host-install(4)
broken-step build-amd64-xsm host-install(4)

Not pushing.


commit 33e31c289cc7b417f110d6da70dc8224443d32d3
Author: xieyuanh 
Date:   Mon Nov 20 12:09:14 2023 +0800

UefiCpuPkg/MpInitLib: Update the comments of _CPU_MP_DATA.

No functional changes in this patch.

Updates the comments of _CPU_MP_DATA to delcared that duplications in
CpuMpData are present to avoid to be direct accessed and comprehended
 in assembly code. CpuMpData: Intended for use in C code while
 ExchangeInfo are used in assembly code in this module.

This patch deletes the unnecessary comments in CpuMpData, since
CpuMpData is no longer responsible for passing information from PEI to
DXE.

Signed-off-by: Yuanhao Xie 
Cc: Laszlo Ersek ler...@redhat.com
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 

commit cb3f41a9378822a0bcf0febf898af254b4994b8d
Author: Yuanhao Xie 
Date:   Fri Nov 10 16:03:02 2023 +0800

UefiCpuPkg/MpInitLib: Enable execute disable bit.

This patch synchronizes the No-Execute bit in the IA32_EFER
register for the APs before the RestoreVolatileRegisters operation.

The commit 964a4f0, titled "Eliminate the second INIT-SIPI-SIPI
sequence," replaces the second INIT-SIPI-SIPI sequence with the BSP
calling the SwitchApContext function to initiate a specialized start-up
sig

[PATCH v5] 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 a spec violation.

This patch fixes the implementation so we follow the x2APIC spec for new
VMs, while preserving the behaviour (buggy or fixed) for migrated-in VMs.

While touching that area, remove the existing printk statement in
vlapic_load_fixup() (as the checks it performed didn't make sense in x2APIC
mode and wouldn't affect the outcome) and put another printk as an else
branch so we get warnings trying to load nonsensical LDR values we don't
know about.

Fixes: f9e0cccf7b35 ("x86/HVM: fix ID handling of x2APIC emulation")
Signed-off-by: Alejandro Vallejo 
Reviewed-by: Roger Pau Monné 
Reviewed-by: Andrew Cooper 
---
v5:

[Andrew]
  * Missing full stop in commit message
  * Specifically mention spec violation in the commit message
  * Use struct vcpu* directly rather than through vlapic_vcpu()
  * Braces in conditionals of lapic_load_fixup()

[Jan]
  * Restored information about id!=0 + ldr==1 being inconsistent.
in the context of older fixup
---
 xen/arch/x86/hvm/vlapic.c | 64 +++
 xen/arch/x86/include/asm/hvm/domain.h |  3 ++
 2 files changed, 48 insertions(+), 19 deletions(-)

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 5cb87f8649..2d705020c8 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -1061,13 +1061,26 @@ static const struct hvm_mmio_ops vlapic_mmio_ops = {
 .write = vlapic_mmio_write,
 };
 
+static uint32_t x2apic_ldr_from_id(uint32_t id)
+{
+return ((id & ~0xf) << 12) | (1 << (id & 0xf));
+}
+
 static void set_x2apic_id(struct vlapic *vlapic)
 {
-u32 id = vlapic_vcpu(vlapic)->vcpu_id;
-u32 ldr = ((id & ~0xf) << 12) | (1 << (id & 0xf));
+const struct vcpu *v = vlapic_vcpu(vlapic);
+uint32_t apic_id = v->vcpu_id * 2;
+uint32_t apic_ldr = x2apic_ldr_from_id(apic_id);
 
-vlapic_set_reg(vlapic, APIC_ID, id * 2);
-vlapic_set_reg(vlapic, APIC_LDR, ldr);
+/*
+ * Workaround for migrated domains to derive LDRs as the source host
+ * would've.
+ */
+if ( v->domain->arch.hvm.bug_x2apic_ldr_vcpu_id )
+apic_ldr = x2apic_ldr_from_id(v->vcpu_id);
+
+vlapic_set_reg(vlapic, APIC_ID, apic_id);
+vlapic_set_reg(vlapic, APIC_LDR, apic_ldr);
 }
 
 int guest_wrmsr_apic_base(struct vcpu *v, uint64_t val)
@@ -1498,27 +1511,40 @@ static int cf_check lapic_save_regs(struct vcpu *v, 
hvm_domain_context_t *h)
  */
 static void lapic_load_fixup(struct vlapic *vlapic)
 {
-uint32_t id = vlapic->loaded.id;
+const struct vcpu *v = vlapic_vcpu(vlapic);
+uint32_t good_ldr = x2apic_ldr_from_id(vlapic->loaded.id);
 
-if ( vlapic_x2apic_mode(vlapic) && id && vlapic->loaded.ldr == 1 )
+/* Skip fixups on xAPIC mode, or if the x2APIC LDR is already correct */
+if ( !vlapic_x2apic_mode(vlapic) ||
+ (vlapic->loaded.ldr == good_ldr) )
+return;
+
+if ( vlapic->loaded.ldr == 1 )
 {
-/*
- * This is optional: ID != 0 contradicts LDR == 1. It's being added
- * to aid in eventual debugging of issues arising from the fixup done
- * here, but can be dropped as soon as it is found to conflict with
- * other (future) changes.
- */
-if ( GET_xAPIC_ID(id) != vlapic_vcpu(vlapic)->vcpu_id * 2 ||
- id != SET_xAPIC_ID(GET_xAPIC_ID(id)) )
-printk(XENLOG_G_WARNING "%pv: bogus APIC ID %#x loaded\n",
-   vlapic_vcpu(vlapic), id);
+   /*
+* Xen <= 4.4 may have a bug by which all the APICs configured in
+* x2APIC mode got LDR = 1, which is inconsistent on every vCPU
+* except for the one with ID = 0. We'll fix fix the bug now and
+* assign an LDR value consistent with the APIC ID.
+*/
 set_x2apic_id(vlapic);
 }
-else /* Undo an eventual earlier fixup. */
+else if ( vlapic->loaded.ldr == x2apic_ldr_from_id(v->vcpu_id) )
 {
-vlapic_set_reg(vlapic, APIC_ID, id);
-vlapic_set_reg(vlapic, APIC_LDR, vlapic->loaded.ldr);
+/*
+ * Migrations from Xen 4.4 to date (4.19 dev window, Nov 2023) may
+ * have LDR drived from the vCPU ID, not the APIC ID. We must preserve
+ * LDRs so new vCPUs use consistent derivations and existing guests,
+ * which may have already read the LDR at the source host, aren't
+ * surprised when interrupts stop working the way they did at the
+ * other end.
+ */
+v->domain->arch.hvm.bug_x2apic_ldr_vcpu_id = true;
 }
+else
+printk(XENLOG_G_WARNING
+   "%pv: bogus x2API

Re: [PATCH] xen/events: fix error codes in xen_bind_pirq_msi_to_irq()

[Dan Carpenter]

On Mon, Nov 27, 2023 at 02:17:05PM +0100, Juergen Gross wrote:
> On 27.11.23 13:57, Dan Carpenter wrote:
> > The error code needs to be set on these error paths.
> > 
> > Fixes: 5dd9ad32d775 ("xen/events: drop xen_allocate_irqs_dynamic()")
> > Fixes: d2ba3166f23b ("xen/events: move drivers/xen/events.c into 
> > drivers/xen/events/")
> 
> Please drop the last Fixes: tag. Said patch didn't introduce any new problem.

Yup.

> 
> > Signed-off-by: Dan Carpenter 
> > ---
> > Are we going to backport these to stable?  Should I split this into two
> > patches?
> > 
> >   drivers/xen/events/events_base.c | 8 ++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/xen/events/events_base.c 
> > b/drivers/xen/events/events_base.c
> > index f5edb9e27e3c..aae62603b461 100644
> > --- a/drivers/xen/events/events_base.c
> > +++ b/drivers/xen/events/events_base.c
> > @@ -1105,13 +1105,17 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, 
> > struct msi_desc *msidesc,
> > mutex_lock(&irq_mapping_update_lock);
> > irq = irq_alloc_descs(-1, 0, nvec, -1);
> > -   if (irq < 0)
> > +   if (irq < 0) {
> > +   ret = irq;
> > goto out;
> > +   }
> 
> Why? The return value for the out: label is in irq.
> 

This patch is full of embarrassment.  I misread this code.  I thought
the out label was in the error handling block.

> > for (i = 0; i < nvec; i++) {
> > info = xen_irq_init(irq + i);
> > -   if (!info)
> > +   if (!info) {
> > +   ret = -ENOMEM;
> > goto error_irq;
> > +   }
> 
> It would be easier to just preset ret with -ENOMEM when defining it.
> 

That only works if it fails on the first iteration.

I'll fix this up and resend.

regards,
dan carpenter

Re: [PATCH v5] xen/x86: On x2APIC mode, derive LDR from APIC ID

[Alejandro Vallejo]

On 27/11/2023 13:46, 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 a spec violation.
> 
> This patch fixes the implementation so we follow the x2APIC spec for new
> VMs, while preserving the behaviour (buggy or fixed) for migrated-in VMs.
> 
> While touching that area, remove the existing printk statement in
> vlapic_load_fixup() (as the checks it performed didn't make sense in x2APIC
> mode and wouldn't affect the outcome) and put another printk as an else
> branch so we get warnings trying to load nonsensical LDR values we don't
> know about.
> 
> Fixes: f9e0cccf7b35 ("x86/HVM: fix ID handling of x2APIC emulation")
> Signed-off-by: Alejandro Vallejo 
> Reviewed-by: Roger Pau Monné 
> Reviewed-by: Andrew Cooper 

While no R-by from Andrew was in the mailing list, it was in the xenbits
patch, of which this is a direct copy except for minor delta suggested
by Jan in lapic_load_fixup()

> https://xenbits.xen.org/gitweb/?p=people/andrewcoop/xen.git;a=commitdiff;h=953dcb0317d20959ffee14e404595cfbb66c607a#patch1

Cheers,
Alejandro

[PATCH] Argo: drop meaningless mfn_valid() check

[Jan Beulich]

Holding a valid struct page_info * in hands already means the referenced
MFN is valid; there's no need to check that again. Convert the checking
logic to a switch(), to help keeping the extra (and questionable) x86-
only check in somewhat tidy shape.

Signed-off-by: Jan Beulich 
---
Initially I had this (with less code churn) as

#ifdef CONFIG_X86
if ( p2mt == p2m_ram_logdirty )
ret = -EAGAIN;
else
#endif
if ( (p2mt != p2m_ram_rw) ||
 !get_page_type(page, PGT_writable_page) )
ret = -EINVAL;

But the "else" placement seemed too ugly to me. Otoh there better
wouldn't be any special casing of log-dirty here (and instead such a
page be converted, perhaps right in check_get_page_from_gfn() when
readonly=false), at which point the odd "else" would go away, and the
if() likely again be preferable over the switch().

--- a/xen/common/argo.c
+++ b/xen/common/argo.c
@@ -1421,15 +1421,24 @@ find_ring_mfn(struct domain *d, gfn_t gf
 return ret;
 
 *mfn = page_to_mfn(page);
-if ( !mfn_valid(*mfn) )
-ret = -EINVAL;
+
+switch ( p2mt )
+{
+case p2m_ram_rw:
+if ( !get_page_and_type(page, d, PGT_writable_page) )
+ret = -EINVAL;
+break;
+
 #ifdef CONFIG_X86
-else if ( p2mt == p2m_ram_logdirty )
+case p2m_ram_logdirty:
 ret = -EAGAIN;
+break;
 #endif
-else if ( (p2mt != p2m_ram_rw) ||
-  !get_page_and_type(page, d, PGT_writable_page) )
+
+default:
 ret = -EINVAL;
+break;
+}
 
 put_page(page);
 

Re: [PATCH v5] xen/x86: On x2APIC mode, derive LDR from APIC ID

[Jan Beulich]

On 27.11.2023 14:49, Alejandro Vallejo wrote:
> On 27/11/2023 13:46, 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 a spec violation.
>>
>> This patch fixes the implementation so we follow the x2APIC spec for new
>> VMs, while preserving the behaviour (buggy or fixed) for migrated-in VMs.
>>
>> While touching that area, remove the existing printk statement in
>> vlapic_load_fixup() (as the checks it performed didn't make sense in x2APIC
>> mode and wouldn't affect the outcome) and put another printk as an else
>> branch so we get warnings trying to load nonsensical LDR values we don't
>> know about.
>>
>> Fixes: f9e0cccf7b35 ("x86/HVM: fix ID handling of x2APIC emulation")
>> Signed-off-by: Alejandro Vallejo 
>> Reviewed-by: Roger Pau Monné 
>> Reviewed-by: Andrew Cooper 
> 
> While no R-by from Andrew was in the mailing list, it was in the xenbits
> patch, of which this is a direct copy except for minor delta suggested
> by Jan in lapic_load_fixup()

Sadly the doubly "fix" is still there in that comment.

Jan

[PATCH v4 00/14] Introduce generic headers

[Oleksii Kurochko]

Some headers are common between several architectures, so the current patch 
series
provide them.

Another one reason to have them as generic is a simplification of adding support
necessary to make a complete Xen build as it was/is being done in the patch 
series [1]
and [2].

Also, instead of providing generic/stub headers, it was used
"#ifdef CONFIG_* #include  #endif" instead of providing empty headers.

Patch related to delay.h [3] was sent separately.

This patch series is a pre-requisite for "Enable build of full Xen for RISC-V" 
[4].

[1] 
https://lore.kernel.org/xen-devel/cover.1694543103.git.sanasta...@raptorengineering.com/
[2] 
https://lore.kernel.org/xen-devel/cover.1692181079.git.oleksii.kuroc...@gmail.com/
[3] 
https://lore.kernel.org/xen-devel/3d55bce44bd6ab9973cbe0ea2fc136cc44d35df2.1698759633.git.oleksii.kuroc...@gmail.com/
[4] 
https://lore.kernel.org/xen-devel/cover.1700761381.git.oleksii.kuroc...@gmail.com/

---
Changes in V4:
 - Update the cover letter message
 - Add Reviewed-by/Acked-by for patches:
[PATCH v3 01/14] xen/asm-generic: introduce stub header paging.h
[PATCH v3 03/14] xen/asm-generic: introduce generic hypercall.h
[PATCH v3 04/14] xen/asm-generic: introduce generic header iocap.h
[PATCH v3 05/14] xen/asm-generic: introduce stub header 
[PATCH v3 06/14] xen/asm-generic: introduce generic header percpu.h
[PATCH v3 07/14] xen/asm-generic: introduce generalized hardirq.h
[PATCH v3 08/14] xen/asm-generic: introduce generic div64.h header
[PATCH v3 09/14] xen/asm-generic: introduce generic header altp2m.h
[PATCH v3 10/14] xen/asm-generic: introduce stub header monitor.h
[PATCH v3 11/14] xen/asm-generic: introduce stub header numa.h
[PATCH v3 12/14] xen/asm-generic: introduce stub header softirq.h
 - Fix some code style and minor issues.
 - Use asm-generic version of device.h for Arm and PPC.
---
Changes in V3:
 - Update the commit message of the cover letter.
 - Drop the following patch as it can be arch-specific enough:
   * [PATCH v2 09/15] xen/asm-generic: introduce generic header smp.h
 - Drop correspondent arch specific headers and use asm-generic version of
   a header.
 - Back to the patch series patches:
   * xen: ifdef inclusion of  in 
   * xen/asm-generic: ifdef inclusion of 
---
Changes in V2:
 - Update the commit message of the cover letter.
 - Drop the following patches because they are arch-specific or was sent as a 
separate patch:
   - xen/asm-generic: introduce stub header event.h
 - xen/asm-generic: introduce stub header spinlock.h
 - [PATCH v1 03/29] xen/asm-generic: introduce stub header cpufeature.h
 - [PATCH v1 07/29] xen/asm-generic: introduce stub header 
guest_atomics.h
 - [PATCH v1 10/29] xen/asm-generic: introduce stub header iommu.h
 - [PATCH v1 12/29] xen/asm-generic: introduce stub header pci.h 
because separate patch was sent [5]
 - [PATCH v1 14/29] xen/asm-generic: introduce stub header setup.h
 - [PATCH v1 15/29] xen/asm-generic: introduce stub header xenoprof.h 
because of [3].
 - [PATCH v1 16/29] xen/asm-generic: introduce stub header flushtlb.h
 - [PATCH v1 22/29] xen/asm-generic: introduce stub header delay.h 
because of [3]
 - [PATCH v1 23/29] xen/asm-generic: introduce stub header domain.h
 - [PATCH v1 24/29] xen/asm-generic: introduce stub header 
guest_access.h
 - [PATCH v1 25/29] xen/asm-generic: introduce stub header irq.h ( 
probably not so generic as I expected, I'll back to it if it will be necessary 
in the future )
 - [PATCH v1 28/29] xen/asm-generic: introduce stub header p2m.h ( 
probably not so generic as I expected, I'll back to it if it will be necessary 
in the future )
 - For the rest of the patches please look at changes for each patch separately.
---

Oleksii Kurochko (14):
  xen/asm-generic: introduce stub header paging.h
  xen/asm-generic: introduce generic device.h
  xen/asm-generic: introduce generic hypercall.h
  xen/asm-generic: introduce generic header iocap.h
  xen/asm-generic: introduce stub header 
  xen/asm-generic: introduce generic header percpu.h
  xen/asm-generic: introduce generalized hardirq.h
  xen/asm-generic: introduce generic div64.h header
  xen/asm-generic: introduce generic header altp2m.h
  xen/asm-generic: introduce stub header monitor.h
  xen/asm-generic: introduce stub header numa.h
  xen/asm-generic: introduce stub header softirq.h
  xen: ifdef inclusion of  in 
  xen/asm-generic: ifdef inclusion of 

 xen/arch/arm/device.c |  15 ++-
 xen/arch/arm/domain_build.c   |   3 +-
 xen/arch/arm/gic-v2.c |   4 +-
 xen/arch/arm/gic-v3.c |   6 +-
 xen/arch/arm/gic.c|   4 +-
 xen/arch/arm/include/asm/Makefile |   9 ++
 xen/arch/arm/include/asm/altp2m.h |  39 --
 xen/arch/arm/include/asm/current.h|   3 +
 xe

[PATCH v4 06/14] xen/asm-generic: introduce generic header percpu.h

[Oleksii Kurochko]

The patch introduces generic percpu.h which was based on Arm's version
with the following changes:
 * makes __per_cpu_data_end[] constant
 * introduce get_per_cpu_offset() for macros this_cpu() and this_cpu_ptr()
 * add inclustion of  as get_per_cpu_offset() is located there.

Also it was changed a place where  is included in 
because asm-generic version of percpu.h started to include  which
requires definition of DECLARE_PER_CPU.

As well the patch switches Arm, PPC and x86 architectures to use asm-generic
version of percpu.h.

Signed-off-by: Oleksii Kurochko 
Acked-by: Julien Grall 
---
Changes in V4:
 - Added FIXME comment in ppc/current.h for get_per_cpu_offset()
 - Added Acked-by: Julien Grall .
---
Changes in V3:
 - switch all architectures to asm-generic version of percpu.h
 - introduce get_per_cpu_offset() and implement it architectures.
 - make __per_cpu_data_end constamt.
 - update the commit message.
---
Changes in V2:
- use smp_processor_id() instead of get_processor_id().
- update commit message .
---
 xen/arch/arm/include/asm/Makefile |  1 +
 xen/arch/arm/include/asm/current.h|  3 +++
 xen/arch/ppc/include/asm/Makefile |  1 +
 xen/arch/ppc/include/asm/current.h|  6 +
 xen/arch/ppc/include/asm/percpu.h | 24 ---
 xen/arch/x86/include/asm/Makefile |  2 ++
 xen/arch/x86/include/asm/current.h|  2 ++
 xen/arch/x86/include/asm/percpu.h | 22 -
 .../asm => include/asm-generic}/percpu.h  | 18 --
 xen/include/xen/percpu.h  |  4 ++--
 10 files changed, 28 insertions(+), 55 deletions(-)
 delete mode 100644 xen/arch/ppc/include/asm/percpu.h
 create mode 100644 xen/arch/x86/include/asm/Makefile
 delete mode 100644 xen/arch/x86/include/asm/percpu.h
 rename xen/{arch/arm/include/asm => include/asm-generic}/percpu.h (57%)

diff --git a/xen/arch/arm/include/asm/Makefile 
b/xen/arch/arm/include/asm/Makefile
index 7593c4e8ee..60bb78fc09 100644
--- a/xen/arch/arm/include/asm/Makefile
+++ b/xen/arch/arm/include/asm/Makefile
@@ -2,5 +2,6 @@
 generic-y += device.h
 generic-y += iocap.h
 generic-y += paging.h
+generic-y += percpu.h
 generic-y += random.h
 generic-y += vm_event.h
diff --git a/xen/arch/arm/include/asm/current.h 
b/xen/arch/arm/include/asm/current.h
index 51d1c8efa8..0be7ad6ef9 100644
--- a/xen/arch/arm/include/asm/current.h
+++ b/xen/arch/arm/include/asm/current.h
@@ -5,6 +5,7 @@
 #include 
 
 #include 
+#include 
 
 /* Tell whether the guest vCPU enabled Workaround 2 (i.e variant 4) */
 #define CPUINFO_WORKAROUND_2_FLAG_SHIFT   0
@@ -60,6 +61,8 @@ do {\
 this_cpu(cpu_id) = (id);\
 } while ( 0 )
 
+#define get_per_cpu_offset()  READ_SYSREG(TPIDR_EL2)
+
 #endif
 
 #endif /* __ARM_CURRENT_H__ */
diff --git a/xen/arch/ppc/include/asm/Makefile 
b/xen/arch/ppc/include/asm/Makefile
index 039b0eb5ee..edef0fab78 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -3,5 +3,6 @@ generic-y += device.h
 generic-y += hypercall.h
 generic-y += iocap.h
 generic-y += paging.h
+generic-y += percpu.h
 generic-y += random.h
 generic-y += vm_event.h
diff --git a/xen/arch/ppc/include/asm/current.h 
b/xen/arch/ppc/include/asm/current.h
index 0ca06033f9..e329a548e6 100644
--- a/xen/arch/ppc/include/asm/current.h
+++ b/xen/arch/ppc/include/asm/current.h
@@ -4,6 +4,8 @@
 
 #include 
 
+#include 
+
 #ifndef __ASSEMBLY__
 
 struct vcpu;
@@ -38,6 +40,10 @@ static inline struct cpu_info *get_cpu_info(void)
 
 #define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs)
 
+#define smp_processor_id()  0 /* TODO: Fix this */
+
+#define get_per_cpu_offset()smp_processor_id() /* TODO: Fix this */
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* __ASM_PPC_CURRENT_H__ */
diff --git a/xen/arch/ppc/include/asm/percpu.h 
b/xen/arch/ppc/include/asm/percpu.h
deleted file mode 100644
index e7c40c0f03..00
--- a/xen/arch/ppc/include/asm/percpu.h
+++ /dev/null
@@ -1,24 +0,0 @@
-#ifndef __PPC_PERCPU_H__
-#define __PPC_PERCPU_H__
-
-#ifndef __ASSEMBLY__
-
-extern char __per_cpu_start[], __per_cpu_data_end[];
-extern unsigned long __per_cpu_offset[NR_CPUS];
-void percpu_init_areas(void);
-
-#define smp_processor_id() 0 /* TODO: Fix this */
-
-#define per_cpu(var, cpu)  \
-(*RELOC_HIDE(&per_cpu__##var, __per_cpu_offset[cpu]))
-#define this_cpu(var) \
-(*RELOC_HIDE(&per_cpu__##var, smp_processor_id()))
-
-#define per_cpu_ptr(var, cpu)  \
-(*RELOC_HIDE(var, __per_cpu_offset[cpu]))
-#define this_cpu_ptr(var) \
-(*RELOC_HIDE(var, smp_processor_id()))
-
-#endif
-
-#endif /* __PPC_PERCPU_H__ */
diff --git a/xen/arch/x86/include/asm/Makefile 
b/xen/arch/x86/include/asm/Makefile
new file mode 100644
index 00..874429ed30
--- /dev/null
+++ b/xen/arch/x86/include/asm/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL

[PATCH v4 04/14] xen/asm-generic: introduce generic header iocap.h

[Oleksii Kurochko]

iocap.h is common for Arm, PPC and RISC-V architectures thereby
it was moved to asm-generic.

Also Arm and PPC were switched to asm-generic version of iocap.h.

Signed-off-by: Oleksii Kurochko 
Reviewed-by: Jan Beulich 
Acked-by: Julien Grall 
---
Changes in V4:
- Added Reviewed-by: Jan Beulich 
- Acked-by: Julien Grall 
---
Changes in V3:
  - Drop Arm and PPC's iocap.h and switch to asm-generic's version.
  - Update the commit message.
---
Changes in V2:
 - update the commit message
---
 xen/arch/arm/include/asm/Makefile | 1 +
 xen/arch/ppc/include/asm/Makefile | 1 +
 xen/arch/ppc/include/asm/iocap.h  | 8 
 xen/{arch/arm/include/asm => include/asm-generic}/iocap.h | 7 ---
 4 files changed, 6 insertions(+), 11 deletions(-)
 delete mode 100644 xen/arch/ppc/include/asm/iocap.h
 rename xen/{arch/arm/include/asm => include/asm-generic}/iocap.h (60%)

diff --git a/xen/arch/arm/include/asm/Makefile 
b/xen/arch/arm/include/asm/Makefile
index df4c1ebb08..2d036045e8 100644
--- a/xen/arch/arm/include/asm/Makefile
+++ b/xen/arch/arm/include/asm/Makefile
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
 generic-y += device.h
+generic-y += iocap.h
 generic-y += paging.h
 generic-y += vm_event.h
diff --git a/xen/arch/ppc/include/asm/Makefile 
b/xen/arch/ppc/include/asm/Makefile
index d5e369128c..209c3e74d8 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 generic-y += device.h
 generic-y += hypercall.h
+generic-y += iocap.h
 generic-y += paging.h
 generic-y += vm_event.h
diff --git a/xen/arch/ppc/include/asm/iocap.h b/xen/arch/ppc/include/asm/iocap.h
deleted file mode 100644
index 76bf13a70f..00
--- a/xen/arch/ppc/include/asm/iocap.h
+++ /dev/null
@@ -1,8 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-#ifndef __ASM_PPC_IOCAP_H__
-#define __ASM_PPC_IOCAP_H__
-
-#define cache_flush_permitted(d)\
-(!rangeset_is_empty((d)->iomem_caps))
-
-#endif /* __ASM_PPC_IOCAP_H__ */
diff --git a/xen/arch/arm/include/asm/iocap.h b/xen/include/asm-generic/iocap.h
similarity index 60%
rename from xen/arch/arm/include/asm/iocap.h
rename to xen/include/asm-generic/iocap.h
index 276fefbc59..dd7cb45488 100644
--- a/xen/arch/arm/include/asm/iocap.h
+++ b/xen/include/asm-generic/iocap.h
@@ -1,10 +1,11 @@
-#ifndef __X86_IOCAP_H__
-#define __X86_IOCAP_H__
+/* 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))
 
-#endif
+#endif /* __ASM_GENERIC_IOCAP_H__ */
 
 /*
  * Local variables:
-- 
2.43.0

[PATCH v4 01/14] xen/asm-generic: introduce stub header paging.h

[Oleksii Kurochko]

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 
Acked-by: Julien Grall 
---
Changes in V4:
 - Added Acked-by: Julien Grall .
---
Changes in V3:
 - Sort xen/arch/{arm,ppc}/include/asm/Makefile alphabetically.
 - Add Reviewed-by: Jan Beulich 
---
Changes in V2:
- evaluate argument of macros
- covert macros to true
- use proper tabs
- switch Arm & PPC to generic paging.h
- update commit message
---
 xen/arch/arm/include/asm/Makefile |  1 +
 xen/arch/arm/include/asm/paging.h | 16 
 xen/arch/ppc/include/asm/Makefile |  1 +
 xen/arch/ppc/include/asm/paging.h |  7 ---
 xen/include/asm-generic/paging.h  | 19 +++
 5 files changed, 21 insertions(+), 23 deletions(-)
 delete mode 100644 xen/arch/arm/include/asm/paging.h
 delete mode 100644 xen/arch/ppc/include/asm/paging.h
 create mode 100644 xen/include/asm-generic/paging.h

diff --git a/xen/arch/arm/include/asm/Makefile 
b/xen/arch/arm/include/asm/Makefile
index 821addb0bf..ece7fa66dd 100644
--- 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 += paging.h
 generic-y += vm_event.h
diff --git a/xen/arch/arm/include/asm/paging.h 
b/xen/arch/arm/include/asm/paging.h
deleted file mode 100644
index 6d1a000246..00
--- a/xen/arch/arm/include/asm/paging.h
+++ /dev/null
@@ -1,16 +0,0 @@
-#ifndef _XEN_PAGING_H
-#define _XEN_PAGING_H
-
-#define paging_mode_translate(d)  (1)
-#define paging_mode_external(d)   (1)
-
-#endif /* XEN_PAGING_H */
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/arch/ppc/include/asm/Makefile 
b/xen/arch/ppc/include/asm/Makefile
index 821addb0bf..ece7fa66dd 100644
--- 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 += paging.h
 generic-y += vm_event.h
diff --git a/xen/arch/ppc/include/asm/paging.h 
b/xen/arch/ppc/include/asm/paging.h
deleted file mode 100644
index eccacece29..00
--- a/xen/arch/ppc/include/asm/paging.h
+++ /dev/null
@@ -1,7 +0,0 @@
-#ifndef __ASM_PPC_PAGING_H__
-#define __ASM_PPC_PAGING_H__
-
-#define paging_mode_translate(d)  (1)
-#define paging_mode_external(d)   (1)
-
-#endif /* __ASM_PPC_PAGING_H__ */
diff --git a/xen/include/asm-generic/paging.h b/xen/include/asm-generic/paging.h
new file mode 100644
index 00..8df534cfdc
--- /dev/null
+++ b/xen/include/asm-generic/paging.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GENERIC_PAGING_H__
+#define __ASM_GENERIC_PAGING_H__
+
+#include 
+
+#define paging_mode_translate(d)((void)(d), true)
+#define paging_mode_external(d) ((void)(d), true)
+
+#endif /* __ASM_GENERIC_PAGING_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.43.0

[PATCH v4 08/14] xen/asm-generic: introduce generic div64.h header

[Oleksii Kurochko]

All archs have the do_div implementation for BITS_PER_LONG == 64
so do_div64.h is moved to asm-generic.

x86 and PPC were switched to asm-generic version of div64.h.

Signed-off-by: Oleksii Kurochko 
Acked-by: Jan Beulich 
---
Changes in V4:
 - Added Acked-by: Jan Beulich .
 - include  in Arm's div64.h for 64-bit case.
---
Changes in V3:
 - Drop x86 and PPC's div64.h.
 - Update the commit message.
---
Changes in V2:
- rename base to divisor
- add "#if BITS_PER_LONG == 64"
- fix code style
---
 xen/arch/arm/include/asm/div64.h  |  8 +---
 xen/arch/ppc/include/asm/Makefile |  1 +
 xen/arch/ppc/include/asm/div64.h  | 14 --
 xen/arch/x86/include/asm/Makefile |  1 +
 xen/arch/x86/include/asm/div64.h  | 14 --
 xen/include/asm-generic/div64.h   | 27 +++
 6 files changed, 30 insertions(+), 35 deletions(-)
 delete mode 100644 xen/arch/ppc/include/asm/div64.h
 delete mode 100644 xen/arch/x86/include/asm/div64.h
 create mode 100644 xen/include/asm-generic/div64.h

diff --git a/xen/arch/arm/include/asm/div64.h b/xen/arch/arm/include/asm/div64.h
index fc667a80f9..0459d5cc01 100644
--- a/xen/arch/arm/include/asm/div64.h
+++ b/xen/arch/arm/include/asm/div64.h
@@ -24,13 +24,7 @@
 
 #if BITS_PER_LONG == 64
 
-# define do_div(n,base) ({  \
-uint32_t __base = (base);   \
-uint32_t __rem; \
-__rem = ((uint64_t)(n)) % __base;   \
-(n) = ((uint64_t)(n)) / __base; \
-__rem;  \
- })
+#include 
 
 #elif BITS_PER_LONG == 32
 
diff --git a/xen/arch/ppc/include/asm/Makefile 
b/xen/arch/ppc/include/asm/Makefile
index 3241236c64..5364bb1d59 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 generic-y += device.h
+generic-y += div64.h
 generic-y += hardirq.h
 generic-y += hypercall.h
 generic-y += iocap.h
diff --git a/xen/arch/ppc/include/asm/div64.h b/xen/arch/ppc/include/asm/div64.h
deleted file mode 100644
index d213e50585..00
--- a/xen/arch/ppc/include/asm/div64.h
+++ /dev/null
@@ -1,14 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-#ifndef __ASM_PPC_DIV64_H__
-#define __ASM_PPC_DIV64_H__
-
-#include 
-
-#define do_div(n, base) ({   \
-uint32_t base_ = (base); \
-uint32_t rem_ = (uint64_t)(n) % base_;   \
-(n) = (uint64_t)(n) / base_; \
-rem_;\
-})
-
-#endif /* __ASM_PPC_DIV64_H__ */
diff --git a/xen/arch/x86/include/asm/Makefile 
b/xen/arch/x86/include/asm/Makefile
index 874429ed30..daab34ff0a 100644
--- a/xen/arch/x86/include/asm/Makefile
+++ b/xen/arch/x86/include/asm/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
+generic-y += div64.h
 generic-y += percpu.h
diff --git a/xen/arch/x86/include/asm/div64.h b/xen/arch/x86/include/asm/div64.h
deleted file mode 100644
index dd49f64a3b..00
--- a/xen/arch/x86/include/asm/div64.h
+++ /dev/null
@@ -1,14 +0,0 @@
-#ifndef __X86_DIV64
-#define __X86_DIV64
-
-#include 
-
-#define do_div(n,base) ({   \
-uint32_t __base = (base);   \
-uint32_t __rem; \
-__rem = ((uint64_t)(n)) % __base;   \
-(n) = ((uint64_t)(n)) / __base; \
-__rem;  \
-})
-
-#endif
diff --git a/xen/include/asm-generic/div64.h b/xen/include/asm-generic/div64.h
new file mode 100644
index 00..068d8a11ad
--- /dev/null
+++ b/xen/include/asm-generic/div64.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GENERIC_DIV64
+#define __ASM_GENERIC_DIV64
+
+#include 
+
+#if BITS_PER_LONG == 64
+
+#define do_div(n, divisor) ({   \
+uint32_t divisor_ = (divisor);  \
+uint32_t rem_ = (uint64_t)(n) % divisor_;   \
+(n) = (uint64_t)(n) / divisor_; \
+rem_;   \
+})
+
+#endif /* BITS_PER_LONG */
+
+#endif
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.43.0

[PATCH v4 12/14] xen/asm-generic: introduce stub header softirq.h

[Oleksii Kurochko]

 is common between Arm, PPC and RISC-V so it is
moved to asm-generic.

Drop Arm and PPC's softirq.h and use asm-generic version instead.

Signed-off-by: Oleksii Kurochko 
Reviewed-by: Michal Orzel 
Added Acked-by: Jan Beulich 
---
Changes in V4:
 - Added Reviewed-by: Michal Orzel 
 - Added Acked-by: Jan Beulich 
---
Changes in V3:
 - Drop Arm and PPC's softirq.h
 - Update the commit message.
---
Changes in V2:
- update the commit message.
---
 xen/arch/arm/include/asm/Makefile | 1 +
 xen/arch/ppc/include/asm/Makefile | 1 +
 xen/arch/ppc/include/asm/softirq.h| 8 
 .../arm/include/asm => include/asm-generic}/softirq.h | 7 ---
 4 files changed, 6 insertions(+), 11 deletions(-)
 delete mode 100644 xen/arch/ppc/include/asm/softirq.h
 rename xen/{arch/arm/include/asm => include/asm-generic}/softirq.h (56%)

diff --git a/xen/arch/arm/include/asm/Makefile 
b/xen/arch/arm/include/asm/Makefile
index b8e0763c77..c3f4291ee2 100644
--- a/xen/arch/arm/include/asm/Makefile
+++ b/xen/arch/arm/include/asm/Makefile
@@ -7,4 +7,5 @@ generic-y += numa.h
 generic-y += paging.h
 generic-y += percpu.h
 generic-y += random.h
+generic-y += softirq.h
 generic-y += vm_event.h
diff --git a/xen/arch/ppc/include/asm/Makefile 
b/xen/arch/ppc/include/asm/Makefile
index a4188dce1d..4da0c70cc0 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -10,4 +10,5 @@ generic-y += numa.h
 generic-y += paging.h
 generic-y += percpu.h
 generic-y += random.h
+generic-y += softirq.h
 generic-y += vm_event.h
diff --git a/xen/arch/ppc/include/asm/softirq.h 
b/xen/arch/ppc/include/asm/softirq.h
deleted file mode 100644
index a0b28a5e51..00
--- a/xen/arch/ppc/include/asm/softirq.h
+++ /dev/null
@@ -1,8 +0,0 @@
-#ifndef __ASM_PPC_SOFTIRQ_H__
-#define __ASM_PPC_SOFTIRQ_H__
-
-#define NR_ARCH_SOFTIRQS 0
-
-#define arch_skip_send_event_check(cpu) 0
-
-#endif /* __ASM_PPC_SOFTIRQ_H__ */
diff --git a/xen/arch/arm/include/asm/softirq.h 
b/xen/include/asm-generic/softirq.h
similarity index 56%
rename from xen/arch/arm/include/asm/softirq.h
rename to xen/include/asm-generic/softirq.h
index 976e0ebd70..83be855e50 100644
--- a/xen/arch/arm/include/asm/softirq.h
+++ b/xen/include/asm-generic/softirq.h
@@ -1,11 +1,12 @@
-#ifndef __ASM_SOFTIRQ_H__
-#define __ASM_SOFTIRQ_H__
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GENERIC_SOFTIRQ_H__
+#define __ASM_GENERIC_SOFTIRQ_H__
 
 #define NR_ARCH_SOFTIRQS   0
 
 #define arch_skip_send_event_check(cpu) 0
 
-#endif /* __ASM_SOFTIRQ_H__ */
+#endif /* __ASM_GENERIC_SOFTIRQ_H__ */
 /*
  * Local variables:
  * mode: C
-- 
2.43.0

[PATCH v4 10/14] xen/asm-generic: introduce stub header monitor.h

[Oleksii Kurochko]

The header is shared between several archs so it is
moved to asm-generic.

Signed-off-by: Oleksii Kurochko 
Acked-by: Jan Beulich .
---
Changes in V4:
 - Removed the double blank line.
 - Added Acked-by: Jan Beulich .
 - Update the commit message
---
Changes in V3:
 - Use forward-declaration of struct domain instead of " #include  
".
 - Add ' include  '
 - Drop PPC's monitor.h.
---
Changes in V2:
- remove inclusion of "+#include "
- add "struct xen_domctl_monitor_op;"
- remove one of SPDX tags.
---
 xen/arch/ppc/include/asm/Makefile  |  1 +
 xen/arch/ppc/include/asm/monitor.h | 43 -
 xen/include/asm-generic/monitor.h  | 62 ++
 3 files changed, 63 insertions(+), 43 deletions(-)
 delete mode 100644 xen/arch/ppc/include/asm/monitor.h
 create mode 100644 xen/include/asm-generic/monitor.h

diff --git a/xen/arch/ppc/include/asm/Makefile 
b/xen/arch/ppc/include/asm/Makefile
index 319e90955b..bcddcc181a 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -5,6 +5,7 @@ generic-y += div64.h
 generic-y += hardirq.h
 generic-y += hypercall.h
 generic-y += iocap.h
+generic-y += monitor.h
 generic-y += paging.h
 generic-y += percpu.h
 generic-y += random.h
diff --git a/xen/arch/ppc/include/asm/monitor.h 
b/xen/arch/ppc/include/asm/monitor.h
deleted file mode 100644
index e5b0282bf1..00
--- a/xen/arch/ppc/include/asm/monitor.h
+++ /dev/null
@@ -1,43 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/* Derived from xen/arch/arm/include/asm/monitor.h */
-#ifndef __ASM_PPC_MONITOR_H__
-#define __ASM_PPC_MONITOR_H__
-
-#include 
-#include 
-
-static inline
-void arch_monitor_allow_userspace(struct domain *d, bool allow_userspace)
-{
-}
-
-static inline
-int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
-{
-/* No arch-specific monitor ops on PPC. */
-return -EOPNOTSUPP;
-}
-
-int arch_monitor_domctl_event(struct domain *d,
-  struct xen_domctl_monitor_op *mop);
-
-static inline
-int arch_monitor_init_domain(struct domain *d)
-{
-/* No arch-specific domain initialization on PPC. */
-return 0;
-}
-
-static inline
-void arch_monitor_cleanup_domain(struct domain *d)
-{
-/* No arch-specific domain cleanup on PPC. */
-}
-
-static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
-{
-BUG_ON("unimplemented");
-return 0;
-}
-
-#endif /* __ASM_PPC_MONITOR_H__ */
diff --git a/xen/include/asm-generic/monitor.h 
b/xen/include/asm-generic/monitor.h
new file mode 100644
index 00..6be8614431
--- /dev/null
+++ b/xen/include/asm-generic/monitor.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * include/asm-GENERIC/monitor.h
+ *
+ * Arch-specific monitor_op domctl handler.
+ *
+ * Copyright (c) 2015 Tamas K Lengyel (ta...@tklengyel.com)
+ * Copyright (c) 2016, Bitdefender S.R.L.
+ *
+ */
+
+#ifndef __ASM_GENERIC_MONITOR_H__
+#define __ASM_GENERIC_MONITOR_H__
+
+#include 
+
+struct domain;
+struct xen_domctl_monitor_op;
+
+static inline
+void arch_monitor_allow_userspace(struct domain *d, bool allow_userspace)
+{
+}
+
+static inline
+int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
+{
+/* No arch-specific monitor ops on GENERIC. */
+return -EOPNOTSUPP;
+}
+
+int arch_monitor_domctl_event(struct domain *d,
+  struct xen_domctl_monitor_op *mop);
+
+static inline
+int arch_monitor_init_domain(struct domain *d)
+{
+/* No arch-specific domain initialization on GENERIC. */
+return 0;
+}
+
+static inline
+void arch_monitor_cleanup_domain(struct domain *d)
+{
+/* No arch-specific domain cleanup on GENERIC. */
+}
+
+static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
+{
+return 0;
+}
+
+#endif /* __ASM_GENERIC_MONITOR_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: BSD
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.43.0

[PATCH v4 03/14] xen/asm-generic: introduce generic hypercall.h

[Oleksii Kurochko]

Introduce an empty generic hypercall.h for archs which don't
implement it.

Drop PPC's hypercall.h and switch to generic one instead.

Signed-off-by: Oleksii Kurochko 
Acked-by: Jan Beulich 
---
Changes in V4:
 - Nothing changed. Only rebase.
---
Changes in V3:
 - Drop PPC's hypercall.h and switch to generic one.
 - Update the commit message
 - Add Acked-by: Jan Beulich 
---
Changes in V2:
 - add check that  isn't included directly.
---
 xen/arch/ppc/include/asm/Makefile|  1 +
 xen/arch/ppc/include/asm/hypercall.h |  5 -
 xen/include/asm-generic/hypercall.h  | 18 ++
 3 files changed, 19 insertions(+), 5 deletions(-)
 delete mode 100644 xen/arch/ppc/include/asm/hypercall.h
 create mode 100644 xen/include/asm-generic/hypercall.h

diff --git a/xen/arch/ppc/include/asm/Makefile 
b/xen/arch/ppc/include/asm/Makefile
index df4c1ebb08..d5e369128c 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
 generic-y += device.h
+generic-y += hypercall.h
 generic-y += paging.h
 generic-y += vm_event.h
diff --git a/xen/arch/ppc/include/asm/hypercall.h 
b/xen/arch/ppc/include/asm/hypercall.h
deleted file mode 100644
index 1e8ca0ce9c..00
--- a/xen/arch/ppc/include/asm/hypercall.h
+++ /dev/null
@@ -1,5 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-#ifndef __ASM_PPC_HYPERCALL_H__
-#define __ASM_PPC_HYPERCALL_H__
-
-#endif /* __ASM_PPC_HYPERCALL_H__ */
diff --git a/xen/include/asm-generic/hypercall.h 
b/xen/include/asm-generic/hypercall.h
new file mode 100644
index 00..7743b35c0d
--- /dev/null
+++ b/xen/include/asm-generic/hypercall.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __XEN_HYPERCALL_H__
+#error "asm/hypercall.h should not be included directly - include 
xen/hypercall.h instead"
+#endif
+
+#ifndef __ASM_GENERIC_HYPERCALL_H__
+#define __ASM_GENERIC_HYPERCALL_H__
+
+#endif /* __ASM_GENERIC_HYPERCALL_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.43.0

[PATCH v4 02/14] xen/asm-generic: introduce generic device.h

[Oleksii Kurochko]

Arm, PPC and RISC-V use the same device.h thereby device.h
was moved to asm-generic. Arm's device.h was taken as a base with
the following changes:
 - #ifdef PCI related things.
 - #ifdef ACPI related things.
 - Rename #ifdef guards.
 - Add SPDX tag.
 - #ifdef CONFIG_HAS_DEVICE_TREE related things.
 - #ifdef-ing iommu related things with CONFIG_HAS_PASSTHROUGH.

Also Arm and PPC are switched to asm-generic version of device.h

Signed-off-by: Oleksii Kurochko 
---
Changes in V4:
 - Updated the commit message
 - Switched Arm and PPC to asm-generic version of device.h
 - Replaced HAS_PCI with CONFIG_HAS_PCI
 - ifdef-ing iommu filed of dev_archdata struct with CONFIG_HAS_PASSTHROUGH
 - ifdef-ing iommu_fwspec of device struct with CONFIG_HAS_PASSTHROUGH
 - ifdef-ing DT related things with CONFIG_HAS_DEVICE_TREE
 - Updated the commit message ( remove a note with question about
   if device.h should be in asm-generic or not )
 - Replaced DEVICE_IC with DEVICE_INTERRUPT_CONTROLLER
 - Rationalized usage of CONFIG_HAS_* in device.h
 - Fixed indents for ACPI_DEVICE_START and ACPI_DEVICE_END
---
Changes in V3:
 - ifdef device tree related things.
 - update the commit message
---
Changes in V2:
- take ( as common ) device.h from Arm as PPC and RISC-V use it as a 
base.
- #ifdef PCI related things.
- #ifdef ACPI related things.
- rename DEVICE_GIC to DEVIC_IC.
- rename #ifdef guards.
- switch Arm and PPC to generic device.h
- add SPDX tag
- update the commit message

---
 xen/arch/arm/device.c |  15 ++-
 xen/arch/arm/domain_build.c   |   2 +-
 xen/arch/arm/gic-v2.c |   4 +-
 xen/arch/arm/gic-v3.c |   6 +-
 xen/arch/arm/gic.c|   4 +-
 xen/arch/arm/include/asm/Makefile |   1 +
 xen/arch/ppc/include/asm/Makefile |   1 +
 xen/arch/ppc/include/asm/device.h |  53 
 xen/arch/ppc/include/asm/irq.h|   4 +
 .../asm => include/asm-generic}/device.h  | 125 +++---
 xen/include/headers++.chk.new |   0
 11 files changed, 106 insertions(+), 109 deletions(-)
 delete mode 100644 xen/arch/ppc/include/asm/device.h
 rename xen/{arch/arm/include/asm => include/asm-generic}/device.h (79%)
 create mode 100644 xen/include/headers++.chk.new

diff --git a/xen/arch/arm/device.c b/xen/arch/arm/device.c
index 1f631d3274..affbe79f9a 100644
--- a/xen/arch/arm/device.c
+++ b/xen/arch/arm/device.c
@@ -16,7 +16,10 @@
 #include 
 
 extern const struct device_desc _sdevice[], _edevice[];
+
+#ifdef CONFIG_ACPI
 extern const struct acpi_device_desc _asdevice[], _aedevice[];
+#endif
 
 int __init device_init(struct dt_device_node *dev, enum device_class class,
const void *data)
@@ -45,6 +48,7 @@ int __init device_init(struct dt_device_node *dev, enum 
device_class class,
 return -EBADF;
 }
 
+#ifdef CONFIG_ACPI
 int __init acpi_device_init(enum device_class class, const void *data, int 
class_type)
 {
 const struct acpi_device_desc *desc;
@@ -61,6 +65,7 @@ int __init acpi_device_init(enum device_class class, const 
void *data, int class
 
 return -EBADF;
 }
+#endif
 
 enum device_class device_get_class(const struct dt_device_node *dev)
 {
@@ -329,9 +334,13 @@ int handle_device(struct domain *d, struct dt_device_node 
*dev, p2m_type_t p2mt,
 struct map_range_data mr_data = {
 .d = d,
 .p2mt = p2mt,
-.skip_mapping = !own_device ||
-(is_pci_passthrough_enabled() &&
-(device_get_class(dev) == DEVICE_PCI_HOSTBRIDGE)),
+.skip_mapping =
+!own_device
+#ifdef CONFIG_HAS_PCI
+|| (is_pci_passthrough_enabled() &&
+(device_get_class(dev) == DEVICE_PCI_HOSTBRIDGE))
+#endif
+,
 .iomem_ranges = iomem_ranges,
 .irq_ranges = irq_ranges
 };
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 2dd2926b41..a26cbff68e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2361,7 +2361,7 @@ static int __init handle_node(struct domain *d, struct 
kernel_info *kinfo,
  * Replace these nodes with our own. Note that the original may be
  * used_by DOMID_XEN so this check comes first.
  */
-if ( device_get_class(node) == DEVICE_GIC )
+if ( device_get_class(node) == DEVICE_INTERRUPT_CONTROLLER )
 return make_gic_node(d, kinfo->fdt, node);
 if ( dt_match_node(timer_matches, node) )
 return make_timer_node(kinfo);
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index cf392bfd1c..5d6885e389 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -1366,7 +1366,7 @@ static const struct dt_device_match gicv2_dt_match[] 
__initconst =
 { /* sentinel */ },
 };
 
-DT_DEVICE_START(gicv2, 

[PATCH v4 14/14] xen/asm-generic: ifdef inclusion of

[Oleksii Kurochko]

ifdefing inclusion of  in 
allows to avoid generation of empty  header
for the case when !CONFIG_MEM_ACCESS.

For Arm it was explicitly added inclusion of  for p2m.c
and traps.c because they require some functions from  which
aren't available in case of !CONFIG_MEM_ACCESS.

Suggested-by: Jan Beulich 
Signed-off-by: Oleksii Kurochko 
---
Changes in V4:
 - Nothing changed. Only rebase.
---
Changes in V3:
 - Remove unnecessary comment.
---
 xen/arch/arm/p2m.c| 1 +
 xen/arch/arm/traps.c  | 1 +
 xen/arch/ppc/include/asm/mem_access.h | 5 -
 xen/include/xen/mem_access.h  | 2 ++
 4 files changed, 4 insertions(+), 5 deletions(-)
 delete mode 100644 xen/arch/ppc/include/asm/mem_access.h

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index b991b76ce4..2465c266e9 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 8492e2b7bb..0ad056b89b 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/xen/arch/ppc/include/asm/mem_access.h 
b/xen/arch/ppc/include/asm/mem_access.h
deleted file mode 100644
index e7986dfdbd..00
--- a/xen/arch/ppc/include/asm/mem_access.h
+++ /dev/null
@@ -1,5 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-#ifndef __ASM_PPC_MEM_ACCESS_H__
-#define __ASM_PPC_MEM_ACCESS_H__
-
-#endif /* __ASM_PPC_MEM_ACCESS_H__ */
diff --git a/xen/include/xen/mem_access.h b/xen/include/xen/mem_access.h
index 4e4811680d..87d93b31f6 100644
--- a/xen/include/xen/mem_access.h
+++ b/xen/include/xen/mem_access.h
@@ -33,7 +33,9 @@
  */
 struct vm_event_st;
 
+#ifdef CONFIG_MEM_ACCESS
 #include 
+#endif
 
 /*
  * Additional access types, which are used to further restrict
-- 
2.43.0

[PATCH v4 11/14] xen/asm-generic: introduce stub header numa.h

[Oleksii Kurochko]

 is common through some archs so it is moved
to asm-generic.

Signed-off-by: Oleksii Kurochko 
Reviewed-by: Michal Orzel 
---
Changes in V4:
 - Updated guards name: *ARCH_GENERIC* -> *ASM_GENERIC*.
 - Moved inclusion of xen/mm-frame.h under "#ifndef CONFIG_NUMA".
 - Added Reviewed-by: Michal Orzel .
---
Changes in V3:
 - Remove old header inclusion in asm-generic numa.h and include
and 
 - Drop Arm and PPC's numa.h and use asm-generic version instead.
---
Changes in V2:
- update the commit message.
- change u8 to uint8_t.
- add ifnded CONFIG_NUMA.
---
 xen/arch/arm/include/asm/Makefile |  1 +
 xen/arch/ppc/include/asm/Makefile |  1 +
 xen/arch/ppc/include/asm/numa.h   | 26 ---
 .../asm => include/asm-generic}/numa.h| 14 ++
 4 files changed, 11 insertions(+), 31 deletions(-)
 delete mode 100644 xen/arch/ppc/include/asm/numa.h
 rename xen/{arch/arm/include/asm => include/asm-generic}/numa.h (76%)

diff --git a/xen/arch/arm/include/asm/Makefile 
b/xen/arch/arm/include/asm/Makefile
index 55d153ed82..b8e0763c77 100644
--- a/xen/arch/arm/include/asm/Makefile
+++ b/xen/arch/arm/include/asm/Makefile
@@ -3,6 +3,7 @@ generic-y += altp2m.h
 generic-y += device.h
 generic-y += hardirq.h
 generic-y += iocap.h
+generic-y += numa.h
 generic-y += paging.h
 generic-y += percpu.h
 generic-y += random.h
diff --git a/xen/arch/ppc/include/asm/Makefile 
b/xen/arch/ppc/include/asm/Makefile
index bcddcc181a..a4188dce1d 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -6,6 +6,7 @@ generic-y += hardirq.h
 generic-y += hypercall.h
 generic-y += iocap.h
 generic-y += monitor.h
+generic-y += numa.h
 generic-y += paging.h
 generic-y += percpu.h
 generic-y += random.h
diff --git a/xen/arch/ppc/include/asm/numa.h b/xen/arch/ppc/include/asm/numa.h
deleted file mode 100644
index 7fdf66c3da..00
--- a/xen/arch/ppc/include/asm/numa.h
+++ /dev/null
@@ -1,26 +0,0 @@
-#ifndef __ASM_PPC_NUMA_H__
-#define __ASM_PPC_NUMA_H__
-
-#include 
-#include 
-
-typedef uint8_t nodeid_t;
-
-/* Fake one node for now. See also node_online_map. */
-#define cpu_to_node(cpu) 0
-#define node_to_cpumask(node)   (cpu_online_map)
-
-/*
- * TODO: make first_valid_mfn static when NUMA is supported on PPC, this
- * is required because the dummy helpers are using it.
- */
-extern mfn_t first_valid_mfn;
-
-/* XXX: implement NUMA support */
-#define node_spanned_pages(nid) (max_page - mfn_x(first_valid_mfn))
-#define node_start_pfn(nid) (mfn_x(first_valid_mfn))
-#define __node_distance(a, b) (20)
-
-#define arch_want_default_dmazone() (false)
-
-#endif /* __ASM_PPC_NUMA_H__ */
diff --git a/xen/arch/arm/include/asm/numa.h b/xen/include/asm-generic/numa.h
similarity index 76%
rename from xen/arch/arm/include/asm/numa.h
rename to xen/include/asm-generic/numa.h
index e2bee2bd82..b00fca4978 100644
--- a/xen/arch/arm/include/asm/numa.h
+++ b/xen/include/asm-generic/numa.h
@@ -1,12 +1,15 @@
-#ifndef __ARCH_ARM_NUMA_H
-#define __ARCH_ARM_NUMA_H
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GENERIC_NUMA_H
+#define __ASM_GENERIC_NUMA_H
 
-#include 
+#include 
 
-typedef u8 nodeid_t;
+typedef uint8_t nodeid_t;
 
 #ifndef CONFIG_NUMA
 
+#include 
+
 /* Fake one node for now. See also node_online_map. */
 #define cpu_to_node(cpu) 0
 #define node_to_cpumask(node)   (cpu_online_map)
@@ -26,7 +29,8 @@ extern mfn_t first_valid_mfn;
 
 #define arch_want_default_dmazone() (false)
 
-#endif /* __ARCH_ARM_NUMA_H */
+#endif /* __ASM_GENERIC_NUMA_H */
+
 /*
  * Local variables:
  * mode: C
-- 
2.43.0

[PATCH v4 09/14] xen/asm-generic: introduce generic header altp2m.h

[Oleksii Kurochko]

 is common between several archs so it is
moved to asm-generic.

Arm and PPC were switched to asm-generic version of altp2m.h.

Signed-off-by: Oleksii Kurochko 
Acked-by: Michal Orzel 
Acked-by: Jan Beulich 
---

 Michal asked the following:
  Shouldn't this copyright be moved to generic header as well?

 It seems to me that it is fine to leave the stub altp2m header
 without the copyright "Copyright (c) 2014, Intel Corporation.".
 
 If an absense of the copyright violates something I will be
 happy to add it during merge.

---
Changes in V4:
 - Update the commit message
 - Added Acked-by: Michal Orzel 
 - Added Acked-by: Jan Beulich 
---
Changes in V3:
 - Drop Arm and PPC's altp2m.h
 - Update the commit message.
---
Changes in V2:
- change uint16_t to unsigned int in declaration of altp2m_vcpu_idx
- update the commit message
---
 xen/arch/arm/include/asm/Makefile |  1 +
 xen/arch/arm/include/asm/altp2m.h | 39 ---
 xen/arch/ppc/include/asm/Makefile |  1 +
 xen/arch/ppc/include/asm/altp2m.h | 25 
 xen/include/asm-generic/altp2m.h  | 34 +++
 5 files changed, 36 insertions(+), 64 deletions(-)
 delete mode 100644 xen/arch/arm/include/asm/altp2m.h
 delete mode 100644 xen/arch/ppc/include/asm/altp2m.h
 create mode 100644 xen/include/asm-generic/altp2m.h

diff --git a/xen/arch/arm/include/asm/Makefile 
b/xen/arch/arm/include/asm/Makefile
index f6f108bbb8..55d153ed82 100644
--- a/xen/arch/arm/include/asm/Makefile
+++ b/xen/arch/arm/include/asm/Makefile
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
+generic-y += altp2m.h
 generic-y += device.h
 generic-y += hardirq.h
 generic-y += iocap.h
diff --git a/xen/arch/arm/include/asm/altp2m.h 
b/xen/arch/arm/include/asm/altp2m.h
deleted file mode 100644
index df50cb2f09..00
--- a/xen/arch/arm/include/asm/altp2m.h
+++ /dev/null
@@ -1,39 +0,0 @@
-/*
- * Alternate p2m
- *
- * Copyright (c) 2014, Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program; If not, see .
- */
-
-#ifndef __ASM_ARM_ALTP2M_H
-#define __ASM_ARM_ALTP2M_H
-
-#include 
-
-/* Alternate p2m on/off per domain */
-static inline bool altp2m_active(const struct domain *d)
-{
-/* Not implemented on ARM. */
-return false;
-}
-
-/* Alternate p2m VCPU */
-static inline uint16_t altp2m_vcpu_idx(const struct vcpu *v)
-{
-/* Not implemented on ARM, should not be reached. */
-BUG();
-return 0;
-}
-
-#endif /* __ASM_ARM_ALTP2M_H */
diff --git a/xen/arch/ppc/include/asm/Makefile 
b/xen/arch/ppc/include/asm/Makefile
index 5364bb1d59..319e90955b 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
+generic-y += altp2m.h
 generic-y += device.h
 generic-y += div64.h
 generic-y += hardirq.h
diff --git a/xen/arch/ppc/include/asm/altp2m.h 
b/xen/arch/ppc/include/asm/altp2m.h
deleted file mode 100644
index bd5c9aa415..00
--- a/xen/arch/ppc/include/asm/altp2m.h
+++ /dev/null
@@ -1,25 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-#ifndef __ASM_PPC_ALTP2M_H__
-#define __ASM_PPC_ALTP2M_H__
-
-#include 
-
-struct domain;
-struct vcpu;
-
-/* Alternate p2m on/off per domain */
-static inline bool altp2m_active(const struct domain *d)
-{
-/* Not implemented on PPC. */
-return false;
-}
-
-/* Alternate p2m VCPU */
-static inline uint16_t altp2m_vcpu_idx(const struct vcpu *v)
-{
-/* Not implemented on PPC, should not be reached. */
-ASSERT_UNREACHABLE();
-return 0;
-}
-
-#endif /* __ASM_PPC_ALTP2M_H__ */
diff --git a/xen/include/asm-generic/altp2m.h b/xen/include/asm-generic/altp2m.h
new file mode 100644
index 00..39865a842a
--- /dev/null
+++ b/xen/include/asm-generic/altp2m.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GENERIC_ALTP2M_H
+#define __ASM_GENERIC_ALTP2M_H
+
+#include 
+
+struct domain;
+struct vcpu;
+
+/* Alternate p2m on/off per domain */
+static inline bool altp2m_active(const struct domain *d)
+{
+/* Not implemented on GENERIC. */
+return false;
+}
+
+/* Alternate p2m VCPU */
+static inline unsigned int altp2m_vcpu_idx(const struct vcpu *v)
+{
+/* Not implemented on GENERIC, should not be reached. */
+BUG();
+return 0;
+}
+
+#endif /* __ASM_GENERIC_ALTP2M_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: BSD
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ 

[PATCH v4 07/14] xen/asm-generic: introduce generalized hardirq.h

[Oleksii Kurochko]

 is common through archs thereby it is moved
to asm-generic.

Arm and PPC were switched to asm generic verstion of hardirq.h.

Signed-off-by: Oleksii Kurochko 
Reviewed-by: Jan Beulich 
Acked-by: Julien Grall 
---
Changes in V4:
 - Added Reviewed-by: Jan Beulich .
 - Added Acked-by: Julien Grall .
---
Changes in V3:
 - Drop Arm and PPC's hardirq.h
 - Update the commit message.
---
Changes in V2:
- add #include .
- update the commit message
---
 xen/arch/arm/include/asm/Makefile |  1 +
 xen/arch/ppc/include/asm/Makefile |  1 +
 xen/arch/ppc/include/asm/hardirq.h| 19 ---
 .../asm => include/asm-generic}/hardirq.h |  8 +---
 4 files changed, 7 insertions(+), 22 deletions(-)
 delete mode 100644 xen/arch/ppc/include/asm/hardirq.h
 rename xen/{arch/arm/include/asm => include/asm-generic}/hardirq.h (79%)

diff --git a/xen/arch/arm/include/asm/Makefile 
b/xen/arch/arm/include/asm/Makefile
index 60bb78fc09..f6f108bbb8 100644
--- a/xen/arch/arm/include/asm/Makefile
+++ b/xen/arch/arm/include/asm/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 generic-y += device.h
+generic-y += hardirq.h
 generic-y += iocap.h
 generic-y += paging.h
 generic-y += percpu.h
diff --git a/xen/arch/ppc/include/asm/Makefile 
b/xen/arch/ppc/include/asm/Makefile
index edef0fab78..3241236c64 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 generic-y += device.h
+generic-y += hardirq.h
 generic-y += hypercall.h
 generic-y += iocap.h
 generic-y += paging.h
diff --git a/xen/arch/ppc/include/asm/hardirq.h 
b/xen/arch/ppc/include/asm/hardirq.h
deleted file mode 100644
index 343efc7e69..00
--- a/xen/arch/ppc/include/asm/hardirq.h
+++ /dev/null
@@ -1,19 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-#ifndef __ASM_PPC_HARDIRQ_H__
-#define __ASM_PPC_HARDIRQ_H__
-
-#include 
-
-typedef struct {
-unsigned long __softirq_pending;
-unsigned int __local_irq_count;
-} __cacheline_aligned irq_cpustat_t;
-
-#include /* Standard mappings for irq_cpustat_t above */
-
-#define in_irq() (local_irq_count(smp_processor_id()) != 0)
-
-#define irq_enter() (local_irq_count(smp_processor_id())++)
-#define irq_exit()  (local_irq_count(smp_processor_id())--)
-
-#endif /* __ASM_PPC_HARDIRQ_H__ */
diff --git a/xen/arch/arm/include/asm/hardirq.h 
b/xen/include/asm-generic/hardirq.h
similarity index 79%
rename from xen/arch/arm/include/asm/hardirq.h
rename to xen/include/asm-generic/hardirq.h
index 67b6a673db..ddccf460b9 100644
--- a/xen/arch/arm/include/asm/hardirq.h
+++ b/xen/include/asm-generic/hardirq.h
@@ -1,5 +1,6 @@
-#ifndef __ASM_HARDIRQ_H
-#define __ASM_HARDIRQ_H
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GENERIC_HARDIRQ_H
+#define __ASM_GENERIC_HARDIRQ_H
 
 #include 
 #include 
@@ -16,7 +17,8 @@ typedef struct {
 #define irq_enter() (local_irq_count(smp_processor_id())++)
 #define irq_exit()  (local_irq_count(smp_processor_id())--)
 
-#endif /* __ASM_HARDIRQ_H */
+#endif /* __ASM_GENERIC_HARDIRQ_H */
+
 /*
  * Local variables:
  * mode: C
-- 
2.43.0

[PATCH v4 13/14] xen: ifdef inclusion of in

[Oleksii Kurochko]

Ifdef-ing inclusion of  allows to avoid
generation of empty  for cases when
CONFIG_GRANT_TABLE is not enabled.

The following changes were done for Arm:
 should be included directly because it contains
gnttab_dom0_frames() macros which is unique for Arm and is used in
arch/arm/domain_build.c.
 is #ifdef-ed with CONFIG_GRANT_TABLE in
 so in case of !CONFIG_GRANT_TABLE gnttab_dom0_frames
won't be available for use in arch/arm/domain_build.c.

Suggested-by: Jan Beulich 
Signed-off-by: Oleksii Kurochko 
---
Changes in V4:
 - Nothing changed. Only rebase.
---
Changes in V3:
 - Remove unnecessary comment.
---
 xen/arch/arm/domain_build.c| 1 +
 xen/arch/ppc/include/asm/grant_table.h | 5 -
 xen/include/xen/grant_table.h  | 3 +++
 3 files changed, 4 insertions(+), 5 deletions(-)
 delete mode 100644 xen/arch/ppc/include/asm/grant_table.h

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index a26cbff68e..646862b10e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -34,6 +34,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #define STATIC_EVTCHN_NODE_SIZE_CELLS 2
diff --git a/xen/arch/ppc/include/asm/grant_table.h 
b/xen/arch/ppc/include/asm/grant_table.h
deleted file mode 100644
index d0ff58dd3d..00
--- a/xen/arch/ppc/include/asm/grant_table.h
+++ /dev/null
@@ -1,5 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-#ifndef __ASM_PPC_GRANT_TABLE_H__
-#define __ASM_PPC_GRANT_TABLE_H__
-
-#endif /* __ASM_PPC_GRANT_TABLE_H__ */
diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h
index 85fe6b7b5e..50edfecfb6 100644
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -26,7 +26,10 @@
 #include 
 #include 
 #include 
+
+#ifdef CONFIG_GRANT_TABLE
 #include 
+#endif
 
 struct grant_table;
 
-- 
2.43.0

[PATCH v4 05/14] xen/asm-generic: introduce stub header

[Oleksii Kurochko]

 is common for Arm, PPC and RISC-V thereby it
is moved to asm-generic.

Signed-off-by: Oleksii Kurochko 
Reviewed-by: Jan Beulich 
Acked-by: Julien Grall 
---
Changes in V4:
 - Added Reviewed-by: Jan Beulich 
 - Added Acked-by: Julien Grall 
---
Changes in V3:
 - Drop Arm and PPC's random.h and switch to asm-generic verison.
---
Changes in V2:
 - update the commit messages
---
 xen/arch/arm/include/asm/Makefile |  1 +
 xen/arch/arm/include/asm/random.h |  9 -
 xen/arch/ppc/include/asm/Makefile |  1 +
 xen/arch/ppc/include/asm/random.h |  9 -
 xen/include/asm-generic/random.h  | 19 +++
 5 files changed, 21 insertions(+), 18 deletions(-)
 delete mode 100644 xen/arch/arm/include/asm/random.h
 delete mode 100644 xen/arch/ppc/include/asm/random.h
 create mode 100644 xen/include/asm-generic/random.h

diff --git a/xen/arch/arm/include/asm/Makefile 
b/xen/arch/arm/include/asm/Makefile
index 2d036045e8..7593c4e8ee 100644
--- a/xen/arch/arm/include/asm/Makefile
+++ b/xen/arch/arm/include/asm/Makefile
@@ -2,4 +2,5 @@
 generic-y += device.h
 generic-y += iocap.h
 generic-y += paging.h
+generic-y += random.h
 generic-y += vm_event.h
diff --git a/xen/arch/arm/include/asm/random.h 
b/xen/arch/arm/include/asm/random.h
deleted file mode 100644
index b4acee276b..00
--- a/xen/arch/arm/include/asm/random.h
+++ /dev/null
@@ -1,9 +0,0 @@
-#ifndef __ASM_RANDOM_H__
-#define __ASM_RANDOM_H__
-
-static inline unsigned int arch_get_random(void)
-{
-return 0;
-}
-
-#endif /* __ASM_RANDOM_H__ */
diff --git a/xen/arch/ppc/include/asm/Makefile 
b/xen/arch/ppc/include/asm/Makefile
index 209c3e74d8..039b0eb5ee 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -3,4 +3,5 @@ generic-y += device.h
 generic-y += hypercall.h
 generic-y += iocap.h
 generic-y += paging.h
+generic-y += random.h
 generic-y += vm_event.h
diff --git a/xen/arch/ppc/include/asm/random.h 
b/xen/arch/ppc/include/asm/random.h
deleted file mode 100644
index 2f9e9bbae4..00
--- a/xen/arch/ppc/include/asm/random.h
+++ /dev/null
@@ -1,9 +0,0 @@
-#ifndef __ASM_PPC_RANDOM_H__
-#define __ASM_PPC_RANDOM_H__
-
-static inline unsigned int arch_get_random(void)
-{
-return 0;
-}
-
-#endif /* __ASM_PPC_RANDOM_H__ */
diff --git a/xen/include/asm-generic/random.h b/xen/include/asm-generic/random.h
new file mode 100644
index 00..d0d35dd217
--- /dev/null
+++ b/xen/include/asm-generic/random.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GENERIC_RANDOM_H__
+#define __ASM_GENERIC_RANDOM_H__
+
+static inline unsigned int arch_get_random(void)
+{
+return 0;
+}
+
+#endif /* __ASM_GENERIC_RANDOM_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: BSD
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.43.0

Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h

[Jan Beulich]

On 27.11.2023 15:13, Oleksii Kurochko wrote:
> Arm, PPC and RISC-V use the same device.h thereby device.h
> was moved to asm-generic. Arm's device.h was taken as a base with
> the following changes:
>  - #ifdef PCI related things.
>  - #ifdef ACPI related things.
>  - Rename #ifdef guards.
>  - Add SPDX tag.
>  - #ifdef CONFIG_HAS_DEVICE_TREE related things.
>  - #ifdef-ing iommu related things with CONFIG_HAS_PASSTHROUGH.
> 
> Also Arm and PPC are switched to asm-generic version of device.h
> 
> Signed-off-by: Oleksii Kurochko 
> ---
> Changes in V4:
>  - Updated the commit message
>  - Switched Arm and PPC to asm-generic version of device.h
>  - Replaced HAS_PCI with CONFIG_HAS_PCI
>  - ifdef-ing iommu filed of dev_archdata struct with CONFIG_HAS_PASSTHROUGH
>  - ifdef-ing iommu_fwspec of device struct with CONFIG_HAS_PASSTHROUGH
>  - ifdef-ing DT related things with CONFIG_HAS_DEVICE_TREE
>  - Updated the commit message ( remove a note with question about
>if device.h should be in asm-generic or not )
>  - Replaced DEVICE_IC with DEVICE_INTERRUPT_CONTROLLER
>  - Rationalized usage of CONFIG_HAS_* in device.h
>  - Fixed indents for ACPI_DEVICE_START and ACPI_DEVICE_END
> ---
> Changes in V3:
>  - ifdef device tree related things.
>  - update the commit message
> ---
> Changes in V2:
>   - take ( as common ) device.h from Arm as PPC and RISC-V use it as a 
> base.
>   - #ifdef PCI related things.
>   - #ifdef ACPI related things.
>   - rename DEVICE_GIC to DEVIC_IC.
>   - rename #ifdef guards.
>   - switch Arm and PPC to generic device.h
>   - add SPDX tag
>   - update the commit message
> 
> ---
>  xen/arch/arm/device.c |  15 ++-
>  xen/arch/arm/domain_build.c   |   2 +-
>  xen/arch/arm/gic-v2.c |   4 +-
>  xen/arch/arm/gic-v3.c |   6 +-
>  xen/arch/arm/gic.c|   4 +-
>  xen/arch/arm/include/asm/Makefile |   1 +
>  xen/arch/ppc/include/asm/Makefile |   1 +
>  xen/arch/ppc/include/asm/device.h |  53 
>  xen/arch/ppc/include/asm/irq.h|   4 +
>  .../asm => include/asm-generic}/device.h  | 125 +++---
>  xen/include/headers++.chk.new |   0
>  11 files changed, 106 insertions(+), 109 deletions(-)
>  delete mode 100644 xen/arch/ppc/include/asm/device.h
>  rename xen/{arch/arm/include/asm => include/asm-generic}/device.h (79%)
>  create mode 100644 xen/include/headers++.chk.new

Stray new file, presumably because of a missing entry in .gitignore?

Overall I think there are too many changes done all in one go here.
But it's mostly Arm which is affected, so I'll leave judging about that
to the Arm maintainers.

> --- a/xen/arch/ppc/include/asm/irq.h
> +++ b/xen/arch/ppc/include/asm/irq.h
> @@ -3,7 +3,9 @@
>  #define __ASM_PPC_IRQ_H__
>  
>  #include 
> +#ifdef CONFIG_HAS_DEVICE_TREE
>  #include 
> +#endif
>  #include 

Why would this #ifdef not cover the public header as well? (Otherwise I'd
be inclined to ask that the conditional be moved inside that header.)

Jan

Re: MISRA: Compatible declarations for sort and bsearch

[Nicola Vetrini]

On 2023-11-24 10:40, Nicola Vetrini wrote:

Hi all,

in xen/lib.h and xen/sort.h there are definitions of the functions 
bsearch and sort that have no prior declarations, and therefore are 
subject to a violation of MISRA C Rule 8.4.


I'm wondering whether it would be preferred

1. to put a declaration just before the definition, in lib.h and sort.h
2. deviate these functions, as their signatures are well-known and 
somewhat standardized


other resolution strategies are possible, but I think these are the 
main ones.


Still on the matter of Rule 8.4, though not related to bsearch or sort:

- the definition of do_mca in x86/cpu/mcheck/mca.c has the following 
header:

  #include  /* for do_mca */
  which in turn leads to x86/include/asm/hypercall.h, which includes the 
following:

  #include  /* for do_mca */

  where I can't see a declaration for do_mca, as I would have expected. 
I'd like to understand what's going on here, since I may be missing some 
piece of information (perhaps something is generated during the build).


- x86/traps.c do_general_protection may want a declaration in 
x86/include/asm/traps.h, or perhaps it should gain the asmlinkage 
attribute, given that it's used only by asm and the TU that defines it.


- function test and variable data in x86/efi/check.c look like they 
should not be MISRA compliant, so they may be added to the 
exclude-list.json


- given the comment in xen/common/page_alloc.c for first_valid_mfn

/*
 * first_valid_mfn is exported because it is use in ARM specific NUMA
 * helpers. See comment in arch/arm/include/asm/numa.h.
 */
mfn_t first_valid_mfn = INVALID_MFN_INITIALIZER;

and the related ARM comment

/*
 * TODO: make first_valid_mfn static when NUMA is supported on Arm, this
 * is required because the dummy helpers are using it.
 */
extern mfn_t first_valid_mfn;

it should probably be deviated.

- compat_set_{px,cx}_pminfo in x86/x86_64/cpufreq.c are perhaps declared 
with an autogenerated header?


--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

Re: [PATCH v4 06/14] xen/asm-generic: introduce generic header percpu.h

[Jan Beulich]

On 27.11.2023 15:13, Oleksii Kurochko wrote:
> The patch introduces generic percpu.h which was based on Arm's version
> with the following changes:
>  * makes __per_cpu_data_end[] constant
>  * introduce get_per_cpu_offset() for macros this_cpu() and this_cpu_ptr()
>  * add inclustion of  as get_per_cpu_offset() is located there.
> 
> Also it was changed a place where  is included in 
> because asm-generic version of percpu.h started to include  
> which
> requires definition of DECLARE_PER_CPU.
> 
> As well the patch switches Arm, PPC and x86 architectures to use asm-generic
> version of percpu.h.
> 
> Signed-off-by: Oleksii Kurochko 
> Acked-by: Julien Grall 

Reviewed-by: Jan Beulich 

Re: [XEN PATCH v2 2/3] xen/arm: add SAF deviation for debugging and logging effects

[Simone Ballarin]

On 27/11/23 11:46, Jan Beulich wrote:

On 24.11.2023 18:29, Simone Ballarin wrote:

Rule 13.1: Initializer lists shall not contain persistent side effects

Effects caused by debug/logging macros and functions (like ASSERT, 
__bad_atomic_size,
LOG, etc ...) that crash execution or produce logs are not dangerous in 
initializer
lists. The evaluation order in abnormal conditions is not relevant. Evaluation 
order
of logging effects is always safe.

This patch deviates violations using SAF commits caused by debug/logging macros 
and
functions.

Asm volatile statements in initializer lists that do not perform any persistent 
side
effect are safe: this patch deviates violations caused by uses of the current 
macro
(that contains an asm volatile) in initializer lists.

No functional changes.

Signed-off-by: Simone Ballarin 

---
Changes in v2:
New patch based on the discussion for "xen/arm: address violations of MISRA C:2012 
Rule 13.1".
---
  docs/misra/safe.json | 16 
  xen/arch/arm/device.c|  1 +
  xen/arch/arm/guestcopy.c |  4 
  xen/arch/x86/hvm/hvm.c   |  1 +
  xen/common/sched/core.c  |  3 +++


The latter two don't really fit the title prefix.


--- a/docs/misra/safe.json
+++ b/docs/misra/safe.json
@@ -28,6 +28,22 @@
  },
  {
  "id": "SAF-3-safe",
+"analyser": {
+"eclair": "MC3R1.R13.1"
+},
+"name": "MC3R1.R13.1: effects for debugging and logging",
+"text": "Effects for debugging and loggings reasons that crash 
execution or produce logs are allowed in initializer lists. The evaluation order in abnormal 
conditions is not relevant."
+},


I'm wary of this statement. Order may not matter much anymore _after_ an
abnormal condition was encountered, but in the course of determining whether
an abnormal condition might have been reached it may very well still matter.



Do you object to the deviation in general? Or just to the wording?


+{
+"id": "SAF-4-safe",
+"analyser": {
+"eclair": "MC3R1.R13.1"
+},
+"name": "MC3R1.R13.1: volatile asm statements that do not perform any 
persistent side effect",
+"text": "Volatile asm statements in an initializer list if do not 
perform persistent side effects are safe."


Since each respective comment ought to affect just a single asm(), I think
this wants writing in singular. I further don't think it is useful for
"text" to largely restate what "name" already says.



Ok.


--- a/xen/arch/arm/device.c
+++ b/xen/arch/arm/device.c
@@ -331,6 +331,7 @@ int handle_device(struct domain *d, struct dt_device_node 
*dev, p2m_type_t p2mt,
  .p2mt = p2mt,
  .skip_mapping = !own_device ||
  (is_pci_passthrough_enabled() &&
+/* SAF-3-safe effects for debugging/logging reasons 
are safe */
  (device_get_class(dev) == DEVICE_PCI_HOSTBRIDGE)),


What's the debugging / logging reason on the commented line?


The "ASSERT(dev != NULL)" in its body.




--- a/xen/arch/arm/guestcopy.c
+++ b/xen/arch/arm/guestcopy.c
@@ -110,18 +110,21 @@ static unsigned long copy_guest(void *buf, uint64_t addr, 
unsigned int len,
  unsigned long raw_copy_to_guest(void *to, const void *from, unsigned int len)
  {
  return copy_guest((void *)from, (vaddr_t)to, len,
+  /* SAF-4-safe No persistent side effects */
GVA_INFO(current), COPY_to_guest | COPY_linear);
  }
  
  unsigned long raw_copy_to_guest_flush_dcache(void *to, const void *from,

   unsigned int len)
  {
+/* SAF-4-safe No persistent side effects */
  return copy_guest((void *)from, (vaddr_t)to, len, GVA_INFO(current),
COPY_to_guest | COPY_flush_dcache | COPY_linear);
  }
  
  unsigned long raw_clear_guest(void *to, unsigned int len)

  {
+/* SAF-4-safe No persistent side effects */
  return copy_guest(NULL, (vaddr_t)to, len, GVA_INFO(current),
COPY_to_guest | COPY_linear);
  }
@@ -129,6 +132,7 @@ unsigned long raw_clear_guest(void *to, unsigned int len)
  unsigned long raw_copy_from_guest(void *to, const void __user *from,
unsigned int len)
  {
+/* SAF-4-safe No persistent side effects */
  return copy_guest(to, (vaddr_t)from, len, GVA_INFO(current),
COPY_from_guest | COPY_linear);
  }


I can only guess that in all four of these it's the use of "current" which
requires the comment. Yet imo that either needs making explicit, or such a
comment shouldn't go on use sites of "current", but on its definition site.



"current" does not contain any violation of R13.1. Its expansion
produces a side-effect, but this is not a problem in itself. The real
problem is that GVA_INFO expands it in an initializer list:
#define GVA_INFO(vcpu) ((copy_inf

Re: [PATCH v4 11/14] xen/asm-generic: introduce stub header numa.h

[Jan Beulich]

On 27.11.2023 15:13, Oleksii Kurochko wrote:
>  is common through some archs so it is moved
> to asm-generic.
> 
> Signed-off-by: Oleksii Kurochko 
> Reviewed-by: Michal Orzel 

Acked-by: Jan Beulich 

Re: [PATCH v4 12/14] xen/asm-generic: introduce stub header softirq.h

[Jan Beulich]

On 27.11.2023 15:13, Oleksii Kurochko wrote:
>  is common between Arm, PPC and RISC-V so it is
> moved to asm-generic.
> 
> Drop Arm and PPC's softirq.h and use asm-generic version instead.
> 
> Signed-off-by: Oleksii Kurochko 
> Reviewed-by: Michal Orzel 
> Added Acked-by: Jan Beulich 

In case a v5 is needed, please drop the stray "Added".

Jan

Re: [PATCH v4 13/14] xen: ifdef inclusion of in

[Jan Beulich]

On 27.11.2023 15:13, Oleksii Kurochko wrote:
> --- a/xen/arch/ppc/include/asm/grant_table.h
> +++ /dev/null
> @@ -1,5 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -#ifndef __ASM_PPC_GRANT_TABLE_H__
> -#define __ASM_PPC_GRANT_TABLE_H__
> -
> -#endif /* __ASM_PPC_GRANT_TABLE_H__ */

Removing this header would be correct only if GRANT_TABLE had a "depends on
!PPC", I'm afraid. Recall that the earlier randconfig adjustment in CI was
actually requested to be undone, at which point what an arch's defconfig
says isn't necessarily what a randconfig should use.

Jan

[PATCH] ubsan: Introduce CONFIG_UBSAN_FATAL to panic on UBSAN failure

[Michal Orzel]

Introduce the CONFIG_UBSAN_FATAL option to cater to scenarios where prompt
attention to undefined behavior issues, notably during CI test runs, is
essential. When enabled, this option causes Xen to panic upon detecting
UBSAN failure (as the last step in ubsan_epilogue()).

Signed-off-by: Michal Orzel 
---
 xen/Kconfig.debug| 7 +++
 xen/common/ubsan/ubsan.c | 4 
 2 files changed, 11 insertions(+)

diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index 94e818ee09b5..e19e9d48817c 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -107,6 +107,13 @@ config UBSAN
 
  If unsure, say N here.
 
+config UBSAN_FATAL
+   bool "Panic on UBSAN failure"
+   depends on UBSAN
+   help
+ Enabling this option will cause Xen to panic when an undefined 
behavior
+ is detected by UBSAN. If unsure, say N here.
+
 config DEBUG_TRACE
bool "Debug trace support"
---help---
diff --git a/xen/common/ubsan/ubsan.c b/xen/common/ubsan/ubsan.c
index a3a80fa99eec..dd5ee0013648 100644
--- a/xen/common/ubsan/ubsan.c
+++ b/xen/common/ubsan/ubsan.c
@@ -174,6 +174,10 @@ static void ubsan_epilogue(unsigned long *flags)
"\n");
spin_unlock_irqrestore(&report_lock, *flags);
current->in_ubsan--;
+
+#ifdef CONFIG_UBSAN_FATAL
+   panic("UBSAN failure detected\n");
+#endif
 }
 
 static void handle_overflow(struct overflow_data *data, unsigned long lhs,
-- 
2.25.1

Re: [PATCH v4 14/14] xen/asm-generic: ifdef inclusion of

[Jan Beulich]

On 27.11.2023 15:13, Oleksii Kurochko wrote:
> --- a/xen/arch/ppc/include/asm/mem_access.h
> +++ /dev/null
> @@ -1,5 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -#ifndef __ASM_PPC_MEM_ACCESS_H__
> -#define __ASM_PPC_MEM_ACCESS_H__
> -
> -#endif /* __ASM_PPC_MEM_ACCESS_H__ */

Same as for the grant table change - if you want to remove this header
(and not put one in place for RISC-V), MEM_ACCESS needs to be impossible
to turn on for PPC and RISC-V.

Jan

Re: [PATCH 1/2] livepatch-build-tools: remove usage of gawk

[Roger Pau Monné]

On Fri, Nov 24, 2023 at 12:10:57PM +, Alejandro Vallejo wrote:
> On 24/11/2023 11:43, Ross Lagerwall wrote:
> > On Thu, Nov 23, 2023 at 4:08 PM Roger Pau Monne 
> wrote:
> >>
> >> And instead use plain awk.
> >>
> >> There's no need to use the --non-decimal-data option for gawk, since the
> >> numbers that we want to print are already prefixed with '0x', and so
> plain awk
> >> will do the conversion from hexadecimal to decimal just fine.
> >
> > I don't think that's true (at least with gnu awk 5.1.1):
> >
> > $ echo '   DW_AT_byte_size   : 0x14' | awk '{printf("%d\n", $4)}'
> > 0
> >
> 
> I think it's a FreeBSD quirk. From their man page:

Seems like BusyBox inherited that behavior:

BusyBox v1.36.1 (2023-07-27 17:12:24 UTC) multi-call binary.
$ echo '   DW_AT_byte_size   : 0x14' | awk '{printf("%d\n", $4)}'
20

I've assumed that Busybox was the lowest common denominator, but not
in this case.

> >Historically,  awk did not accept "0x" as a hex string. However,
> since
> >One True Awk used strtod to convert strings to floats, and since
> "0x12"
> >is a valid hexadecimal representation of a floating  point number,
> On
> >FreeBSD, awk has accepted this notation as an extension since One
> True
> >Awk was imported in FreeBSD 5.0. Upstream One True  Awk has
> restored
> >the  historical behavior for better compatibility between the
> different
> >awk implementations.  Both gawk and mawk already  behave similarly.
> >Starting with FreeBSD 14.0 awk will no longer accept this
> extension.
> 
> I'm guessing the latest FreeBSD doesn't have that behaviour either?

Hm, I guess my FreeBSD world is not new enough, as the awk (version
20210724) I have does parse hex numbers:

# echo '   DW_AT_byte_size   : 0x14' | awk '{printf("%d\n", $4)}'
20

Will see what I can do to address this without requiring gawk if
possible.

I'm not sure if there are other issues that would prevent
livepatch-build-tools from working on FreeBSD however.

Thanks, Roger.

Re: [PATCH] x86/x2apic: introduce a mixed physical/cluster mode

[Roger Pau Monné]

On Mon, Nov 27, 2023 at 11:49:03AM +, Andrew Cooper wrote:
> On 24/11/2023 7:54 pm, Neowutran wrote:
> > Hi, 
> > I did some more tests and research, indeed this patch improved/solved my 
> > specific case. 
> >
> > Starting point: 
> >
> > I am using Xen version 4.17.2 (exactly this source 
> > https://github.com/QubesOS/qubes-vmm-xen).
> > In the bios (a Asus motherboard), I configured the "local apic" parameter 
> > to "X2APIC".
> > For Xen, I did not set the parameter "x2apic-mode" nor the parameter 
> > "x2apic_phys". 
> >
> > Case 1:
> > I tryied to boot just like that, result: system is unusuably slow
> >
> > Case 2:
> > Then, I applied a backport of the patch  
> > https://lore.kernel.org/xen-devel/20231106142739.19650-1-roger@citrix.com/raw
> >  
> > to the original Xen version of QubesOS and I recompiled. 
> > (https://github.com/neowutran/qubes-vmm-xen/blob/x2apic3/X2APIC.patch)
> > Result: it work, the system is usable. 
> >
> > Case 3:
> > Then, I applied the patch 
> > https://github.com/xen-project/xen/commit/26a449ce32cef33f2cb50602be19fcc0c4223ba9
> > to the original Xen version of QubesOS and I recompiled.
> > (https://github.com/neowutran/qubes-vmm-xen/blob/x2apic4/X2APIC.patch)
> > Result: system is  
> > unusuably slow. 
> >
> >
> > In "Case 2", the value returned by the function "apic_x2apic_probe" is 
> > "&apic_x2apic_mixed". 
> > In "Case 3", the value returned by the function "apic_x2apic_probe" is 
> > "&apic_x2apic_cluster". 
> >
> >
> > ---
> > If you want / need, details for the function "apic_x2apic_probe":
> >
> > Known "input" value:
> >
> > "CONFIG_X2APIC_PHYSICAL" is not defined
> > "iommu_intremap == iommu_intremap_off" = false
> > "acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL" -> 0
> > "acpi_gbl_FADT.flags" = 247205 (in decimal)
> > "CONFIG_X2APIC_PHYSICAL" is not defined
> > "CONFIG_X2APIC_MIXED" is defined, because it is the default choice
> > "x2apic_mode" = 0
> > "x2apic_phys" = -1
> >
> >
> >
> > Trace log (I did some call "printk" to trace what was going on)
> > Case 2:
> > (XEN) NEOWUTRAN: X2APIC_MODE: 0 
> > (XEN) NEOWUTRAN: X2APIC_PHYS: -1 
> > (XEN) NEOWUTRAN: acpi_gbl_FADT.flags: 247205 
> > (XEN) NEOWUTRAN IOMMU_INTREMAP: different 
> > (XEN) Neowutran: PASSE 2 
> > (XEN) Neowutran: PASSE 4 
> > (XEN) NEOWUTRAN: X2APIC_MODE: 3 
> > (XEN) Neowutran: PASSE 7 
> > (XEN) NEOWUTRAN: X2APIC_MODE: 3 
> >  
> > (XEN) NEOWUTRAN: X2APIC_PHYS: -1 
> > (XEN) NEOWUTRAN: acpi_gbl_FADT.flags: 247205 
> > (XEN) NEOWUTRAN IOMMU_INTREMAP: different 
> >
> > Case 3:
> > (XEN) NEOWUTRAN2: X2APIC_PHYS: -1 
> > (XEN) NEOWUTRAN2: acpi_gbl_FADT.flags: 247205 
> > (XEN) NEOWUTRAN2 IOMMU_INTREMAP: different 
> > (XEN) Neowutran2: Passe 1 
> > (XEN) NEOWUTRAN2: X2APIC_PHYS: 0 
> > (XEN) Neowutran2: Passe 6 
> > (XEN) Neowutran2: Passe 7 
> > (XEN) NEOWUTRAN2: X2APIC_PHYS: 0 
> > (XEN) NEOWUTRAN2: acpi_gbl_FADT.flags: 247205 
> > (XEN) NEOWUTRAN2 IOMMU_INTREMAP: different 
> > (XEN) Neowutran2: Passe 2 
> > (XEN) Neowutran2: Passe 4 
> > (XEN) Neowutran2: Passe 7
> >
> >
> >
> > If you require the full logs, I could publish the full logs somewhere.
> > --
> >
> > ( However I do not understand if the root issue is a buggy motherboard, a
> > bug in xen, or if the parameter "X2APIC_PHYSICAL" should have been set
> > by the QubesOS project, or something else)
> 
> Hello,
> 
> Thankyou for the analysis.
> 
> For your base version of QubeOS Xen, was that 4.13.2-5 ?   I can't see
> any APIC changes in the patchqueue, and I believe all relevant bugfixes
> are in 4.17.2, but I'd just like to confirm.
> 
> First, by "unusable slow", other than the speed, did everything else
> appear to operate adequately?  Any chance you could guess the slowdown. 
> i.e. was it half the speed, or "seconds per log console line during
> boot" levels of slow?
> 
> 
> Having re-reviewed 26a449ce32, the patch is correct but the reasoning is
> wrong.
> 
> ACPI_FADT_APIC_CLUSTER predates x2APIC by almost a decade (it appeared
> in ACPI 3.0), and is not relevant outside of xAPIC mode.  xAPIC has 2
> different logical destination modes, cluster and flat, and their
> applicability is dependent on whether you have fewer or more than 8
> local APICs, hence that property being called out in the ACPI spec.
> 
> x2APIC does not have this property.  DFR was removed from the
> architecture, and logical mode is strictly cluster.  So the bit should
> never have been interpreted on an x2APIC code path.

FWIW, Jan also pointed out that the ACPI spec mentions xAPIC strictly,
even for ACPI_FADT_APIC_PHYSICAL.  It's possible APIC_PHYSICAL should
also be enforced only in xAPIC mode.  Or it's also possible the ACPI
spec was not updated to mention both xAPIC and x2APIC modes.

Roger.

Re: [PATCH] x86/xen: fix percpu vcpu_info allocation

[Boris Ostrovsky]

On 11/24/23 2:48 AM, Juergen Gross wrote:

Today the percpu struct vcpu_info is allocated via DEFINE_PER_CPU(),
meaning that it could cross a page boundary. In this case registering
it with the hypervisor will fail, resulting in a panic().

This can easily be fixed by using DEFINE_PER_CPU_ALIGNED() instead,
as struct vcpu_info is guaranteed to have a size of 64 bytes, matching
the cache line size of x86 64-bit processors (Xen doesn't support
32-bit processors).

Fixes: 5ead97c84fa7 ("xen: Core Xen implementation")
Signed-off-by: Juergen Gross 


Reviewed-by: Boris Ostrovsky 

although I am not sure in usefulness of BUILD_BUG_ON --- 64 bytes is part of 
ABI and hypervisor already has its own BUILD_BUG_ON for this.


-boris



@@ -160,6 +163,7 @@ void xen_vcpu_setup(int cpu)
int err;
struct vcpu_info *vcpup;
  
+	BUILD_BUG_ON(sizeof(*vcpup) > SMP_CACHE_BYTES);

BUG_ON(HYPERVISOR_shared_info == &xen_dummy_shared_info);