[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH 1/1] xen:vtd: missing RMRR mapping while share EPT
On 2014/7/24 15:00, Chen, Tiejun wrote:
On 2014/7/24 14:14, Jan Beulich wrote:
On 24.07.14 at 03:23, <tiejun.chen@xxxxxxxxx> wrote:
On 2014/7/23 23:42, Jan Beulich wrote:
On 23.07.14 at 11:35, <tiejun.chen@xxxxxxxxx> wrote:
@@ -1867,7 +1869,19 @@ static int rmrr_identity_mapping(struct
domain *d,
while ( base_pfn < end_pfn )
{
- if ( intel_iommu_map_page(d, base_pfn, base_pfn,
+ if ( iommu_use_hap_pt(d) ) {
+ dprintk(XENLOG_DEBUG VTDPREFIX,
+ "Set RMRR mapping: pfn:0x%lx mfn:0x%lx.\n",
+ base_pfn, mfn_x(_mfn(base_pfn)));
Do we really need this message, even more so not at guest level?
Its useful to debug as I think, but if you insist on this point, I'm
fine to remove this as well.
The main question is how frequently this may get printed vs how
useful the message is.
Apart from the above there are several Indentation issues here.
Are you saying this thing?
if ()
{
}
Yes, among other things.
So what about this?
Almost:
@@ -1867,7 +1869,21 @@ static int rmrr_identity_mapping(struct domain
*d,
while ( base_pfn < end_pfn )
{
- if ( intel_iommu_map_page(d, base_pfn, base_pfn,
+ if ( iommu_use_hap_pt(d) )
Don't you, btw, need to extend this condition by
&& (!iommu_passthrough || !is_hardware_domain(d))?
Why do we need these checks here?
Current problem I met is issued when do GFX passthrough for Windows Guest.
+ {
+ dprintk(XENLOG_DEBUG VTDPREFIX,
This still (if you absolutely want to retain the message) needs
I will remove this simply since this is not a big deal :)
changing to XENLOG_G_DEBUG, and you want to include the domain
ID in what gets printed for the message to be of any practical use.
+ "Set RMRR mapping: pfn:%#lx mfn:%#lx.\n",
Additionally please omit the stop at the end. Also, with VTDPREFIX
not ending with a space, you want the message to be starting
with one.
+ base_pfn, mfn_x(_mfn(base_pfn)));
+ p2m_lock(p2m);
+ if ( p2m_set_entry(p2m, base_pfn, _mfn(base_pfn),
PAGE_ORDER_4K,
+ p2m_mmio_direct, p2m_access_rw) )
Indentation.
+ {
+ p2m_unlock(p2m);
+ return -1;
+ }
+ p2m_unlock(p2m);
+ }
+ else if ( intel_iommu_map_page(d, base_pfn, base_pfn,
IOMMUF_readable|IOMMUF_writable) )
Again (here you need you also adjust the second line for indentation
to match up again).
I'm not familiar with our xen coding style so I'm wondering if we have
such a similar .pl like checkpatch.pl.
Anyway, what about this?
@@ -1867,8 +1869,20 @@ static int rmrr_identity_mapping(struct domain *d,
while ( base_pfn < end_pfn )
{
- if ( intel_iommu_map_page(d, base_pfn, base_pfn,
- IOMMUF_readable|IOMMUF_writable) )
+ if ( iommu_use_hap_pt(d) && (!iommu_passthrough ||
+ !is_hardware_domain(d)) )
+ {
+ p2m_lock(p2m);
+ if ( p2m_set_entry(p2m, base_pfn, _mfn(base_pfn),
PAGE_ORDER_4K,
+ p2m_mmio_direct, p2m_access_rw) )
+ {
+ p2m_unlock(p2m);
+ return -1;
+ }
+ p2m_unlock(p2m);
+ }
+ else if ( intel_iommu_map_page(d, base_pfn, base_pfn,
+ IOMMUF_readable|IOMMUF_writable) )
return -1;
base_pfn++;
}
Thanks
Tiejun
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
