Re: [PATCH v2 3/3] svm/nestedsvm: Introduce nested capabilities bit

[Jan Beulich]

On 28.03.2024 11:57, George Dunlap wrote:
> On Thu, Mar 28, 2024 at 6:44 AM Jan Beulich  wrote:
>>> As to why to have each vendor independent code check for HAP -- there
>>> are in fact two implementations of the code; it's nice to be able to
>>> look in one place for each implementation to determine the
>>> requirements.  Additionally, it would be possible, in the future, for
>>> one of the nested virt implementations to enable shadow mode, while
>>> the other one didn't.  The current arrangement makes that easy.
>>
>> Well, first - how likely is this, when instead we've been considering
>> whether we could get rid of shadow mode? And then this is balancing
>> between ease of future changes vs avoiding redundancy where it can be
>> avoided. I'm not going to insist on centralizing the HAP check, but I
>> certainly wanted the option to be considered.
> 
> I considered it before replying to you; so consider it considered. :-)
> 
> --- a/xen/arch/x86/hvm/nestedhvm.c
> +++ b/xen/arch/x86/hvm/nestedhvm.c
> @@ -150,6 +150,16 @@ static int __init cf_check nestedhvm_setup(void)
>  __clear_bit(0x80, shadow_io_bitmap[0]);
>  __clear_bit(0xed, shadow_io_bitmap[1]);
>
> +/*
> + * NB this must be called after all command-line processing has been
> + * done, so that if (for example) HAP is disabled, nested virt is
> + * disabled as well.
> + */
> +if ( cpu_has_vmx )
> +start_nested_vmx(_funcs);
> +else if ( cpu_has_svm )
> +start_nested_svm(_funcs);

 Instead of passing the pointer, couldn't you have the functions return
 bool, and then set hvm_funcs.caps.nested_virt from that? Passing that
 pointer looks somewhat odd to me.
>>>
>>> For one, it makes start_nested_XXX symmetric to start_XXX, which also
>>> has access to the full hvm_funcs structure (albeit in the former case
>>> because it's creating the structure).
>>
>> This last aspect is the crucial aspect: start{svm,vmx}() are indeed quite
>> special because of this. Everywhere else we have accessor helpers when
>> hvm_funcs needs consulting, e.g. ...
>>
>>>  For two, both of them need to read caps.hap.
>>
>> ... caps _reads_ would want using (an) accessor(s), too.
>>
>>>  For three, it's just more flexible -- there may be
>>> future things that we want to modify in the start_nested_*()
>>> functions.  If we did as you suggest, and then added (say)
>>> caps.nested_virt_needs_hap, then we'd either need to add a second
>>> function, or refactor it to look like this.
>>
>> Right, I can see this as an argument when taking, as you do, speculation
>> on future needs into account. Albeit I'm then inclined to say that even
>> such modifications may better be done through accessor helpers.
> 
> Why access it through accessor helpers?
> 
> I consider that it's the SVM and VMX "construction/setup" code
> respectively which "own" hvm_funcs (as evidenced by the fact that they
> create the structures and then return them); and I consider the
> start_nested_{vmx/svm} to be a part of the same code; so I consider it
> valid for them to have direct access to the structure.
> 
 Is there a reason to use direct calls here rather than a true hook
 (seeing that hvm_funcs must have been populated by the time we make it
 here)? I understand we're (remotely) considering to switch away from
 using hooks at some point, but right now this feels somewhat
 inconsistent if not further justified.
>>>
>>> Again it mimics the calls to start_vmx/svm in hvm_enable.  But I could
>>> look at adding a function pointer to see if it works.
>>
>> Andrew - do you have any input here towards those somewhat vague plans
>> of getting rid of the hook functions? I guess they're more relevant in
>> places where we can't easily use the altcall machinery (i.e. not here).
> 
> Rather than do all that work collecting information, why don't we just
> check it in as it is?  Obviously if we're thinking about getting rid
> of hook functions at some point anyway, it can't be all that bad.
> There is an aesthetic justification for the lack of hook, in that it's
> similar to the start_vmx/svm() functions.
> 
> So far I really don't think the remaining "open" changes we're
> discussing are worth either of our efforts.  I don't plan to make any
> of these changes unless another x86 maintainer seconds your request
> (or you can convince me they're worth making).
> 
> (The other two changes -- correcting the function name in the commit
> message, and removing the extra blank line -- I've already changed
> locally, so could check in with your ack.)

After some mumbling to myself over the holidays
Acked-by: Jan Beulich 

Jan

[ovmf test] 185223: all pass - PUSHED

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf b3871141136ac643332587f33f2e4fa78d72eb07
baseline version:
 ovmf 8f698f0a646124ede518d3e255ef725de1239639

Last test of basis   185212  2024-04-01 03:13:04 Z2 days
Testing same since   185223  2024-04-03 02:42:59 Z0 days1 attempts


People who touched revisions under test:
  Qingyu 

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



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   8f698f0a64..b387114113  b3871141136ac643332587f33f2e4fa78d72eb07 -> 
xen-tested-master

[xen-4.17-testing test] 185217: tolerable FAIL - PUSHED

[osstest service owner]

flight 185217 xen-4.17-testing real [real]
flight 185221 xen-4.17-testing real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/185217/
http://logs.test-lab.xenproject.org/osstest/logs/185221/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl18 guest-start/debian.repeat fail pass in 185221-retest

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

version targeted for testing:
 xen  9bc40dbcf9eafccc1923b2555286bf6a2af03b7a
baseline version:
 xen  f38a815a54000ca51ff5165b2863d60b6bbea49c

Last test of basis   185180  2024-03-28 10:52:41 Z5 days
Testing same since   185217  2024-04-02 14:38:50 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Roger Pau Monné 

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

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

[osstest service owner]

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

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  17cf285d87e28a9ee9ab1f192982fc6dfc9e4193
baseline version:
 xen  0a8b92d0a47797ec4d70c7fd7b05abbc135b51dc

Last test of basis   185183  2024-03-28 16:05:48 Z5 days
Testing same since   185218  2024-04-02 14:38:50 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Roger Pau Monné 

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

Re: [XEN PATCH 6/7] xen/vm-event: address a violation of MISRA C:2012 Rule 16.3

[Tamas K Lengyel]

On Tue, Apr 2, 2024 at 3:24 AM Federico Serafini
 wrote:
>
> Add break statement to address a violation of MISRA C:2012 Rule 16.3
> ("An unconditional `break' statement shall terminate every
> switch-clause ").
>
> No functional change.
>
> Signed-off-by: Federico Serafini 

Acked-by: Tamas K Lengyel 

[PATCH] docs/misra: add 13.6 to rules.rst

[Stefano Stabellini]

As agreed during MISRA C meetings, add Rule 13.6 to rules.rst.

Signed-off-by: Stefano Stabellini 
---
There was a request to expand the scope to also include alignof and
typeof. Depending on whether the MISRA C scanners support it, and under
which rules violations will be listed, rules.rst will be updated
accordingly (either updating the notes section of 13.6 or adding a new
entry.)

diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index 1e134ccebc..415b5b63c3 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -445,6 +445,12 @@ maintainers if you want to suggest a change.
  - Initializer lists shall not contain persistent side effects
  -
 
+   * - `Rule 13.6 
`_
+ - Required
+ - The operand of the sizeof operator shall not contain any
+   expression which has potential side-effects
+ -
+
* - `Rule 14.1 
`_
  - Required
  - A loop counter shall not have essentially floating type

Re: [PATCH 2/3] drivers: serial: add Qualcomm GENI-based serial driver

[Volodymyr Babchuk]

Hi Michal,

Michal Orzel  writes:

> Hello,
>
> On 29/03/2024 01:08, Volodymyr Babchuk wrote:
>> 
>> 
>> Generic Interface (GENI) is a newer interface for low speed interfaces
>> like UART, I2C or SPI. This patch adds the simple driver for the UART
>> instance of GENI. Code is based on similar drivers in U-Boot and Linux
>> kernel.
> Do you have a link to a manual?

I wish I had. Qualcomm provides HW manuals only under very strict
NDAs. At the time of writing I don't have access to the manual at
all. Those patches are based solely on similar drivers in U-Boot and
Linux kernel.

>> 
>> This driver implements only simple synchronous mode, because although
>> GENI supports FIFO mode, it needs to know number of
>> characters **before** starting TX transaction. This is a stark
>> contrast when compared to other UART peripherals, which allow adding
>> characters to a FIFO while TX operation is running.
>> 
>> The patch adds both normal UART driver and earlyprintk version.
>> 
>> Signed-off-by: Volodymyr Babchuk 
>> ---
>>  xen/arch/arm/Kconfig.debug   |  19 +-
>>  xen/arch/arm/arm64/debug-qcom.inc|  76 +++
> Shouldn't all the files (+ other places) have geni in their names? Could qcom 
> refer to other type of serial device?

AFAIK, there is an older type of serial device. Both Linux and U-Boot
use "msm" instead of "qcom" in drivers names for those devices.

But I can add "geni" to the names, no big deal.

>
>>  xen/arch/arm/include/asm/qcom-uart.h |  48 +
>>  xen/drivers/char/Kconfig |   8 +
>>  xen/drivers/char/Makefile|   1 +
>>  xen/drivers/char/qcom-uart.c | 288 +++
>>  6 files changed, 439 insertions(+), 1 deletion(-)
>>  create mode 100644 xen/arch/arm/arm64/debug-qcom.inc
>>  create mode 100644 xen/arch/arm/include/asm/qcom-uart.h
>>  create mode 100644 xen/drivers/char/qcom-uart.c
>> 
>> diff --git a/xen/arch/arm/Kconfig.debug b/xen/arch/arm/Kconfig.debug
>> index eec860e88e..f6ab0bb30e 100644
>> --- a/xen/arch/arm/Kconfig.debug
>> +++ b/xen/arch/arm/Kconfig.debug
>> @@ -119,6 +119,19 @@ choice
>> selecting one of the platform specific options below 
>> if
>> you know the parameters for the port.
>> 
>> +   This option is preferred over the platform specific
>> +   options; the platform specific options are deprecated
>> +   and will soon be removed.
>> +   config EARLY_UART_CHOICE_QCOM
> The list is sorted alphabetically. Please adhere to that.
>

Sure

>> +   select EARLY_UART_QCOM
>> +   bool "Early printk via Qualcomm debug UART"
> Shouldn't you add depends on ARM_64? Isn't this device Arm64 specific like in 
> Linux?
>

In linux it depends on ARCH_QCOM which can be enabled both for arm and
arm64. But for Xen case... I believe it is better to make ARM_64
dependency.

>> +   help
>> +   Say Y here if you wish the early printk to direct 
>> their
> help text here should be indented by 2 tabs and 2 spaces (do not take example 
> from surrounding code)
>

Would anyone mind if I'll send patch that aligns surrounding code as well?

>> +   output to a Qualcomm debug UART. You can use this 
>> option to
>> +   provide the parameters for the debug UART rather than
>> +   selecting one of the platform specific options below 
>> if
>> +   you know the parameters for the port.
>> +
>> This option is preferred over the platform specific
>> options; the platform specific options are deprecated
>> and will soon be removed.
>> @@ -211,6 +224,9 @@ config EARLY_UART_PL011
>>  config EARLY_UART_SCIF
>> select EARLY_PRINTK
>> bool
>> +config EARLY_UART_QCOM
>> +   select EARLY_PRINTK
>> +   bool
> The list is sorted alphabetically. Please adhere to that.
>
>> 
>>  config EARLY_PRINTK
>> bool
>> @@ -261,7 +277,7 @@ config EARLY_UART_PL011_MMIO32
>>   will be done using 32-bit only accessors.
>> 
>>  config EARLY_UART_INIT
>> -   depends on EARLY_UART_PL011 && EARLY_UART_PL011_BAUD_RATE != 0
>> +   depends on (EARLY_UART_PL011 && EARLY_UART_PL011_BAUD_RATE != 0) || 
>> EARLY_UART_QCOM
>> def_bool y
>> 
>>  config EARLY_UART_8250_REG_SHIFT
>> @@ -308,3 +324,4 @@ config EARLY_PRINTK_INC
>> default "debug-mvebu.inc" if EARLY_UART_MVEBU
>> default "debug-pl011.inc" if EARLY_UART_PL011
>> default "debug-scif.inc" if EARLY_UART_SCIF
>> +   default "debug-qcom.inc" if EARLY_UART_QCOM
>> diff --git a/xen/arch/arm/arm64/debug-qcom.inc 
>> b/xen/arch/arm/arm64/debug-qcom.inc
>> new file mode 100644
>> index 00..130d08d6e9
>> --- /dev/null
>> +++ b/xen/arch/arm/arm64/debug-qcom.inc
>> @@ -0,0 +1,76 @@
>> +/*
>> + * xen/arch/arm/arm64/debug-qcom.inc

Re: [PATCH net] xen-netfront: Add missing skb_mark_for_recycle

[Arthur Borsboom]

After having a better look, I have found the patch in linux-next

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=0cd74ffcf4fb0536718241d59d2c124578624d83

On Tue, 2 Apr 2024 at 10:20, Arthur Borsboom  wrote:
>
> On Fri, 29 Mar 2024 at 10:47, Arthur Borsboom  
> wrote:
> >
> > On Wed, 27 Mar 2024 at 13:15, Jesper Dangaard Brouer  
> > wrote:
> > >
> > > Notice that skb_mark_for_recycle() is introduced later than fixes tag in
> > > 6a5bcd84e886 ("page_pool: Allow drivers to hint on SKB recycling").
> > >
> > > It is believed that fixes tag were missing a call to 
> > > page_pool_release_page()
> > > between v5.9 to v5.14, after which is should have used 
> > > skb_mark_for_recycle().
> > > Since v6.6 the call page_pool_release_page() were removed (in 535b9c61bdef
> > > ("net: page_pool: hide page_pool_release_page()") and remaining callers
> > > converted (in commit 6bfef2ec0172 ("Merge branch
> > > 'net-page_pool-remove-page_pool_release_page'")).
> > >
> > > This leak became visible in v6.8 via commit dba1b8a7ab68 ("mm/page_pool: 
> > > catch
> > > page_pool memory leaks").
> > >
> > > Fixes: 6c5aa6fc4def ("xen networking: add basic XDP support for 
> > > xen-netfront")
> > > Reported-by: Arthur Borsboom 
> > > Signed-off-by: Jesper Dangaard Brouer 
> > > ---
> > > Compile tested only, can someone please test this
> >
> > I have tested this patch on Xen 4.18.1 with VM (Arch Linux) kernel 
> > 6.9.0-rc1.
> >
> > Without the patch there are many trace traces and cloning the Linux
> > mainline git repository resulted in failures (same with kernel 6.8.1).
> > The patched kernel 6.9.0-rc1 performs as expected; cloning the git
> > repository was successful and no kernel traces observed.
> > Hereby my tested by:
> >
> > Tested-by: Arthur Borsboom 
> >
> >
> >
> > >  drivers/net/xen-netfront.c |1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> > > index ad29f370034e..8d2aee88526c 100644
> > > --- a/drivers/net/xen-netfront.c
> > > +++ b/drivers/net/xen-netfront.c
> > > @@ -285,6 +285,7 @@ static struct sk_buff 
> > > *xennet_alloc_one_rx_buffer(struct netfront_queue *queue)
> > > return NULL;
> > > }
> > > skb_add_rx_frag(skb, 0, page, 0, 0, PAGE_SIZE);
> > > +   skb_mark_for_recycle(skb);
> > >
> > > /* Align ip header to a 16 bytes boundary */
> > > skb_reserve(skb, NET_IP_ALIGN);
> > >
> > >
>
> I don't see this patch yet in linux-next.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log
>
> Any idea in which kernel release this patch will be included?

Re: [XEN PATCH v3 1/7] x86/msi: address violation of MISRA C Rule 20.7 and coding style

[Nicola Vetrini]

On 2024-04-02 17:05, Jan Beulich wrote:

On 29.03.2024 10:11, Nicola Vetrini wrote:

MISRA C Rule 20.7 states: "Expressions resulting from the expansion
of macro parameters shall be enclosed in parentheses". Therefore, some
macro definitions should gain additional parentheses to ensure that 
all

current and future users will be safe with respect to expansions that
can possibly alter the semantics of the passed-in macro parameter.

While at it, the style of these macros has been somewhat uniformed.

No functional change.

Signed-off-by: Nicola Vetrini 
---
Changes in v2:
- Make the style change more consistent
---
 xen/arch/x86/include/asm/msi.h | 49 
+-

 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/xen/arch/x86/include/asm/msi.h 
b/xen/arch/x86/include/asm/msi.h

index 997ccb87be0c..bd110c357ce4 100644
--- a/xen/arch/x86/include/asm/msi.h
+++ b/xen/arch/x86/include/asm/msi.h
@@ -147,33 +147,34 @@ int msi_free_irq(struct msi_desc *entry);
  */
 #define NR_HP_RESERVED_VECTORS 20

-#define msi_control_reg(base)  (base + PCI_MSI_FLAGS)
-#define msi_lower_address_reg(base)(base + PCI_MSI_ADDRESS_LO)
-#define msi_upper_address_reg(base)(base + PCI_MSI_ADDRESS_HI)
-#define msi_data_reg(base, is64bit)\
-   ( (is64bit == 1) ? base+PCI_MSI_DATA_64 : base+PCI_MSI_DATA_32 )
-#define msi_mask_bits_reg(base, is64bit) \
-   ( (is64bit == 1) ? base+PCI_MSI_MASK_BIT : base+PCI_MSI_MASK_BIT-4)
+#define msi_control_reg(base)((base) + PCI_MSI_FLAGS)
+#define msi_lower_address_reg(base)  ((base) + PCI_MSI_ADDRESS_LO)
+#define msi_upper_address_reg(base)  ((base) + PCI_MSI_ADDRESS_HI)
+#define msi_data_reg(base, is64bit) \
+(((is64bit) == 1) ? (base) + PCI_MSI_DATA_64 : (base) + 
PCI_MSI_DATA_32)

+#define msi_mask_bits_reg(base, is64bit)\
+(((is64bit) == 1) ? (base) + PCI_MSI_MASK_BIT   \
+  : (base) + PCI_MSI_MASK_BIT - 4)
 #define msi_pending_bits_reg(base, is64bit) \
-   ((base) + PCI_MSI_MASK_BIT + ((is64bit) ? 4 : 0))
-#define msi_disable(control)   control &= ~PCI_MSI_FLAGS_ENABLE
+((base) + PCI_MSI_MASK_BIT + ((is64bit) ? 4 : 0))
+#define msi_disable(control) ({ (control) &= 
~PCI_MSI_FLAGS_ENABLE })

 #define multi_msi_capable(control) \
-   (1 << ((control & PCI_MSI_FLAGS_QMASK) >> 1))
+(1 << (((control) & PCI_MSI_FLAGS_QMASK) >> 1))
 #define multi_msi_enable(control, num) \
-   control |= (((fls(num) - 1) << 4) & PCI_MSI_FLAGS_QSIZE);
-#define is_64bit_address(control)  (!!(control & PCI_MSI_FLAGS_64BIT))
-#define is_mask_bit_support(control)	(!!(control & 
PCI_MSI_FLAGS_MASKBIT))

