From mboxrd@z Thu Jan 1 00:00:00 1970 From: Manish Jaggi Subject: Re: PCI Pass-through in Xen ARM - Draft 2. Date: Thu, 30 Jul 2015 18:21:01 +0530 Message-ID: <55BA1DB5.7090102@caviumnetworks.com> References: <55903F12.7010908@caviumnetworks.com> <55911E66.2040009@citrix.com> <5598C6CD.2040207@caviumnetworks.com> <1436173886.25646.24.camel@citrix.com> <559A532F.50305@caviumnetworks.com> <1436178036.25646.28.camel@citrix.com> <55B89ED5.4040904@caviumnetworks.com> <1438250078.11600.272.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1438250078.11600.272.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Prasun Kapoor , "Kumar, Vijaya" , "xen-devel@lists.xen.org" , Julien Grall , Stefano Stabellini , "Kulkarni, Ganapatrao" List-Id: xen-devel@lists.xenproject.org On Thursday 30 July 2015 03:24 PM, Ian Campbell wrote: > On Wed, 2015-07-29 at 15:07 +0530, Manish Jaggi wrote: >> On Monday 06 July 2015 03:50 PM, Ian Campbell wrote: >>> On Mon, 2015-07-06 at 15:36 +0530, Manish Jaggi wrote: >>>> On Monday 06 July 2015 02:41 PM, Ian Campbell wrote: >>>>> On Sun, 2015-07-05 at 11:25 +0530, Manish Jaggi wrote: >>>>>> On Monday 29 June 2015 04:01 PM, Julien Grall wrote: >>>>>>> Hi Manish, >>>>>>> >>>>>>> On 28/06/15 19:38, Manish Jaggi wrote: >>>>>>>> 4.1 Holes in guest memory space >>>>>>>> ---------------------------- >>>>>>>> Holes are added in the guest memory space for mapping pci >>>>>>>> device's BAR >>>>>>>> regions. >>>>>>>> These are defined in arch-arm.h >>>>>>>> >>>>>>>> /* For 32bit */ >>>>>>>> GUEST_MMIO_HOLE0_BASE, GUEST_MMIO_HOLE0_SIZE >>>>>>>> >>>>>>>> /* For 64bit */ >>>>>>>> GUEST_MMIO_HOLE1_BASE , GUEST_MMIO_HOLE1_SIZE >>>>>>> The memory layout for 32bit and 64bit are exactly the same. Why >>>>>>> do you >>>>>>> need to differ here? >>>>>> I think Ian has already replied. I will change the name of macro >>>>>>>> 4.2 New entries in xenstore for device BARs >>>>>>>> -------------------------------------------- >>>>>>>> toolkit also updates the xenstore information for the device >>>>>>>> (virtualbar:physical bar). >>>>>>>> This information is read by xenpciback and returned to the >>>>>>>> pcifront >>>>>>>> driver configuration >>>>>>>> space accesses. >>>>>>> Can you details what do you plan to put in xenstore and how? >>>>>> It is implementation . But I plan to put under domU / device / >>>>>> heirarchy >>>>> Actually, xenstore is an API of sorts which needs to be maintained >>>>> going >>>>> forward (since front and backend can evolve separately, so it does >>>>> need >>>>> some level of design and documentation. >>>>> >>>>>>> What about the expansion ROM? >>>>>> Do you want to put some restriction on not using expansion ROM as >>>>>> a >>>>>> passthrough device. >>>>> "expansion ROM as a passthrough device" doesn't make sense to me, >>>>> passthrough devices may _have_ an expansion ROM. >>>>> >>>>> The expansion ROM is just another BAR. I don't know how >>>>> pcifront/back >>>>> deal with those today on PV x86, but I see no reason for ARM to >>>>> deviate. >>>>> >>>>> >>>>>>>> 4.3 Hypercall for bdf mapping notification to xen >>>>>>>> ----------------------------------------------- >>>>>>>> #define PHYSDEVOP_map_sbdf 43 >>>>>>>> typedef struct { >>>>>>>> u32 s; >>>>>>>> u8 b; >>>>>>>> u8 df; >>>>>>>> u16 res; >>>>>>>> } sbdf_t; >>>>>>>> struct physdev_map_sbdf { >>>>>>>> int domain_id; >>>>>>>> sbdf_t sbdf; >>>>>>>> sbdf_t gsbdf; >>>>>>>> }; >>>>>>>> >>>>>>>> Each domain has a pdev list, which contains the list of all >>>>>>>> pci devices. >>>>>>>> The >>>>>>>> pdev structure already has a sbdf information. The >>>>>>>> arch_pci_dev is >>>>>>>> updated to >>>>>>>> contain the gsbdf information. (gs- guest segment id) >>>>>>>> >>>>>>>> Whenever there is trap from guest or an interrupt has to be >>>>>>>> injected, >>>>>>>> the pdev >>>>>>>> list is iterated to find the gsbdf. >>>>>>> Can you give more background for this section? i.e: >>>>>>> - Why do you need this? >>>>>>> - How xen will translate the gbdf to a vDeviceID? >>>>>> In the context of the hypercall processing. >>>>>>> - Who will call this hypercall? >>>>>>> - Why not setting the gsbdf when the device is >>>>>>> assigned? >>>>>> Can the maintainer of the pciback suggest an alternate. >>>>> That's not me, but I don't think this belongs here, I think it can >>>>> be >>>>> done from the toolstack. If you think not then please explain what >>>>> information the toolstack doesn't have in its possession which >>>>> prevents >>>>> this mapping from being done there. >>>> The toolstack does not have the guest sbdf information. I could only >>>> find it in xenpciback. >>> Are you sure? The sbdf relates to the physical device, correct? If so >>> then surely the toolstack knows it -- it's written in the config file >>> and is the primary parameter to all of the related libxl passthrough >>> APIs. The toolstack wouldn't be able to do anything about passing >>> through a given device without knowing which device it should be >>> passing >>> through. >>> >>> Perhaps this info needs plumbing through to some new bit of the >>> toolstack, but it is surely available somewhere. >>> >>> If you meant the virtual SBDF then that is in libxl_device_pci.vdevfn. >> I added prints in libxl__device_pci_add. vdevfn is always 0 so this may >> not be the right variable to use. >> Can you please recheck. >> >> Also the vdev-X entry in xenstore appears to be created from pciback >> code and not from xl. >> Check function xen_pcibk_publish_pci_dev. >> >> So I have to send a hypercall from pciback only. > I don't think the necessarily follows. > > You could have the tools read the vdev-X node back on plug. I have been trying to get the flow of caller of libxl__device_pci_add during pci device assignemnt from cfg file(cold boot). It should be called form xl create flow. Is it called from C code or Python code. libxl__device_pci_add calls xc_assign_device Secondly, the vdev-X entry is created async by dom0 watching on event. So how the tools could read back and call assign device again. static void xen_pcibk_be_watch(struct xenbus_watch *watch, const char **vec, unsigned int len) { ... switch (xenbus_read_driver_state(pdev->xdev->nodename)) { case XenbusStateInitWait: xen_pcibk_setup_backend(pdev); break; } > > Or you could change things such that vdevfn is always chosen by the > toolstack for ARM, not optionally like it is on x86. > > Please evaluate the options. > > Ian. > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel