Re: [Xen-devel] [PATCH] hvmloader / qemu-xen: Getting rid of resource conflict for OpRegion.

[Ross Philipson <Ross.Philipson@xxxxxxxxxx>]

> -----Original Message-----
> From: xen-devel-bounces@xxxxxxxxxxxxx [mailto:xen-devel-
> bounces@xxxxxxxxxxxxx] On Behalf Of G.R.
> Sent: Thursday, December 20, 2012 10:06 AM
> To: Keir (Xen.org)
> Cc: Stefano Stabellini; Ian Campbell; Jean.guyader@xxxxxxxxx; xen-devel
> Subject: Re: [Xen-devel] [PATCH] hvmloader / qemu-xen: Getting rid of
> resource conflict for OpRegion.
> 
> On Thu, Dec 20, 2012 at 10:19 PM, Keir Fraser <keir@xxxxxxx> wrote:
> > On 20/12/2012 13:31, "G.R." <firemeteor@xxxxxxxxxxxxxxxxxxxxx> wrote:
> >
> >> If concern is about security, the same argument should apply to the
> >> first page (the portion before the page offset).
> >> The problem is that I have no idea what is around the mapped page.
> >> Not sure who has the knowledge.
> >
> > Well we can't do better than mapping some whole number of pages,
> really.
> > Unless we trap to qemu on every access. I don't think we'd go there
> > unless there really were a known security issue. But mapping only the
> > exact number of pages we definitely need is a good principle.
> >
> >> What's the standard flow to handle such map with offset?
> >> I expect this to be a common case, since the ioremap function in
> >> linux kernel accept this.
> >
> > map_size = ((host_opregion & 0xfff) + 8096 + 0xfff) >> 12
> >
> Keir, I believe this expression should give the same result.
> First of all, 8096 should be 8192 :-), and this part should result in an
> constant number 2 after right shift.
> The remaining part is ((host_opregion & 0xfff) + 0xfff) >> 12 As long as
> the first sub-expression is non zero, the result of the add should range
> from [0x1000, 0x1ffe].
> And this will yield a result '1' after the right shift.
> So as long as there is known security risk (which I'm not sure about),
> the patch should be fine.
> 
> > Possibly with suitable macros used instead of magic numbers (e.g.,
> > XC_PAGE_* and a macro for the opregion size).
> 
> I guess there is no predefined macro for OpRegion size. And I guess I
> need to define it twice for both code?

In addition we should think about defining the IGD OpRegion in ACPI per the 
spec (cited earlier). Guest drivers seem to find the region just by reading the 
ASLS register in the gfx device's config space but it would be more correct to 
define it in ACPI too. Just a thought.

Ross

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

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