-#define msi_enable(control, num) multi_msi_enable(control, num); \
-   control |= PCI_MSI_FLAGS_ENABLE
-
-#define msix_control_reg(base) (base + PCI_MSIX_FLAGS)
-#define msix_table_offset_reg(base)(base + PCI_MSIX_TABLE)
-#define msix_pba_offset_reg(base)  (base + PCI_MSIX_PBA)
-#define msix_enable(control)   control |= PCI_MSIX_FLAGS_ENABLE
-#define msix_disable(control)  control &= ~PCI_MSIX_FLAGS_ENABLE
-#define msix_table_size(control) 	((control & 
PCI_MSIX_FLAGS_QSIZE)+1)

-#define msix_unmask(address)   (address & ~PCI_MSIX_VECTOR_BITMASK)
-#define msix_mask(address) (address | PCI_MSIX_VECTOR_BITMASK)
+({ (control) |= (((fls(num) - 1) << 4) & PCI_MSI_FLAGS_QSIZE) })
+#define is_64bit_address(control)(!!((control) & 
PCI_MSI_FLAGS_64BIT))
+#define is_mask_bit_support(control) (!!((control) & 
PCI_MSI_FLAGS_MASKBIT))
+#define msi_enable(control, num) ({ multi_msi_enable(control, 
num); \
+(control) |= 
PCI_MSI_FLAGS_ENABLE })


Neither this nor ...


+#define msix_control_reg(base)   ((base) + PCI_MSIX_FLAGS)
+#define msix_table_offset_reg(base)  ((base) + PCI_MSIX_TABLE)
+#define msix_pba_offset_reg(base)((base) + PCI_MSIX_PBA)
+#define msix_enable(control) ({ (control) |= 
PCI_MSIX_FLAGS_ENABLE })
+#define msix_disable(control)({ (control) &= 
~PCI_MSIX_FLAGS_ENABLE })


... these would compile afaict, if  they were used.

Once again - before fiddling with these we need to settle on which of 
these
we want to keep (and then also use, rather than open-coding), and which 
to

drop (instead of massaging).

Jan


Ok, we can drop this patch from this for now.

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

[PATCH] Revert "evtchn: refuse EVTCHNOP_status for Xen-bound event channels"

[Andrew Cooper]

The commit makes a claim without any kind of justification.

The claim is false, and the commit broke lsevtchn in dom0.  It is also quite
obvious from XSM_TARGET that it has broken device model stubdoms too.

Whether to return information about a xen-owned evtchn is a matter of policy,
and it's not acceptable to short circuit the XSM on the matter.

This reverts commit f60ab5337f968e2f10c639ab59db7afb0fe4f7c3.

