All of lore.kernel.org
 help / color / mirror / Atom feed
From: Avi Kivity <avi@qumranet.com>
To: Ben-Ami Yassour1 <BENAMI@il.ibm.com>
Cc: kvm-devel@lists.sourceforge.net, andrea@qumranet.com,
	allen.m.kay@intel.com
Subject: Re: [PATCH 1/1] direct mmio for passthrough - kernel part
Date: Tue, 01 Apr 2008 22:43:19 +0300	[thread overview]
Message-ID: <47F29057.7050004@qumranet.com> (raw)
In-Reply-To: <OF1AFF42D8.0AC7A193-ONC225741E.006AA79E-C225741E.006AF73C@il.ibm.com>

Ben-Ami Yassour1 wrote:
>>
>> Can you explain why you're not using the regular memory slot mechanism?
>> i.e. have userspace mmap(/dev/mem) and create a memslot containing that
>> at the appropriate guest physical address?
>>
>>     
> Our initial approach was to mmap /sys/bus/pci/devices/.../resource#
> and create a memory slot for it. However eventually we realized that for
> mmio we don't need hva mapped to the mmio region, we can map the gpa
> directly to hpa.
>
> As far as I understand, the memory slots mechanism is used to map
> gpa to hva. Then gfn_to_page uses get_user_pages to map hva to hpa.
> However, get_user_pages does not work for mmio, and in addition we
> know the hpa to begin with, so there is no real need to map an hva
> for the mmio region.
>
> In addition there is an assumption in the code that there is a page
> struct for the frame which is not the case for mmio. So it was
> easier to simply add a list of mmio gpa-hpa mapping.
>
> I guess we can use the memory slots array to holds the gpa to hpa
> mapping but it is not necessarily natural, and would probably
> require more hooks in the code to handle an mmio memory slot. BTW,
> note that for a guest that has multiple passthrough devices each
> with a set of mmio regions, it might become a long list, so there
> might be value to keep it separate from that respect as well.
>
> With regards to the potential security issue Anthony pointed out
> (ioctls taking hpa's) we can change the interface so that they will
> take (device, BAR) instead and the kernel will translate to hpa
>
>   

Not enough.  How do you know if this calling process has permissions to 
access that pci device (I retract my previous "pci passthrough is as 
rootish as you get" remark).

> What do you think? Given that the shadow page table code has to be
> modified anyway (due to the struct page issue), is it worthwhile to
> experiment with mmap(...region) or is the current approach sufficient?
>   

As Anthony points out, the advantage to mmap() is that whatever security 
is needed can be applied at that level.  Passing host physical addresses 
from userspace requires a whole new security model.

The issue with gfn_to_page() is real.  We can either try to work around 
it somehow, or we can try to back mmio regions with struct pages.  Now 
that it looks like mem_map is becoming virtually mapped, the cost is 
minimal, but I expect this approach will meet some resistance.

-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

  reply	other threads:[~2008-04-01 19:43 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-01 11:52 [RFC] direct mmio for passthrough benami
2008-04-01 11:52 ` [PATCH 1/1] direct mmio for passthrough - kernel part benami
2008-04-01 13:30   ` Avi Kivity
2008-04-01 14:42     ` Anthony Liguori
2008-04-01 15:20       ` Anthony Liguori
2008-04-01 17:05         ` Avi Kivity
2008-04-01 18:18         ` Andrea Arcangeli
2008-04-01 18:28           ` Anthony Liguori
2008-04-01 17:03       ` Avi Kivity
2008-04-01 17:18         ` Daniel P. Berrange
2008-04-01 18:10           ` Andrea Arcangeli
2008-04-01 18:18             ` Daniel P. Berrange
2008-04-01 18:23             ` Anthony Liguori
2008-04-01 18:21         ` Anthony Liguori
2008-04-01 19:22           ` Avi Kivity
2008-04-01 22:38             ` Andrea Arcangeli
2008-04-01 22:22           ` Andrea Arcangeli
2008-04-01 22:29             ` Anthony Liguori
2008-04-02  4:00               ` Avi Kivity
2008-04-01 19:28     ` Ben-Ami Yassour1
2008-04-01 19:43       ` Avi Kivity [this message]
2008-04-01 20:04         ` Anthony Liguori
2008-04-02  4:32           ` Avi Kivity
2008-04-02  7:03             ` Andrea Arcangeli
2008-04-02  9:50               ` Avi Kivity
2008-04-02 10:28                 ` Andrea Arcangeli
2008-04-02 10:59                   ` Avi Kivity
2008-04-02 11:16                     ` Avi Kivity
2008-04-02 11:50                       ` Andrea Arcangeli
2008-04-02 11:53                         ` Andrea Arcangeli
2008-04-03  8:51                         ` Avi Kivity
2008-04-02 14:59                     ` Anthony Liguori

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=47F29057.7050004@qumranet.com \
    --to=avi@qumranet.com \
    --cc=BENAMI@il.ibm.com \
    --cc=allen.m.kay@intel.com \
    --cc=andrea@qumranet.com \
    --cc=kvm-devel@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.