There is an issue of calculate 2^n roundup of CONFIG_MAX_PT_IRQ_ENTRIES,
and the code style is very ugly when we use macro to fix it.

So this patch move MAX_IR_ENTRIES to offline tool which could do align
check and calculate it automatically.

v1-->v2:
1. remove ROUND2/4/8 MACROs of powerof2_roundup

Signed-off-by: Chenli Wei <chenli.wei@...>
---
hypervisor/include/arch/x86/asm/vtd.h | 7 -------
hypervisor/include/lib/util.h | 21 ---------------------
2 files changed, 28 deletions(-)

diff --git a/hypervisor/include/arch/x86/asm/vtd.h b/hypervisor/include/arch/x86/asm/vtd.h
index 4d3518286..284fc62db 100644
--- a/hypervisor/include/arch/x86/asm/vtd.h
+++ b/hypervisor/include/arch/x86/asm/vtd.h
@@ -42,13 +42,6 @@
#define DMAR_ICS_REG 0x9cU /* Invalidation complete status register */
#define DMAR_IRTA_REG 0xb8U /* Interrupt remapping table addr register */

-/* Make sure all PT IRQs work w/ interrupt remapping or post interrupt */
-#if (CONFIG_MAX_PT_IRQ_ENTRIES <= 256)
-#define MAX_IR_ENTRIES 256
-#else
-#define MAX_IR_ENTRIES powerof2_roundup(CONFIG_MAX_PT_IRQ_ENTRIES)
-#endif
-
/* Values for entry_type in ACPI_DMAR_DEVICE_SCOPE - device types */
enum acpi_dmar_scope_type {
ACPI_DMAR_SCOPE_TYPE_NOT_USED = 0,
diff --git a/hypervisor/include/lib/util.h b/hypervisor/include/lib/util.h
index ca3296556..d702ae253 100644
--- a/hypervisor/include/lib/util.h
+++ b/hypervisor/include/lib/util.h
@@ -25,27 +25,6 @@
/** Replaces 'x' by the string "x". */
#define STRINGIFY(x) #x

-/*
- * This algorithm get the round up 2^n
- *
- * n = input - 1; 0x1002 ----> 0x1001
- * n |= n >> 1; 0x1001 | 0x800
- * n |= n >> 2; 0x1801 | 0x600
- * n |= n >> 4; 0x1e01 | 0x1e0
- * n |= n >> 8; 0x1fe1 | 0x1f
- * n |= n >> 16; 0x1fff
- * n |= n >> 32; 0x1fff
- * n += 1; 0x2000
- */
-#define ROUND0(x) ((x)-1)
-#define ROUND1(x) (ROUND0(x) |(ROUND0(x)>>1))
-#define ROUND2(x) (ROUND1(x) |(ROUND1(x)>>2))
-#define ROUND4(x) (ROUND2(x) |(ROUND2(x)>>4))
-#define ROUND8(x) (ROUND4(x) |(ROUND4(x)>>8))
-#define ROUND16(x) (ROUND8(x) |(ROUND8(x)>>16))
-#define ROUND32(x) (ROUND16(x) |(ROUND16(x)>>32))
-#define powerof2_roundup(x) ((ROUND32(x) == (~0UL)) ? ~0UL : (ROUND32(x) +1))
-
/* Macro used to check if a value is aligned to the required boundary.
* Returns TRUE if aligned; FALSE if not aligned
* NOTE: The required alignment must be a power of 2 (2, 4, 8, 16, 32, etc)
--
2.17.1

On Mon, May 16, 2022 at 02:07:12PM +0800, Chenli Wei wrote:

There is an issue of calculate 2^n roundup of CONFIG_MAX_PT_IRQ_ENTRIES,
and the code style is very ugly when we use macro to fix it.

So this patch move MAX_IR_ENTRIES to offline tool which could do align
check and calculate it automatically.

This patch is LGTM.
One question: would it better to add ACFG_ prefix to MAX_IR_ENTRIES ?

v1-->v2:
1. remove ROUND2/4/8 MACROs of powerof2_roundup

Signed-off-by: Chenli Wei <chenli.wei@...>
---
hypervisor/include/arch/x86/asm/vtd.h | 7 -------
hypervisor/include/lib/util.h | 21 ---------------------
2 files changed, 28 deletions(-)

diff --git a/hypervisor/include/arch/x86/asm/vtd.h b/hypervisor/include/arch/x86/asm/vtd.h
index 4d3518286..284fc62db 100644
--- a/hypervisor/include/arch/x86/asm/vtd.h
+++ b/hypervisor/include/arch/x86/asm/vtd.h
@@ -42,13 +42,6 @@
#define DMAR_ICS_REG 0x9cU /* Invalidation complete status register */
#define DMAR_IRTA_REG 0xb8U /* Interrupt remapping table addr register */

-/* Make sure all PT IRQs work w/ interrupt remapping or post interrupt */
-#if (CONFIG_MAX_PT_IRQ_ENTRIES <= 256)
-#define MAX_IR_ENTRIES 256
-#else
-#define MAX_IR_ENTRIES powerof2_roundup(CONFIG_MAX_PT_IRQ_ENTRIES)
-#endif
-
/* Values for entry_type in ACPI_DMAR_DEVICE_SCOPE - device types */
enum acpi_dmar_scope_type {
ACPI_DMAR_SCOPE_TYPE_NOT_USED = 0,
diff --git a/hypervisor/include/lib/util.h b/hypervisor/include/lib/util.h
index ca3296556..d702ae253 100644
--- a/hypervisor/include/lib/util.h
+++ b/hypervisor/include/lib/util.h
@@ -25,27 +25,6 @@
/** Replaces 'x' by the string "x". */
#define STRINGIFY(x) #x

-/*
- * This algorithm get the round up 2^n
- *
- * n = input - 1; 0x1002 ----> 0x1001
- * n |= n >> 1; 0x1001 | 0x800
- * n |= n >> 2; 0x1801 | 0x600
- * n |= n >> 4; 0x1e01 | 0x1e0
- * n |= n >> 8; 0x1fe1 | 0x1f
- * n |= n >> 16; 0x1fff
- * n |= n >> 32; 0x1fff
- * n += 1; 0x2000
- */
-#define ROUND0(x) ((x)-1)
-#define ROUND1(x) (ROUND0(x) |(ROUND0(x)>>1))
-#define ROUND2(x) (ROUND1(x) |(ROUND1(x)>>2))
-#define ROUND4(x) (ROUND2(x) |(ROUND2(x)>>4))
-#define ROUND8(x) (ROUND4(x) |(ROUND4(x)>>8))
-#define ROUND16(x) (ROUND8(x) |(ROUND8(x)>>16))
-#define ROUND32(x) (ROUND16(x) |(ROUND16(x)>>32))
-#define powerof2_roundup(x) ((ROUND32(x) == (~0UL)) ? ~0UL : (ROUND32(x) +1))
-
/* Macro used to check if a value is aligned to the required boundary.
* Returns TRUE if aligned; FALSE if not aligned
* NOTE: The required alignment must be a power of 2 (2, 4, 8, 16, 32, etc)
--
2.17.1

On 5/16/2022 2:23 PM, Li, Fei1 wrote:

On Mon, May 16, 2022 at 02:07:12PM +0800, Chenli Wei wrote:

There is an issue of calculate 2^n roundup of CONFIG_MAX_PT_IRQ_ENTRIES,
and the code style is very ugly when we use macro to fix it.

So this patch move MAX_IR_ENTRIES to offline tool which could do align
check and calculate it automatically.

This patch is LGTM.
One question: would it better to add ACFG_ prefix to MAX_IR_ENTRIES ?

The name "MAX_IR_ENTRIES " was rename from "CONFIG_MAX_IR_ENTRIES".

That's because the "MAX_IR_ENTRIES " was not config by user after we align it with CONFIG_MAX_PT_IRQ_ENTRIES

Now we move it too config tool, by it's not user config, so I think it's better to keep the current name.

v1-->v2:
1. remove ROUND2/4/8 MACROs of powerof2_roundup

Signed-off-by: Chenli Wei <chenli.wei@...>
---
hypervisor/include/arch/x86/asm/vtd.h | 7 -------
hypervisor/include/lib/util.h | 21 ---------------------
2 files changed, 28 deletions(-)

diff --git a/hypervisor/include/arch/x86/asm/vtd.h b/hypervisor/include/arch/x86/asm/vtd.h
index 4d3518286..284fc62db 100644
--- a/hypervisor/include/arch/x86/asm/vtd.h
+++ b/hypervisor/include/arch/x86/asm/vtd.h
@@ -42,13 +42,6 @@
#define DMAR_ICS_REG 0x9cU /* Invalidation complete status register */
#define DMAR_IRTA_REG 0xb8U /* Interrupt remapping table addr register */
-/* Make sure all PT IRQs work w/ interrupt remapping or post interrupt */
-#if (CONFIG_MAX_PT_IRQ_ENTRIES <= 256)
-#define MAX_IR_ENTRIES 256
-#else
-#define MAX_IR_ENTRIES powerof2_roundup(CONFIG_MAX_PT_IRQ_ENTRIES)
-#endif
-
/* Values for entry_type in ACPI_DMAR_DEVICE_SCOPE - device types */
enum acpi_dmar_scope_type {
ACPI_DMAR_SCOPE_TYPE_NOT_USED = 0,
diff --git a/hypervisor/include/lib/util.h b/hypervisor/include/lib/util.h
index ca3296556..d702ae253 100644
--- a/hypervisor/include/lib/util.h
+++ b/hypervisor/include/lib/util.h
@@ -25,27 +25,6 @@
/** Replaces 'x' by the string "x". */
#define STRINGIFY(x) #x
-/*
- * This algorithm get the round up 2^n
- *
- * n = input - 1; 0x1002 ----> 0x1001
- * n |= n >> 1; 0x1001 | 0x800
- * n |= n >> 2; 0x1801 | 0x600
- * n |= n >> 4; 0x1e01 | 0x1e0
- * n |= n >> 8; 0x1fe1 | 0x1f
- * n |= n >> 16; 0x1fff
- * n |= n >> 32; 0x1fff
- * n += 1; 0x2000
- */
-#define ROUND0(x) ((x)-1)
-#define ROUND1(x) (ROUND0(x) |(ROUND0(x)>>1))
-#define ROUND2(x) (ROUND1(x) |(ROUND1(x)>>2))
-#define ROUND4(x) (ROUND2(x) |(ROUND2(x)>>4))
-#define ROUND8(x) (ROUND4(x) |(ROUND4(x)>>8))
-#define ROUND16(x) (ROUND8(x) |(ROUND8(x)>>16))
-#define ROUND32(x) (ROUND16(x) |(ROUND16(x)>>32))
-#define powerof2_roundup(x) ((ROUND32(x) == (~0UL)) ? ~0UL : (ROUND32(x) +1))
-
/* Macro used to check if a value is aligned to the required boundary.
* Returns TRUE if aligned; FALSE if not aligned
* NOTE: The required alignment must be a power of 2 (2, 4, 8, 16, 32, etc)
--
2.17.1

On Wed, May 18, 2022 at 02:10:32AM +0800, Dong, Eddie wrote:

-----Original Message-----
From: acrn-dev@... <acrn-dev@...> On
Behalf Of chenli.wei
Sent: Sunday, May 15, 2022 11:07 PM
To: Mao, Junjie <junjie.mao@...>; Li, Fei1 <fei1.li@...>; acrn-
dev@...
Cc: Wei, Chenli <chenli.wei@...>
Subject: [acrn-dev] [PATCH v2 2/2] hv: move the define of MAX_IR_ENTRIES
to offline tool

There is an issue of calculate 2^n roundup of
CONFIG_MAX_PT_IRQ_ENTRIES, and the code style is very ugly when we use
macro to fix it.

So this patch move MAX_IR_ENTRIES to offline tool which could do align
check and calculate it automatically.

v1-->v2:
1. remove ROUND2/4/8 MACROs of powerof2_roundup

Signed-off-by: Chenli Wei <chenli.wei@...>
---
hypervisor/include/arch/x86/asm/vtd.h | 7 -------
hypervisor/include/lib/util.h | 21 ---------------------
2 files changed, 28 deletions(-)

diff --git a/hypervisor/include/arch/x86/asm/vtd.h
b/hypervisor/include/arch/x86/asm/vtd.h
index 4d3518286..284fc62db 100644
--- a/hypervisor/include/arch/x86/asm/vtd.h
+++ b/hypervisor/include/arch/x86/asm/vtd.h
@@ -42,13 +42,6 @@
#define DMAR_ICS_REG 0x9cU /* Invalidation complete status register
*/
#define DMAR_IRTA_REG 0xb8U /* Interrupt remapping table addr
register */

-/* Make sure all PT IRQs work w/ interrupt remapping or post interrupt */ -
#if (CONFIG_MAX_PT_IRQ_ENTRIES <= 256) -#define MAX_IR_ENTRIES 256 -
#else -#define MAX_IR_ENTRIES
powerof2_roundup(CONFIG_MAX_PT_IRQ_ENTRIES)
-#endif
-

Can you add following comments at the 1st place (vtd.c) where the MACRO is used?
MAX_IR_ENTRIES is roundup (to power of 2) of CONFIG_MAX_PT_IRQ_ENTRIES.


Also, I want you to add a check somewhere:

ASSERT( ~(MAX_IR_ENTRIES-1UL) & CONFIG_MAX_PT_IRQ_ENTRIES == 0);

Now we did this check
#if ((MAX_IR_ENTRIES < 256U) || (MAX_IR_ENTRIES > 0x10000U) || (MAX_IR_ENTRIES & (MAX_IR_ENTRIES -1)) != 0U)
#error "MAX_IR_ENTRIES must in the region of [256,0x10000] and be 2^n"
#endif
in misc/hv_prebuild/static_checks.c already.

You can PR with above changes.

/* Values for entry_type in ACPI_DMAR_DEVICE_SCOPE - device types */
enum acpi_dmar_scope_type {
ACPI_DMAR_SCOPE_TYPE_NOT_USED = 0,
diff --git a/hypervisor/include/lib/util.h b/hypervisor/include/lib/util.h index
ca3296556..d702ae253 100644
--- a/hypervisor/include/lib/util.h
+++ b/hypervisor/include/lib/util.h
@@ -25,27 +25,6 @@
/** Replaces 'x' by the string "x". */
#define STRINGIFY(x) #x

-/*
- * This algorithm get the round up 2^n
- *
- * n = input - 1; 0x1002 ----> 0x1001
- * n |= n >> 1; 0x1001 | 0x800
- * n |= n >> 2; 0x1801 | 0x600
- * n |= n >> 4; 0x1e01 | 0x1e0
- * n |= n >> 8; 0x1fe1 | 0x1f
- * n |= n >> 16; 0x1fff
- * n |= n >> 32; 0x1fff
- * n += 1; 0x2000
- */
-#define ROUND0(x) ((x)-1)
-#define ROUND1(x) (ROUND0(x) |(ROUND0(x)>>1)) -#define ROUND2(x)
(ROUND1(x) |(ROUND1(x)>>2)) -#define ROUND4(x) (ROUND2(x)
|(ROUND2(x)>>4)) -#define ROUND8(x) (ROUND4(x) |(ROUND4(x)>>8)) -
#define ROUND16(x) (ROUND8(x) |(ROUND8(x)>>16)) -#define ROUND32(x)
(ROUND16(x) |(ROUND16(x)>>32)) -#define powerof2_roundup(x)
((ROUND32(x) == (~0UL)) ? ~0UL : (ROUND32(x) +1))
-
/* Macro used to check if a value is aligned to the required boundary.
* Returns TRUE if aligned; FALSE if not aligned
* NOTE: The required alignment must be a power of 2 (2, 4, 8, 16, 32, etc)
--
2.17.1

On 5/18/2022 9:07 AM, Li, Fei1 wrote:

On Wed, May 18, 2022 at 02:10:32AM +0800, Dong, Eddie wrote:

-----Original Message-----
From: acrn-dev@... <acrn-dev@...> On
Behalf Of chenli.wei
Sent: Sunday, May 15, 2022 11:07 PM
To: Mao, Junjie <junjie.mao@...>; Li, Fei1 <fei1.li@...>; acrn-
dev@...
Cc: Wei, Chenli <chenli.wei@...>
Subject: [acrn-dev] [PATCH v2 2/2] hv: move the define of MAX_IR_ENTRIES
to offline tool

There is an issue of calculate 2^n roundup of
CONFIG_MAX_PT_IRQ_ENTRIES, and the code style is very ugly when we use
macro to fix it.

So this patch move MAX_IR_ENTRIES to offline tool which could do align
check and calculate it automatically.

v1-->v2:
1. remove ROUND2/4/8 MACROs of powerof2_roundup

Signed-off-by: Chenli Wei <chenli.wei@...>
---
hypervisor/include/arch/x86/asm/vtd.h | 7 -------
hypervisor/include/lib/util.h | 21 ---------------------
2 files changed, 28 deletions(-)

diff --git a/hypervisor/include/arch/x86/asm/vtd.h
b/hypervisor/include/arch/x86/asm/vtd.h
index 4d3518286..284fc62db 100644
--- a/hypervisor/include/arch/x86/asm/vtd.h
+++ b/hypervisor/include/arch/x86/asm/vtd.h
@@ -42,13 +42,6 @@
#define DMAR_ICS_REG 0x9cU /* Invalidation complete status register
*/
#define DMAR_IRTA_REG 0xb8U /* Interrupt remapping table addr
register */

-/* Make sure all PT IRQs work w/ interrupt remapping or post interrupt */ -
#if (CONFIG_MAX_PT_IRQ_ENTRIES <= 256) -#define MAX_IR_ENTRIES 256 -
#else -#define MAX_IR_ENTRIES
powerof2_roundup(CONFIG_MAX_PT_IRQ_ENTRIES)
-#endif
-

Can you add following comments at the 1st place (vtd.c) where the MACRO is used?
MAX_IR_ENTRIES is roundup (to power of 2) of CONFIG_MAX_PT_IRQ_ENTRIES.

OK

Also, I want you to add a check somewhere:

ASSERT( ~(MAX_IR_ENTRIES-1UL) & CONFIG_MAX_PT_IRQ_ENTRIES == 0);

Now we did this check
#if ((MAX_IR_ENTRIES < 256U) || (MAX_IR_ENTRIES > 0x10000U) || (MAX_IR_ENTRIES & (MAX_IR_ENTRIES -1)) != 0U)
#error "MAX_IR_ENTRIES must in the region of [256,0x10000] and be 2^n"
#endif
in misc/hv_prebuild/static_checks.c already.

We also check the CONFIG_MAX_PT_IRQ_ENTRIES by config tool before we align it.

You can PR with above changes.

/* Values for entry_type in ACPI_DMAR_DEVICE_SCOPE - device types */
enum acpi_dmar_scope_type {
ACPI_DMAR_SCOPE_TYPE_NOT_USED = 0,
diff --git a/hypervisor/include/lib/util.h b/hypervisor/include/lib/util.h index
ca3296556..d702ae253 100644
--- a/hypervisor/include/lib/util.h
+++ b/hypervisor/include/lib/util.h
@@ -25,27 +25,6 @@
/** Replaces 'x' by the string "x". */
#define STRINGIFY(x) #x

-/*
- * This algorithm get the round up 2^n
- *
- * n = input - 1; 0x1002 ----> 0x1001
- * n |= n >> 1; 0x1001 | 0x800
- * n |= n >> 2; 0x1801 | 0x600
- * n |= n >> 4; 0x1e01 | 0x1e0
- * n |= n >> 8; 0x1fe1 | 0x1f
- * n |= n >> 16; 0x1fff
- * n |= n >> 32; 0x1fff
- * n += 1; 0x2000
- */
-#define ROUND0(x) ((x)-1)
-#define ROUND1(x) (ROUND0(x) |(ROUND0(x)>>1)) -#define ROUND2(x)
(ROUND1(x) |(ROUND1(x)>>2)) -#define ROUND4(x) (ROUND2(x)
|(ROUND2(x)>>4)) -#define ROUND8(x) (ROUND4(x) |(ROUND4(x)>>8)) -
#define ROUND16(x) (ROUND8(x) |(ROUND8(x)>>16)) -#define ROUND32(x)
(ROUND16(x) |(ROUND16(x)>>32)) -#define powerof2_roundup(x)
((ROUND32(x) == (~0UL)) ? ~0UL : (ROUND32(x) +1))
-
/* Macro used to check if a value is aligned to the required boundary.
* Returns TRUE if aligned; FALSE if not aligned
* NOTE: The required alignment must be a power of 2 (2, 4, 8, 16, 32, etc)
--
2.17.1