Fixes: f60ab5337f96 ("evtchn: refuse EVTCHNOP_status for Xen-bound event 
channels")
Signed-off-by: Andrew Cooper 
---
CC: George Dunlap 
CC: Jan Beulich 
CC: Stefano Stabellini 
CC: Julien Grall 
CC: Daniel Smith 
---
 xen/common/event_channel.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index 20f586cf5ecd..ae6c2f902645 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -1040,12 +1040,6 @@ int evtchn_status(evtchn_status_t *status)
 
 read_lock(>event_lock);
 
-if ( consumer_is_xen(chn) )
-{
-rc = -EACCES;
-goto out;
-}
-
 rc = xsm_evtchn_status(XSM_TARGET, d, chn);
 if ( rc )
 goto out;

base-commit: 7a09966e7b2823b70f6d56d0cf66c11124f4a3c1
-- 
2.30.2

Re: [PATCH] x86: Address MISRA Rule 13.6

[Federico Serafini]

On 02/04/24 17:54, Andrew Cooper wrote:

On 02/04/2024 4:46 pm, Jan Beulich wrote:

On 02.04.2024 17:43, Andrew Cooper wrote:

MISRA Rule 13.6 doesn't like having an expression in a sizeof() which
potentially has side effects.

Address several violations by pulling the expression out into a local
variable.

No functional change.

Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 
with one caveat:


--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -1150,8 +1150,9 @@ static void cf_check irq_guest_eoi_timer_fn(void *data)
  {
  struct domain *d = action->guest[i];
  unsigned int pirq = domain_irq_to_pirq(d, irq);
+struct pirq *pirq_info = pirq_info(d, pirq);

Misra won't like the var's name matching the macro's. Can we go with just
"info"?


Ah - missed that.

I can name it to just info, but I considered "struct pirq *info" to be a
little odd.


The local variable is a non-callable entity;
"clashes" between function-like macros and non-callable entities
will be deviated (as agreed during MISRA meetings).

--
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)

Re: [PATCH] x86: Address MISRA Rule 13.6

[Andrew Cooper]

On 02/04/2024 5:06 pm, Jan Beulich wrote:
> On 02.04.2024 17:54, Andrew Cooper wrote:
>> On 02/04/2024 4:46 pm, Jan Beulich wrote:
>>> On 02.04.2024 17:43, Andrew Cooper wrote:
 MISRA Rule 13.6 doesn't like having an expression in a sizeof() which
 potentially has side effects.

 Address several violations by pulling the expression out into a local
 variable.

 No functional change.

 Signed-off-by: Andrew Cooper 
>>> Reviewed-by: Jan Beulich 
>>> with one caveat:
>>>
 --- a/xen/arch/x86/irq.c
 +++ b/xen/arch/x86/irq.c
 @@ -1150,8 +1150,9 @@ static void cf_check irq_guest_eoi_timer_fn(void 
 *data)
  {
  struct domain *d = action->guest[i];
  unsigned int pirq = domain_irq_to_pirq(d, irq);
 +struct pirq *pirq_info = pirq_info(d, pirq);
>>> Misra won't like the var's name matching the macro's. Can we go with just
>>> "info"?
>> Ah - missed that.
>>
>> I can name it to just info, but I considered "struct pirq *info" to be a
>> little odd.
> I agree, but what do you do with another "pirq" already there.
>
> Or wait, what about
>
> struct pirq *pirq = pirq_info(d, domain_irq_to_pirq(d, irq));
>
> ?

That should work.  I'll switch to this locally, and wait for the
feedback on whether the patch works for 13.6.

~Andrew

Re: [PATCH] x86: Address MISRA Rule 13.6

[Jan Beulich]

On 02.04.2024 17:54, Andrew Cooper wrote:
> On 02/04/2024 4:46 pm, Jan Beulich wrote:
>> On 02.04.2024 17:43, Andrew Cooper wrote:
>>> MISRA Rule 13.6 doesn't like having an expression in a sizeof() which
>>> potentially has side effects.
>>>
>>> Address several violations by pulling the expression out into a local
>>> variable.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Andrew Cooper 
>> Reviewed-by: Jan Beulich 
>> with one caveat:
>>
>>> --- a/xen/arch/x86/irq.c
>>> +++ b/xen/arch/x86/irq.c
>>> @@ -1150,8 +1150,9 @@ static void cf_check irq_guest_eoi_timer_fn(void 
>>> *data)
>>>  {
>>>  struct domain *d = action->guest[i];
>>>  unsigned int pirq = domain_irq_to_pirq(d, irq);
>>> +struct pirq *pirq_info = pirq_info(d, pirq);
>> Misra won't like the var's name matching the macro's. Can we go with just
>> "info"?
> 
> Ah - missed that.
> 
> I can name it to just info, but I considered "struct pirq *info" to be a
> little odd.

I agree, but what do you do with another "pirq" already there.

Or wait, what about

struct pirq *pirq = pirq_info(d, domain_irq_to_pirq(d, irq));

?

Jan

Re: [PATCH] x86: Address MISRA Rule 13.6

[Andrew Cooper]

On 02/04/2024 4:46 pm, Jan Beulich wrote:
> On 02.04.2024 17:43, Andrew Cooper wrote:
>> MISRA Rule 13.6 doesn't like having an expression in a sizeof() which
>> potentially has side effects.
>>
>> Address several violations by pulling the expression out into a local
>> variable.
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper 
> Reviewed-by: Jan Beulich 
> with one caveat:
>
>> --- a/xen/arch/x86/irq.c
>> +++ b/xen/arch/x86/irq.c
>> @@ -1150,8 +1150,9 @@ static void cf_check irq_guest_eoi_timer_fn(void *data)
>>  {
>>  struct domain *d = action->guest[i];
>>  unsigned int pirq = domain_irq_to_pirq(d, irq);
>> +struct pirq *pirq_info = pirq_info(d, pirq);
> Misra won't like the var's name matching the macro's. Can we go with just
> "info"?

Ah - missed that.

I can name it to just info, but I considered "struct pirq *info" to be a
little odd.

~Andrew

Re: [PATCH] x86: Address MISRA Rule 13.6

[Jan Beulich]

On 02.04.2024 17:43, Andrew Cooper wrote:
> MISRA Rule 13.6 doesn't like having an expression in a sizeof() which
> potentially has side effects.
> 
> Address several violations by pulling the expression out into a local
> variable.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 
with one caveat:

> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -1150,8 +1150,9 @@ static void cf_check irq_guest_eoi_timer_fn(void *data)
>  {
>  struct domain *d = action->guest[i];
>  unsigned int pirq = domain_irq_to_pirq(d, irq);
> +struct pirq *pirq_info = pirq_info(d, pirq);

Misra won't like the var's name matching the macro's. Can we go with just
"info"?

Jan

[PATCH] x86: Address MISRA Rule 13.6

[Andrew Cooper]

MISRA Rule 13.6 doesn't like having an expression in a sizeof() which
potentially has side effects.

Address several violations by pulling the expression out into a local
variable.

No functional change.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Roger Pau Monné 
CC: Stefano Stabellini 
CC: Roberto Bagnara 
CC: consult...@bugseng.com 

Slightly RFC.
---
 xen/arch/x86/irq.c   | 3 ++-
 xen/arch/x86/time.c  | 6 --
 xen/drivers/passthrough/amd/iommu_intr.c | 3 ++-
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 0487f734a5d2..d73f687f7617 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -1150,8 +1150,9 @@ static void cf_check irq_guest_eoi_timer_fn(void *data)
 {
 struct domain *d = action->guest[i];
 unsigned int pirq = domain_irq_to_pirq(d, irq);
+struct pirq *pirq_info = pirq_info(d, pirq);
 
-if ( test_and_clear_bool(pirq_info(d, pirq)->masked) )
+if ( test_and_clear_bool(pirq_info->masked) )
 action->in_flight--;
 }
 
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 60870047894b..6f136f4b14bf 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -2503,7 +2503,9 @@ static long cmos_utc_offset; /* in seconds */
 
 int time_suspend(void)
 {
-if ( smp_processor_id() == 0 )
+unsigned int cpu = smp_processor_id();
+
+if ( cpu == 0 )
 {
 cmos_utc_offset = -get_wallclock_time();
 cmos_utc_offset += get_sec();
@@ -2514,7 +2516,7 @@ int time_suspend(void)
 }
 
 /* Better to cancel calibration timer for accuracy. */
-clear_bit(TIME_CALIBRATE_SOFTIRQ, _pending(smp_processor_id()));
+clear_bit(TIME_CALIBRATE_SOFTIRQ, _pending(cpu));
 
 return 0;
 }
diff --git a/xen/drivers/passthrough/amd/iommu_intr.c 
b/xen/drivers/passthrough/amd/iommu_intr.c
index d9eefcd8e411..7fc796dec25b 100644
--- a/xen/drivers/passthrough/amd/iommu_intr.c
+++ b/xen/drivers/passthrough/amd/iommu_intr.c
@@ -183,6 +183,7 @@ static void free_intremap_entry(const struct amd_iommu 
*iommu,
 unsigned int bdf, unsigned int index)
 {
 union irte_ptr entry = get_intremap_entry(iommu, bdf, index);
+struct ivrs_mappings *ivrs = get_ivrs_mappings(iommu->seg);
 
 if ( iommu->ctrl.ga_en )
 {
@@ -201,7 +202,7 @@ static void free_intremap_entry(const struct amd_iommu 
*iommu,
 else
 ACCESS_ONCE(entry.ptr32->raw) = 0;
 
-__clear_bit(index, get_ivrs_mappings(iommu->seg)[bdf].intremap_inuse);
+__clear_bit(index, ivrs[bdf].intremap_inuse);
 }
 
 static void update_intremap_entry(const struct amd_iommu *iommu,

base-commit: 7a09966e7b2823b70f6d56d0cf66c11124f4a3c1
-- 
2.30.2

Re: [PATCH v6 7/8] xen/rwlock: raise the number of possible cpus

[Jürgen Groß]

On 02.04.24 16:52, Jan Beulich wrote:

On 27.03.2024 16:22, Juergen Gross wrote:

@@ -36,14 +36,21 @@ void queue_write_lock_slowpath(rwlock_t *lock);
  
  static inline bool _is_write_locked_by_me(unsigned int cnts)

  {
-BUILD_BUG_ON(_QW_CPUMASK < NR_CPUS);
+BUILD_BUG_ON((_QW_CPUMASK + 1) < NR_CPUS);
+BUILD_BUG_ON(NR_CPUS * _QR_BIAS > INT_MAX);
  return (cnts & _QW_WMASK) == _QW_LOCKED &&
 (cnts & _QW_CPUMASK) == smp_processor_id();
  }
  
  static inline bool _can_read_lock(unsigned int cnts)

  {
-return !(cnts & _QW_WMASK) || _is_write_locked_by_me(cnts);
+/*
+ * If write locked by the caller, no other readers are possible.
+ * Not allowing the lock holder to read_lock() another 32768 times ought
+ * to be fine.
+ */
+return cnts <= INT_MAX &&
+   (!(cnts & _QW_WMASK) || _is_write_locked_by_me(cnts));
  }


What is the 32768 in the comment relating to? INT_MAX is quite a bit higher,
yet the comparison against it is the only thing you add. Whereas the reader
count is, with the sign bit unused, 17 bits, though (bits 14..30). I think


You missed:

#define_QR_SHIFT(_QW_SHIFT + 2) /* Reader count shift */

So the reader's shift is 16, resulting in 15 bits for the reader count.


even in such a comment rather than using a literal number the corresponding
expression would better be stated.


Hmm, you mean replacing the 32768 with INT_MAX >> _QR_SHIFT? This would be
fine with me.


Juergen

Re: [PATCH v6 6/8] xen/spinlock: support higher number of cpus

[Jürgen Groß]

On 02.04.24 16:42, Jan Beulich wrote:

On 27.03.2024 16:22, Juergen Gross wrote:

Allow 16 bits per cpu number, which is the limit imposed by
spinlock_tickets_t.

This will allow up to 65535 cpus, while increasing only the size of
recursive spinlocks in debug builds from 8 to 12 bytes.

The current Xen limit of 4095 cpus is imposed by SPINLOCK_CPU_BITS
being 12. There are machines available with more cpus than the current
Xen limit, so it makes sense to have the possibility to use more cpus.

Signed-off-by: Juergen Gross 


Reviewed-by: Jan Beulich 
albeit I have to say that I'm not entirely convinced of ...


--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -485,7 +485,9 @@ bool _rspin_trylock(rspinlock_t *lock)
  
  /* Don't allow overflow of recurse_cpu field. */

  BUILD_BUG_ON(NR_CPUS > SPINLOCK_NO_CPU);
+BUILD_BUG_ON(SPINLOCK_CPU_BITS > sizeof(lock->recurse_cpu) * 8);
  BUILD_BUG_ON(SPINLOCK_RECURSE_BITS < 3);
+BUILD_BUG_ON(SPINLOCK_MAX_RECURSE > ((1u << SPINLOCK_RECURSE_BITS) - 1));
  
  check_lock(>debug, true);


... the two additions here: The two checks we had verify independent
properties, whereas the new ones basically check that struct rspinlock
and its associated #define-s were got right. We don't check such
elsewhere, I don't think.


I think we do.

What about:
  BUILD_BUG_ON(sizeof(hwp_req) != sizeof(hwp_req.raw))
checking that two union elements are of the same size (and both elements don't
contain any other structs).

Additionally it is not obvious at a first glance that SPINLOCK_CPU_BITS defined
in line 11 is relevant for the definition of recurse_cpu in line 217.

Regarding the second added BUILD_BUG_ON() there was a comment by Julien related
to the definition of SPINLOCK_MAX_RECURSE in V4 of this patch. We settled to use
the current form including the added BUILD_BUG_ON().


Juergen

Re: [XEN PATCH v3 1/7] x86/msi: address violation of MISRA C Rule 20.7 and coding style

[Jan Beulich]

On 29.03.2024 10:11, Nicola Vetrini wrote:
> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> of macro parameters shall be enclosed in parentheses". Therefore, some
> macro definitions should gain additional parentheses to ensure that all
> current and future users will be safe with respect to expansions that
> can possibly alter the semantics of the passed-in macro parameter.
> 
> While at it, the style of these macros has been somewhat uniformed.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini 
> ---
> Changes in v2:
> - Make the style change more consistent
> ---
>  xen/arch/x86/include/asm/msi.h | 49 +-
>  1 file changed, 25 insertions(+), 24 deletions(-)
> 
> diff --git a/xen/arch/x86/include/asm/msi.h b/xen/arch/x86/include/asm/msi.h
> index 997ccb87be0c..bd110c357ce4 100644
> --- a/xen/arch/x86/include/asm/msi.h
> +++ b/xen/arch/x86/include/asm/msi.h
> @@ -147,33 +147,34 @@ int msi_free_irq(struct msi_desc *entry);
>   */
>  #define NR_HP_RESERVED_VECTORS   20
>  
> -#define msi_control_reg(base)(base + PCI_MSI_FLAGS)
> -#define msi_lower_address_reg(base)  (base + PCI_MSI_ADDRESS_LO)
> -#define msi_upper_address_reg(base)  (base + PCI_MSI_ADDRESS_HI)
> -#define msi_data_reg(base, is64bit)  \
> - ( (is64bit == 1) ? base+PCI_MSI_DATA_64 : base+PCI_MSI_DATA_32 )
> -#define msi_mask_bits_reg(base, is64bit) \
> - ( (is64bit == 1) ? base+PCI_MSI_MASK_BIT : base+PCI_MSI_MASK_BIT-4)
> +#define msi_control_reg(base)((base) + PCI_MSI_FLAGS)
> +#define msi_lower_address_reg(base)  ((base) + PCI_MSI_ADDRESS_LO)
> +#define msi_upper_address_reg(base)  ((base) + PCI_MSI_ADDRESS_HI)
> +#define msi_data_reg(base, is64bit) \
> +(((is64bit) == 1) ? (base) + PCI_MSI_DATA_64 : (base) + PCI_MSI_DATA_32)
> +#define msi_mask_bits_reg(base, is64bit)\
> +(((is64bit) == 1) ? (base) + PCI_MSI_MASK_BIT   \
> +  : (base) + PCI_MSI_MASK_BIT - 4)
>  #define msi_pending_bits_reg(base, is64bit) \
> - ((base) + PCI_MSI_MASK_BIT + ((is64bit) ? 4 : 0))
> -#define msi_disable(control) control &= ~PCI_MSI_FLAGS_ENABLE
> +((base) + PCI_MSI_MASK_BIT + ((is64bit) ? 4 : 0))
> +#define msi_disable(control) ({ (control) &= ~PCI_MSI_FLAGS_ENABLE })
>  #define multi_msi_capable(control) \
> - (1 << ((control & PCI_MSI_FLAGS_QMASK) >> 1))
> +(1 << (((control) & PCI_MSI_FLAGS_QMASK) >> 1))
>  #define multi_msi_enable(control, num) \
> - control |= (((fls(num) - 1) << 4) & PCI_MSI_FLAGS_QSIZE);
> -#define is_64bit_address(control)(!!(control & PCI_MSI_FLAGS_64BIT))
> -#define is_mask_bit_support(control) (!!(control & PCI_MSI_FLAGS_MASKBIT))
> -#define msi_enable(control, num) multi_msi_enable(control, num); \
> - control |= PCI_MSI_FLAGS_ENABLE
> -
> -#define msix_control_reg(base)   (base + PCI_MSIX_FLAGS)
> -#define msix_table_offset_reg(base)  (base + PCI_MSIX_TABLE)
> -#define msix_pba_offset_reg(base)(base + PCI_MSIX_PBA)
> -#define msix_enable(control) control |= PCI_MSIX_FLAGS_ENABLE
> -#define msix_disable(control)control &= 
> ~PCI_MSIX_FLAGS_ENABLE
> -#define msix_table_size(control) ((control & PCI_MSIX_FLAGS_QSIZE)+1)
> -#define msix_unmask(address) (address & ~PCI_MSIX_VECTOR_BITMASK)
> -#define msix_mask(address)   (address | PCI_MSIX_VECTOR_BITMASK)
> +({ (control) |= (((fls(num) - 1) << 4) & PCI_MSI_FLAGS_QSIZE) })
> +#define is_64bit_address(control)(!!((control) & PCI_MSI_FLAGS_64BIT))
> +#define is_mask_bit_support(control) (!!((control) & PCI_MSI_FLAGS_MASKBIT))
> +#define msi_enable(control, num) ({ multi_msi_enable(control, num); \
> +(control) |= PCI_MSI_FLAGS_ENABLE })

Neither this nor ...

> +#define msix_control_reg(base)   ((base) + PCI_MSIX_FLAGS)
> +#define msix_table_offset_reg(base)  ((base) + PCI_MSIX_TABLE)
> +#define msix_pba_offset_reg(base)((base) + PCI_MSIX_PBA)
> +#define msix_enable(control) ({ (control) |= PCI_MSIX_FLAGS_ENABLE })
> +#define msix_disable(control)({ (control) &= ~PCI_MSIX_FLAGS_ENABLE 
> })

... these would compile afaict, if  they were used.

Once again - before fiddling with these we need to settle on which of these
we want to keep (and then also use, rather than open-coding), and which to
drop (instead of massaging).

Jan

Re: [XEN PATCH v3 7/7] x86/amd: address violations of MISRA C Rule 20.7

[Jan Beulich]

On 29.03.2024 10:11, Nicola Vetrini wrote:
> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> of macro parameters shall be enclosed in parentheses". Therefore, some
> macro definitions should gain additional parentheses to ensure that all
> current and future users will be safe with respect to expansions that
> can possibly alter the semantics of the passed-in macro parameter.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini 

Acked-by: Jan Beulich 

Re: [XEN PATCH v3 6/7] xen/mm: address violations of MISRA C Rule 20.7

[Jan Beulich]

On 29.03.2024 10:11, Nicola Vetrini wrote:
> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> of macro parameters shall be enclosed in parentheses". Therefore, some
> macro definitions should gain additional parentheses to ensure that all
> current and future users will be safe with respect to expansions that
> can possibly alter the semantics of the passed-in macro parameter.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini 

Acked-by: Jan Beulich 

Re: [XEN PATCH v3 4/7] x86/hvm: address violations of MISRA C Rule 20.7

[Jan Beulich]

On 29.03.2024 10:11, Nicola Vetrini wrote:
> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> of macro parameters shall be enclosed in parentheses". Therefore, some
> macro definitions should gain additional parentheses to ensure that all
> current and future users will be safe with respect to expansions that
> can possibly alter the semantics of the passed-in macro parameter.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini 

Acked-by: Jan Beulich 
albeit once again ...

> --- a/xen/arch/x86/hvm/domain.c
> +++ b/xen/arch/x86/hvm/domain.c
> @@ -132,9 +132,9 @@ int arch_set_info_hvm_guest(struct vcpu *v, const struct 
> vcpu_hvm_context *ctx)
>  s = (struct segment_register)   \
>  { 0, { (r)->s ## _ar }, (r)->s ## _limit, (r)->s ## _base };\
>  /* Set accessed / busy bit for present segments. */ \
> -if ( s.p )  \
> -s.type |= (x86_seg_##s != x86_seg_tr ? 1 : 2);  \
> -check_segment(, x86_seg_ ## s); })
> +if ( (s).p )\
> +(s).type |= (x86_seg_##s != x86_seg_tr ? 1 : 2);\

... addressing the style issue (blanks around ##) while touching code would have
been nice.

Jan

> +check_segment(&(s), x86_seg_ ## s); })
>  
>  rc = SEG(cs, regs);
>  rc |= SEG(ds, regs);

Re: [XEN PATCH v3 3/7] x86/vPMU: address violations of MISRA C Rule 20.7

[Jan Beulich]

On 29.03.2024 10:11, Nicola Vetrini wrote:
> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> of macro parameters shall be enclosed in parentheses". Therefore, some
> macro definitions should gain additional parentheses to ensure that all
> current and future users will be safe with respect to expansions that
> can possibly alter the semantics of the passed-in macro parameter.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini 

Acked-by: Jan Beulich 

Re: [PATCH v6 7/8] xen/rwlock: raise the number of possible cpus

[Jan Beulich]

On 27.03.2024 16:22, Juergen Gross wrote:
> @@ -36,14 +36,21 @@ void queue_write_lock_slowpath(rwlock_t *lock);
>  
>  static inline bool _is_write_locked_by_me(unsigned int cnts)
>  {
> -BUILD_BUG_ON(_QW_CPUMASK < NR_CPUS);
> +BUILD_BUG_ON((_QW_CPUMASK + 1) < NR_CPUS);
> +BUILD_BUG_ON(NR_CPUS * _QR_BIAS > INT_MAX);
>  return (cnts & _QW_WMASK) == _QW_LOCKED &&
> (cnts & _QW_CPUMASK) == smp_processor_id();
>  }
>  
>  static inline bool _can_read_lock(unsigned int cnts)
>  {
> -return !(cnts & _QW_WMASK) || _is_write_locked_by_me(cnts);
> +/*
> + * If write locked by the caller, no other readers are possible.
> + * Not allowing the lock holder to read_lock() another 32768 times ought
> + * to be fine.
> + */
> +return cnts <= INT_MAX &&
> +   (!(cnts & _QW_WMASK) || _is_write_locked_by_me(cnts));
>  }

What is the 32768 in the comment relating to? INT_MAX is quite a bit higher,
yet the comparison against it is the only thing you add. Whereas the reader
count is, with the sign bit unused, 17 bits, though (bits 14..30). I think
even in such a comment rather than using a literal number the corresponding
expression would better be stated.

Jan

Re: [PATCH v6 6/8] xen/spinlock: support higher number of cpus

[Jan Beulich]

On 27.03.2024 16:22, Juergen Gross wrote:
> Allow 16 bits per cpu number, which is the limit imposed by
> spinlock_tickets_t.
> 
> This will allow up to 65535 cpus, while increasing only the size of
> recursive spinlocks in debug builds from 8 to 12 bytes.
> 
> The current Xen limit of 4095 cpus is imposed by SPINLOCK_CPU_BITS
> being 12. There are machines available with more cpus than the current
> Xen limit, so it makes sense to have the possibility to use more cpus.
> 
> Signed-off-by: Juergen Gross 

Reviewed-by: Jan Beulich 
albeit I have to say that I'm not entirely convinced of ...

> --- a/xen/common/spinlock.c
> +++ b/xen/common/spinlock.c
> @@ -485,7 +485,9 @@ bool _rspin_trylock(rspinlock_t *lock)
>  
>  /* Don't allow overflow of recurse_cpu field. */
>  BUILD_BUG_ON(NR_CPUS > SPINLOCK_NO_CPU);
> +BUILD_BUG_ON(SPINLOCK_CPU_BITS > sizeof(lock->recurse_cpu) * 8);
>  BUILD_BUG_ON(SPINLOCK_RECURSE_BITS < 3);
> +BUILD_BUG_ON(SPINLOCK_MAX_RECURSE > ((1u << SPINLOCK_RECURSE_BITS) - 1));
>  
>  check_lock(>debug, true);

... the two additions here: The two checks we had verify independent
properties, whereas the new ones basically check that struct rspinlock
and its associated #define-s were got right. We don't check such
elsewhere, I don't think.

Jan

Re: [PATCH v6 3/8] xen/spinlock: add missing rspin_is_locked() and rspin_barrier()

[Jan Beulich]

On 27.03.2024 16:22, Juergen Gross wrote:
> Add rspin_is_locked() and rspin_barrier() in order to prepare differing
> spinlock_t and rspinlock_t types.
> 
> Signed-off-by: Juergen Gross 

Reviewed-by: Jan Beulich 

Re: [PATCH v6 4/4] x86/PVH: Support relocatable dom0 kernels

[Jan Beulich]

On 27.03.2024 22:51, Jason Andryuk wrote:
> v6:
> Select alignment from, in order, Note, PHDRs, then default

The comment in the public header also needs to reflect this change.

> +static bool __init check_and_adjust_load_address(
> +const struct domain *d, struct elf_binary *elf, struct elf_dom_parms 
> *parms)
> +{
> +paddr_t reloc_base;
> +
> +if ( check_load_address(d, elf) )
> +return true;
> +
> +if ( !parms->phys_reloc )
> +{
> +printk("%pd kernel: Address conflict and not relocatable\n", d);
> +return false;
> +}
> +
> +reloc_base = find_kernel_memory(d, elf, parms);
> +if ( !reloc_base )
> +{
> +printk("%pd kernel: Failed find a load address\n", d);
> +return false;
> +}
> +
> +if ( opt_dom0_verbose )
> +printk("%pd kernel: Moving [%p, %p] -> [%"PRIpaddr", 
> %"PRIpaddr"]\n", d,
> +   elf->dest_base, elf->dest_base + elf->dest_size - 1,
> +   reloc_base, reloc_base + elf->dest_size - 1);
> +
> +parms->phys_entry = reloc_base +
> +(parms->phys_entry - (uintptr_t)elf->dest_base);

I think this would be easier to read as

parms->phys_entry =
reloc_base + (parms->phys_entry - (uintptr_t)elf->dest_base);

Everything else looks good to me now.

Jan

Re: preparations for 4.17.4

[Jan Beulich]

On 27.03.2024 13:33, Andrew Cooper wrote:
> 1) livepatching of .rodata:
> 
> 989556c6f8ca - xen/virtual-region: Rename the start/end fields
> ef969144a425 - xen/virtual-region: Include rodata pointers
> b083b1c393dc - x86/livepatch: Relax permissions on rodata too
> 
> And technically "x86/mm: fix detection of last L1 entry in
> modify_xen_mappings_lite()" too but you've already backported this one.
> 
> Patching .rodata worked before Xen 4.17, and was broken (left as a TODO)
> when I adjusted Xen to stop using CR0.WP=0 for patching.
> 
> 
> 2) Policy fixes:
> 
> e2d8a6522516 - x86/cpu-policy: Fix visibility of HTT/CMP_LEGACY in max
> policies
> 
> This is a real bugfix for a real regression we found updating from Xen
> 4.13 -> 4.17.  It has a dependency on
> 
> 5420aa165dfa - x86/cpu-policy: Hide x2APIC from PV guests
> 
> which I know you had more concern with.  FWIW, I'm certain its a good
> fix, and should be backported.
> 
> 
> 3) Test fixes:
> 
> 0263dc9069dd - tests/resource: Fix HVM guest in !SHADOW builds
> 
> It's minor, but does make a difference for those of us who run these
> tests regularly.
> 
> 
> 4) Watchdog fixes:
> 
> 9e18f339830c - x86/boot: Improve the boot watchdog determination of
> stuck cpus
> 131892e0dcc1 - x86/boot: Support the watchdog on newer AMD systems
> 
> You took "x86/boot: Fix setup_apic_nmi_watchdog() to fail more cleanly"
> and the first of the two patches is in the same category IMO.  The
> second I also feel ok to take for the in-support releases, particularly
> as all it is doing is dropping a family list.

I've pushed all of the above.

> 5) Ucode scan stability  (For 4.18 only)
> 
> Xen 4.18 had "x86/ucode: Refresh raw CPU policy after microcode load" in
> it's .0 release, so should also gain:
> 
> cf7fe8b72dea - x86/ucode: Fix stability of the raw CPU Policy rescan

This already is in 4.18.1, ...

> I've only noticed because I've got them both backported to 4.17 in
> XenServer, but I don't think upstream wants to take that route.

... while, as you suggest, not (and not intended to be) in 4.17.

Jan

Re: [PATCH v6 08/20] xen/riscv: introduce cmpxchg.h

[Oleksii]

On Tue, 2024-04-02 at 14:54 +0200, Jan Beulich wrote:
> On 02.04.2024 13:43, Oleksii wrote:
> > On Thu, 2024-03-21 at 13:07 +0100, Jan Beulich wrote:
> > > > + * If resulting 4-byte access is still misalgined, it will
> > > > fault
> > > > just as
> > > > + * non-emulated 4-byte access would.
> > > > + */
> > > > +#define emulate_xchg_1_2(ptr, new, lr_sfx, sc_sfx) \
> > > > +({ \
> > > > +    uint32_t *aligned_ptr = (uint32_t *)((unsigned long)ptr &
> > > > ~(0x4 - sizeof(*(ptr; \
> > > > +    unsigned int new_val_pos = ((unsigned long)(ptr) & (0x4 -
> > > > sizeof(*(ptr * BITS_PER_BYTE; \
> > > 
> > > You parenthesize ptr here correctly, but not in the line above.
> > > 
> > > Instead of "_pos" in the name, maybe better "_bit"?
> > > 
> > > Finally, here and elsewhere, please limit line length to 80
> > > chars.
> > > (Omitting
> > > the 0x here would help a little, but not quite enough. Question
> > > is
> > > whether
> > > these wouldn't better be sizeof(*aligned_ptr) anyway.)
> > 
> > I'm unsure if using sizeof(*aligned_ptr) is correct in this
> > context.
> > The intention was to determine the position for the value we're
> > attempting to exchange.
> > 
> > Let's consider a scenario where we have 0xAABBCCDD at address 0x6.
> > Even
> > though this would result in misaligned access, I believe
> > new_val_pos
> > should still be calculated accurately. Let's say we want to
> > exchange
> > two bytes (AABB) with .
> > 
> > With the current implementation, we would calculate:
> > 0x6 & 2 = 2 * 8 = 16, which is the value on which the new value
> > should
> > be shifted.
> > 
> > However, if this value is then ANDed with sizeof(*aligned_ptr):
> > 0x6 & 4 = 6 * 8 = 48, which is not the expected value.
> > 
> > Am I overlooking something?
> 
> I'm afraid I can only reply with a counter question (feels like there
> is
> some misunderstanding): Do you agree that 0x4 == 4 ==
> sizeof(*aligned_ptr)?
> It's the 0x4 that the latter part of my earlier reply was about. I'm
> pretty
> sure you have, over the various reviews I've done, noticed my dislike
> for
> the use of literal numbers, when those aren't truly "magic".
Yes, it was misunderstading. Thanks for clarifying.

~ Oleksii

[ANNOUNCE] Call for agenda items - Community Call 4th April 2024

[Kelly Choi]

Hi all,

*Please add your proposed agenda items below.*

https://cryptpad.fr/pad/#/2/pad/edit/YWBsde6D8Y8R8z3PLBG2akrt/

If any action items are missing or have been resolved, please add/remove
them from the sheet.

*CALL LINK: https://meet.jit.si/XenProjectCommunityCall
*

*DATE: Thursday 4th April 2024*

*TIME: 1500 UTC (4 pm UK time)*
*Note the following administrative conventions for the call:*


** To allow time to switch between meetings, we plan on starting theagenda
at 15:05 UTC sharp.  Aim to join by 15:03 UTC if possible to allocatetime
to sort out technical difficulties.*








** If you want to be CC'ed please add or remove yourself from
thesign-up-sheet
at https://cryptpad.fr/pad/#/2/pad/edit/D9vGzihPxxAOe6RFPz0sRCf+/
== Dial-in
Information ==## Meeting time16:00 - 17:00 British timeFurther
International meeting times:*
https://www.timeanddate.com/worldclock/meetingdetails.html?year=2024=4=4=4=15=0=0=1234=37=224=179

## Dial in details
https://meet.jit.si/static/dialInInfo.html?room=XenProjectCommunityCall

Many thanks,
Kelly Choi

Community Manager
Xen Project

Re: [PATCH v6 08/20] xen/riscv: introduce cmpxchg.h

[Jan Beulich]

On 02.04.2024 13:43, Oleksii wrote:
> On Thu, 2024-03-21 at 13:07 +0100, Jan Beulich wrote:
>>> + * If resulting 4-byte access is still misalgined, it will fault
>>> just as
>>> + * non-emulated 4-byte access would.
>>> + */
>>> +#define emulate_xchg_1_2(ptr, new, lr_sfx, sc_sfx) \
>>> +({ \
>>> +    uint32_t *aligned_ptr = (uint32_t *)((unsigned long)ptr &
>>> ~(0x4 - sizeof(*(ptr; \
>>> +    unsigned int new_val_pos = ((unsigned long)(ptr) & (0x4 -
>>> sizeof(*(ptr * BITS_PER_BYTE; \
>>
>> You parenthesize ptr here correctly, but not in the line above.
>>
>> Instead of "_pos" in the name, maybe better "_bit"?
>>
>> Finally, here and elsewhere, please limit line length to 80 chars.
>> (Omitting
>> the 0x here would help a little, but not quite enough. Question is
>> whether
>> these wouldn't better be sizeof(*aligned_ptr) anyway.)
> 
> I'm unsure if using sizeof(*aligned_ptr) is correct in this context.
> The intention was to determine the position for the value we're
> attempting to exchange.
> 
> Let's consider a scenario where we have 0xAABBCCDD at address 0x6. Even
> though this would result in misaligned access, I believe new_val_pos
> should still be calculated accurately. Let's say we want to exchange
> two bytes (AABB) with .
> 
> With the current implementation, we would calculate:
> 0x6 & 2 = 2 * 8 = 16, which is the value on which the new value should
> be shifted.
> 
> However, if this value is then ANDed with sizeof(*aligned_ptr):
> 0x6 & 4 = 6 * 8 = 48, which is not the expected value.
> 
> Am I overlooking something?

I'm afraid I can only reply with a counter question (feels like there is
some misunderstanding): Do you agree that 0x4 == 4 == sizeof(*aligned_ptr)?
It's the 0x4 that the latter part of my earlier reply was about. I'm pretty
sure you have, over the various reviews I've done, noticed my dislike for
the use of literal numbers, when those aren't truly "magic".

Jan

[PATCH 2/2] xen/arm: Add i.MX UART driver

[Oleksandr Tyshchenko]

From: Oleksandr Tyshchenko 

The i.MX UART Documentation:
https://www.nxp.com/webapp/Download?colCode=IMX8MMRM
Chapter 16.2 Universal Asynchronous Receiver/Transmitter (UART)

Tested on i.MX 8M Mini only, but I guess, it should be
suitable for other i.MX8M* SoCs (those UART device tree nodes
contain "fsl,imx6q-uart" compatible string).

Signed-off-by: Oleksandr Tyshchenko 
---
I used the "earlycon=ec_imx6q,0x3089" cmd arg and
selected CONFIG_SERIAL_IMX_EARLYCON in Linux for enabling vUART.
---
---
 MAINTAINERS |   1 +
 xen/drivers/char/Kconfig|   7 +
 xen/drivers/char/Makefile   |   1 +
 xen/drivers/char/imx-uart.c | 299 
 4 files changed, 308 insertions(+)
 create mode 100644 xen/drivers/char/imx-uart.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 1bd22fd75f..bd4084fd20 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -249,6 +249,7 @@ F:  xen/drivers/char/arm-uart.c
 F: xen/drivers/char/cadence-uart.c
 F: xen/drivers/char/exynos4210-uart.c
 F: xen/drivers/char/imx-lpuart.c
+F: xen/drivers/char/imx-uart.c
 F: xen/drivers/char/meson-uart.c
 F: xen/drivers/char/mvebu-uart.c
 F: xen/drivers/char/omap-uart.c
diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig
index e18ec3788c..f51a1f596a 100644
--- a/xen/drivers/char/Kconfig
+++ b/xen/drivers/char/Kconfig
@@ -20,6 +20,13 @@ config HAS_IMX_LPUART
help
  This selects the i.MX LPUART. If you have i.MX8QM based board, say Y.
 
+config HAS_IMX_UART
+   bool "i.MX UART driver"
+   default y
+   depends on ARM_64
+   help
+ This selects the i.MX UART. If you have i.MX8M* based board, say Y.
+
 config HAS_MVEBU
bool "Marvell MVEBU UART driver"
default y
diff --git a/xen/drivers/char/Makefile b/xen/drivers/char/Makefile
index e7e374775d..147530a1ed 100644
--- a/xen/drivers/char/Makefile
+++ b/xen/drivers/char/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_HAS_SCIF) += scif-uart.o
 obj-$(CONFIG_HAS_EHCI) += ehci-dbgp.o
 obj-$(CONFIG_XHCI) += xhci-dbc.o
 obj-$(CONFIG_HAS_IMX_LPUART) += imx-lpuart.o
+obj-$(CONFIG_HAS_IMX_UART) += imx-uart.o
 obj-$(CONFIG_ARM) += arm-uart.o
 obj-y += serial.o
 obj-$(CONFIG_XEN_GUEST) += xen_pv_console.o
diff --git a/xen/drivers/char/imx-uart.c b/xen/drivers/char/imx-uart.c
new file mode 100644
index 00..13bb189063
--- /dev/null
+++ b/xen/drivers/char/imx-uart.c
@@ -0,0 +1,299 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * xen/drivers/char/imx-uart.c
+ *
+ * Driver for i.MX UART.
+ *
+ * Based on Linux's drivers/tty/serial/imx.c
+ *
+ * Copyright (C) 2024 EPAM Systems Inc.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define imx_uart_read(uart, off)  readl((uart)->regs + (off))
+#define imx_uart_write(uart, off, val)writel((val), (uart)->regs + (off))
+
+static struct imx_uart {
+uint32_t baud, clock_hz, data_bits, parity, stop_bits, fifo_size;
+uint32_t irq;
+char __iomem *regs;
+struct irqaction irqaction;
+struct vuart_info vuart;
+} imx_com;
+
+static void imx_uart_interrupt(int irq, void *data)
+{
+struct serial_port *port = data;
+struct imx_uart *uart = port->uart;
+uint32_t usr1, usr2;
+
+usr1 = imx_uart_read(uart, USR1);
+usr2 = imx_uart_read(uart, USR2);
+
+if ( usr1 & (USR1_RRDY | USR1_AGTIM) )
+{
+imx_uart_write(uart, USR1, USR1_AGTIM);
+serial_rx_interrupt(port);
+}
+
+if ( (usr1 & USR1_TRDY) || (usr2 & USR2_TXDC) )
+serial_tx_interrupt(port);
+}
+
+static void imx_uart_clear_rx_errors(struct serial_port *port)
+{
+struct imx_uart *uart = port->uart;
+uint32_t usr1, usr2;
+
+usr1 = imx_uart_read(uart, USR1);
+usr2 = imx_uart_read(uart, USR2);
+
+if ( usr2 & USR2_BRCD )
+imx_uart_write(uart, USR2, USR2_BRCD);
+else if ( usr1 & USR1_FRAMERR )
+imx_uart_write(uart, USR1, USR1_FRAMERR);
+else if ( usr1 & USR1_PARITYERR )
+imx_uart_write(uart, USR1, USR1_PARITYERR);
+
+if ( usr2 & USR2_ORE )
+imx_uart_write(uart, USR2, USR2_ORE);
+}
+
+static void __init imx_uart_init_preirq(struct serial_port *port)
+{
+struct imx_uart *uart = port->uart;
+uint32_t reg;
+
+/*
+ * Wait for the transmission to complete. This is needed for a smooth
+ * transition when we come from early printk.
+ */
+while ( !(imx_uart_read(uart, USR2) & USR2_TXDC) )
+cpu_relax();
+
+/* Set receiver/transmitter trigger level */
+reg = imx_uart_read(uart, UFCR);
+reg &= (UFCR_RFDIV | UFCR_DCEDTE);
+reg |= TXTL_DEFAULT << UFCR_TXTL_SHF | RXTL_DEFAULT;
+imx_uart_write(uart, UFCR, reg);
+
+/* Enable UART and disable interrupts/DMA */
+reg = imx_uart_read(uart, UCR1);
+reg |= UCR1_UARTEN;
+reg &= ~(UCR1_TRDYEN | UCR1_TXMPTYEN | UCR1_RTSDEN | UCR1_RRDYEN |
+ UCR1_RXDMAEN | UCR1_TXDMAEN | UCR1_ATDMAEN);
+

[PATCH 0/2] Add UART support for i.MX 8M Mini EVKB

[Oleksandr Tyshchenko]

From: Oleksandr Tyshchenko 

Hello all.

This small series contains early printk and full UART support
for i.MX 8M Mini EVKB [1].

Tested on i.MX 8M Mini only to which I had an access, but from my
understanding, this UART support should be suitable for other i.MX8M* SoCs
(those UART device tree nodes contain "fsl,imx6q-uart" compatible string).

[1] 
https://www.nxp.com/document/guide/getting-started-with-the-i-mx-8m-mini-evkb:GS-iMX-8M-Mini-EVK

Oleksandr Tyshchenko (2):
  xen/arm: Add i.MX UART early printk support
  xen/arm: Add i.MX UART driver

 MAINTAINERS   |   1 +
 xen/arch/arm/Kconfig.debug|  14 ++
 xen/arch/arm/arm64/debug-imx-uart.inc |  38 
 xen/arch/arm/include/asm/imx-uart.h   |  76 +++
 xen/drivers/char/Kconfig  |   7 +
 xen/drivers/char/Makefile |   1 +
 xen/drivers/char/imx-uart.c   | 299 ++
 7 files changed, 436 insertions(+)
 create mode 100644 xen/arch/arm/arm64/debug-imx-uart.inc
 create mode 100644 xen/arch/arm/include/asm/imx-uart.h
 create mode 100644 xen/drivers/char/imx-uart.c

-- 
2.34.1

[PATCH 1/2] xen/arm: Add i.MX UART early printk support

[Oleksandr Tyshchenko]

From: Oleksandr Tyshchenko 

Tested on i.MX 8M Mini only, but I guess, it should be
suitable for other i.MX8M* SoCs (those UART device tree nodes
contain "fsl,imx6q-uart" compatible string).

Signed-off-by: Oleksandr Tyshchenko 
---
I selected the following configs for enabling early printk:

 CONFIG_EARLY_UART_CHOICE_IMX_UART=y
 CONFIG_EARLY_UART_IMX_UART=y
 CONFIG_EARLY_PRINTK=y
 CONFIG_EARLY_UART_BASE_ADDRESS=0x3089
 CONFIG_EARLY_PRINTK_INC="debug-imx-uart.inc"
---
---
 xen/arch/arm/Kconfig.debug| 14 +
 xen/arch/arm/arm64/debug-imx-uart.inc | 38 ++
 xen/arch/arm/include/asm/imx-uart.h   | 76 +++
 3 files changed, 128 insertions(+)
 create mode 100644 xen/arch/arm/arm64/debug-imx-uart.inc
 create mode 100644 xen/arch/arm/include/asm/imx-uart.h

diff --git a/xen/arch/arm/Kconfig.debug b/xen/arch/arm/Kconfig.debug
index eec860e88e..a15d08f214 100644
--- a/xen/arch/arm/Kconfig.debug
+++ b/xen/arch/arm/Kconfig.debug
@@ -68,6 +68,16 @@ choice
provide the parameters for the i.MX LPUART rather than
selecting one of the platform specific options below if
you know the parameters for the port.
+   config EARLY_UART_CHOICE_IMX_UART
+   select EARLY_UART_IMX_UART
+   depends on ARM_64
+   bool "Early printk via i.MX UART"
+   help
+   Say Y here if you wish the early printk to direct their
+   output to a i.MX UART. You can use this option to
+   provide the parameters for the i.MX UART rather than
+   selecting one of the platform specific options below if
+   you know the parameters for the port.
config EARLY_UART_CHOICE_MESON
select EARLY_UART_MESON
depends on ARM_64
@@ -199,6 +209,9 @@ config EARLY_UART_EXYNOS4210
 config EARLY_UART_IMX_LPUART
select EARLY_PRINTK
bool
+config EARLY_UART_IMX_UART
+   select EARLY_PRINTK
+   bool
 config EARLY_UART_MESON
select EARLY_PRINTK
bool
@@ -304,6 +317,7 @@ config EARLY_PRINTK_INC
default "debug-cadence.inc" if EARLY_UART_CADENCE
default "debug-exynos4210.inc" if EARLY_UART_EXYNOS4210
default "debug-imx-lpuart.inc" if EARLY_UART_IMX_LPUART
+   default "debug-imx-uart.inc" if EARLY_UART_IMX_UART
default "debug-meson.inc" if EARLY_UART_MESON
default "debug-mvebu.inc" if EARLY_UART_MVEBU
default "debug-pl011.inc" if EARLY_UART_PL011
diff --git a/xen/arch/arm/arm64/debug-imx-uart.inc 
b/xen/arch/arm/arm64/debug-imx-uart.inc
new file mode 100644
index 00..27a68b1ed5
--- /dev/null
+++ b/xen/arch/arm/arm64/debug-imx-uart.inc
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * xen/arch/arm/arm64/debug-imx-uart.inc
+ *
+ * i.MX8M* specific debug code
+ *
+ * Copyright (C) 2024 EPAM Systems Inc.
+ */
+
+#include 
+
+/*
+ * Wait UART to be ready to transmit
+ * rb: register which contains the UART base address
+ * rc: scratch register
+ */
+.macro early_uart_ready xb, c
+1:
+ldr   w\c, [\xb, #IMX21_UTS] /* <- Test register */
+tst   w\c, #UTS_TXFULL   /* Check TxFIFO FULL bit */
+bne   1b /* Wait for the UART to be ready */
+.endm
+
+/*
+ * UART transmit character
+ * rb: register which contains the UART base address
+ * rt: register which contains the character to transmit
+ */
+.macro early_uart_transmit xb, wt
+str   \wt, [\xb, #URTX0] /* -> Transmitter Register */
+.endm
+
+/*
+ * Local variables:
+ * mode: ASM
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/include/asm/imx-uart.h 
b/xen/arch/arm/include/asm/imx-uart.h
new file mode 100644
index 00..413a81dd44
--- /dev/null
+++ b/xen/arch/arm/include/asm/imx-uart.h
@@ -0,0 +1,76 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * xen/arch/arm/include/asm/imx-uart.h
+ *
+ * Common constant definition between early printk and the UART driver
+ *
+ * Copyright (C) 2024 EPAM Systems Inc.
+ */
+
+#ifndef __ASM_ARM_IMX_UART_H__
+#define __ASM_ARM_IMX_UART_H__
+
+/* 32-bit register definition */
+#define URXD0(0x00) /* Receiver Register */
+#define URTX0(0x40) /* Transmitter Register */
+#define UCR1 (0x80) /* Control Register 1 */
+#define UCR2 (0x84) /* Control Register 2 */
+#define UCR3 (0x88) /* Control Register 3 */
+#define UCR4 (0x8c) /* Control Register 4 */
+#define UFCR (0x90) /* FIFO Control Register */
+#define USR1 (0x94) /* Status Register 1 */
+#define USR2 (0x98) /* Status Register 2 */
+#define IMX21_UTS(0xb4) /* Test Register */
+
+#define URXD_ERRBIT(14, UL) /* Error detect */
+#define URXD_RX_DATAGENMASK(7, 0) /* Received data mask */
+
+#define UCR1_TRDYEN  BIT(13, UL) /* Transmitter ready interrupt 

Re: [PATCH v6 08/20] xen/riscv: introduce cmpxchg.h

[Oleksii]

On Thu, 2024-03-21 at 13:07 +0100, Jan Beulich wrote:
> > + * If resulting 4-byte access is still misalgined, it will fault
> > just as
> > + * non-emulated 4-byte access would.
> > + */
> > +#define emulate_xchg_1_2(ptr, new, lr_sfx, sc_sfx) \
> > +({ \
> > +    uint32_t *aligned_ptr = (uint32_t *)((unsigned long)ptr &
> > ~(0x4 - sizeof(*(ptr; \
> > +    unsigned int new_val_pos = ((unsigned long)(ptr) & (0x4 -
> > sizeof(*(ptr * BITS_PER_BYTE; \
> 
> You parenthesize ptr here correctly, but not in the line above.
> 
> Instead of "_pos" in the name, maybe better "_bit"?
> 
> Finally, here and elsewhere, please limit line length to 80 chars.
> (Omitting
> the 0x here would help a little, but not quite enough. Question is
> whether
> these wouldn't better be sizeof(*aligned_ptr) anyway.)

I'm unsure if using sizeof(*aligned_ptr) is correct in this context.
The intention was to determine the position for the value we're
attempting to exchange.

Let's consider a scenario where we have 0xAABBCCDD at address 0x6. Even
though this would result in misaligned access, I believe new_val_pos
should still be calculated accurately. Let's say we want to exchange
two bytes (AABB) with .

With the current implementation, we would calculate:
0x6 & 2 = 2 * 8 = 16, which is the value on which the new value should
be shifted.

However, if this value is then ANDed with sizeof(*aligned_ptr):
0x6 & 4 = 6 * 8 = 48, which is not the expected value.

Am I overlooking something?

~ Oleksii

Re: [PATCH 2/3] drivers: serial: add Qualcomm GENI-based serial driver

[Michal Orzel]

Hello,

On 29/03/2024 01:08, Volodymyr Babchuk wrote:
> 
> 
> Generic Interface (GENI) is a newer interface for low speed interfaces
> like UART, I2C or SPI. This patch adds the simple driver for the UART
> instance of GENI. Code is based on similar drivers in U-Boot and Linux
> kernel.
Do you have a link to a manual?

> 
> This driver implements only simple synchronous mode, because although
> GENI supports FIFO mode, it needs to know number of
> characters **before** starting TX transaction. This is a stark
> contrast when compared to other UART peripherals, which allow adding
> characters to a FIFO while TX operation is running.
> 
> The patch adds both normal UART driver and earlyprintk version.
> 
> Signed-off-by: Volodymyr Babchuk 
> ---
>  xen/arch/arm/Kconfig.debug   |  19 +-
>  xen/arch/arm/arm64/debug-qcom.inc|  76 +++
Shouldn't all the files (+ other places) have geni in their names? Could qcom 
refer to other type of serial device?


>  xen/arch/arm/include/asm/qcom-uart.h |  48 +
>  xen/drivers/char/Kconfig |   8 +
>  xen/drivers/char/Makefile|   1 +
>  xen/drivers/char/qcom-uart.c | 288 +++
>  6 files changed, 439 insertions(+), 1 deletion(-)
>  create mode 100644 xen/arch/arm/arm64/debug-qcom.inc
>  create mode 100644 xen/arch/arm/include/asm/qcom-uart.h
>  create mode 100644 xen/drivers/char/qcom-uart.c
> 
> diff --git a/xen/arch/arm/Kconfig.debug b/xen/arch/arm/Kconfig.debug
> index eec860e88e..f6ab0bb30e 100644
> --- a/xen/arch/arm/Kconfig.debug
> +++ b/xen/arch/arm/Kconfig.debug
> @@ -119,6 +119,19 @@ choice
> selecting one of the platform specific options below 
> if
> you know the parameters for the port.
> 
> +   This option is preferred over the platform specific
> +   options; the platform specific options are deprecated
> +   and will soon be removed.
> +   config EARLY_UART_CHOICE_QCOM
The list is sorted alphabetically. Please adhere to that.

> +   select EARLY_UART_QCOM
> +   bool "Early printk via Qualcomm debug UART"
Shouldn't you add depends on ARM_64? Isn't this device Arm64 specific like in 
Linux?

> +   help
> +   Say Y here if you wish the early printk to direct 
> their
help text here should be indented by 2 tabs and 2 spaces (do not take example 
from surrounding code)

> +   output to a Qualcomm debug UART. You can use this 
> option to
> +   provide the parameters for the debug UART rather than
> +   selecting one of the platform specific options below 
> if
> +   you know the parameters for the port.
> +
> This option is preferred over the platform specific
> options; the platform specific options are deprecated
> and will soon be removed.
> @@ -211,6 +224,9 @@ config EARLY_UART_PL011
>  config EARLY_UART_SCIF
> select EARLY_PRINTK
> bool
> +config EARLY_UART_QCOM
> +   select EARLY_PRINTK
> +   bool
The list is sorted alphabetically. Please adhere to that.

> 
>  config EARLY_PRINTK
> bool
> @@ -261,7 +277,7 @@ config EARLY_UART_PL011_MMIO32
>   will be done using 32-bit only accessors.
> 
>  config EARLY_UART_INIT
> -   depends on EARLY_UART_PL011 && EARLY_UART_PL011_BAUD_RATE != 0
> +   depends on (EARLY_UART_PL011 && EARLY_UART_PL011_BAUD_RATE != 0) || 
> EARLY_UART_QCOM
> def_bool y
> 
>  config EARLY_UART_8250_REG_SHIFT
> @@ -308,3 +324,4 @@ config EARLY_PRINTK_INC
> default "debug-mvebu.inc" if EARLY_UART_MVEBU
> default "debug-pl011.inc" if EARLY_UART_PL011
> default "debug-scif.inc" if EARLY_UART_SCIF
> +   default "debug-qcom.inc" if EARLY_UART_QCOM
> diff --git a/xen/arch/arm/arm64/debug-qcom.inc 
> b/xen/arch/arm/arm64/debug-qcom.inc
> new file mode 100644
> index 00..130d08d6e9
> --- /dev/null
> +++ b/xen/arch/arm/arm64/debug-qcom.inc
> @@ -0,0 +1,76 @@
> +/*
> + * xen/arch/arm/arm64/debug-qcom.inc
> + *
> + * Qualcomm debug UART specific debug code
> + *
> + * Volodymyr Babchuk 
> + * Copyright (C) 2024, EPAM Systems.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that 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.
No need for the license text. You should use SPDX identifier instead at the top 
of the file:
/* SPDX-License-Identifier: 

[xen-unstable test] 185215: tolerable FAIL

[osstest service owner]

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

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  7a09966e7b2823b70f6d56d0cf66c11124f4a3c1
baseline version:
 xen  7a09966e7b2823b70f6d56d0cf66c11124f4a3c1

Last test of basis   185215  2024-04-02 01:54:28 Z0 days
Testing same since  (not found) 0 attempts

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

[OSSTEST PATCH 37/36] production-config: Set Bookworm's debian-installer version

[Anthony PERARD]

Signed-off-by: Anthony PERARD 
---

I've got this one more patch to add to the series, which I forgot to
write. I'll put it before "Switch to Debian Bookworm as default suite"
when I'll push the rest of the series.

 production-config | 1 +
 1 file changed, 1 insertion(+)

diff --git a/production-config b/production-config
index 6345c40c..0eb8f0f3 100644
--- a/production-config
+++ b/production-config
@@ -92,6 +92,7 @@ TftpDiVersion_wheezy 2016-06-08
 TftpDiVersion_jessie 2018-06-26
 TftpDiVersion_stretch 2020-09-24
 TftpDiVersion_buster 2023-06-20
+TftpDiVersion_bookworm 2024-03-26
 
 DebianSnapshotBackports_jessie 
http://snapshot.debian.org/archive/debian/20190206T211314Z/
 
-- 
Anthony PERARD

Re: [PATCH 2/2] x86/video: do not assume a video mode to be unconditionally present

[Andrew Cooper]

On 02/04/2024 10:54 am, Jan Beulich wrote:
> On 28.03.2024 20:58, Andrew Cooper wrote:
>> On 28/03/2024 3:35 pm, Roger Pau Monne wrote:
>>> There's no reason to assume VGA text mode 3 to be unconditionally available.
>>> With the addition of booting Xen itself in PVH mode there's a boot path that
>>> explicitly short-circuits all the real-mode logic, including the VGA 
>>> detection.
>>>
>>> Leave the default user selected mode as text mode 3 in boot_vid_mode, but do
>>> not populate boot_vid_info with any default settings.  It will either be
>>> populated by the real-mode video detection code, or left zeroed in case
>>> real-mode code is skipped.
>>>
>>> Note that only PVH skips the real-mode portion of the boot trampoline,
>>> otherwise the only way to skip it is to set `no-real-mode` on the command 
>>> line,
>>> and the description for the option already notes that VGA would be disabled 
>>> as
>>> a result of skipping real-mode bootstrap.
>> IIRC, Grub defaults to using no-real-mode for xen.efi.
> That's our MB2 entry path which forces skip_realmode to 1. The xen.efi boot
> path doesn't, but similarly skips entering real mode (retrieving desired
> data via EFI protocols instead). Imo if to be retained, the above paragraph
> would want extending some, to cover all the cases.

What I mean is that Grub's 20_linux_xen script writes a stanza which
includes "no-real-mode edd=off" for any non-PC platform, which includes EFI.

~Andrew

Re: [PATCH 2/2] x86/video: do not assume a video mode to be unconditionally present

[Jan Beulich]

On 28.03.2024 20:58, Andrew Cooper wrote:
> On 28/03/2024 3:35 pm, Roger Pau Monne wrote:
>> There's no reason to assume VGA text mode 3 to be unconditionally available.
>> With the addition of booting Xen itself in PVH mode there's a boot path that
>> explicitly short-circuits all the real-mode logic, including the VGA 
>> detection.
>>
>> Leave the default user selected mode as text mode 3 in boot_vid_mode, but do
>> not populate boot_vid_info with any default settings.  It will either be
>> populated by the real-mode video detection code, or left zeroed in case
>> real-mode code is skipped.
>>
>> Note that only PVH skips the real-mode portion of the boot trampoline,
>> otherwise the only way to skip it is to set `no-real-mode` on the command 
>> line,
>> and the description for the option already notes that VGA would be disabled 
>> as
>> a result of skipping real-mode bootstrap.
> 
> IIRC, Grub defaults to using no-real-mode for xen.efi.

That's our MB2 entry path which forces skip_realmode to 1. The xen.efi boot
path doesn't, but similarly skips entering real mode (retrieving desired
data via EFI protocols instead). Imo if to be retained, the above paragraph
would want extending some, to cover all the cases.

Jan

Re: [PATCH 2/2] x86/video: do not assume a video mode to be unconditionally present

[Jan Beulich]

On 28.03.2024 16:35, Roger Pau Monne wrote:
> There's no reason to assume VGA text mode 3 to be unconditionally available.
> With the addition of booting Xen itself in PVH mode there's a boot path that
> explicitly short-circuits all the real-mode logic, including the VGA 
> detection.
> 
> Leave the default user selected mode as text mode 3 in boot_vid_mode, but do
> not populate boot_vid_info with any default settings.  It will either be
> populated by the real-mode video detection code, or left zeroed in case
> real-mode code is skipped.
> 
> Note that only PVH skips the real-mode portion of the boot trampoline,
> otherwise the only way to skip it is to set `no-real-mode` on the command 
> line,
> and the description for the option already notes that VGA would be disabled as
> a result of skipping real-mode bootstrap.
> 
> This fixes Xen incorrectly reporting:
> 
> (XEN) Video information:
> (XEN)  VGA is text mode 80x25, font 8x16
> 
> When booted as a PVH guest.

And what effect does this have on a bare-metal boot with no-real-mode in use?
The default on x86 hardware still is that in the absence of other information,
a VGA of some kind can be assumed to be there. Yes, there are headless
systems, but better assume VGA is there when there's not than the other way
around.

What I would have expected is for the PVH boot path to clear boot_vid_info.

Jan

Re: [PATCH 1/2] x86/video: add boot_video_info offset generation to asm-offsets

[Jan Beulich]

On 02.04.2024 11:38, Jan Beulich wrote:
> On 28.03.2024 16:35, Roger Pau Monne wrote:
>> Currently the offsets into the boot_video_info struct are manually encoded in
>> video.S, which is fragile.  Generate them in asm-offsets.c and switch the
>> current code to use those instead.
> 
> Just to mention it (without asking for immediate action): Defining 
> boot_vid_info
> in assembly code then is as fragile. Moving to C would likely be problematic
> because it needs to be in the trampoline range. But at least its size should 
> (at
> some point) perhaps better be tied to the C struct's sizeof().

Actually I overlooked that you partly do this. The use of BVI_capabilities there
looks odd to me, though. Why not

.space  BVI_size - (. - boot_vid_info)

? I realize it becomes just BVI_size in patch 2, but I have some question there,
too.

Jan

> The fields, with
> some effort, could also be converted using the new BVI_* constants. That would
> still leave the field sizes; maybe those could at least be cross-checked by 
> some
> BUILD_BUG_ONs.
> 
> Jan

Re: [PATCH 1/2] x86/video: add boot_video_info offset generation to asm-offsets

[Jan Beulich]

On 28.03.2024 16:35, Roger Pau Monne wrote:
> Currently the offsets into the boot_video_info struct are manually encoded in
> video.S, which is fragile.  Generate them in asm-offsets.c and switch the
> current code to use those instead.

Just to mention it (without asking for immediate action): Defining boot_vid_info
in assembly code then is as fragile. Moving to C would likely be problematic
because it needs to be in the trampoline range. But at least its size should (at
some point) perhaps better be tied to the C struct's sizeof(). The fields, with
some effort, could also be converted using the new BVI_* constants. That would
still leave the field sizes; maybe those could at least be cross-checked by some
BUILD_BUG_ONs.

Jan

[OSSTEST PATCH] ap-common: Fix libvirt's git repo URL

[Anthony PERARD]

The current URL doesn't work anymore and just timeout, switch to the
new main repo URL.

Signed-off-by: Anthony PERARD 
---

Notes:
I'll push that later today.

Last libvirt test by osstest was on 20 February.

 ap-common | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ap-common b/ap-common
index 5384891b..eabb85c6 100644
--- a/ap-common
+++ b/ap-common
@@ -41,7 +41,7 @@
 : ${PUSH_TREE_FREEBSD:=$XENBITS:/home/xen/git/freebsd.git}
 : ${BASE_TREE_FREEBSD:=git://xenbits.xen.org/freebsd.git}
 
-: ${TREE_LIBVIRT:=git://libvirt.org/libvirt.git}
+: ${TREE_LIBVIRT:=https://gitlab.com/libvirt/libvirt.git}
 : ${PUSH_TREE_LIBVIRT:=$XENBITS:/home/xen/git/libvirt.git}
 : ${BASE_TREE_LIBVIRT:=git://xenbits.xen.org/libvirt.git}
 
-- 
Anthony PERARD

Re: [PATCH v2 5/5] xen/memory, tools: Make init-dom0less consume XEN_DOMCTL_get_mem_map

[Henry Wang]

Hi Jan,

On 4/2/2024 4:51 PM, Jan Beulich wrote:

On 02.04.2024 10:43, Henry Wang wrote:

On 4/2/2024 3:05 PM, Jan Beulich wrote:

On 29.03.2024 06:11, Henry Wang wrote:

On 3/12/2024 1:07 AM, Jan Beulich wrote:

+/*
+ * Flag to force populate physmap to use pages from domheap instead of 1:1
+ * or static allocation.
+ */
+#define XENMEMF_force_heap_alloc  (1<<19)
#endif

If this is for populate_physmap only, then other sub-ops need to reject
its use.

I have to admit I'm a little wary of allocating another flag here and ...


--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -205,6 +205,8 @@ struct npfec {
#define  MEMF_no_icache_flush (1U<<_MEMF_no_icache_flush)
#define _MEMF_no_scrub8
#define  MEMF_no_scrub(1U<<_MEMF_no_scrub)
+#define _MEMF_force_heap_alloc 9
+#define  MEMF_force_heap_alloc (1U<<_MEMF_force_heap_alloc)
#define _MEMF_node16
#define  MEMF_node_mask   ((1U << (8 * sizeof(nodeid_t))) - 1)
#define  MEMF_node(n) n) + 1) & MEMF_node_mask) << _MEMF_node)

... here - we don't have that many left. Since other sub-ops aren't
intended to support this flag, did you consider adding another (perhaps
even arch-specific) sub-op instead?

While revisiting this comment when trying to come up with a V3, I
realized adding a sub-op here in the same level as
XENMEM_populate_physmap will basically duplicate the function
populate_physmap() with just the "else" (the non-1:1 allocation) part,
also a similar xc_domain_populate_physmap_exact() & co will be needed
from the toolstack side to call the new sub-op. So I am having the
concern of the duplication of code and not sure if I understand you
correctly. Would you please elaborate a bit more or clarify if I
understand you correctly? Thanks!

Well, the goal is to avoid both code duplication and introduction of a new,
single-use flag. The new sub-op suggestion, I realize now, would mainly have
helped with avoiding the new flag in the public interface. That's still
desirable imo. Internally, have you checked which MEMF_* are actually used
by populate_physmap()? Briefly looking, e.g. MEMF_no_dma and MEMF_no_refcount
aren't. It therefore would be possible to consider re-purposing one that
isn't (likely to be) used there. Of course doing so requires care to avoid
passing that flag down to other code (page_alloc.c functions in particular),
where the meaning would be the original one.

I think you made a good point, however, to be honest I am not sure about
the repurposing flags such as MEMF_no_dma and MEMF_no_refcount, because
I think the name and the purpose of the flag should be clear and
less-misleading. Reusing either one for another meaning, namely forcing
a non-heap allocation in populate_physmap() would be confusing in the
future. Also if one day these flags will be needed in
populate_physmap(), current repurposing approach will lead to a even
confusing code base.

For the latter - hence "(likely to be)" in my earlier reply.


Agreed.


For the naming - of course an aliasing #define ought to be used then, to
make the purpose clear at the use sites.


Well I have to admit the alias #define approach is clever (thanks) and I 
am getting persuaded gradually, as there will be also another benefit 
for me to do less modification from my side :) I will firstly try this 
approach in v3 if ...



I also do agree very much that we need to also avoid the code
duplication, so compared to other two suggested approach, adding a new
MEMF would be the cleanest solution IMHO, as it is just one bit and MEMF
flags are not added very often.

I would also curious what the other common code maintainers will say on
this issue: @Andrew, @Stefano, @Julien, any ideas? Thanks!


...not receiving any other input, and we can continue the discussion in 
v3. Thanks!


Kind regards,
Henry


Jan

Re: [PATCH v2 5/5] xen/memory, tools: Make init-dom0less consume XEN_DOMCTL_get_mem_map

[Jan Beulich]

On 02.04.2024 10:43, Henry Wang wrote:
> On 4/2/2024 3:05 PM, Jan Beulich wrote:
>> On 29.03.2024 06:11, Henry Wang wrote:
>>> On 3/12/2024 1:07 AM, Jan Beulich wrote:
> +/*
> + * Flag to force populate physmap to use pages from domheap instead of 
> 1:1
> + * or static allocation.
> + */
> +#define XENMEMF_force_heap_alloc  (1<<19)
>#endif
 If this is for populate_physmap only, then other sub-ops need to reject
 its use.

 I have to admit I'm a little wary of allocating another flag here and ...

> --- a/xen/include/xen/mm.h
> +++ b/xen/include/xen/mm.h
> @@ -205,6 +205,8 @@ struct npfec {
>#define  MEMF_no_icache_flush (1U<<_MEMF_no_icache_flush)
>#define _MEMF_no_scrub8
>#define  MEMF_no_scrub(1U<<_MEMF_no_scrub)
> +#define _MEMF_force_heap_alloc 9
> +#define  MEMF_force_heap_alloc (1U<<_MEMF_force_heap_alloc)
>#define _MEMF_node16
>#define  MEMF_node_mask   ((1U << (8 * sizeof(nodeid_t))) - 1)
>#define  MEMF_node(n) n) + 1) & MEMF_node_mask) << _MEMF_node)
 ... here - we don't have that many left. Since other sub-ops aren't
 intended to support this flag, did you consider adding another (perhaps
 even arch-specific) sub-op instead?
>>> While revisiting this comment when trying to come up with a V3, I
>>> realized adding a sub-op here in the same level as
>>> XENMEM_populate_physmap will basically duplicate the function
>>> populate_physmap() with just the "else" (the non-1:1 allocation) part,
>>> also a similar xc_domain_populate_physmap_exact() & co will be needed
>>> from the toolstack side to call the new sub-op. So I am having the
>>> concern of the duplication of code and not sure if I understand you
>>> correctly. Would you please elaborate a bit more or clarify if I
>>> understand you correctly? Thanks!
>> Well, the goal is to avoid both code duplication and introduction of a new,
>> single-use flag. The new sub-op suggestion, I realize now, would mainly have
>> helped with avoiding the new flag in the public interface. That's still
>> desirable imo. Internally, have you checked which MEMF_* are actually used
>> by populate_physmap()? Briefly looking, e.g. MEMF_no_dma and MEMF_no_refcount
>> aren't. It therefore would be possible to consider re-purposing one that
>> isn't (likely to be) used there. Of course doing so requires care to avoid
>> passing that flag down to other code (page_alloc.c functions in particular),
>> where the meaning would be the original one.
> 
> I think you made a good point, however, to be honest I am not sure about 
> the repurposing flags such as MEMF_no_dma and MEMF_no_refcount, because 
> I think the name and the purpose of the flag should be clear and 
> less-misleading. Reusing either one for another meaning, namely forcing 
> a non-heap allocation in populate_physmap() would be confusing in the 
> future. Also if one day these flags will be needed in 
> populate_physmap(), current repurposing approach will lead to a even 
> confusing code base.

For the latter - hence "(likely to be)" in my earlier reply.

For the naming - of course an aliasing #define ought to be used then, to
make the purpose clear at the use sites.

Jan

> I also do agree very much that we need to also avoid the code 
> duplication, so compared to other two suggested approach, adding a new 
> MEMF would be the cleanest solution IMHO, as it is just one bit and MEMF 
> flags are not added very often.
> 
> I would also curious what the other common code maintainers will say on 
> this issue: @Andrew, @Stefano, @Julien, any ideas? Thanks!
> 
> Kind regards,
> Henry

Re: [PATCH v2 5/5] xen/memory, tools: Make init-dom0less consume XEN_DOMCTL_get_mem_map

[Henry Wang]

Hi Jan,

On 4/2/2024 3:05 PM, Jan Beulich wrote:

On 29.03.2024 06:11, Henry Wang wrote:

On 3/12/2024 1:07 AM, Jan Beulich wrote:

+/*
+ * Flag to force populate physmap to use pages from domheap instead of 1:1
+ * or static allocation.
+ */
+#define XENMEMF_force_heap_alloc  (1<<19)
   #endif

If this is for populate_physmap only, then other sub-ops need to reject
its use.

I have to admit I'm a little wary of allocating another flag here and ...


--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -205,6 +205,8 @@ struct npfec {
   #define  MEMF_no_icache_flush (1U<<_MEMF_no_icache_flush)
   #define _MEMF_no_scrub8
   #define  MEMF_no_scrub(1U<<_MEMF_no_scrub)
+#define _MEMF_force_heap_alloc 9
+#define  MEMF_force_heap_alloc (1U<<_MEMF_force_heap_alloc)
   #define _MEMF_node16
   #define  MEMF_node_mask   ((1U << (8 * sizeof(nodeid_t))) - 1)
   #define  MEMF_node(n) n) + 1) & MEMF_node_mask) << _MEMF_node)

... here - we don't have that many left. Since other sub-ops aren't
intended to support this flag, did you consider adding another (perhaps
even arch-specific) sub-op instead?

While revisiting this comment when trying to come up with a V3, I
realized adding a sub-op here in the same level as
XENMEM_populate_physmap will basically duplicate the function
populate_physmap() with just the "else" (the non-1:1 allocation) part,
also a similar xc_domain_populate_physmap_exact() & co will be needed
from the toolstack side to call the new sub-op. So I am having the
concern of the duplication of code and not sure if I understand you
correctly. Would you please elaborate a bit more or clarify if I
understand you correctly? Thanks!

Well, the goal is to avoid both code duplication and introduction of a new,
single-use flag. The new sub-op suggestion, I realize now, would mainly have
helped with avoiding the new flag in the public interface. That's still
desirable imo. Internally, have you checked which MEMF_* are actually used
by populate_physmap()? Briefly looking, e.g. MEMF_no_dma and MEMF_no_refcount
aren't. It therefore would be possible to consider re-purposing one that
isn't (likely to be) used there. Of course doing so requires care to avoid
passing that flag down to other code (page_alloc.c functions in particular),
where the meaning would be the original one.


I think you made a good point, however, to be honest I am not sure about 
the repurposing flags such as MEMF_no_dma and MEMF_no_refcount, because 
I think the name and the purpose of the flag should be clear and 
less-misleading. Reusing either one for another meaning, namely forcing 
a non-heap allocation in populate_physmap() would be confusing in the 
future. Also if one day these flags will be needed in 
populate_physmap(), current repurposing approach will lead to a even 
confusing code base.


I also do agree very much that we need to also avoid the code 
duplication, so compared to other two suggested approach, adding a new 
MEMF would be the cleanest solution IMHO, as it is just one bit and MEMF 
flags are not added very often.


I would also curious what the other common code maintainers will say on 
this issue: @Andrew, @Stefano, @Julien, any ideas? Thanks!


Kind regards,
Henry


Jan

Re: [PATCH net] xen-netfront: Add missing skb_mark_for_recycle

[Arthur Borsboom]

On Fri, 29 Mar 2024 at 10:47, Arthur Borsboom  wrote:
>
> On Wed, 27 Mar 2024 at 13:15, Jesper Dangaard Brouer  wrote:
> >
> > Notice that skb_mark_for_recycle() is introduced later than fixes tag in
> > 6a5bcd84e886 ("page_pool: Allow drivers to hint on SKB recycling").
> >
> > It is believed that fixes tag were missing a call to 
> > page_pool_release_page()
> > between v5.9 to v5.14, after which is should have used 
> > skb_mark_for_recycle().
> > Since v6.6 the call page_pool_release_page() were removed (in 535b9c61bdef
> > ("net: page_pool: hide page_pool_release_page()") and remaining callers
> > converted (in commit 6bfef2ec0172 ("Merge branch
> > 'net-page_pool-remove-page_pool_release_page'")).
> >
> > This leak became visible in v6.8 via commit dba1b8a7ab68 ("mm/page_pool: 
> > catch
> > page_pool memory leaks").
> >
> > Fixes: 6c5aa6fc4def ("xen networking: add basic XDP support for 
> > xen-netfront")
> > Reported-by: Arthur Borsboom 
> > Signed-off-by: Jesper Dangaard Brouer 
> > ---
> > Compile tested only, can someone please test this
>
> I have tested this patch on Xen 4.18.1 with VM (Arch Linux) kernel 6.9.0-rc1.
>
> Without the patch there are many trace traces and cloning the Linux
> mainline git repository resulted in failures (same with kernel 6.8.1).
> The patched kernel 6.9.0-rc1 performs as expected; cloning the git
> repository was successful and no kernel traces observed.
> Hereby my tested by:
>
> Tested-by: Arthur Borsboom 
>
>
>
> >  drivers/net/xen-netfront.c |1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> > index ad29f370034e..8d2aee88526c 100644
> > --- a/drivers/net/xen-netfront.c
> > +++ b/drivers/net/xen-netfront.c
> > @@ -285,6 +285,7 @@ static struct sk_buff 
> > *xennet_alloc_one_rx_buffer(struct netfront_queue *queue)
> > return NULL;
> > }
> > skb_add_rx_frag(skb, 0, page, 0, 0, PAGE_SIZE);
> > +   skb_mark_for_recycle(skb);
> >
> > /* Align ip header to a 16 bytes boundary */
> > skb_reserve(skb, NET_IP_ALIGN);
> >
> >

I don't see this patch yet in linux-next.

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log

Any idea in which kernel release this patch will be included?

Re: [PATCH 2/3] drivers: serial: add Qualcomm GENI-based serial driver

[Jan Beulich]

On 29.03.2024 01:08, Volodymyr Babchuk wrote:
> Generic Interface (GENI) is a newer interface for low speed interfaces
> like UART, I2C or SPI. This patch adds the simple driver for the UART
> instance of GENI. Code is based on similar drivers in U-Boot and Linux
> kernel.
> 
> This driver implements only simple synchronous mode, because although
> GENI supports FIFO mode, it needs to know number of
> characters **before** starting TX transaction. This is a stark
> contrast when compared to other UART peripherals, which allow adding
> characters to a FIFO while TX operation is running.
> 
> The patch adds both normal UART driver and earlyprintk version.
> 
> Signed-off-by: Volodymyr Babchuk 
> ---
>  xen/arch/arm/Kconfig.debug   |  19 +-
>  xen/arch/arm/arm64/debug-qcom.inc|  76 +++
>  xen/arch/arm/include/asm/qcom-uart.h |  48 +
>  xen/drivers/char/Kconfig |   8 +
>  xen/drivers/char/Makefile|   1 +
>  xen/drivers/char/qcom-uart.c | 288 +++
>  6 files changed, 439 insertions(+), 1 deletion(-)
>  create mode 100644 xen/arch/arm/arm64/debug-qcom.inc
>  create mode 100644 xen/arch/arm/include/asm/qcom-uart.h
>  create mode 100644 xen/drivers/char/qcom-uart.c

This last new file wants mentioning in ./MAINTAINERS'es "ARM (W/
VIRTUALISATION EXTENSIONS) ARCHITECTURE" section, I suppose.

Jan

Re: [PATCH 1/3] arm: smmu: allow SMMU to have more IRQs than context banks

[Michal Orzel]

Hello,

On 29/03/2024 01:08, Volodymyr Babchuk wrote:
> 
> 
> I encountered platform, namely Qualcomm SA8155P where SMMU-compatible
NIT: a commit msg should be written in imperative mood

> IO-MMU advertises more context IQRs than there are context banks. This
> should not be an issue, we need to relax the check in the SMMU driver
> to allow such configuration.
> 
> Signed-off-by: Volodymyr Babchuk 
> ---
>  xen/drivers/passthrough/arm/smmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/drivers/passthrough/arm/smmu.c 
> b/xen/drivers/passthrough/arm/smmu.c
> index 32e2ff279b..2dd3688f3b 100644
> --- a/xen/drivers/passthrough/arm/smmu.c
> +++ b/xen/drivers/passthrough/arm/smmu.c
> @@ -2550,7 +2550,7 @@ static int arm_smmu_device_dt_probe(struct 
> platform_device *pdev)
> parse_driver_options(smmu);
> 
> if (smmu->version > ARM_SMMU_V1 &&
> -   smmu->num_context_banks != smmu->num_context_irqs) {
> +   smmu->num_context_banks > smmu->num_context_irqs) {
This was done in Linux by commit:
d1e20222d537 ("iommu/arm-smmu: Error out only if not enough context interrupts")

However, they also ignore superfluous interrupts. Shouldn't we do the same?

~Michal

Re: [XEN PATCH 3/6] xen/arm: ffa: separate memory sharing routines

[Bertrand Marquis]

Hi Julien,

> On 28 Mar 2024, at 18:58, Julien Grall  wrote:
> 
> Hi Bertrand,
> 
> On 27/03/2024 13:40, Bertrand Marquis wrote:
>> Hi Jens,
>>> On 25 Mar 2024, at 10:39, Jens Wiklander  wrote:
>>> 
>>> Move memory sharing routines into a separate file for easier navigation
>>> in the source code.
>>> 
>>> Add ffa_shm_domain_destroy() to isolate the ffa_shm things in
>>> ffa_domain_teardown_continue().
>>> 
>>> Signed-off-by: Jens Wiklander 
>> With the copyright date mentioned after fixed (which can be done on commit):
>> Reviewed-by: Bertrand Marquis  
> I have fixed and committed the series. However, it wasn't trivial to find the 
> comment in your reply. In the future, can you try to trim your reply?

Thanks a lot and very sorry for that, I will remember to trim next time.

Cheers
Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall

[XEN PATCH 7/7] vsprintf: address violations of MISRA C:2012 Rule 16.3

[Federico Serafini]

MISRA C:2012 Rule 16.3 states: "An unconditional `break' statement
shall terminate every switch-clause".

Add break statement to address violations of the rule or add
pseudo-keyword fallthrough to meet the requirements to deviate it.

No functional change.

Signed-off-by: Federico Serafini 
---
 xen/common/vsprintf.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/xen/common/vsprintf.c b/xen/common/vsprintf.c
index c49631c0a4..612751c90f 100644
--- a/xen/common/vsprintf.c
+++ b/xen/common/vsprintf.c
@@ -377,7 +377,7 @@ static char *pointer(char *str, const char *end, const char 
**fmt_ptr,
 str = number(str, end, hex_buffer[i], 16, 2, -1, ZEROPAD);
 
 if ( ++i == field_width )
-return str;
+break;
 
 if ( sep )
 {
@@ -386,6 +386,8 @@ static char *pointer(char *str, const char *end, const char 
**fmt_ptr,
 ++str;
 }
 }
+
+return str;
 }
 
 case 'p': /* PCI SBDF. */
@@ -619,6 +621,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, 
va_list args)
 
 case 'X':
 flags |= LARGE;
+fallthrough;
 case 'x':
 base = 16;
 break;
@@ -626,6 +629,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, 
va_list args)
 case 'd':
 case 'i':
 flags |= SIGN;
+fallthrough;
 case 'u':
 break;
 
-- 
2.34.1

Re: [PATCH v3 1/3] xen/arm: Add imx8q{m,x} platform glue

[Michal Orzel]

Hi John,

On 28/03/2024 17:34, John Ernberg wrote:
> 
> 
> When using Linux for dom0 there are a bunch of drivers that need to do SMC
> SIP calls into the firmware to enable certain hardware bits like the
> watchdog.
> 
> Provide a basic platform glue that implements the needed SMC forwarding.
> 
> The format of these calls are as follows:
>  - reg 0: function ID
>  - reg 1: subfunction ID (when there's a subfunction)
>  remaining regs: args
> 
> For now we only allow Dom0 to make these calls as they are all managing
> hardware. There is no specification for these SIP calls, the IDs and names
> have been extracted from the upstream linux kernel and the vendor kernel.
> 
> Most of the SIP calls are only available for the iMX8M series of SoCs, so
> they are easy to reject and they need to be revisited when iMX8M series
> support is added.
I just realized that you pinged me in v2 for clarification, sorry I missed that.
I still believe we shouldn't list FIDs that are meant for IMX8M, given that
the driver is written for IMX8QM/QXP.

> 
> From the other calls we can reject CPUFREQ because Dom0 cannot make an
> informed decision regarding CPU frequency scaling, WAKEUP_SRC is to wake
> up from suspend, which Xen doesn't support at this time.
> 
> This leaves the TIME SIP, OTP SIPs, BUILDINFO SIP and TEMP ALARM SIP, which
> for now are allowed to Dom0.
> 
> NOTE: This code is based on code found in NXP Xen tree located here:
> https://github.com/nxp-imx/imx-xen/blob/lf-5.10.y_4.13/xen/arch/arm/platforms/imx8qm.c
> 
> Signed-off-by: Peng Fan 
> [jernberg: Add SIP call filtering]
> Signed-off-by: John Ernberg 
> 
> ---
> 
> v3:
>  - Adhere to style guidelines for line length and label indentation (Michal 
> Orzel)
>  - Use smccc macros to build the SIP function identifier (Michal Orzel)
>  - Adjust platform name to be specific to QM and QXP variants (Michal Orzel)
>  - Pick up additional SIPs which may be used by other Linux versions - for 
> review purposes
>  - Changes to the commit message due to above
> 
> v2:
>  - Reword the commit message to be a bit clearer
>  - Include the link previously added as a context note to the commit message 
> (Julien Grall)
>  - Add Pengs signed off (Julien Grall, Peng Fan)
>  - Add basic SIP call filter (Julien Grall)
>  - Expand the commit message a whole bunch because of the changes to the code
> ---
>  xen/arch/arm/platforms/Makefile |   1 +
>  xen/arch/arm/platforms/imx8qm.c | 168 
>  2 files changed, 169 insertions(+)
>  create mode 100644 xen/arch/arm/platforms/imx8qm.c
> 
> diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
> index 8632f4115f..bec6e55d1f 100644
> --- a/xen/arch/arm/platforms/Makefile
> +++ b/xen/arch/arm/platforms/Makefile
> @@ -9,5 +9,6 @@ obj-$(CONFIG_ALL_PLAT)   += sunxi.o
>  obj-$(CONFIG_ALL64_PLAT) += thunderx.o
>  obj-$(CONFIG_ALL64_PLAT) += xgene-storm.o
>  obj-$(CONFIG_ALL64_PLAT) += brcm-raspberry-pi.o
> +obj-$(CONFIG_ALL64_PLAT) += imx8qm.o
>  obj-$(CONFIG_MPSOC_PLATFORM)  += xilinx-zynqmp.o
>  obj-$(CONFIG_MPSOC_PLATFORM)  += xilinx-zynqmp-eemi.o
> diff --git a/xen/arch/arm/platforms/imx8qm.c b/xen/arch/arm/platforms/imx8qm.c
> new file mode 100644
> index 00..0992475474
> --- /dev/null
> +++ b/xen/arch/arm/platforms/imx8qm.c
> @@ -0,0 +1,168 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * xen/arch/arm/platforms/imx8qm.c
> + *
> + * i.MX 8QM setup
> + *
> + * Copyright (c) 2016 Freescale Inc.
> + * Copyright 2018-2019 NXP
> + *
> + *
> + * Peng Fan 
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +static const char * const imx8qm_dt_compat[] __initconst =
> +{
> +"fsl,imx8qm",
> +"fsl,imx8qxp",
> +NULL
> +};
> +
> +#define IMX_SIP_FID(fid) \
> +ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> +   ARM_SMCCC_CONV_64, \
> +   ARM_SMCCC_OWNER_SIP, \
> +   fid)
macro parameter should be enclosed within parenthesis

> +
> +#define IMX_SIP_F_GPC0x
> +#define IMX_SIP_F_CPUFREQ0x0001
> +#define IMX_SIP_F_TIME   0x0002
> +#define IMX_SIP_F_BUILDINFO  0x0003
> +#define IMX_SIP_F_DDR_DVFS   0x0004
> +#define IMX_SIP_F_SRC0x0005
> +#define IMX_SIP_F_GET_SOC_INFO   0x0006
> +#define IMX_SIP_F_NOC0x0008
> +#define IMX_SIP_F_WAKEUP_SRC 0x0009
> +#define IMX_SIP_F_OTP_READ   0x000A
> +#define IMX_SIP_F_OTP_WRITE  0x000B
> +#define IMX_SIP_F_SET_TEMP_ALARM 0x000C
Is there specific reason for leading zeros?

> +
> +#define IMX_SIP_TIME_SF_RTC_SET_TIME 0x00
> +#define IMX_SIP_TIME_SF_WDOG_START   0x01
> +#define IMX_SIP_TIME_SF_WDOG_STOP0x02
> +#define IMX_SIP_TIME_SF_WDOG_SET_ACT 0x03
> +#define IMX_SIP_TIME_SF_WDOG_PING0x04
> +#define IMX_SIP_TIME_SF_WDOG_SET_TIMEOUT 0x05
> +#define IMX_SIP_TIME_SF_WDOG_GET_STAT0x06
> +#define IMX_SIP_TIME_SF_WDOG_SET_PRETIME 0x07
> +
> +static bool 

[XEN PATCH 3/7] xen/sched: address a violation of MISRA C:2012 Rule 16.3

[Federico Serafini]

Use pseudo-keyword fallthrough to meet the requirements to deviate
MISRA C:2012 Rule 16.3 ("An unconditional `break' statement shall
terminate every switch-clause").

No functional change.

Signed-off-by: Federico Serafini 
---
 xen/common/sched/credit2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/common/sched/credit2.c b/xen/common/sched/credit2.c
index c76330d79d..0962b52415 100644
--- a/xen/common/sched/credit2.c
+++ b/xen/common/sched/credit2.c
@@ -3152,8 +3152,8 @@ static int cf_check csched2_sys_cntl(
 printk(XENLOG_INFO "Disabling context switch rate limiting\n");
 prv->ratelimit_us = params->ratelimit_us;
 write_unlock_irqrestore(>lock, flags);
+fallthrough;
 
-/* FALLTHRU */
 case XEN_SYSCTL_SCHEDOP_getinfo:
 params->ratelimit_us = prv->ratelimit_us;
 break;
-- 
2.34.1

[XEN PATCH 4/7] xen/evtchn: address a violation of MISRA C:2012 Rule 16.3

[Federico Serafini]

Add break statement to address a violation of MISRA C:2012 Rule 16.3
("An unconditional `break' statement shall terminate every
switch-clause ").

No functional change.

Signed-off-by: Federico Serafini 
---
 xen/common/event_channel.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index 20f586cf5e..aceee0695f 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -853,6 +853,7 @@ int evtchn_send(struct domain *ld, unsigned int lport)
 break;
 default:
 ret = -EINVAL;
+break;
 }
 
 out:
-- 
2.34.1

[XEN PATCH 2/7] console: address a violation of MISRA C:2012 Rule 16.3

[Federico Serafini]

Add break statement to address a violation of MISRA C:2012 Rule 16.3
("An unconditional `break' statement shall terminate every
switch-clause ").

No functional change.

Signed-off-by: Federico Serafini 
---
 xen/drivers/char/console.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index ccd5f8cc14..e185f80efe 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -568,6 +568,8 @@ static void __serial_rx(char c)
 
 if ( d != NULL )
 rcu_unlock_domain(d);
+
+break;
 }
 #endif
 }
-- 
2.34.1

[XEN PATCH 6/7] xen/vm-event: address a violation of MISRA C:2012 Rule 16.3

[Federico Serafini]

Add break statement to address a violation of MISRA C:2012 Rule 16.3
("An unconditional `break' statement shall terminate every
switch-clause ").

No functional change.

Signed-off-by: Federico Serafini 
---
 xen/common/vm_event.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index ecf49c38a9..fbf1aa0848 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -770,6 +770,7 @@ int vm_event_domctl(struct domain *d, struct 
xen_domctl_vm_event_op *vec)
 
 default:
 rc = -ENOSYS;
+break;
 }
 
 return rc;
-- 
2.34.1

[XEN PATCH 5/7] xen/sched: address a violation of MISRA C:2012 Rule 16.3

[Federico Serafini]

Add break statement to address a violation of MISRA C:2012 Rule 16.3
("An unconditional `break' statement shall terminate every
switch-clause ").

No functional change.

Signed-off-by: Federico Serafini 
---
 xen/common/sched/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
index 3e48da1cdd..0cb33831d2 100644
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -2007,6 +2007,7 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) 
arg)
 
 default:
 ret = -ENOSYS;
+break;
 }
 
 return ret;
-- 
2.34.1

[XEN PATCH 1/7] xen/domctl: address a violation of MISRA C:2012 Rule 16.3

[Federico Serafini]

Add break statement to address a violation of MISRA C:2012 Rule 16.3
("An unconditional `break' statement shall terminate every
switch-clause ").

No functional change.

Signed-off-by: Federico Serafini 
---
 xen/common/domctl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index d94a9dae91..f2e0e36a17 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -316,6 +316,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
u_domctl)
 d = rcu_lock_domain_by_id(op->domain);
 if ( !d )
 return -ESRCH;
+break;
 }
 
 ret = xsm_domctl(XSM_OTHER, d, op->cmd);
-- 
2.34.1

[XEN PATCH 0/7] xen: address violations of MISRA C:2012 Rule 16.3

[Federico Serafini]

This patch series addresses some violations of Rule 16.3 by adding the missing
break statements and pseudo-keyword fallthrough.

Federico Serafini (7):
  xen/domctl: address a violation of MISRA C:2012 Rule 16.3
  console: address a violation of MISRA C:2012 Rule 16.3
  xen/sched: address a violation of MISRA C:2012 Rule 16.3
  xen/evtchn: address a violation of MISRA C:2012 Rule 16.3
  xen/sched: address a violation of MISRA C:2012 Rule 16.3
  xen/vm-event: address a violation of MISRA C:2012 Rule 16.3
  vsprintf: address violations of MISRA C:2012 Rule 16.3

 xen/common/domctl.c| 1 +
 xen/common/event_channel.c | 1 +
 xen/common/sched/core.c| 1 +
 xen/common/sched/credit2.c | 2 +-
 xen/common/vm_event.c  | 1 +
 xen/common/vsprintf.c  | 6 +-
 xen/drivers/char/console.c | 2 ++
 7 files changed, 12 insertions(+), 2 deletions(-)

-- 
2.34.1

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

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-arndale  10 host-ping-check-xen fail pass in 185216-retest

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds18 guest-start/debian.repeat fail REGR. vs. 185211

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

version targeted for testing:
 linux026e680b0a08a62b1d948e5a8ca78700bfac0e6e
baseline version:
 linux39cd87c4eb2b893354f3b850f916353f2658ae6f

Last test of basis   185211  2024-04-01 02:02:09 Z1 days
Testing same since   185214  2024-04-01 22:11:44 Z0 days1 attempts


People who touched revisions under test:
  Karel Balej 
  Linus Torvalds 
  Uwe Kleine-König 

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

Re: [PATCH v2 5/5] xen/memory, tools: Make init-dom0less consume XEN_DOMCTL_get_mem_map

[Jan Beulich]

On 29.03.2024 06:11, Henry Wang wrote:
> On 3/12/2024 1:07 AM, Jan Beulich wrote:
>>> +/*
>>> + * Flag to force populate physmap to use pages from domheap instead of 1:1
>>> + * or static allocation.
>>> + */
>>> +#define XENMEMF_force_heap_alloc  (1<<19)
>>>   #endif
>> If this is for populate_physmap only, then other sub-ops need to reject
>> its use.
>>
>> I have to admit I'm a little wary of allocating another flag here and ...
>>
>>> --- a/xen/include/xen/mm.h
>>> +++ b/xen/include/xen/mm.h
>>> @@ -205,6 +205,8 @@ struct npfec {
>>>   #define  MEMF_no_icache_flush (1U<<_MEMF_no_icache_flush)
>>>   #define _MEMF_no_scrub8
>>>   #define  MEMF_no_scrub(1U<<_MEMF_no_scrub)
>>> +#define _MEMF_force_heap_alloc 9
>>> +#define  MEMF_force_heap_alloc (1U<<_MEMF_force_heap_alloc)
>>>   #define _MEMF_node16
>>>   #define  MEMF_node_mask   ((1U << (8 * sizeof(nodeid_t))) - 1)
>>>   #define  MEMF_node(n) n) + 1) & MEMF_node_mask) << _MEMF_node)
>> ... here - we don't have that many left. Since other sub-ops aren't
>> intended to support this flag, did you consider adding another (perhaps
>> even arch-specific) sub-op instead?
> 
> While revisiting this comment when trying to come up with a V3, I 
> realized adding a sub-op here in the same level as 
> XENMEM_populate_physmap will basically duplicate the function 
> populate_physmap() with just the "else" (the non-1:1 allocation) part, 
> also a similar xc_domain_populate_physmap_exact() & co will be needed 
> from the toolstack side to call the new sub-op. So I am having the 
> concern of the duplication of code and not sure if I understand you 
> correctly. Would you please elaborate a bit more or clarify if I 
> understand you correctly? Thanks!

Well, the goal is to avoid both code duplication and introduction of a new,
single-use flag. The new sub-op suggestion, I realize now, would mainly have
helped with avoiding the new flag in the public interface. That's still
desirable imo. Internally, have you checked which MEMF_* are actually used
by populate_physmap()? Briefly looking, e.g. MEMF_no_dma and MEMF_no_refcount
aren't. It therefore would be possible to consider re-purposing one that
isn't (likely to be) used there. Of course doing so requires care to avoid
passing that flag down to other code (page_alloc.c functions in particular),
where the meaning would be the original one.

Jan

Jan

Re: [PATCH v6 06/20] xen/bitops: put __ffs() and ffz() into linux compatible header

[Jan Beulich]

On 29.03.2024 19:23, Oleksii wrote:
> On Wed, 2024-03-20 at 16:42 +0100, Jan Beulich wrote:
>> On 15.03.2024 19:06, Oleksii Kurochko wrote:
>>> --- a/xen/lib/find-next-bit.c
>>> +++ b/xen/lib/find-next-bit.c
>>> @@ -9,6 +9,7 @@
>>>   * 2 of the License, or (at your option) any later version.
>>>   */
>>>  #include 
>>> +#include 
>>>  
>>>  #include 
>>
>> Hmm, no, a library source would better not include this header.
>> Surely
>> the ffz() can be taken care of locally here?
> Except that __ffs() from xen/linux-compat.h is used in find-next-bit.c,
> so it seems that it is still need to include the header.

Hmm, no - that __ffs() use, if we mean that to become a Linux compat thing
only, should then be replaced (or covered locally), too, imo.

Jan

Re: [PATCH v6 3/4] libelf: Store maximum PHDR p_align

[Jan Beulich]

On 29.03.2024 15:41, Jason Andryuk wrote:
> On 2024-03-28 12:47, Jan Beulich wrote:
>> On 27.03.2024 22:51, Jason Andryuk wrote:
>>> --- a/xen/common/libelf/libelf-loader.c
>>> +++ b/xen/common/libelf/libelf-loader.c
>>> @@ -468,6 +468,7 @@ void elf_parse_binary(struct elf_binary *elf)
>>>   {
>>>   ELF_HANDLE_DECL(elf_phdr) phdr;
>>>   uint64_t low = -1, high = 0, paddr, memsz;
>>> +uint64_t max_align = 0, palign;
>>>   unsigned i, count;
>>>   
>>>   count = elf_phdr_count(elf);
>>> @@ -481,17 +482,23 @@ void elf_parse_binary(struct elf_binary *elf)
>>>   continue;
>>>   paddr = elf_uval(elf, phdr, p_paddr);
>>>   memsz = elf_uval(elf, phdr, p_memsz);
>>> -elf_msg(elf, "ELF: phdr: paddr=%#" PRIx64 " memsz=%#" PRIx64 "\n",
>>> -paddr, memsz);
>>> +palign = elf_uval(elf, phdr, p_align);
>>> +elf_msg(elf,
>>> +"ELF: phdr: paddr=%#" PRIx64 " memsz=%#" PRIx64 " 
>>> palign=%#" PRIx64 "\n",
>>> +paddr, memsz, palign);
>>>   if ( low > paddr )
>>>   low = paddr;
>>>   if ( high < paddr + memsz )
>>>   high = paddr + memsz;
>>> +if ( max_align < palign )
>>> +max_align = palign;
>>>   }
>>>   elf->pstart = low;
>>>   elf->pend = high;
>>> -elf_msg(elf, "ELF: memory: %#" PRIx64 " -> %#" PRIx64 "\n",
>>> -elf->pstart, elf->pend);
>>> +elf->palign = max_align;
>>> +elf_msg(elf,
>>> +"ELF: memory: %#" PRIx64 " -> %#" PRIx64 " align:%#" PRIx64 
>>> "\n",
>>> +elf->pstart, elf->pend, elf->palign);
>>>   }
>>
>> Hmm, it's just this final logging change which I'm a little concerned by:
>> Having looked at Linux'es phdr, I noticed that the addresses there aren't
>> necessarily matching the corresponding alignment. Therefore I'm a little
>> concerned that the output here might raise questions when people see
>> seemingly inconsistent values in the log. Could you/we at least make it
>> read like e.g. "align (max): ..."?
> 
> max_align?
> 
> Looking at my test vmlinux, it looks like PHDR 0 (.text) and PHDR 1 
> (.data) are aligned.  It's the PHDR 2 (.init) that isn't aligned.  Is 
> that what you see?
> 
> This line is already printing the min and max across all the PHDRs, so 
> it would only look confusing if the start didn't match the align.
> 
> I'm not sure how useful it is to print the alignment, and I considered 
> not printing it.  It's only used with PVH Dom0 right now, so it's not 
> relevant in most cases.

Well, yes, not printing the alignment would be fine with me. I just didn't
suggest that as an alternative since I was assuming you put its printing
there for a reason.

Jan