xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: "Jan Beulich" <JBeulich@suse.com>
To: "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>,
	xen-devel <xen-devel@lists.xen.org>
Subject: Re: PCI passthrough for HVM with stubdomain broken by "tools/libxl: handle the iomem parameter with the memory_mapping hcall"
Date: Thu, 23 Jun 2016 03:46:46 -0600	[thread overview]
Message-ID: <576BCC2602000078000F7FC9@prv-mh.provo.novell.com> (raw)
In-Reply-To: <20160623092353.GI1593@mail-itl>

>>> On 23.06.16 at 11:23, <marmarek@invisiblethingslab.com> wrote:
> On Thu, Jun 23, 2016 at 11:18:24AM +0200, Marek Marczykowski-Górecki wrote:
>> On Thu, Jun 23, 2016 at 03:12:47AM -0600, Jan Beulich wrote:
>> > >>> On 23.06.16 at 10:57, <marmarek@invisiblethingslab.com> wrote:
>> > > On Thu, Jun 23, 2016 at 02:32:29AM -0600, Jan Beulich wrote:
>> > >> I wonder what good the duplication of the returned domain ID does: I'm
>> > >> tempted to remove the one in the command-specific structure. Does
>> > >> anyone have insight into why it was done that way?
>> > > 
>> > > Isn't XEN_DOMCTL_getdomaininfo supposed to return info about first
>> > > existing domain with ID >= passed one? Reading various comments in code
>> > > it looks to be used to domain enumeration. This patch changes this
>> > > behaviour.
>> > 
>> > No, it doesn't. It adjusts the behavior only for the DM case (which
>> > isn't supposed to get information on other than the target domain,
>> > i.e. in this one specific case the very domain ID needs to be passed
>> > in).
>> 
>> int xc_domain_getinfo(xc_interface *xch,
>>                       uint32_t first_domid,
>>                       unsigned int max_doms,
>>                       xc_dominfo_t *info)
>> {
>>     unsigned int nr_doms;
>>     uint32_t next_domid = first_domid;
>>     DECLARE_DOMCTL;
>>     int rc = 0;
>> 
>>     memset(info, 0, max_doms*sizeof(xc_dominfo_t));
>> 
>>     for ( nr_doms = 0; nr_doms < max_doms; nr_doms++ )
>>     {   
>>         domctl.cmd = XEN_DOMCTL_getdomaininfo;
>>         domctl.domain = (domid_t)next_domid;
>>         if ( (rc = do_domctl(xch, &domctl)) < 0 )
>>             break;
>>         info->domid      = (uint16_t)domctl.domain;
>> (...)
>>         next_domid = (uint16_t)domctl.domain + 1;
>> 
>> 
>> Looks like heavily dependent on XEN_DOMCTL_getdomaininfo returning next 
> valid
>> domain.
> 
> Hmm, looks like I've misread you patch. Reading again...
> 
> But now I see rcu_read_lock(&domlist_read_lock) is gets called only when
> looping over domains, but rcu_read_unlock is called in any case. Is it
> correct?

How that? There is this third hunk:

@@ -885,6 +892,9 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe
         copyback = 1;
 
     getdomaininfo_out:
+        if ( dom == DOMID_INVALID )
+            break;
+
         rcu_read_unlock(&domlist_read_lock);
         d = NULL;
         break;

Jan

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

  reply	other threads:[~2016-06-23  9:46 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-22 13:03 PCI passthrough for HVM with stubdomain broken by "tools/libxl: handle the iomem parameter with the memory_mapping hcall" Marek Marczykowski-Górecki
2016-06-22 13:50 ` Jan Beulich
2016-06-22 14:13   ` Marek Marczykowski-Górecki
2016-06-22 15:23     ` Jan Beulich
2016-06-22 18:24       ` Daniel De Graaf
2016-06-23  8:32         ` Jan Beulich
2016-06-23  8:39           ` Jan Beulich
2016-06-23 14:33             ` Daniel De Graaf
2016-06-23  8:57           ` Marek Marczykowski-Górecki
2016-06-23  9:12             ` Jan Beulich
2016-06-23  9:18               ` Marek Marczykowski-Górecki
2016-06-23  9:23                 ` Marek Marczykowski-Górecki
2016-06-23  9:46                   ` Jan Beulich [this message]
2016-06-23 13:25                     ` Marek Marczykowski-Górecki
2016-06-23 14:12                       ` Andrew Cooper
2016-06-23 14:59                         ` Marek Marczykowski-Górecki
2016-06-23 15:01                           ` Andrew Cooper
2016-06-23 14:45                       ` Jan Beulich
2016-06-23 15:00                       ` Daniel De Graaf
2016-06-23 15:22                         ` Marek Marczykowski-Górecki
2016-06-23 15:30                           ` Daniel De Graaf
2016-06-23 15:37                           ` Jan Beulich
2016-06-23 15:45                             ` Marek Marczykowski-Górecki
2016-06-23 15:49                               ` Marek Marczykowski-Górecki
2016-06-23 16:02                               ` Jan Beulich
2016-06-23  9:45                 ` Jan Beulich
2016-06-23  9:44           ` Andrew Cooper
2016-06-23  9:50             ` Jan Beulich
2016-06-23 14:15               ` Andrew Cooper

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=576BCC2602000078000F7FC9@prv-mh.provo.novell.com \
    --to=jbeulich@suse.com \
    --cc=dgdegra@tycho.nsa.gov \
    --cc=marmarek@invisiblethingslab.com \
    --cc=xen-devel@lists.xen.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).