With the wider testing, some more violations have been spotted. This
addresses violations of Rule 7.2 (suffixes required) and Rule 7.3 (L must be
uppercase).
For ELF64_R_TYPE(), cast to uint32_t matching the surrounding examples.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: consulting@bugseng.com <consulting@bugseng.com>
CC: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
xen/arch/x86/mm/shadow/common.c | 4 ++--
xen/arch/x86/pv/descriptor-tables.c | 2 +-
xen/drivers/passthrough/vtd/iommu.h | 2 +-
xen/include/xen/elfstructs.h | 2 +-
xen/include/xen/sizes.h | 2 +-
5 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index 0176e33bc9c7..423764a32653 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -1961,7 +1961,7 @@ int sh_remove_write_access(struct domain *d, mfn_t gmfn,
/* FreeBSD 64bit: linear map 0xffff800000000000 */
switch ( level )
{
- case 1: GUESS(0xffff800000000000
+ case 1: GUESS(0xffff800000000000UL
+ ((fault_addr & VADDR_MASK) >> 9), 6); break;
case 2: GUESS(0xffff804000000000UL
+ ((fault_addr & VADDR_MASK) >> 18), 6); break;
@@ -1969,7 +1969,7 @@ int sh_remove_write_access(struct domain *d, mfn_t gmfn,
+ ((fault_addr & VADDR_MASK) >> 27), 6); break;
}
/* FreeBSD 64bit: direct map at 0xffffff0000000000 */
- GUESS(0xffffff0000000000 + gfn_to_gaddr(gfn), 6);
+ GUESS(0xffffff0000000000UL + gfn_to_gaddr(gfn), 6);
}
#undef GUESS
diff --git a/xen/arch/x86/pv/descriptor-tables.c b/xen/arch/x86/pv/descriptor-tables.c
index 02647a2c5047..26f7d18b11b5 100644
--- a/xen/arch/x86/pv/descriptor-tables.c
+++ b/xen/arch/x86/pv/descriptor-tables.c
@@ -216,7 +216,7 @@ static bool check_descriptor(const struct domain *dom, seg_desc_t *d)
* 0xf6800000. Extend these to allow access to the larger read-only
* M2P table available in 32on64 mode.
*/
- base = (b & 0xff000000) | ((b & 0xff) << 16) | (a >> 16);
+ base = (b & 0xff000000U) | ((b & 0xff) << 16) | (a >> 16);
limit = (b & 0xf0000) | (a & 0xffff);
limit++; /* We add one because limit is inclusive. */
diff --git a/xen/drivers/passthrough/vtd/iommu.h b/xen/drivers/passthrough/vtd/iommu.h
index 29d350b23db6..4f41360c53c0 100644
--- a/xen/drivers/passthrough/vtd/iommu.h
+++ b/xen/drivers/passthrough/vtd/iommu.h
@@ -266,7 +266,7 @@ struct dma_pte {
#define DMA_PTE_PROT (DMA_PTE_READ | DMA_PTE_WRITE)
#define DMA_PTE_SP (1 << 7)
#define DMA_PTE_SNP (1 << 11)
-#define DMA_PTE_CONTIG_MASK (0xfull << PADDR_BITS)
+#define DMA_PTE_CONTIG_MASK (0xfULL << PADDR_BITS)
#define dma_clear_pte(p) do {(p).val = 0;} while(0)
#define dma_set_pte_readable(p) do {(p).val |= DMA_PTE_READ;} while(0)
#define dma_set_pte_writable(p) do {(p).val |= DMA_PTE_WRITE;} while(0)
diff --git a/xen/include/xen/elfstructs.h b/xen/include/xen/elfstructs.h
index eb6b87a823a8..8770e7454672 100644
--- a/xen/include/xen/elfstructs.h
+++ b/xen/include/xen/elfstructs.h
@@ -360,7 +360,7 @@ typedef struct {
} Elf64_Rela;
#define ELF64_R_SYM(info) ((info) >> 32)
-#define ELF64_R_TYPE(info) ((info) & 0xFFFFFFFF)
+#define ELF64_R_TYPE(info) ((uint32_t)(info))
#define ELF64_R_INFO(s,t) (((s) << 32) + (uint32_t)(t))
/*
diff --git a/xen/include/xen/sizes.h b/xen/include/xen/sizes.h
index f7b728ddab06..d309ebf04406 100644
--- a/xen/include/xen/sizes.h
+++ b/xen/include/xen/sizes.h
@@ -43,6 +43,6 @@
#define SZ_512M 0x20000000
#define SZ_1G 0x40000000
-#define SZ_2G 0x80000000
+#define SZ_2G 0x80000000U
#endif /* __XEN_SIZES_H__ */
--
2.39.5
On 2025-12-10 19:30, Andrew Cooper wrote:
> With the wider testing, some more violations have been spotted. This
> addresses violations of Rule 7.2 (suffixes required) and Rule 7.3 (L
> must be
> uppercase).
>
> For ELF64_R_TYPE(), cast to uint32_t matching the surrounding examples.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: consulting@bugseng.com <consulting@bugseng.com>
> CC: Nicola Vetrini <nicola.vetrini@bugseng.com>
> ---
> xen/arch/x86/mm/shadow/common.c | 4 ++--
> xen/arch/x86/pv/descriptor-tables.c | 2 +-
> xen/drivers/passthrough/vtd/iommu.h | 2 +-
> xen/include/xen/elfstructs.h | 2 +-
> xen/include/xen/sizes.h | 2 +-
> 5 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/xen/arch/x86/mm/shadow/common.c
> b/xen/arch/x86/mm/shadow/common.c
> index 0176e33bc9c7..423764a32653 100644
> --- a/xen/arch/x86/mm/shadow/common.c
> +++ b/xen/arch/x86/mm/shadow/common.c
> @@ -1961,7 +1961,7 @@ int sh_remove_write_access(struct domain *d,
> mfn_t gmfn,
> /* FreeBSD 64bit: linear map 0xffff800000000000 */
> switch ( level )
> {
> - case 1: GUESS(0xffff800000000000
> + case 1: GUESS(0xffff800000000000UL
> + ((fault_addr & VADDR_MASK) >> 9), 6);
> break;
> case 2: GUESS(0xffff804000000000UL
> + ((fault_addr & VADDR_MASK) >> 18), 6);
> break;
> @@ -1969,7 +1969,7 @@ int sh_remove_write_access(struct domain *d,
> mfn_t gmfn,
> + ((fault_addr & VADDR_MASK) >> 27), 6);
> break;
> }
> /* FreeBSD 64bit: direct map at 0xffffff0000000000 */
> - GUESS(0xffffff0000000000 + gfn_to_gaddr(gfn), 6);
> + GUESS(0xffffff0000000000UL + gfn_to_gaddr(gfn), 6);
> }
>
> #undef GUESS
> diff --git a/xen/arch/x86/pv/descriptor-tables.c
> b/xen/arch/x86/pv/descriptor-tables.c
> index 02647a2c5047..26f7d18b11b5 100644
> --- a/xen/arch/x86/pv/descriptor-tables.c
> +++ b/xen/arch/x86/pv/descriptor-tables.c
> @@ -216,7 +216,7 @@ static bool check_descriptor(const struct domain
> *dom, seg_desc_t *d)
> * 0xf6800000. Extend these to allow access to the larger
> read-only
> * M2P table available in 32on64 mode.
> */
> - base = (b & 0xff000000) | ((b & 0xff) << 16) | (a >> 16);
> + base = (b & 0xff000000U) | ((b & 0xff) << 16) | (a >> 16);
>
> limit = (b & 0xf0000) | (a & 0xffff);
> limit++; /* We add one because limit is inclusive. */
> diff --git a/xen/drivers/passthrough/vtd/iommu.h
> b/xen/drivers/passthrough/vtd/iommu.h
> index 29d350b23db6..4f41360c53c0 100644
> --- a/xen/drivers/passthrough/vtd/iommu.h
> +++ b/xen/drivers/passthrough/vtd/iommu.h
> @@ -266,7 +266,7 @@ struct dma_pte {
> #define DMA_PTE_PROT (DMA_PTE_READ | DMA_PTE_WRITE)
> #define DMA_PTE_SP (1 << 7)
> #define DMA_PTE_SNP (1 << 11)
> -#define DMA_PTE_CONTIG_MASK (0xfull << PADDR_BITS)
> +#define DMA_PTE_CONTIG_MASK (0xfULL << PADDR_BITS)
> #define dma_clear_pte(p) do {(p).val = 0;} while(0)
> #define dma_set_pte_readable(p) do {(p).val |= DMA_PTE_READ;} while(0)
> #define dma_set_pte_writable(p) do {(p).val |= DMA_PTE_WRITE;}
> while(0)
> diff --git a/xen/include/xen/elfstructs.h
> b/xen/include/xen/elfstructs.h
> index eb6b87a823a8..8770e7454672 100644
> --- a/xen/include/xen/elfstructs.h
> +++ b/xen/include/xen/elfstructs.h
> @@ -360,7 +360,7 @@ typedef struct {
> } Elf64_Rela;
>
> #define ELF64_R_SYM(info) ((info) >> 32)
> -#define ELF64_R_TYPE(info) ((info) & 0xFFFFFFFF)
> +#define ELF64_R_TYPE(info) ((uint32_t)(info))
> #define ELF64_R_INFO(s,t) (((s) << 32) + (uint32_t)(t))
>
> /*
> diff --git a/xen/include/xen/sizes.h b/xen/include/xen/sizes.h
> index f7b728ddab06..d309ebf04406 100644
> --- a/xen/include/xen/sizes.h
> +++ b/xen/include/xen/sizes.h
> @@ -43,6 +43,6 @@
> #define SZ_512M 0x20000000
>
> #define SZ_1G 0x40000000
> -#define SZ_2G 0x80000000
> +#define SZ_2G 0x80000000U
>
> #endif /* __XEN_SIZES_H__ */
--
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253
On 2025-12-10 21:09, Nicola Vetrini wrote:
> On 2025-12-10 19:30, Andrew Cooper wrote:
>> With the wider testing, some more violations have been spotted. This
>> addresses violations of Rule 7.2 (suffixes required) and Rule 7.3 (L
>> must be
>> uppercase).
>>
>> For ELF64_R_TYPE(), cast to uint32_t matching the surrounding
>> examples.
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Stefano Stabellini <sstabellini@kernel.org>
>> CC: consulting@bugseng.com <consulting@bugseng.com>
>> CC: Nicola Vetrini <nicola.vetrini@bugseng.com>
>> ---
>> xen/arch/x86/mm/shadow/common.c | 4 ++--
>> xen/arch/x86/pv/descriptor-tables.c | 2 +-
>> xen/drivers/passthrough/vtd/iommu.h | 2 +-
>> xen/include/xen/elfstructs.h | 2 +-
>> xen/include/xen/sizes.h | 2 +-
>> 5 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/xen/arch/x86/mm/shadow/common.c
>> b/xen/arch/x86/mm/shadow/common.c
>> index 0176e33bc9c7..423764a32653 100644
>> --- a/xen/arch/x86/mm/shadow/common.c
>> +++ b/xen/arch/x86/mm/shadow/common.c
>> @@ -1961,7 +1961,7 @@ int sh_remove_write_access(struct domain *d,
>> mfn_t gmfn,
>> /* FreeBSD 64bit: linear map 0xffff800000000000 */
>> switch ( level )
>> {
>> - case 1: GUESS(0xffff800000000000
>> + case 1: GUESS(0xffff800000000000UL
>> + ((fault_addr & VADDR_MASK) >> 9), 6);
>> break;
>> case 2: GUESS(0xffff804000000000UL
>> + ((fault_addr & VADDR_MASK) >> 18), 6);
>> break;
>> @@ -1969,7 +1969,7 @@ int sh_remove_write_access(struct domain *d,
>> mfn_t gmfn,
>> + ((fault_addr & VADDR_MASK) >> 27), 6);
>> break;
>> }
>> /* FreeBSD 64bit: direct map at 0xffffff0000000000 */
>> - GUESS(0xffffff0000000000 + gfn_to_gaddr(gfn), 6);
>> + GUESS(0xffffff0000000000UL + gfn_to_gaddr(gfn), 6);
>> }
>>
>> #undef GUESS
>> diff --git a/xen/arch/x86/pv/descriptor-tables.c
>> b/xen/arch/x86/pv/descriptor-tables.c
>> index 02647a2c5047..26f7d18b11b5 100644
>> --- a/xen/arch/x86/pv/descriptor-tables.c
>> +++ b/xen/arch/x86/pv/descriptor-tables.c
>> @@ -216,7 +216,7 @@ static bool check_descriptor(const struct domain
>> *dom, seg_desc_t *d)
>> * 0xf6800000. Extend these to allow access to the larger
>> read-only
>> * M2P table available in 32on64 mode.
>> */
>> - base = (b & 0xff000000) | ((b & 0xff) << 16) | (a >> 16);
>> + base = (b & 0xff000000U) | ((b & 0xff) << 16) | (a >>
>> 16);
>>
>> limit = (b & 0xf0000) | (a & 0xffff);
>> limit++; /* We add one because limit is inclusive. */
>> diff --git a/xen/drivers/passthrough/vtd/iommu.h
>> b/xen/drivers/passthrough/vtd/iommu.h
>> index 29d350b23db6..4f41360c53c0 100644
>> --- a/xen/drivers/passthrough/vtd/iommu.h
>> +++ b/xen/drivers/passthrough/vtd/iommu.h
>> @@ -266,7 +266,7 @@ struct dma_pte {
>> #define DMA_PTE_PROT (DMA_PTE_READ | DMA_PTE_WRITE)
>> #define DMA_PTE_SP (1 << 7)
>> #define DMA_PTE_SNP (1 << 11)
>> -#define DMA_PTE_CONTIG_MASK (0xfull << PADDR_BITS)
>> +#define DMA_PTE_CONTIG_MASK (0xfULL << PADDR_BITS)
>> #define dma_clear_pte(p) do {(p).val = 0;} while(0)
>> #define dma_set_pte_readable(p) do {(p).val |= DMA_PTE_READ;}
>> while(0)
>> #define dma_set_pte_writable(p) do {(p).val |= DMA_PTE_WRITE;}
>> while(0)
>> diff --git a/xen/include/xen/elfstructs.h
>> b/xen/include/xen/elfstructs.h
>> index eb6b87a823a8..8770e7454672 100644
>> --- a/xen/include/xen/elfstructs.h
>> +++ b/xen/include/xen/elfstructs.h
>> @@ -360,7 +360,7 @@ typedef struct {
>> } Elf64_Rela;
>>
>> #define ELF64_R_SYM(info) ((info) >> 32)
>> -#define ELF64_R_TYPE(info) ((info) & 0xFFFFFFFF)
>> +#define ELF64_R_TYPE(info) ((uint32_t)(info))
Actually I think this doesn't build:
arch/x86/livepatch.c: In function ‘arch_livepatch_perform_rela’:
././include/xen/config.h:55:24: error: format ‘%lu’ expects argument of
type ‘long unsigned int’, but argument 3 has type ‘unsigned int’
[-Werror=format=]
55 | #define XENLOG_ERR "<0>"
| ^~~~~
arch/x86/livepatch.c:332:20: note: in expansion of macro ‘XENLOG_ERR’
332 | printk(XENLOG_ERR LIVEPATCH "%s: Unhandled
relocation %lu\n",
| ^~~~~~~~~~
arch/x86/livepatch.c:332:69: note: format string is defined here
332 | printk(XENLOG_ERR LIVEPATCH "%s: Unhandled
relocation %lu\n",
|
~~^
|
|
|
long unsigned int
|
%u
the error location is a bit unclear, but the cast is the culprit
>> #define ELF64_R_INFO(s,t) (((s) << 32) + (uint32_t)(t))
>>
>> /*
>> diff --git a/xen/include/xen/sizes.h b/xen/include/xen/sizes.h
>> index f7b728ddab06..d309ebf04406 100644
>> --- a/xen/include/xen/sizes.h
>> +++ b/xen/include/xen/sizes.h
>> @@ -43,6 +43,6 @@
>> #define SZ_512M 0x20000000
>>
>> #define SZ_1G 0x40000000
>> -#define SZ_2G 0x80000000
>> +#define SZ_2G 0x80000000U
>>
>> #endif /* __XEN_SIZES_H__ */
--
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253
On 10/12/2025 8:31 pm, Nicola Vetrini wrote:
> On 2025-12-10 21:09, Nicola Vetrini wrote:
>> On 2025-12-10 19:30, Andrew Cooper wrote:
>>> diff --git a/xen/include/xen/elfstructs.h
>>> b/xen/include/xen/elfstructs.h
>>> index eb6b87a823a8..8770e7454672 100644
>>> --- a/xen/include/xen/elfstructs.h
>>> +++ b/xen/include/xen/elfstructs.h
>>> @@ -360,7 +360,7 @@ typedef struct {
>>> } Elf64_Rela;
>>>
>>> #define ELF64_R_SYM(info) ((info) >> 32)
>>> -#define ELF64_R_TYPE(info) ((info) & 0xFFFFFFFF)
>>> +#define ELF64_R_TYPE(info) ((uint32_t)(info))
>
> Actually I think this doesn't build:
>
> arch/x86/livepatch.c: In function ‘arch_livepatch_perform_rela’:
> ././include/xen/config.h:55:24: error: format ‘%lu’ expects argument
> of type ‘long unsigned int’, but argument 3 has type ‘unsigned int’
> [-Werror=format=]
> 55 | #define XENLOG_ERR "<0>"
> | ^~~~~
> arch/x86/livepatch.c:332:20: note: in expansion of macro ‘XENLOG_ERR’
> 332 | printk(XENLOG_ERR LIVEPATCH "%s: Unhandled
> relocation %lu\n",
> | ^~~~~~~~~~
> arch/x86/livepatch.c:332:69: note: format string is defined here
> 332 | printk(XENLOG_ERR LIVEPATCH "%s: Unhandled
> relocation %lu\n",
>
> | ~~^
>
> | |
>
> |
> long unsigned int
>
> | %u
>
> the error location is a bit unclear, but the cast is the culprit
Yeah, I spotted that just as I heading out, and ran
https://gitlab.com/xen-project/hardware/xen-staging/-/pipelines/2207521982
instead.
I've swapped back to using 0xFFFFFFFFU. info ends up being long, and
the result of the expression needs to stay that way.
However, looking at the report for that, I still missed one. I've
folded in this hunk too.
diff --git a/xen/arch/x86/pv/emulate.c b/xen/arch/x86/pv/emulate.c
index 8c44dea12330..e741e686c1af 100644
--- a/xen/arch/x86/pv/emulate.c
+++ b/xen/arch/x86/pv/emulate.c
@@ -37,7 +37,7 @@ int pv_emul_read_descriptor(unsigned int sel, const struct vcpu *v,
if ( !(desc.b & _SEGMENT_L) )
{
*base = ((desc.a >> 16) + ((desc.b & 0xff) << 16) +
- (desc.b & 0xff000000));
+ (desc.b & 0xff000000U));
*limit = (desc.a & 0xffff) | (desc.b & 0x000f0000);
if ( desc.b & _SEGMENT_G )
*limit = ((*limit + 1) << 12) - 1;
~Andrew
On 2025-12-11 00:48, Andrew Cooper wrote:
> On 10/12/2025 8:31 pm, Nicola Vetrini wrote:
>> On 2025-12-10 21:09, Nicola Vetrini wrote:
>>> On 2025-12-10 19:30, Andrew Cooper wrote:
>>>> diff --git a/xen/include/xen/elfstructs.h
>>>> b/xen/include/xen/elfstructs.h
>>>> index eb6b87a823a8..8770e7454672 100644
>>>> --- a/xen/include/xen/elfstructs.h
>>>> +++ b/xen/include/xen/elfstructs.h
>>>> @@ -360,7 +360,7 @@ typedef struct {
>>>> } Elf64_Rela;
>>>>
>>>> #define ELF64_R_SYM(info) ((info) >> 32)
>>>> -#define ELF64_R_TYPE(info) ((info) & 0xFFFFFFFF)
>>>> +#define ELF64_R_TYPE(info) ((uint32_t)(info))
>>
>> Actually I think this doesn't build:
>>
>> arch/x86/livepatch.c: In function ‘arch_livepatch_perform_rela’:
>> ././include/xen/config.h:55:24: error: format ‘%lu’ expects argument
>> of type ‘long unsigned int’, but argument 3 has type ‘unsigned int’
>> [-Werror=format=]
>> 55 | #define XENLOG_ERR "<0>"
>> | ^~~~~
>> arch/x86/livepatch.c:332:20: note: in expansion of macro ‘XENLOG_ERR’
>> 332 | printk(XENLOG_ERR LIVEPATCH "%s: Unhandled
>> relocation %lu\n",
>> | ^~~~~~~~~~
>> arch/x86/livepatch.c:332:69: note: format string is defined here
>> 332 | printk(XENLOG_ERR LIVEPATCH "%s: Unhandled
>> relocation %lu\n",
>>
>> |
>> ~~^
>>
>> |
>> |
>>
>> |
>> long unsigned int
>>
>> | %u
>>
>> the error location is a bit unclear, but the cast is the culprit
>
> Yeah, I spotted that just as I heading out, and ran
> https://gitlab.com/xen-project/hardware/xen-staging/-/pipelines/2207521982
> instead.
>
> I've swapped back to using 0xFFFFFFFFU. info ends up being long, and
> the result of the expression needs to stay that way.
>
> However, looking at the report for that, I still missed one. I've
> folded in this hunk too.
>
> diff --git a/xen/arch/x86/pv/emulate.c b/xen/arch/x86/pv/emulate.c
> index 8c44dea12330..e741e686c1af 100644
> --- a/xen/arch/x86/pv/emulate.c
> +++ b/xen/arch/x86/pv/emulate.c
> @@ -37,7 +37,7 @@ int pv_emul_read_descriptor(unsigned int sel, const
> struct vcpu *v,
> if ( !(desc.b & _SEGMENT_L) )
> {
> *base = ((desc.a >> 16) + ((desc.b & 0xff) << 16) +
> - (desc.b & 0xff000000));
> + (desc.b & 0xff000000U));
> *limit = (desc.a & 0xffff) | (desc.b & 0x000f0000);
> if ( desc.b & _SEGMENT_G )
> *limit = ((*limit + 1) << 12) - 1;
>
Makes sense, feel free to retain my R-by with these two changes.
>
> ~Andrew
--
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253
© 2016 - 2025 Red Hat, Inc.