Re: [Xen-devel] [PATCH 2/4] AMD/IOMMU: Delete iommu_{get, set, clear}_bit() helpers

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.02.2020 15:43, Andrew Cooper wrote:
> @@ -85,16 +85,14 @@ static void flush_command_buffer(struct amd_iommu *iommu)
>      loop_count = 1000;
>      do {
>          status = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
> -        comp_wait = get_field_from_reg_u32(status,
> -                                           IOMMU_STATUS_COMP_WAIT_INT_MASK,
> -                                           IOMMU_STATUS_COMP_WAIT_INT_SHIFT);
> +        comp_wait = status & IOMMU_STATUS_COMP_WAIT_INT;

Unless you also change comp_wait to bool, this just happens to
be correct this way because of the bit checked being at a low
enough position.

> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -351,13 +351,12 @@ static void iommu_reset_log(struct amd_iommu *iommu,
>      BUG_ON(!iommu || ((log != &iommu->event_log) && (log != 
> &iommu->ppr_log)));
>  
>      run_bit = ( log == &iommu->event_log ) ?
> -        IOMMU_STATUS_EVENT_LOG_RUN_SHIFT :
> -        IOMMU_STATUS_PPR_LOG_RUN_SHIFT;
> +        IOMMU_STATUS_EVENT_LOG_RUN : IOMMU_STATUS_PPR_LOG_RUN;
>  
>      /* wait until EventLogRun bit = 0 */
>      do {
>          entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
> -        log_run = iommu_get_bit(entry, run_bit);
> +        log_run = entry & run_bit;

Same here for log_run then. I also think run_bit would better
become unsigned int then.

With these taken care of
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel