* [PATCH 1/1] x86/iommu: use bit structures for context_entry
@ 2013-12-20 8:45 Li, Zhen-Hua
[not found] ` <1387529101-29769-1-git-send-email-zhen-hual-VXdhtT5mjnY@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Li, Zhen-Hua @ 2013-12-20 8:45 UTC (permalink / raw)
To: David Woodhouse, Joerg Roedel,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Li, Zhen-Hua
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 <zhen-hual-VXdhtT5mjnY@public.gmane.org>
---
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
^ permalink raw reply related [flat|nested] 12+ messages in thread[parent not found: <1387529101-29769-1-git-send-email-zhen-hual-VXdhtT5mjnY@public.gmane.org>]
* Re: [PATCH 1/1] x86/iommu: use bit structures for context_entry [not found] ` <1387529101-29769-1-git-send-email-zhen-hual-VXdhtT5mjnY@public.gmane.org> @ 2013-12-26 1:55 ` Kai Huang [not found] ` <CAOtp4KqUgSnwadP0T=aWYX7rRzDBn3qkuqMzceDBO0c_bhpFwQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2014-01-07 14:41 ` Joerg Roedel 1 sibling, 1 reply; 12+ messages in thread From: Kai Huang @ 2013-12-26 1:55 UTC (permalink / raw) To: Li, Zhen-Hua Cc: David Woodhouse, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-kernel-u79uwXL29TY76Z2rM5mHXA [-- Attachment #1.1: Type: text/plain, Size: 3459 bytes --] Just curious, why would |= operation cause problem? :) On Fri, Dec 20, 2013 at 4:45 PM, Li, Zhen-Hua <zhen-hual-VXdhtT5mjnY@public.gmane.org> 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 <zhen-hual-VXdhtT5mjnY@public.gmane.org> > --- > 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 > iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu > [-- Attachment #1.2: Type: text/html, Size: 4475 bytes --] [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <CAOtp4KqUgSnwadP0T=aWYX7rRzDBn3qkuqMzceDBO0c_bhpFwQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* RE: [PATCH 1/1] x86/iommu: use bit structures for context_entry [not found] ` <CAOtp4KqUgSnwadP0T=aWYX7rRzDBn3qkuqMzceDBO0c_bhpFwQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2013-12-26 5:12 ` Wu, Feng [not found] ` <E959C4978C3B6342920538CF579893F001D3C26D-0J0gbvR4kTg/UvCtAeCM4rfspsVTdybXVpNB7YpNyf8@public.gmane.org> 0 siblings, 1 reply; 12+ messages in thread From: Wu, Feng @ 2013-12-26 5:12 UTC (permalink / raw) To: Kai Huang, Li, Zhen-Hua Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, David Woodhouse, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [-- Attachment #1.1: Type: text/plain, Size: 4073 bytes --] I think if using |= operation, it should use &= operation to clear those bits first. Thanks, Feng From: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org [mailto:iommu-bounces@lists.linux-foundation.org] On Behalf Of Kai Huang Sent: Thursday, December 26, 2013 9:55 AM To: Li, Zhen-Hua Cc: David Woodhouse; iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; linux-kernel-fy+rA21nqHI@public.gmane.orgrnel.org Subject: Re: [PATCH 1/1] x86/iommu: use bit structures for context_entry Just curious, why would |= operation cause problem? :) On Fri, Dec 20, 2013 at 4:45 PM, Li, Zhen-Hua <zhen-hual-VXdhtT5mjnY@public.gmane.org<mailto:zhen-hual-VXdhtT5mjnY@public.gmane.org>> 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 <zhen-hual-VXdhtT5mjnY@public.gmane.org<mailto:zhen-hual-VXdhtT5mjnY@public.gmane.org>> --- 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 iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org<mailto:iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org> https://lists.linuxfoundation.org/mailman/listinfo/iommu [-- Attachment #1.2: Type: text/html, Size: 10678 bytes --] [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply related [flat|nested] 12+ messages in thread
[parent not found: <E959C4978C3B6342920538CF579893F001D3C26D-0J0gbvR4kTg/UvCtAeCM4rfspsVTdybXVpNB7YpNyf8@public.gmane.org>]
* Re: [PATCH 1/1] x86/iommu: use bit structures for context_entry [not found] ` <E959C4978C3B6342920538CF579893F001D3C26D-0J0gbvR4kTg/UvCtAeCM4rfspsVTdybXVpNB7YpNyf8@public.gmane.org> @ 2014-01-06 1:41 ` Li, ZhenHua 0 siblings, 0 replies; 12+ messages in thread From: Li, ZhenHua @ 2014-01-06 1:41 UTC (permalink / raw) To: Wu, Feng Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, David Woodhouse, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Yes, that's the problem. And some other structures like this, see "union irte". On 12/26/2013 01:12 PM, Wu, Feng wrote: > I think if using |= operation, it should use &= operation to clear those > bits first. > > Thanks, > > Feng > > *From:*iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org > [mailto:iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org] *On Behalf Of *Kai Huang > *Sent:* Thursday, December 26, 2013 9:55 AM > *To:* Li, Zhen-Hua > *Cc:* David Woodhouse; iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; > linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > *Subject:* Re: [PATCH 1/1] x86/iommu: use bit structures for context_entry > > Just curious, why would |= operation cause problem? :) > > On Fri, Dec 20, 2013 at 4:45 PM, Li, Zhen-Hua <zhen-hual-VXdhtT5mjnY@public.gmane.org > <mailto:zhen-hual-VXdhtT5mjnY@public.gmane.org>> 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 <zhen-hual-VXdhtT5mjnY@public.gmane.org <mailto:zhen-hual-VXdhtT5mjnY@public.gmane.org>> > --- > 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 > iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org <mailto:iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org> > https://lists.linuxfoundation.org/mailman/listinfo/iommu > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/1] x86/iommu: use bit structures for context_entry [not found] ` <1387529101-29769-1-git-send-email-zhen-hual-VXdhtT5mjnY@public.gmane.org> 2013-12-26 1:55 ` Kai Huang @ 2014-01-07 14:41 ` Joerg Roedel [not found] ` <20140107144108.GF2742-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> 1 sibling, 1 reply; 12+ messages in thread From: Joerg Roedel @ 2014-01-07 14:41 UTC (permalink / raw) To: Li, Zhen-Hua Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, David Woodhouse, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Fri, Dec 20, 2013 at 04:45:01PM +0800, Li, Zhen-Hua 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. What is the problem you are trying to fix here? Is it an actual bug? Joerg ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <20140107144108.GF2742-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH 1/1] x86/iommu: use bit structures for context_entry [not found] ` <20140107144108.GF2742-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> @ 2014-01-10 7:29 ` Li, ZhenHua [not found] ` <52CFA140.5060700-VXdhtT5mjnY@public.gmane.org> 0 siblings, 1 reply; 12+ messages in thread From: Li, ZhenHua @ 2014-01-10 7:29 UTC (permalink / raw) To: Joerg Roedel Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, David Woodhouse, linux-kernel-u79uwXL29TY76Z2rM5mHXA I have not seen such a bug yet . but obviously a '=' should be used when you want to set a value. for example, if x != 0, x=10 and x|=10 will cause different result. ZhenHua On 01/07/2014 10:41 PM, Joerg Roedel wrote: > On Fri, Dec 20, 2013 at 04:45:01PM +0800, Li, Zhen-Hua 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. > > What is the problem you are trying to fix here? Is it an actual bug? > > > Joerg > > ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <52CFA140.5060700-VXdhtT5mjnY@public.gmane.org>]
* Re: [PATCH 1/1] x86/iommu: use bit structures for context_entry [not found] ` <52CFA140.5060700-VXdhtT5mjnY@public.gmane.org> @ 2014-01-10 7:32 ` Jiang Liu 0 siblings, 0 replies; 12+ messages in thread From: Jiang Liu @ 2014-01-10 7:32 UTC (permalink / raw) To: Li, ZhenHua, Joerg Roedel Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, David Woodhouse, linux-kernel-u79uwXL29TY76Z2rM5mHXA I have noticed the same issue too. But after second check, seems it's safe with current implementation, but obviously it's not future safe. On 2014/1/10 15:29, Li, ZhenHua wrote: > I have not seen such a bug yet . but obviously a '=' should be used when > you want to set a value. > > for example, if x != 0, > x=10 > and > x|=10 > will cause different result. > > ZhenHua > > On 01/07/2014 10:41 PM, Joerg Roedel wrote: >> On Fri, Dec 20, 2013 at 04:45:01PM +0800, Li, Zhen-Hua 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. >> >> What is the problem you are trying to fix here? Is it an actual bug? >> >> >> Joerg >> >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/1] x86/iommu: use bit structures for context_entry
@ 2014-01-10 8:27 Li, Zhen-Hua
[not found] ` <1389342475-8540-1-git-send-email-zhen-hual-VXdhtT5mjnY@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Li, Zhen-Hua @ 2014-01-10 8:27 UTC (permalink / raw)
To: David Woodhouse, Joerg Roedel,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Li, Zhen-Hua
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 <zhen-hual-VXdhtT5mjnY@public.gmane.org>
---
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
^ permalink raw reply related [flat|nested] 12+ messages in thread[parent not found: <1389342475-8540-1-git-send-email-zhen-hual-VXdhtT5mjnY@public.gmane.org>]
* Re: [PATCH 1/1] x86/iommu: use bit structures for context_entry [not found] ` <1389342475-8540-1-git-send-email-zhen-hual-VXdhtT5mjnY@public.gmane.org> @ 2014-01-10 8:31 ` Li, ZhenHua 0 siblings, 0 replies; 12+ messages in thread From: Li, ZhenHua @ 2014-01-10 8:31 UTC (permalink / raw) To: Li, Zhen-Hua Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, David Woodhouse, jiang.liu-VuQAYsv1563Yd54FQh9/CA This patch is sent again because a ';' was missed in my last mail. > + context->asr = value >> VTD_PAGE_SHIFT I corrected it and created a new one. And it is tested . Please use the latest one. Sorry for that. ZhenHua On 01/10/2014 04:27 PM, Li, Zhen-Hua 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 <zhen-hual-VXdhtT5mjnY@public.gmane.org> > --- > 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) > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/1] x86/iommu: use bit structures for context_entry
@ 2013-05-24 0:22 Li, Zhen-Hua
[not found] ` <1369354931-19373-1-git-send-email-zhen-hual-VXdhtT5mjnY@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Li, Zhen-Hua @ 2013-05-24 0:22 UTC (permalink / raw)
To: David Woodhouse,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Li, Zhen-Hua
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.
Signed-off-by: Li, Zhen-Hua <zhen-hual-VXdhtT5mjnY@public.gmane.org>
---
drivers/iommu/intel-iommu.c | 88 +++++++++++++++----------------------------
1 file changed, 31 insertions(+), 57 deletions(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index b4f0e28..ae10471 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -221,55 +221,28 @@ 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);
-}
-static inline void context_set_present(struct context_entry *context)
-{
- context->lo |= 1;
-}
-
-static inline void context_set_fault_enable(struct context_entry *context)
-{
- context->lo &= (((u64)-1) << 2) | 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;
-}
-
-static inline void context_set_address_root(struct context_entry *context,
- unsigned long value)
-{
- context->lo |= value & VTD_PAGE_MASK;
-}
-
-static inline void context_set_address_width(struct context_entry *context,
- unsigned long value)
-{
- context->hi |= value & 7;
-}
-
-static inline void context_set_domain_id(struct context_entry *context,
- unsigned long value)
-{
- context->hi |= (value & ((1 << 16) - 1)) << 8;
-}
-
-static inline void context_clear_entry(struct context_entry *context)
-{
- context->lo = 0;
- context->hi = 0;
-}
-
/*
* 0: readable
* 1: writable
@@ -727,7 +700,7 @@ static int device_context_mapped(struct intel_iommu *iommu, u8 bus, u8 devfn)
ret = 0;
goto out;
}
- ret = context_present(&context[devfn]);
+ ret = context->present;
out:
spin_unlock_irqrestore(&iommu->lock, flags);
return ret;
@@ -743,7 +716,8 @@ static void clear_context_table(struct intel_iommu *iommu, u8 bus, u8 devfn)
root = &iommu->root_entry[bus];
context = get_context_addr_from_root(root);
if (context) {
- context_clear_entry(&context[devfn]);
+ context[devfn].lo = 0;
+ context[devfn].hi = 0;
__iommu_flush_cache(iommu, &context[devfn], \
sizeof(*context));
}
@@ -1573,7 +1547,7 @@ static int domain_context_mapping_one(struct dmar_domain *domain, int segment,
if (!context)
return -ENOMEM;
spin_lock_irqsave(&iommu->lock, flags);
- if (context_present(context)) {
+ if (context->present) {
spin_unlock_irqrestore(&iommu->lock, flags);
return 0;
}
@@ -1623,7 +1597,7 @@ static int domain_context_mapping_one(struct dmar_domain *domain, int segment,
}
}
- context_set_domain_id(context, id);
+ context->did = id;
if (translation != CONTEXT_TT_PASS_THROUGH) {
info = iommu_support_dev_iotlb(domain, segment, bus, devfn);
@@ -1635,15 +1609,15 @@ static int domain_context_mapping_one(struct dmar_domain *domain, int segment,
* AGAW value supported by hardware. And ASR is ignored by hardware.
*/
if (unlikely(translation == CONTEXT_TT_PASS_THROUGH))
- context_set_address_width(context, iommu->msagaw);
+ context->aw = iommu->msagaw;
else {
- context_set_address_root(context, virt_to_phys(pgd));
- context_set_address_width(context, iommu->agaw);
+ context->asr = virt_to_phys(pgd) >> VTD_PAGE_SHIFT;
+ context->aw = iommu->agaw;
}
- context_set_translation_type(context, translation);
- context_set_fault_enable(context);
- context_set_present(context);
+ context->translation_type = translation;
+ context->fpd = 0;
+ context->present = 1;
domain_flush_cache(domain, context, sizeof(*context));
/*
--
1.7.10.4
^ permalink raw reply related [flat|nested] 12+ messages in thread[parent not found: <1369354931-19373-1-git-send-email-zhen-hual-VXdhtT5mjnY@public.gmane.org>]
* Re: [PATCH 1/1] x86/iommu: use bit structures for context_entry [not found] ` <1369354931-19373-1-git-send-email-zhen-hual-VXdhtT5mjnY@public.gmane.org> @ 2013-05-24 0:36 ` Joe Perches 2013-05-24 0:43 ` ZhenHua 0 siblings, 1 reply; 12+ messages in thread From: Joe Perches @ 2013-05-24 0:36 UTC (permalink / raw) To: Li, Zhen-Hua Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, David Woodhouse, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Fri, 2013-05-24 at 08:22 +0800, Li, Zhen-Hua 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. [] > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c [] > @@ -221,55 +221,28 @@ 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, why use struct and union at all? Why not just: struct context_entry { u64 present:1, fpd:1, translation_type:2, reserved_l:8, asr:52; u64 aw:3, avail:4, reserved_h1:1, did:16, reserved_h2:40; }; > @@ -743,7 +716,8 @@ static void clear_context_table(struct intel_iommu *iommu, u8 bus, u8 devfn) [] > if (context) { > - context_clear_entry(&context[devfn]); > + context[devfn].lo = 0; > + context[devfn].hi = 0; memset(&context[devfn], 0, sizeof(...)) ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/1] x86/iommu: use bit structures for context_entry 2013-05-24 0:36 ` Joe Perches @ 2013-05-24 0:43 ` ZhenHua 0 siblings, 0 replies; 12+ messages in thread From: ZhenHua @ 2013-05-24 0:43 UTC (permalink / raw) To: Joe Perches Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, David Woodhouse, linux-kernel-u79uwXL29TY76Z2rM5mHXA Use lo and hi for clear, may run faster than memset. But it is not a big problem. Never mind. Thanks ZhenHua On 05/24/2013 08:36 AM, Joe Perches wrote: > On Fri, 2013-05-24 at 08:22 +0800, Li, Zhen-Hua 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. > [] >> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c > [] >> @@ -221,55 +221,28 @@ 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, > why use struct and union at all? > > Why not just: > > struct context_entry { > u64 present:1, > fpd:1, > translation_type:2, > reserved_l:8, > asr:52; > u64 aw:3, > avail:4, > reserved_h1:1, > did:16, > reserved_h2:40; > }; > >> @@ -743,7 +716,8 @@ static void clear_context_table(struct intel_iommu *iommu, u8 bus, u8 devfn) > [] >> if (context) { >> - context_clear_entry(&context[devfn]); >> + context[devfn].lo = 0; >> + context[devfn].hi = 0; > memset(&context[devfn], 0, sizeof(...)) > > ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-01-10 8:31 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-20 8:45 [PATCH 1/1] x86/iommu: use bit structures for context_entry Li, Zhen-Hua
[not found] ` <1387529101-29769-1-git-send-email-zhen-hual-VXdhtT5mjnY@public.gmane.org>
2013-12-26 1:55 ` Kai Huang
[not found] ` <CAOtp4KqUgSnwadP0T=aWYX7rRzDBn3qkuqMzceDBO0c_bhpFwQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-12-26 5:12 ` Wu, Feng
[not found] ` <E959C4978C3B6342920538CF579893F001D3C26D-0J0gbvR4kTg/UvCtAeCM4rfspsVTdybXVpNB7YpNyf8@public.gmane.org>
2014-01-06 1:41 ` Li, ZhenHua
2014-01-07 14:41 ` Joerg Roedel
[not found] ` <20140107144108.GF2742-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
2014-01-10 7:29 ` Li, ZhenHua
[not found] ` <52CFA140.5060700-VXdhtT5mjnY@public.gmane.org>
2014-01-10 7:32 ` Jiang Liu
-- strict thread matches above, loose matches on Subject: below --
2014-01-10 8:27 Li, Zhen-Hua
[not found] ` <1389342475-8540-1-git-send-email-zhen-hual-VXdhtT5mjnY@public.gmane.org>
2014-01-10 8:31 ` Li, ZhenHua
2013-05-24 0:22 Li, Zhen-Hua
[not found] ` <1369354931-19373-1-git-send-email-zhen-hual-VXdhtT5mjnY@public.gmane.org>
2013-05-24 0:36 ` Joe Perches
2013-05-24 0:43 ` ZhenHua
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).