hw/riscv/riscv-iommu.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-)
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
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;
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
>
>
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
>>
>>
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.
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
© 2016 - 2024 Red Hat, Inc.