1. Patchew
  2. QEMU
  3. View series

[PATCH v2] hw/riscv: fix build error with clang

Pierrick Bouvier posted 1 patch 1 year, 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20241104222225.1523751-1-pierrick.bouvier@linaro.org
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
hw/riscv/riscv-iommu.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
[PATCH v2] hw/riscv: fix build error with clang
Posted by Pierrick Bouvier 1 year, 3 months ago 
Introduced in 0c54ac, "hw/riscv: add RISC-V IOMMU base emulation"

../hw/riscv/riscv-iommu.c:187:17: error: redefinition of '_pext_u64'

  187 | static uint64_t _pext_u64(uint64_t val, uint64_t ext)

      |                 ^

D:/a/_temp/msys64/clang64/lib/clang/18/include/bmi2intrin.h:217:1: note: previous definition is here

  217 | _pext_u64(unsigned long long __X, unsigned long long __Y)

      | ^

After a conversation on the mailing list, it was decided to rename and
add a comment for this function.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 hw/riscv/riscv-iommu.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
index feb650549ac..12f01a75f5d 100644
--- a/hw/riscv/riscv-iommu.c
+++ b/hw/riscv/riscv-iommu.c
@@ -183,8 +183,25 @@ static void riscv_iommu_pri(RISCVIOMMUState *s,
     }
 }
 
