Re: [Xen-devel] [PATCH v1 0/2] Extend ioreq-server to support page write protection

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Tim Deegan [mailto:tim@xxxxxxx]
> Sent: Thursday, August 14, 2014 1:08 AM
> 
> At 16:14 +0000 on 13 Aug (1407942882), Tian, Kevin wrote:
> > > From: Tim Deegan [mailto:tim@xxxxxxx]
> > > Sent: Wednesday, August 13, 2014 1:39 AM
> > >
> > > At 23:15 +0000 on 12 Aug (1407881757), Ye, Wei wrote:
> > > > Some summary of the discussion:
> > > >
> > > > Without vt-d page fault support, a general write-protection is not
> > > > possible. It's impossible to protect the write of both CPU and DMA
> > > > path.  But note that XenGT itself doesn't use VT-d, and there is no
> > > > another usage of having another device DMA to GPU resource.
> > >
> > > None that you're aware of.  But there might be some out-of-tree users?
> >
> > If there is, then it has to be in the unsupported feature list. I think 
> > it's fair
> > enough since we all know w/o complete VT-d page fault support there's
> > no solution for such usage. Pretty much like old days big real mode tricks
> > before hardware support is ready.
> 
> I think there's confusion here about what we're taling about.  Yes,
> until we have h/w support for restarting faulting DMA the IOMMU
> feature set will be incomplete.  The important thing is that it should
> fail safe -- i.e. if a guest has a read-only mapping of a page it is
> better to disallow DMA altogether than to allow read-write DMA.

Yes, there did be some confusions here. :-)

I think we are on the same page that, complete 'read-only' or
complete 'write-protection' is impossible w/o restarting faulting
DMA, because there is no way to enforce the access permission 
in the IOMMU side.

Then whatever 'read-only' or 'write-protection" discussion now
is a partial solution where only CPU path is protected, with the
assumption that no valid usage of using DMA to access the concerned
memory area. And then I agree disallowing DMA becomes a fail-safe 
means to capture violating usages.

> 
> > > > Or even
> > > > other device use the protect page for its DMA buffer, like a
> > > > malicious guest, there is no security hole since it can only access
> > > > it's our memory range.
> > >
> > > Not true at all!  If the guest has a read-only mapping it must be
> > > read-only for a reason.  For example, it might have a read-only grant
> > > mapping of another VM's memory.  Or an out-of-VM security tool might
> > > be enforcing W^X permissions.
> >
> > sorry original statement is inaccurate. there is no cross-VM security hole
> > since EPT/VT-d are fully under VMM's control, but yes out-of-VM security
> > tool might be broken...
> 
> As I just said: "it might have a read-only grant mapping of another
> VM's memory".  We can't let it DMA into that mapping.
> 
> > > >  So, just keep protecting the CPU write path may be feasible for this
> > > moment.
> > >
> > > I don't think so.
> >
> > Could we introduce a cpu-write-protection attribute only for functional
> > usages which don't care about DMA path like in XenGT?
> 
> Possibly.  Sorry for going over things that have probably already been
> covered but AIUI you are saying that XenGT needs to allow full DMA access
> to this memory area but only read-only CPU access.  If that's what's
> needed then I think a new type with these semantics is the best way of
> doing it (and requiring no shared IOMMU/HAP tables).  But:

It's about GPU page table write-protection, so XenGT needs to trap
writes in both CPU/DMA paths:

* CPU writes are captured by read-only permission in EPT
* GPU writes are captured by scanning GPU commands before they're
submitted for execution, as part of the 'mediation' concept in XenGT
mediated pass-through architecture.

Note XenGT doesn't use IOMMU because w/o SR-IOV a single device
entry can't be used to server multiple VMs.

So XenGT can achieve true 'write-protection' w/o IOMMU restarting
fault support. However it's very GPU specific. As a general implementation, 
I think a new type with below semantics should be enough:

- only CPU access to the protected area, so it's removed from IOMMU
page table
- require IOMMU/HAP seperate

How would you call such a new type then? just p2m_write_protection
or p2m_cpu_write_protection? (still need to differentiate write-protection
from read-only, since the latter implies no writes)

>  - it's pointless if the guest can make the device issue DMA for it,
>    (with a blitting engine or similar) since it can just use that DMA
>    to make the writes it wants; and

Based on above explanation it's not a problem

>  - for the same reason, you will need to give the GPU its own set
>    of IOMMU tables, separate from the ones used for other devices that
>    the guest controls.  Otherwise the guest can use its NIC to DMA
>    into the read-only area.

XenGT doesn't use IOMMU. So it's enough as long as we zap the range
from all IOMMU page tables

in the future when we have SR-IOV GPU support, there's no need to
shadow GPU page table then, and thus no such a requirement of
write-protection.

> 
> > > > Another way is treat the page as MMIO ranges(means removing
> mapping
> > > > in both EPD/VT-D page table). In this way, both read/write access
> > > > should be forwarded and emulated. For XenGT usage case, we observed
> > > > that for the Linux guest, there is no read access to the ppgtt page
> > > > tables. And for the windows guest, the read access is just about 20%
> > > > of write. The read overhead is relatively small, so treat the pages
> > > > as MMIO range may be possible.
> > >
> > > That might work for XenGT but it doesn't address the general problem
> > > at all.  AFAICT the only _safe_ way to make a page read-only to a guest
> > > that can issue DMA is to remove it from the IOMMU tables - and that
> > > means having separate EPT and vtD tables, until hardware support
> > > for restartable IOMMU faults comes along.
> >
> > Aren't we talking about a similar option? ours is more conservative, not 
> > only
> > removing from IOMMU table, but also removing from EPT so both r/w are
> > trapped which serves the read-only requirements with performance penalty.
> 
> Yes they are similar.  I would say mine is more conservative in a
> different way - not using the emulator unneccesarily. :)
> 

yeah from different view. Our original thought is if new p2m type is tricky, 
then treating it as mmio could be an easier thing to extend. Now since you
also agree adding a new type is reasonable, then we can continue this
direction (actually original patch is along this road, but didn't zap IOMMU
pages. :-))

Thanks
Kevin

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel