Re: [Xen-devel] pci-passthrough loses msi-x interrupts ability after domain destroy

2017-09-25 Thread Ross Philipson
>
> ​[snip]​
>
>
> > So i think David's NACK was mostly for the patchset having some hackish
> cosmetics.
>
> He didn't like 'do_flr' which made sense as the patchset did not do FLR.
> It made a bus-reset
> for more than one device (if those devices were assigned to pciback).
>

​When I first wrote this, FLR was not yet implemented in the kernel so I
was actually adding FLR support; thus the name do_flr. So that is just
cargo from years ago :)​



>
> >
> > On the upside one can conclude that this patchset is now pretty well
> tested over the years ;)
> >
> > Since David has left, perhaps Jurgen/Boris/Konrad could express their
> views (again) ?
> > (CC'ed them as well)
>
> I've asked Govinda (CC-ed) to refresh the patchset against the lastest
> kernel and
> repost it and see where it goes.
>
> >
> > > As noted in the original LKML threads, vfio has similar relevant pci
> > > device reset retry logic. (Thanks to Rich Persaud for this pointer:)
> > > http://elixir.free-electrons.com/linux/v4.14-rc1/source/
> drivers/vfio/pci/vfio_pci.c#L1353
> > >
> > > libvirt also performs similar reset logic, using a direct low level
> > > interface to config space (Thanks to Marek for this pointer, libvirt
> > > is used by Qubes:)
> > > https://github.com/libvirt/libvirt/blob/master/src/util/virpci.c#L929
> > > I thinks this indicates that it would be possible to extend libxl to
> > > do something similar, but that seems less satisfactory compared to
> > > performing the work in a kernel-provided implementation.
> > >
> > > Is there a way forward to providing this functionality within Xen
> > > software or Linux> Christopher
> > > --
> > >
> > > openxt.org
> > >
> >
>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
>



-- 
Ross Philipson
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] xsm: policy hooks to require an IOMMU and interrupt remapping

2017-08-23 Thread Ross Philipson
I did test and review this submission back when I worked on the OpenXT
project with Christopher so I can add a reviewed by.

Reviewed-by: Ross Philipson <ross.philip...@gmail.com>

On Tue, Aug 22, 2017 at 4:18 AM, Jan Beulich <jbeul...@suse.com> wrote:

> >>> On 18.08.17 at 23:55, <dgde...@tycho.nsa.gov> wrote:
> > On 08/18/2017 05:02 PM, christopher.w.cl...@gmail.com wrote:
> >> From: Christopher Clark <christopher.cla...@baesystems.com>
> >>
> >> Isolation of devices passed through to domains usually requires an
> >> active IOMMU. The existing method of requiring an IOMMU is via a Xen
> >> boot parameter ("iommu=force") which will abort boot if an IOMMU is not
> >> available.
> >>
> >> More graceful degradation of behaviour when an IOMMU is absent can be
> >> achieved by enabling XSM to perform enforcement of IOMMU requirement.
> >>
> >> This patch enables an enforceable XSM policy to specify that an IOMMU is
> >> required for particular domains to access devices and how capable that
> >> IOMMU must be. This allows a Xen system to boot whilst still
> >> ensuring that an IOMMU is active before permitting device use.
> >>
> >> Using a XSM policy ensures that the isolation properties remain enforced
> >> even when the large, complex toolstack software changes.
> >>
> >> For some hardware platforms interrupt remapping is a strict requirement
> >> for secure isolation. Not all IOMMUs provide interrupt remapping.
> >> The XSM policy can now optionally require interrupt remapping.
> >>
> >> The device use hooks now check whether an IOMMU is:
> >>   * Active and securely isolating:
> >>  -- current criteria for this is that interrupt remapping is ok
> >>   * Active but interrupt remapping is not available
> >>   * Not active
> >>
> >> This patch also updates the reference XSM policy to use the new
> >> primitives, with policy entries that do not require an active IOMMU.
> >>
> >> Signed-off-by: Christopher Clark <christopher.cla...@baesystems.com>
> >
> > Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
>
> To be honest, for this kind of a change I would have hoped for
> a Reviewed-by (by you or someone else), not just an Acked-by.
> Hence I'm hesitant to put the patch in right away.
>
> Jan
>
>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
>



-- 
Ross Philipson
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] EFI + tboot + Xen