-/* Portable implementation of pext_u64, bit-mask extraction. */
-static uint64_t _pext_u64(uint64_t val, uint64_t ext)
+/*
+ * Discards all bits from 'val' whose matching bits in the same
+ * positions in the mask 'ext' are zeros, and packs the remaining
+ * bits from 'val' contiguously at the least-significant end of the
+ * result, keeping the same bit order as 'val' and filling any
+ * other bits at the most-significant end of the result with zeros.
+ *
+ * For example, for the following 'val' and 'ext', the return 'ret'
+ * will be:
+ *
+ * val = a b c d e f g h
+ * ext = 1 0 1 0 0 1 1 0
+ * ret = 0 0 0 0 a c f g
+ *
+ * This function, taken from the riscv-iommu 1.0 spec, section 2.3.3
+ * "Process to translate addresses of MSIs", is similar to bit manip
+ * function PEXT (Parallel bits extract) from x86.
+ */
+static uint64_t riscv_iommu_pext_u64(uint64_t val, uint64_t ext)
 {
     uint64_t ret = 0;
     uint64_t rot = 1;
@@ -528,7 +545,7 @@ static MemTxResult riscv_iommu_msi_write(RISCVIOMMUState *s,
     int cause;
 
     /* Interrupt File Number */
-    intn = _pext_u64(PPN_DOWN(gpa), ctx->msi_addr_mask);
+    intn = riscv_iommu_pext_u64(PPN_DOWN(gpa), ctx->msi_addr_mask);
     if (intn >= 256) {
         /* Interrupt file number out of range */
         res = MEMTX_ACCESS_ERROR;
-- 
2.39.5
Re: [PATCH v2] hw/riscv: fix build error with clang
Posted by LIU Zhiwei 1 year, 3 months ago 
On 2024/11/5 06:22, Pierrick Bouvier wrote:
> Introduced in 0c54ac, "hw/riscv: add RISC-V IOMMU base emulation"
>
> ../hw/riscv/riscv-iommu.c:187:17: error: redefinition of '_pext_u64'
>
>    187 | static uint64_t _pext_u64(uint64_t val, uint64_t ext)
>
>        |                 ^
>
> D:/a/_temp/msys64/clang64/lib/clang/18/include/bmi2intrin.h:217:1: note: previous definition is here
>
>    217 | _pext_u64(unsigned long long __X, unsigned long long __Y)
>
>        | ^
>
> After a conversation on the mailing list, it was decided to rename and
> add a comment for this function.
>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>

Zhiwei

> ---
>   hw/riscv/riscv-iommu.c | 23 ++++++++++++++++++++---
>   1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> index feb650549ac..12f01a75f5d 100644
> --- a/hw/riscv/riscv-iommu.c
> +++ b/hw/riscv/riscv-iommu.c
> @@ -183,8 +183,25 @@ static void riscv_iommu_pri(RISCVIOMMUState *s,
>       }
>   }
>   
> -/* Portable implementation of pext_u64, bit-mask extraction. */
> -static uint64_t _pext_u64(uint64_t val, uint64_t ext)
> +/*
> + * Discards all bits from 'val' whose matching bits in the same
> + * positions in the mask 'ext' are zeros, and packs the remaining
> + * bits from 'val' contiguously at the least-significant end of the
> + * result, keeping the same bit order as 'val' and filling any
> + * other bits at the most-significant end of the result with zeros.
> + *
> + * For example, for the following 'val' and 'ext', the return 'ret'
> + * will be:
> + *
> + * val = a b c d e f g h
> + * ext = 1 0 1 0 0 1 1 0
> + * ret = 0 0 0 0 a c f g
> + *
> + * This function, taken from the riscv-iommu 1.0 spec, section 2.3.3
> + * "Process to translate addresses of MSIs", is similar to bit manip
> + * function PEXT (Parallel bits extract) from x86.
> + */
> +static uint64_t riscv_iommu_pext_u64(uint64_t val, uint64_t ext)
>   {
>       uint64_t ret = 0;
>       uint64_t rot = 1;
> @@ -528,7 +545,7 @@ static MemTxResult riscv_iommu_msi_write(RISCVIOMMUState *s,
>       int cause;
>   
>       /* Interrupt File Number */
> -    intn = _pext_u64(PPN_DOWN(gpa), ctx->msi_addr_mask);
> +    intn = riscv_iommu_pext_u64(PPN_DOWN(gpa), ctx->msi_addr_mask);
>       if (intn >= 256) {
>           /* Interrupt file number out of range */
>           res = MEMTX_ACCESS_ERROR;
Re: [PATCH v2] hw/riscv: fix build error with clang
Posted by Alistair Francis 1 year, 3 months ago 
On Tue, Nov 5, 2024 at 8:23 AM Pierrick Bouvier
<pierrick.bouvier@linaro.org> wrote:
>
> Introduced in 0c54ac, "hw/riscv: add RISC-V IOMMU base emulation"
>
> ../hw/riscv/riscv-iommu.c:187:17: error: redefinition of '_pext_u64'
>
>   187 | static uint64_t _pext_u64(uint64_t val, uint64_t ext)
>
>       |                 ^
>
> D:/a/_temp/msys64/clang64/lib/clang/18/include/bmi2intrin.h:217:1: note: previous definition is here
>
>   217 | _pext_u64(unsigned long long __X, unsigned long long __Y)
>
>       | ^
>
> After a conversation on the mailing list, it was decided to rename and
> add a comment for this function.
>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/riscv/riscv-iommu.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> index feb650549ac..12f01a75f5d 100644
> --- a/hw/riscv/riscv-iommu.c
> +++ b/hw/riscv/riscv-iommu.c
> @@ -183,8 +183,25 @@ static void riscv_iommu_pri(RISCVIOMMUState *s,
>      }
>  }
>
> -/* Portable implementation of pext_u64, bit-mask extraction. */
> -static uint64_t _pext_u64(uint64_t val, uint64_t ext)
> +/*
> + * Discards all bits from 'val' whose matching bits in the same
> + * positions in the mask 'ext' are zeros, and packs the remaining
> + * bits from 'val' contiguously at the least-significant end of the
> + * result, keeping the same bit order as 'val' and filling any
> + * other bits at the most-significant end of the result with zeros.
> + *
> + * For example, for the following 'val' and 'ext', the return 'ret'
> + * will be:
> + *
> + * val = a b c d e f g h
> + * ext = 1 0 1 0 0 1 1 0
> + * ret = 0 0 0 0 a c f g
> + *
> + * This function, taken from the riscv-iommu 1.0 spec, section 2.3.3
> + * "Process to translate addresses of MSIs", is similar to bit manip
> + * function PEXT (Parallel bits extract) from x86.
> + */
> +static uint64_t riscv_iommu_pext_u64(uint64_t val, uint64_t ext)
>  {
>      uint64_t ret = 0;
>      uint64_t rot = 1;
> @@ -528,7 +545,7 @@ static MemTxResult riscv_iommu_msi_write(RISCVIOMMUState *s,
>      int cause;
>
>      /* Interrupt File Number */
> -    intn = _pext_u64(PPN_DOWN(gpa), ctx->msi_addr_mask);
> +    intn = riscv_iommu_pext_u64(PPN_DOWN(gpa), ctx->msi_addr_mask);
>      if (intn >= 256) {
>          /* Interrupt file number out of range */
>          res = MEMTX_ACCESS_ERROR;
> --
> 2.39.5
>
>
Re: [PATCH v2] hw/riscv: fix build error with clang
Posted by Pierrick Bouvier 1 year, 3 months ago 
Thanks for the review.
Feel free to pull the patch in your next PR, so it can be available for 
release 9.2.

Regards,
Pierrick

On 11/4/24 18:37, Alistair Francis wrote:
> On Tue, Nov 5, 2024 at 8:23 AM Pierrick Bouvier
> <pierrick.bouvier@linaro.org> wrote:
>>
>> Introduced in 0c54ac, "hw/riscv: add RISC-V IOMMU base emulation"
>>
>> ../hw/riscv/riscv-iommu.c:187:17: error: redefinition of '_pext_u64'
>>
>>    187 | static uint64_t _pext_u64(uint64_t val, uint64_t ext)
>>
>>        |                 ^
>>
>> D:/a/_temp/msys64/clang64/lib/clang/18/include/bmi2intrin.h:217:1: note: previous definition is here
>>
>>    217 | _pext_u64(unsigned long long __X, unsigned long long __Y)
>>
>>        | ^
>>
>> After a conversation on the mailing list, it was decided to rename and
>> add a comment for this function.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> 
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> 
> Alistair
> 
>> ---
>>   hw/riscv/riscv-iommu.c | 23 ++++++++++++++++++++---
>>   1 file changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
>> index feb650549ac..12f01a75f5d 100644
>> --- a/hw/riscv/riscv-iommu.c
>> +++ b/hw/riscv/riscv-iommu.c
>> @@ -183,8 +183,25 @@ static void riscv_iommu_pri(RISCVIOMMUState *s,
>>       }
>>   }
>>
>> -/* Portable implementation of pext_u64, bit-mask extraction. */
>> -static uint64_t _pext_u64(uint64_t val, uint64_t ext)
>> +/*
>> + * Discards all bits from 'val' whose matching bits in the same
>> + * positions in the mask 'ext' are zeros, and packs the remaining
>> + * bits from 'val' contiguously at the least-significant end of the
>> + * result, keeping the same bit order as 'val' and filling any
>> + * other bits at the most-significant end of the result with zeros.
>> + *
>> + * For example, for the following 'val' and 'ext', the return 'ret'
>> + * will be:
>> + *
>> + * val = a b c d e f g h
>> + * ext = 1 0 1 0 0 1 1 0
>> + * ret = 0 0 0 0 a c f g
>> + *
>> + * This function, taken from the riscv-iommu 1.0 spec, section 2.3.3
>> + * "Process to translate addresses of MSIs", is similar to bit manip
>> + * function PEXT (Parallel bits extract) from x86.
>> + */
>> +static uint64_t riscv_iommu_pext_u64(uint64_t val, uint64_t ext)
>>   {
>>       uint64_t ret = 0;
>>       uint64_t rot = 1;
>> @@ -528,7 +545,7 @@ static MemTxResult riscv_iommu_msi_write(RISCVIOMMUState *s,
>>       int cause;
>>
>>       /* Interrupt File Number */
>> -    intn = _pext_u64(PPN_DOWN(gpa), ctx->msi_addr_mask);
>> +    intn = riscv_iommu_pext_u64(PPN_DOWN(gpa), ctx->msi_addr_mask);
>>       if (intn >= 256) {
>>           /* Interrupt file number out of range */
>>           res = MEMTX_ACCESS_ERROR;
>> --
>> 2.39.5
>>
>>
Re: [PATCH v2] hw/riscv: fix build error with clang
Posted by Philippe Mathieu-Daudé 1 year, 3 months ago 
On 5/11/24 05:29, Pierrick Bouvier wrote:
> Thanks for the review.
> Feel free to pull the patch in your next PR, so it can be available for 
> release 9.2.
> 
> Regards,
> Pierrick
> 
> On 11/4/24 18:37, Alistair Francis wrote:
>> On Tue, Nov 5, 2024 at 8:23 AM Pierrick Bouvier
>> <pierrick.bouvier@linaro.org> wrote:
>>>
>>> Introduced in 0c54ac, "hw/riscv: add RISC-V IOMMU base emulation"
>>>
>>> ../hw/riscv/riscv-iommu.c:187:17: error: redefinition of '_pext_u64'
>>>
>>>    187 | static uint64_t _pext_u64(uint64_t val, uint64_t ext)
>>>
>>>        |                 ^
>>>
>>> D:/a/_temp/msys64/clang64/lib/clang/18/include/bmi2intrin.h:217:1: 
>>> note: previous definition is here
>>>
>>>    217 | _pext_u64(unsigned long long __X, unsigned long long __Y)
>>>
>>>        | ^
>>>
>>> After a conversation on the mailing list, it was decided to rename and
>>> add a comment for this function.
>>>
>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>
>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Fix queued via hw-misc tree.
Re: [PATCH v2] hw/riscv: fix build error with clang
Posted by Daniel Henrique Barboza 1 year, 3 months ago 

On 11/5/24 7:55 PM, Philippe Mathieu-Daudé wrote:
> On 5/11/24 05:29, Pierrick Bouvier wrote:
>> Thanks for the review.
>> Feel free to pull the patch in your next PR, so it can be available for release 9.2.
>>
>> Regards,
>> Pierrick
>>
>> On 11/4/24 18:37, Alistair Francis wrote:
>>> On Tue, Nov 5, 2024 at 8:23 AM Pierrick Bouvier
>>> <pierrick.bouvier@linaro.org> wrote:
>>>>
>>>> Introduced in 0c54ac, "hw/riscv: add RISC-V IOMMU base emulation"
>>>>
>>>> ../hw/riscv/riscv-iommu.c:187:17: error: redefinition of '_pext_u64'
>>>>
>>>>    187 | static uint64_t _pext_u64(uint64_t val, uint64_t ext)
>>>>
>>>>        |                 ^
>>>>
>>>> D:/a/_temp/msys64/clang64/lib/clang/18/include/bmi2intrin.h:217:1: note: previous definition is here
>>>>
>>>>    217 | _pext_u64(unsigned long long __X, unsigned long long __Y)
>>>>
>>>>        | ^
>>>>
>>>> After a conversation on the mailing list, it was decided to rename and
>>>> add a comment for this function.
>>>>
>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>
>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> 
> Fix queued via hw-misc tree.

Do you fancy taking the riscv-iommu Coverity fixes as well? They're somewhat trivial and it'll
spare Alistair from making a PR with just a handful of patches.


Thanks,

Daniel

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.