Just curious, why would |= operation cause problem? :)

On Fri, Dec 20, 2013 at 4:45 PM, Li, Zhen-Hua <[email protected]> wrote:

> There is a structure named context_entry used by intel iommu, and there
> are some bit operations on it. Use bit structure may make these operations
> easy.
> Also the function context_set_address_root may cause problem because it
> uses
>  |= operation, not set the new value. And this patch can fix it.
>
> Signed-off-by: Li, Zhen-Hua <[email protected]>
> ---
>  drivers/iommu/intel-iommu.c | 37 +++++++++++++++++++++++++++----------
>  1 file changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 43b9bfe..65cd480 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -221,47 +221,64 @@ get_context_addr_from_root(struct root_entry *root)
>   * 8-23: domain id
>   */
>  struct context_entry {
> -       u64 lo;
> -       u64 hi;
> +       union {
> +               struct {
> +                       u64     present:1,
> +                               fpd:1,
> +                               translation_type:2,
> +                               reserved_l:8,
> +                               asr:52;
> +               };
> +               u64 lo;
> +       };
> +       union {
> +               struct {
> +                       u64     aw:3,
> +                               avail:4,
> +                               reserved_h1:1,
> +                               did:16,
> +                               reserved_h2:40;
> +               };
> +               u64 hi;
> +       };
>  };
>
>  static inline bool context_present(struct context_entry *context)
>  {
> -       return (context->lo & 1);
> +       return context->present;
>  }
>  static inline void context_set_present(struct context_entry *context)
>  {
> -       context->lo |= 1;
> +       context->present = 1;
>  }
>
>  static inline void context_set_fault_enable(struct context_entry *context)
>  {
> -       context->lo &= (((u64)-1) << 2) | 1;
> +       context->fpd = 1;
>  }
>
>  static inline void context_set_translation_type(struct context_entry
> *context,
>                                                 unsigned long value)
>  {
> -       context->lo &= (((u64)-1) << 4) | 3;
> -       context->lo |= (value & 3) << 2;
> +       context->translation_type = value;
>  }
>
>  static inline void context_set_address_root(struct context_entry *context,
>                                             unsigned long value)
>  {
> -       context->lo |= value & VTD_PAGE_MASK;
> +       context->asr = value >> VTD_PAGE_SHIFT
>  }
>
>  static inline void context_set_address_width(struct context_entry
> *context,
>                                              unsigned long value)
>  {
> -       context->hi |= value & 7;
> +       context->aw  = value;
>  }
>
>  static inline void context_set_domain_id(struct context_entry *context,
>                                          unsigned long value)
>  {
> -       context->hi |= (value & ((1 << 16) - 1)) << 8;
> +       context->did = value;
>  }
>
>  static inline void context_clear_entry(struct context_entry *context)
> --
> 1.8.4.3
>
> _______________________________________________
> iommu mailing list
> [email protected]
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to