Re: [Xen-devel] [PATCH] Don't track all memory when enabling log dirty to track vram

["Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx>]

Jan Beulich wrote on 2014-05-21:
>>>> On 21.05.14 at 03:02, <yang.z.zhang@xxxxxxxxx> wrote:
>> Jan Beulich wrote on 2014-05-20:
>>>>>> On 20.05.14 at 12:12, <George.Dunlap@xxxxxxxxxxxxx> wrote:
>>>> On Tue, May 20, 2014 at 8:20 AM, Jan Beulich <JBeulich@xxxxxxxx>
> wrote:
>>>>>>>> On 20.05.14 at 05:13, <yang.z.zhang@xxxxxxxxx> wrote:
>>>>>> George Dunlap wrote on 2014-05-19:
>>>>>>> Avoiding these by "hoping" that the guest OS doesn't DMA into a
>>>>>>> video buffer isn't really robust enough.  I think that was Tim
>>>>>>> and Jan's
>>>>>> 
>>>>>> Video buffer is only one case. How we can prevent the DMA to other
>>>>>> reserved region?
>>>>> 
>>>>> You continue to neglect the difference: Accessing VRAM this way is
>>>>> legitimate (and potentially useful). And - as just said in the
>>>>> other reply - ideally we'd also simply ignore accesses to reserved
>> 
>> Can you give an example of what legitmate case you are mentioned twice(You
>> mentioned it also in other reply)?
> 
> What's wrong with loading some graphics data right from an I/O device
> (disk, network) into the frame buffer?

Yes, it is ok. But we are talking about the DMA access to a 'readonly' buffer. 
It is totally a wrong usage model to me.

> 
>> I cannot understand why we need to
>> restrict the CPU access to VRAM region but allow accessing from device.
> 
> We'd love to also do this for device accesses, but to date IOMMU
> fault are unrecoverable, and hence this is not an option.
> 
>> As I known, for gfx passthrough, both device and CPU are able to access
>> them. And for emulated gfx, only software will access it which same as
>> current we see in Xen.
> 
> Why's that? I.e. why should a guest with emulated graphics not be
> able to program a passed through device to access the VRAM directly?

I think you confused the background of the issue: the essence of the issue is 
that we marked all pages as read-only and meanwhile we allow DMA to access 
those page. So the right solution is only mark the required page (in this case 
means VRAM region) as readonly and if guest still DMA to that region, then any 
unpredictable behavior to that guest is reasonable. What you said before 
totally ignore the 'read-only' thing. If there is no read-only operation, there 
will be no problem and no discussion around it.

> 
>>>>> regions (and in fact we try to, by not immediately bringing down a
>>>>> guest device doing such).
>>>> 
>>>> On the other hand, just to play devil's advocate here: Implementing
>>>> separate IOMMU tables (including superpages) isn't free; it has a
>>>> non-negligible cost, both in initial developer time, continuing
>>>> maintenance (code complexity, fixing bugs), extra memory at
>>>> run-time, &c.
>>>> 
>>>> Of all the things we could invest that developer time doing, why
>>>> should we make it possible to DMA into VRAM, rather than doing
>>>> something else?
>>> 
>>> While I agree that the question is valid, my position really is that
>>> it was a mistake to implement the IOMMU code without superpage
>> 
>> We support the superpage via sharing EPT and VT-d pagetable.
> 
> I.e. without EPT no superpages. Afaict there's no such limitation on
> the AMD side.

I don't think this is a 'limitation'. The original motivation to sharing page 
table is to use memory efficiently and easy for code maintain. And when we 
tried to add superpage support, we follow this way (we only need to change one 
copy of page table). But you think it is a limitation. I cannot understand.

> 
>>> support, i.e. I view this as a shortcoming independent of the VRAM
>>> issue, and I would want to see this fixed rather sooner than later.
>>> Had it been done properly from the beginning (like one would expect
>>> for non-experimental code), a lot of this discussion could have been
>>> avoided, and we wouldn't have had to take the respective workaround
>>> close to the 4.4 release.
>> 
>> I still think the best solution is fixing the VRAM global log dirty
>> mechanism which my previous patch already did. Because I cannot see any
>> benefit with separating the page table.
> 
> You didn't in any way negate the condition of superpage support to be
> added post-4.4 in order for your other change to go in: Neither
> http://lists.xenproject.org/archives/html/xen-devel/2014-02/msg01230.html
>  nor
> http://lists.xenproject.org/archives/html/xen-devel/2014-02/msg01236.html
>  have been responded to by you. By not doing so, to me at least you
> implicitly accepted the condition. And by now refusing to meet it, you
> basically tell us that we shouldn't be doing compromises like this with
> you in the future.

I have said before I am totally unware of it. And I know it only two days ago 
after someone told me. So please do not confuse this with the thing what we are 
discussing now. If you think I gave a promise implicitly at that time, I am 
sorry to let you think so.

As I said in previous thread, if we can prove that add hugepage for the 
separate VT-d page table is the only choice to solve problem, then now I am 
promising that I will do it ASAP. But till now, I didn't see any point that we 
must to have it. To me, it is still a nice to have feature.

> 
> Jan


Best regards,
Yang



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