2017-04-25 Thread Ross Philipson
On 04/17/2017 06:05 PM, Rich Persaud wrote:
> On Apr 14, 2017, at 16:43, Daniel Kiper <daniel.ki...@oracle.com
> <mailto:daniel.ki...@oracle.com>> wrote:
> 
>> On Fri, Apr 14, 2017 at 04:17:54PM +0100, Andrew Cooper wrote:
>>> On 14/04/2017 15:54, Daniel Kiper wrote:
>>>> Hey,
>>>>
>>>> Has anybody tried to run EFI + tboot + Xen?
>>>> I have a feeling that it does not work because
>>>> tboot shuts down EFI boot services. However,
>>>> even if it works then efibootmgr is unusable
>>>> due to lack of EFI runtime services. Do we care?
>>>> Is it possible to make it work with full blown
>>>> EFI infrastructure available for Xen?
>>>
>>> Judging by
>>> http://hg.code.sf.net/p/tboot/code/file/9352e6391332/tboot/common/boot.S#l83
>>> it will be grub exiting boot services.  tboot needs rather more
>>> multiboot2 knowledge before it could participate in a hand-off to Xen
>>> while keeping boot services active.
>>
>> Sure, it is not a problem. However, I was told that it was (not) done
>> deliberately because we cannot trust EFI due to lack of its measurement.
>> I am not sure it is true or not. I though that somebody played with tboot
>> and Xen and has some knowledge in that area. Anyway, I will investigate
>> this further. However, any knowledge sharing is greatly appreciated.
> 
> On the OpenXT project, Ross Philipson has an early PoC:
> https://github.com/rossphilipson/efi-tboot
> 
> From the README:
> ---
> 
> EFI TBOOT is mostly a proof of concept at this point. It is not currently
> functional. It can be built and installed as an EFI boot loader. It only works
> in conjunction with Xen at the moment. The current development work is being
> done on Fedora 25 x64. The status as of March 14, 2017 is: 
> 
> 
> - EFI TBOOT will boot, but it needs a few key strokes to get going (this is 
> for
> debugging purposes). 
> 
> - EFI TBOOT will relocate itself to EFI runtime memory and setup a shared
> runtime variable with Xen. 
> 
> - EFI related configuration setup is done as well as standard TBOOT pre- 
> launch
> configuration. 
> 
> - Xen is launched and has code to call EFI TBOOT back after EBS. 
> 
> - EFI TBOOT then does the SENTER successfully in the callback. 
> 
> - The post launch entry point is reached but the switch back to long mode is 
> not
> working
> 
> ---
> 
> 
> Rich
> 

So this project is a proof of concept at the moment. Currently the readme is out
of date (I will fix that). The SENTER returns correctly now and rebuilds the
world to get back into long mode. It then calls into the post launch function in
tboot.c and does a bit more before dying because it is incomplete.

Anyway I will work on fixing the readme with more details on what it is all 
about.

-- 
Ross Philipson

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block

2016-04-06 Thread Ross Philipson
On 03/29/2016 07:26 AM, George Dunlap wrote:
> On Fri, Mar 25, 2016 at 4:34 PM, Ross Philipson
> <ross.philip...@gmail.com> wrote:
>> On 03/25/2016 12:11 PM, Wei Liu wrote:
>>> On Thu, Mar 17, 2016 at 06:10:59PM -0400, Ross Philipson wrote:
>>>> It seems the logic is meant to detect sector unaligned buffers for block
>>>> writes. The NOTing of the logic instead masks off any unaligned bits and
>>>> also would cause the function to always fail. It seems the function is not
>>>> used in any of the tools so that is probably why the problem is not seen.
>>>> In the vhd_read_block function it is correct.
>>>>
>>>> Signed-off-by: Ross Philipson <ross.philip...@ainfosec.com>
>>>
>>> This seems to fall under tools umbrella. I've look at blktap2 code,
>>> the reasoning is convincing to me so:
>>>
>>>   Acked-by: Wei Liu <wei.l...@citrix.com>
>>>
>>> But I've never used blktap2 and we don't normally test it so I would
>>> also wait a bit longer to see if anybody objects to this change.
>>>
>>> OOI if no code was using this, how did you discover this problem?
>>
>> We have an old fork of blktap2 from way back when. I was working to extract 
>> our
>> changes and turn them into patches so we could rebase on upstream blktap2.
>> Someone long ago found that - I have no idea how but it was a valid fix so I
>> upstreamed it.
>>
>> There are a number of other ones that were found that are still in upstream
>> blktap2 - I plan to send them too.
> 
> Ross,
> 
> Instead of cross-porting fixes from XenServer's blktap3 to the
> bitrotting blktap2, would you consider taking up my work on replacing
> blktap2 with building blktap3 as an external project?

George, a few questions below to get started...

> 
> The basic shape of things that need to happen for that are:
> 
> 1. Allow qemu to provide emulated devices for disks which use hotplug
> scripts (just checked in last week)
> 
> 2. Do the tweaks necessary to allow blktap3 to be built as an
> independent project (outside the XenServer build system)

I am not sure where the latest and greatest blktap3 work can be found. I have
found some old repos that have activity up until the work stopped on that
project in 2013. Can you give me some pointers on this?

Also it seems the overall idea is to not have blktap3 (or any blktap) live in
the Xen tree but rather be external. Where would blktap3 live?

> 
> 3. Switch libxl to use the already-in-tree block-tap script (which
> calls tapctl) rather than linking against the blktap library.

I believe I found this work:

http://lists.xenproject.org/archives/html/xen-devel/2015-07/msg00805.html

It looks like 4 of the set made it in and patches 3 & 5 still need work? I can
re-base them.

> 
> 4. Add a blktap target to raisin.
> 
> #1 is done and was just checked in last week.  #2 shouldn't be a
> terribly large amount of work.  For #3, I've posted patches already,
> and I can probably do a quick rebase for you to pick up if you wanted.
> #4 shouldn't be too hard once you have an out-of-tree build working; I
> can help with that as well.
> 
> If we then add tests for upstream blktap to osstest, then we'll catch
> any breakages, and automatically get updates; and we can delete the
> bitrotting blktap2 out of the tree.
> 
> What do you think?
> 
>  -George
> 


-- 
Ross Philipson

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


Re: [Xen-devel] [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block

2016-04-06 Thread Ross Philipson
On 04/06/2016 10:39 AM, Ian Jackson wrote:
> Ross Philipson writes ("[Xen-devel] [PATCH] blktap2: Invalid logic detecting 
> unaligned buffers in vhd_write_block"):
>> It seems the logic is meant to detect sector unaligned buffers for block
>> writes. The NOTing of the logic instead masks off any unaligned bits and
>> also would cause the function to always fail. It seems the function is not
>> used in any of the tools so that is probably why the problem is not seen.
>> In the vhd_read_block function it is correct.
>>
>> Signed-off-by: Ross Philipson <ross.philip...@ainfosec.com>
>> ---
>> diff --git a/tools/blktap2/vhd/lib/libvhd.c b/tools/blktap2/vhd/lib/libvhd.c
>> index 1fd5b4e..4ebe012 100644
>> --- a/tools/blktap2/vhd/lib/libvhd.c
>> +++ b/tools/blktap2/vhd/lib/libvhd.c
>> @@ -2188,7 +2188,7 @@ vhd_write_block(vhd_context_t *ctx, uint32_t block, 
>> char *data)
>> if (block >= ctx->bat.entries)
>> return -ERANGE;
>>
>> -   if ((unsigned long)data & ~(VHD_SECTOR_SIZE -1))
>> +   if ((unsigned long)data & (VHD_SECTOR_SIZE -1))
> 
> Notwithstanding the discussion in the rest of the thread, this is a
> clearly-correct bugfix to the code in tree, so I have queued it.
> 
> However, I had to do so rather manually because something had mangled
> the whitespace.  Ross, can you check your patch submission path,
> please ?
> 
> Ian.
> 

Never mind, I found the problem. Tabs got turned into spaces. Sorry about that.

-- 
Ross Philipson

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


Re: [Xen-devel] [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block

2016-04-06 Thread Ross Philipson
On 04/06/2016 10:39 AM, Ian Jackson wrote:
> Ross Philipson writes ("[Xen-devel] [PATCH] blktap2: Invalid logic detecting 
> unaligned buffers in vhd_write_block"):
>> It seems the logic is meant to detect sector unaligned buffers for block
>> writes. The NOTing of the logic instead masks off any unaligned bits and
>> also would cause the function to always fail. It seems the function is not
>> used in any of the tools so that is probably why the problem is not seen.
>> In the vhd_read_block function it is correct.
>>
>> Signed-off-by: Ross Philipson <ross.philip...@ainfosec.com>
>> ---
>> diff --git a/tools/blktap2/vhd/lib/libvhd.c b/tools/blktap2/vhd/lib/libvhd.c
>> index 1fd5b4e..4ebe012 100644
>> --- a/tools/blktap2/vhd/lib/libvhd.c
>> +++ b/tools/blktap2/vhd/lib/libvhd.c
>> @@ -2188,7 +2188,7 @@ vhd_write_block(vhd_context_t *ctx, uint32_t block, 
>> char *data)
>> if (block >= ctx->bat.entries)
>> return -ERANGE;
>>
>> -   if ((unsigned long)data & ~(VHD_SECTOR_SIZE -1))
>> +   if ((unsigned long)data & (VHD_SECTOR_SIZE -1))
> 
> Notwithstanding the discussion in the rest of the thread, this is a
> clearly-correct bugfix to the code in tree, so I have queued it.
> 
> However, I had to do so rather manually because something had mangled
> the whitespace.  Ross, can you check your patch submission path,
> please ?
> 
> Ian.
> 

I went back and checked the original email I sent and I don't see any problems.
What part was mangled?

Thanks

-- 
Ross Philipson

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


Re: [Xen-devel] [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block

2016-03-30 Thread Ross Philipson
On 03/29/2016 07:26 AM, George Dunlap wrote:
> On Fri, Mar 25, 2016 at 4:34 PM, Ross Philipson
> <ross.philip...@gmail.com> wrote:
>> On 03/25/2016 12:11 PM, Wei Liu wrote:
>>> On Thu, Mar 17, 2016 at 06:10:59PM -0400, Ross Philipson wrote:
>>>> It seems the logic is meant to detect sector unaligned buffers for block
>>>> writes. The NOTing of the logic instead masks off any unaligned bits and
>>>> also would cause the function to always fail. It seems the function is not
>>>> used in any of the tools so that is probably why the problem is not seen.
>>>> In the vhd_read_block function it is correct.
>>>>
>>>> Signed-off-by: Ross Philipson <ross.philip...@ainfosec.com>
>>>
>>> This seems to fall under tools umbrella. I've look at blktap2 code,
>>> the reasoning is convincing to me so:
>>>
>>>   Acked-by: Wei Liu <wei.l...@citrix.com>
>>>
>>> But I've never used blktap2 and we don't normally test it so I would
>>> also wait a bit longer to see if anybody objects to this change.
>>>
>>> OOI if no code was using this, how did you discover this problem?
>>
>> We have an old fork of blktap2 from way back when. I was working to extract 
>> our
>> changes and turn them into patches so we could rebase on upstream blktap2.
>> Someone long ago found that - I have no idea how but it was a valid fix so I
>> upstreamed it.
>>
>> There are a number of other ones that were found that are still in upstream
>> blktap2 - I plan to send them too.
> 
> Ross,
> 
> Instead of cross-porting fixes from XenServer's blktap3 to the
> bitrotting blktap2, would you consider taking up my work on replacing
> blktap2 with building blktap3 as an external project?

George, sorry I had to sort out some red tape before I could reply. For the
record I am not cross porting from blktap3 but upstreaming from a very old fork
of blktap2 that came from XenServer.

But really that is of little concern because I think your idea is a really good
one. Yes I would be up for taking on that work and we would very much like to
re-base our project on blktap3 too. I cannot give you an exact date when I would
start on it but probably I could start at the beginning of May. In the mean time
I will spend time familiarizing myself with blktap3 and I will probably have
some question for you too...

Thanks

> 
> The basic shape of things that need to happen for that are:
> 
> 1. Allow qemu to provide emulated devices for disks which use hotplug
> scripts (just checked in last week)
> 
> 2. Do the tweaks necessary to allow blktap3 to be built as an
> independent project (outside the XenServer build system)
> 
> 3. Switch libxl to use the already-in-tree block-tap script (which
> calls tapctl) rather than linking against the blktap library.
> 
> 4. Add a blktap target to raisin.
> 
> #1 is done and was just checked in last week.  #2 shouldn't be a
> terribly large amount of work.  For #3, I've posted patches already,
> and I can probably do a quick rebase for you to pick up if you wanted.
> #4 shouldn't be too hard once you have an out-of-tree build working; I
> can help with that as well.
> 
> If we then add tests for upstream blktap to osstest, then we'll catch
> any breakages, and automatically get updates; and we can delete the
> bitrotting blktap2 out of the tree.
> 
> What do you think?
> 
>  -George
> 


-- 
Ross Philipson

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


Re: [Xen-devel] [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block

2016-03-25 Thread Ross Philipson
On 03/25/2016 12:11 PM, Wei Liu wrote:
> On Thu, Mar 17, 2016 at 06:10:59PM -0400, Ross Philipson wrote:
>> It seems the logic is meant to detect sector unaligned buffers for block
>> writes. The NOTing of the logic instead masks off any unaligned bits and
>> also would cause the function to always fail. It seems the function is not
>> used in any of the tools so that is probably why the problem is not seen.
>> In the vhd_read_block function it is correct.
>>
>> Signed-off-by: Ross Philipson <ross.philip...@ainfosec.com>
> 
> This seems to fall under tools umbrella. I've look at blktap2 code,
> the reasoning is convincing to me so:
> 
>   Acked-by: Wei Liu <wei.l...@citrix.com>
> 
> But I've never used blktap2 and we don't normally test it so I would
> also wait a bit longer to see if anybody objects to this change.
> 
> OOI if no code was using this, how did you discover this problem?

We have an old fork of blktap2 from way back when. I was working to extract our
changes and turn them into patches so we could rebase on upstream blktap2.
Someone long ago found that - I have no idea how but it was a valid fix so I
upstreamed it.

There are a number of other ones that were found that are still in upstream
blktap2 - I plan to send them too.

> 
> Wei.
> 
>> ---
>> diff --git a/tools/blktap2/vhd/lib/libvhd.c b/tools/blktap2/vhd/lib/libvhd.c
>> index 1fd5b4e..4ebe012 100644
>> --- a/tools/blktap2/vhd/lib/libvhd.c
>> +++ b/tools/blktap2/vhd/lib/libvhd.c
>> @@ -2188,7 +2188,7 @@ vhd_write_block(vhd_context_t *ctx, uint32_t block, 
>> char *data)
>> if (block >= ctx->bat.entries)
>> return -ERANGE;
>>
>> -   if ((unsigned long)data & ~(VHD_SECTOR_SIZE -1))
>> +   if ((unsigned long)data & (VHD_SECTOR_SIZE -1))
>> return -EINVAL;
>>
>> blk  = ctx->bat.bat[block];
>>
>> ___
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel


-- 
Ross Philipson

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


[Xen-devel] [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block

2016-03-19 Thread Ross Philipson
It seems the logic is meant to detect sector unaligned buffers for block
writes. The NOTing of the logic instead masks off any unaligned bits and
also would cause the function to always fail. It seems the function is not
used in any of the tools so that is probably why the problem is not seen.
In the vhd_read_block function it is correct.

Signed-off-by: Ross Philipson <ross.philip...@ainfosec.com>
---
diff --git a/tools/blktap2/vhd/lib/libvhd.c b/tools/blktap2/vhd/lib/libvhd.c
index 1fd5b4e..4ebe012 100644
--- a/tools/blktap2/vhd/lib/libvhd.c
+++ b/tools/blktap2/vhd/lib/libvhd.c
@@ -2188,7 +2188,7 @@ vhd_write_block(vhd_context_t *ctx, uint32_t block, char 
*data)
if (block >= ctx->bat.entries)
return -ERANGE;

-   if ((unsigned long)data & ~(VHD_SECTOR_SIZE -1))
+   if ((unsigned long)data & (VHD_SECTOR_SIZE -1))
return -EINVAL;

blk  = ctx->bat.bat[block];

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


Re: [Xen-devel] [PATCH] tools: remove xenpmd

2016-02-22 Thread Ross Philipson
On 02/19/2016 12:12 PM, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [PATCH] tools: remove xenpmd"):
>> On Wed, 2016-02-17 at 12:05 -0500, Konrad Rzeszutek Wilk wrote:
>>> On Wed, Feb 17, 2016 at 02:41:14PM +, Ian Campbell wrote:
>>>> It has four instances of the same CID 1198719 (strncpy(..., 32) into a
>>>> 32-byte buffer).
>>>>
>>>> However ever since it was checked in in 2008 it has contained a
>>>> /tmp instead of the real ones in /proc. I therefore conclude that it
>>>> has not ever been used in its current form in the last 8 years and
>>>> might as well therefore be deleted. The code remains in the git
>>>> history should anyone one to reanimate it.
>>>>
>>>> Searching online shows no references to the use of this tool.
>>>>
>>>> Signed-off-by: Ian Campbell <ian.campb...@citrix.com>
>>>
>>> Acked-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
>>>
>>> .. Can I commit this in now :-)
>>
>> I think formally it probably needs an Ack from Wei or Ian?
> 
> I think it probably does, and:
> 
> Acked-by: Ian Jackson <ian.jack...@eu.citrix.com>
> 
> But:
> 
>> I usually give this sort of thing (removals etc) a little longer than
>> normal to give more of a chance for objections, although the counter to
>> that is that once this is committed it could just as easily be reverted the
>> next day as in 6 months time if someone turns up with a good reason it
>> shouldn't have been nuked.
> 
> Deleting it right away seems a bit rude.  Maybe someone is using it
> with the #define sorted out.  So if nothing intervenes I guess we
> should commit this late next week ?
> 
> Ian.

I was around when this was first created. It requires bits in QEMU to
make it work which I don't think were ever upstreamed. It is also tied
to one of the ACPI SSDTs. If you are removing it you may need to look
into that module too:

tools/firmware/hvmloader/acpi/ssdt_pm.asl

I am not weighing in one whether or not to remove it, just giving some
supplemental info...

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


-- 
Ross Philipson

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


Re: [Xen-devel] [RFC Design Doc] Add vNVDIMM support for Xen

2016-02-06 Thread Ross Philipson
On 02/05/2016 08:43 PM, Haozhong Zhang wrote:
> On 02/05/16 09:40, Ross Philipson wrote:
>> On 02/03/2016 09:09 AM, Andrew Cooper wrote:
> [...]
>>> I agree.
>>>
>>> There has to be a single entity responsible for collating the eventual
>>> ACPI handed to the guest, and this is definitely HVMLoader.
>>>
>>> However, it is correct that Qemu create the ACPI tables for the devices
>>> it emulates for the guest.
>>>
>>> We need to agree on a mechanism whereby each entity can provide their
>>> own subset of the ACPI tables to HVMLoader, and have HVMLoader present
>>> the final set properly to the VM.
>>>
>>> There is an existing usecase of passing the Host SLIC table to a VM, for
>>> OEM Versions of Windows.  I believe this is achieved with
>>> HVM_XS_ACPI_PT_{ADDRESS,LENGTH}, but that mechanism is a little
>>> inflexible and could probably do with being made a little more generic.
>>
>> A while back I added a generic mechanism to load extra ACPI tables into a
>> guest, configurable at runtime. It looks like the functionality is still
>> present. That might be an option.
>>
>> Also, following the thread, it wasn't clear if some of the tables like the
>> SSDT for the NVDIMM device and it's _FIT/_DSM methods were something that
>> could be statically created at build time. If it is something that needs to
>> be generated at runtime (e.g. platform specific), I have a library that can
>> generate any AML on the fly and create SSDTs.
>>
>> Anyway just FYI in case this is helpful.
>>
> 
> Hi Ross,
> 
> Thanks for the information!
> 
> SSDT for NVDIMM devices can not be created statically, because the
> number of some items in it can not be determined at build time. For
> example, the number of NVDIMM ACPI namespace devices (_DSM is under it)
> defined in SSDT is determined by the number of vNVDIMM devices in domain
> configuration. FYI, a sample SSDT for NVDIMM looks like
> 
>   Scope (\_SB){
>   Device (NVDR) // NVDIMM Root device
>   {
>   Name (_HID, “ACPI0012”)
>   Method (_STA) {...}
>   Method (_FIT) {...}
>   Method (_DSM, ...) {
>   ...
>   }
>   }
> 
>   Device (NVD0) // 1st NVDIMM Device
>   {
>   Name(_ADR, h0)
>   Method (_DSM, ...) {
>   ...
>   }
>   }
> 
>   Device (NVD1) // 2nd NVDIMM Device
>   {
>   Name(_ADR, h1)
>   Method (_DSM, ...) {
>   ...
>   }
>   }
> 
>   ...
>   }

Makes sense.

> 
> I had ported QEMU's AML builder code as well as NVDIMM ACPI building
> code to hvmloader and it did work, but then there was too much
> duplicated code for vNVDIMM between QEMU and hvmloader for vNVDIMM.
> Therefore, I prefer to let QEMU that emulates vNVDIMM devices
> to build those tables, as in Andrew and Jan's replies.

Yea it looks like QEUM's AML generating code is quite complete nowadays.
Back when I wrote my library there wasn't really much out there. Anyway
this is where it is if there is something that I might generate that is
missing:

https://github.com/OpenXT/xctools/tree/master/libxenacpi


> 
> Thanks,
> Haozhong
> 


-- 
Ross Philipson

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


Re: [Xen-devel] [RFC Design Doc] Add vNVDIMM support for Xen

2016-02-05 Thread Ross Philipson

On 02/03/2016 09:09 AM, Andrew Cooper wrote:

On 03/02/16 09:13, Jan Beulich wrote:

On 03.02.16 at 08:00, <haozhong.zh...@intel.com> wrote:

On 02/02/16 17:11, Stefano Stabellini wrote:

Once upon a time somebody made the decision that ACPI tables
on Xen should be static and included in hvmloader. That might have been
a bad decision but at least it was coherent. Loading only *some* tables
from QEMU, but not others, it feels like an incomplete design to me.

For example, QEMU is currently in charge of emulating the PCI bus, why
shouldn't it be QEMU that generates the PRT and MCFG?


To Keir, Jan and Andrew:

Are there anything related to ACPI that must be done (or are better to
be done) in hvmloader?

Some of the static tables (FADT and HPET come to mind) likely would
better continue to live in hvmloader. MCFG (for example) coming from
qemu, otoh, would be quite natural (and would finally allow MMCFG
support for guests in the first place). I.e. ...


I prefer switching to QEMU building all ACPI tables for devices that it
is emulating. However this alternative is good too because it is
coherent with the current design.

I would prefer to this one if the final conclusion is that only one
agent should be allowed to build guest ACPI. As I said above, it looks
like a big change to switch to QEMU for all ACPI tables and I'm afraid
it would break some existing guests.

... I indeed think that tables should come from qemu for components
living in qemu, and from hvmloader for components coming from Xen.


I agree.

There has to be a single entity responsible for collating the eventual
ACPI handed to the guest, and this is definitely HVMLoader.

However, it is correct that Qemu create the ACPI tables for the devices
it emulates for the guest.

We need to agree on a mechanism whereby each entity can provide their
own subset of the ACPI tables to HVMLoader, and have HVMLoader present
the final set properly to the VM.

There is an existing usecase of passing the Host SLIC table to a VM, for
OEM Versions of Windows.  I believe this is achieved with
HVM_XS_ACPI_PT_{ADDRESS,LENGTH}, but that mechanism is a little
inflexible and could probably do with being made a little more generic.


A while back I added a generic mechanism to load extra ACPI tables into 
a guest, configurable at runtime. It looks like the functionality is 
still present. That might be an option.


Also, following the thread, it wasn't clear if some of the tables like 
the SSDT for the NVDIMM device and it's _FIT/_DSM methods were something 
that could be statically created at build time. If it is something that 
needs to be generated at runtime (e.g. platform specific), I have a 
library that can generate any AML on the fly and create SSDTs.


Anyway just FYI in case this is helpful.



~Andrew

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




--
Ross Philipson

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


Re: [Xen-devel] [PATCH V4 3/7] libxl: add pvusb API

2015-06-17 Thread Ross Philipson

On 06/17/2015 12:03 AM, Jürgen Groß wrote:

On 06/16/2015 06:51 PM, Ross Philipson wrote:

On 06/16/2015 12:39 PM, George Dunlap wrote:

On 06/16/2015 05:32 PM, Ian Jackson wrote:

I have just discovered that the value used in /dev/disk/by-path is not
from sysfs, or at least, not directly.

udev cobbles it together with a bunch of string mangling, from
information mostly from sysfs.  There is no corresponding thing for
usb devices.

So Linux, the kernel, does not actually provide a stable device name
string.  This is obviously absurd, but I think fixing it is out of
scope.

I suggest we provide a facility to allow a user to specify a fnmatch
glob pattern to be applied to the sysfs path.  That way when they see
their device is
   /devices/pci:00/:00:1d.7/usb1/1-1
they can write
   /devices/pci:00/:00:1d.7/usb*/*-1
which will match exactly and only the right thing.


What about Juergen's system that has two usbN directories in a single
pci node?

Quoting:
---
Hmm, perhaps. On my system I've got:

/sys/devices/pci:00/:00:14.0/usb3/
/sys/devices/pci:00/:00:14.0/usb4/

So two busses on one pci bus address. Are usb3 and usb4 always in this
order or are they sometimes just numbered the other way round?
---

Assuming that usb3 and usb4 are actually distinct busses, and they might
both have something plugged into port; in which case a glob like this:

devices/pci:00/:00:14.0/usb*/*-1

Might match both of the following:

/sys/devices/pci:00/:00:14.0/usb3/3-1
/sys/devices/pci:00/:00:14.0/usb4/4-1


Is that an xHCI host controller? If so that might be how the system
represents the 2 logical (USB2/USB3) root hubs - each as its own
separate bus.


See my other reply: this is the case. So Ian's suggestion would still
work.


Ack, saw it, thanks.




Juergen




--
Ross Philipson

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


Re: [Xen-devel] [PATCH V4 3/7] libxl: add pvusb API

2015-06-16 Thread Ross Philipson

On 06/16/2015 12:39 PM, George Dunlap wrote:

On 06/16/2015 05:32 PM, Ian Jackson wrote:

I have just discovered that the value used in /dev/disk/by-path is not
from sysfs, or at least, not directly.

udev cobbles it together with a bunch of string mangling, from
information mostly from sysfs.  There is no corresponding thing for
usb devices.

So Linux, the kernel, does not actually provide a stable device name
string.  This is obviously absurd, but I think fixing it is out of
scope.

I suggest we provide a facility to allow a user to specify a fnmatch
glob pattern to be applied to the sysfs path.  That way when they see
their device is
   /devices/pci:00/:00:1d.7/usb1/1-1
they can write
   /devices/pci:00/:00:1d.7/usb*/*-1
which will match exactly and only the right thing.


What about Juergen's system that has two usbN directories in a single
pci node?

Quoting:
---
Hmm, perhaps. On my system I've got:

/sys/devices/pci:00/:00:14.0/usb3/
/sys/devices/pci:00/:00:14.0/usb4/

So two busses on one pci bus address. Are usb3 and usb4 always in this
order or are they sometimes just numbered the other way round?
---

Assuming that usb3 and usb4 are actually distinct busses, and they might
both have something plugged into port; in which case a glob like this:

devices/pci:00/:00:14.0/usb*/*-1

Might match both of the following:

/sys/devices/pci:00/:00:14.0/usb3/3-1
/sys/devices/pci:00/:00:14.0/usb4/4-1


Is that an xHCI host controller? If so that might be how the system 
represents the 2 logical (USB2/USB3) root hubs - each as its own 
separate bus.




  -George

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




--
Ross Philipson

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


Re: [Xen-devel] ACPI shutdown unreliable with win7?

2015-05-29 Thread Ross Philipson

On 05/29/2015 12:00 PM, Paul Durrant wrote:

-Original Message-
From: Ross Philipson [mailto:ross.philip...@gmail.com]
Sent: 29 May 2015 16:35
To: Ian Campbell; Paul Durrant
Cc: Andrew Cooper; xen-devel; Jan Beulich; Ian Jackson
Subject: Re: [Xen-devel] ACPI shutdown unreliable with win7?

On 05/29/2015 11:11 AM, Ian Campbell wrote:

On Fri, 2015-05-29 at 16:06 +0100, Paul Durrant wrote:

FWIW We appear to wait 200s, if we were seeing failures due to

windows

update then I'd be inclined to extend that, but I think right now that
would be premature, unless WU happens with no status on the screen.



No, you'd see something. Perhaps our ACPI lid/power switch code is just

buggy then?


It seems to work reliably for the WinXP tests, FWIW...

Ian.



One thing I find confusing is that the firmware code does not even have
a power button device (PNP0C0C) or the fixed feature power button that
is enabled in the FADT (flag == PWR_BUTTON bit 4). So I don't see how
the shutdown is purely an ACPI function. Is there something else to the
story? Is it relying on PV tools to do it?



Xen (and QEMU seemingly) implement the 'Fixed Power Button' (section 
4.8.2.2.1.1 on my spec) and this requires the PWR_BUTTON flag to be clear 
(according table 4-13). It also does not require a power button device to be 
implemented (which is presumably why this way of doing it was chosen).


Ah got it. I read the spec backwards - that the bit was set not clear 
for the fixed feature power button :)





   Paul


Ross




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




--
Ross Philipson



--
Ross Philipson

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


Re: [Xen-devel] ACPI shutdown unreliable with win7?

2015-05-29 Thread Ross Philipson

On 05/29/2015 11:11 AM, Ian Campbell wrote:

On Fri, 2015-05-29 at 16:06 +0100, Paul Durrant wrote:

FWIW We appear to wait 200s, if we were seeing failures due to windows
update then I'd be inclined to extend that, but I think right now that
would be premature, unless WU happens with no status on the screen.



No, you'd see something. Perhaps our ACPI lid/power switch code is just buggy 
then?


It seems to work reliably for the WinXP tests, FWIW...

Ian.



One thing I find confusing is that the firmware code does not even have 
a power button device (PNP0C0C) or the fixed feature power button that 
is enabled in the FADT (flag == PWR_BUTTON bit 4). So I don't see how 
the shutdown is purely an ACPI function. Is there something else to the 
story? Is it relying on PV tools to do it?


Ross




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




--
Ross Philipson

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


Re: [Xen-devel] ACPI shutdown unreliable with win7?

2015-05-29 Thread Ross Philipson

On 05/29/2015 12:00 PM, Paul Durrant wrote:

-Original Message-
From: Ross Philipson [mailto:ross.philip...@gmail.com]
Sent: 29 May 2015 16:35
To: Ian Campbell; Paul Durrant
Cc: Andrew Cooper; xen-devel; Jan Beulich; Ian Jackson
Subject: Re: [Xen-devel] ACPI shutdown unreliable with win7?

On 05/29/2015 11:11 AM, Ian Campbell wrote:

On Fri, 2015-05-29 at 16:06 +0100, Paul Durrant wrote:

FWIW We appear to wait 200s, if we were seeing failures due to

windows

update then I'd be inclined to extend that, but I think right now that
would be premature, unless WU happens with no status on the screen.



No, you'd see something. Perhaps our ACPI lid/power switch code is just

buggy then?


It seems to work reliably for the WinXP tests, FWIW...

Ian.



One thing I find confusing is that the firmware code does not even have
a power button device (PNP0C0C) or the fixed feature power button that
is enabled in the FADT (flag == PWR_BUTTON bit 4). So I don't see how
the shutdown is purely an ACPI function. Is there something else to the
story? Is it relying on PV tools to do it?



Xen (and QEMU seemingly) implement the 'Fixed Power Button' (section 
4.8.2.2.1.1 on my spec) and this requires the PWR_BUTTON flag to be clear 
(according table 4-13). It also does not require a power button device to be 
implemented (which is presumably why this way of doing it was chosen).


which is presumably why this way of doing it was chosen

Yea it is simpler plus the power button device would then need a bunch 
of GPE plumbing to get events.


Ross


   Paul


Ross




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




--
Ross Philipson



--
Ross Philipson

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