[Xen-devel] XC_PAGE_SIZE or XEN_PAGE_SIZE?

[Oleksandr Andrushchenko]

Hi, all!

While working on display protocol I found that there is no(?) common

???_PAGE_SIZE define I can use for both Xen and Linux kernel:

Xen defines XC_PAGE_SIZE which is also used in Linux user-space and

kernel has XEN_PAGE_SIZE, but no XC_PAGE_SIZE.

So, the question is which define should I use? Or just replace it with

some number, like fbif does:

#define XENFB_OUT_RING_SIZE 2048

Thank you,

Oleksandr


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

Re: [Xen-devel] [PATCH v4 06/15] domctl: Add XEN_DOMCTL_acpi_access

[Jan Beulich]

>>> On 01.12.16 at 17:43,  wrote:
> On 12/01/2016 11:06 AM, Jan Beulich wrote:
>>
>>> +++ b/xen/include/public/domctl.h
>>> @@ -1144,6 +1144,29 @@ struct xen_domctl_psr_cat_op {
>>>  typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
>>>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
>>>  
>>> +/* ACPI Generic Address Structure */
>>> +typedef struct gas {
>> xen_acpi_gas
>>
>>> +#define XEN_ACPI_SYSTEM_MEMORY 0
>>> +#define XEN_ACPI_SYSTEM_IO 1
>>> +uint8_tspace_id;   /* Address space */
>>> +uint8_tbit_width;  /* Size in bits of given register */
>>> +uint8_tbit_offset; /* Bit offset within the register */
>>> +uint8_taccess_width;   /* Minimum Access size (ACPI 3.0) */
>>> +uint64_t   address;/* 64-bit address of register */
>> uint64_aligned_t with explicit padding added ahead of it.
>>
>> And then there's the question of what uses of this will look like:
>> I'm not convinced we need to stick to the exact ACPI layout
>> here, unless you expect (or could imagine) for the tool stack to
>> hold GAS structures coming from elsewhere in its hands. If we
>> don't follow the layout as strictly, we could namely widen
>> bit_width (and maybe bit_offset) to allow for larger transfers
>> in one go. And in such a relaxed model I don't think we'd need
>> access_width at all as a field.
> 
> There is indeed no current need to use actual ACPI GAS layout but then
> it's not GAS, really, and should be named something else.

Which of course is fine by me; I had referred to that structure only
for the underlying principle of specifying how to access the data.

Jan


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

Re: [Xen-devel] [PATCH] AMD IOMMU: Support IOAPIC IDs larger than 128

[Jan Beulich]

>>> On 02.12.16 at 04:48,  wrote:
> On 12/01/2016 06:58 PM, Jan Beulich wrote:
> On 01.12.16 at 12:04,  wrote:
>>> Currently, the driver uses the APIC ID to index into the ioapic_sbdf array.
>>> The current MAX_IO_APICS is 128, which causes the driver initialization
>>> to fail on the system with IOAPIC ID >= 128.
>>>
>>> Instead, this patch adds APIC ID in the struct ioapic_sbdf,
>>> which is used to match the entry when searching through the array.
>>
>> Wouldn't it have been a lot simpler to just bump the array size to
>> 256? I'll comment on the rest of the patch anyway ...
> 
> Yes, it would, and that's what I originally tried. However, I was 
> thinking that it would be unnecessarily waste of space since that would 
> affect serveral structures i.e.
>  * mp_ioapic_routing[MAX_IO_APICS]
>  * mp_ioapics[MAX_IO_APICS]
>  * nr_ioapic_entries[MAX_IO_APICS]
>  * vector_map[MAX_IO_APICS]

I don't understand - these arrays aren't indexed by IO-APIC ID,
so why would they need to grow? Note that I specifically did not
suggest to bump MAX_IO_APICS, but only to ...

>  * ioapic_sbdf[MAX_IO_APICS]

... grow this one array (or alternatively index it the same way
the others are being indexed).

> If you think this is a reasonable change, I can simplify the patch. The 
> number 256 should be reasonable for x1APIC since it only supports 8-bit 
> APIC ID. However, this might be an issue later on with 32-bit APIC ID w/ 
> x2APIC.

How would such a wider ID be communicated? I don't see any
MADT sub-table structure other than type 1, which has an 8-bit
ID field.

Jan


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

[Xen-devel] [ovmf baseline-only test] 68147: all pass

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 68147 ovmf real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/68147/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf eae631bc687c3cbfab01632ce3bc21783d22b590
baseline version:
 ovmf e148e6e9625f8a0054f131bacba4e5c9a21a4377

Last test of basis68132  2016-12-01 00:21:34 Z1 days
Testing same since68147  2016-12-01 19:23:26 Z0 days1 attempts


People who touched revisions under test:
  Ard Biesheuvel 
  Feng Tian 
  Jan Dabros 
  Jeff Fan 
  Jiewen Yao 
  Laszlo Ersek 
  Marcin Wojtas 
  Michael Kinney 
  Richard Thomaiyar 
  Ryan Harkin 
  Thomaiyar, Richard Marian 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass



sg-report-flight on osstest.xs.citrite.net
logs: /home/osstest/logs
images: /home/osstest/images

Logs, config files, etc. are available at
http://osstest.xs.citrite.net/~osstest/testlogs/logs

Test harness code can be found at
http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary


Push not applicable.


commit eae631bc687c3cbfab01632ce3bc21783d22b590
Author: Michael Kinney 
Date:   Tue Nov 29 12:01:44 2016 -0800

Vlv2TbltDevicePkg: Add /m flag for multi-processor build

https://bugzilla.tianocore.org/show_bug.cgi?id=274

Add support for multi-processor builds using a /m flag.

Cc: Jiewen Yao 
Cc: David Wei 
Cc: Mang Guo 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael Kinney 
Reviewed-by: Jiewen Yao 

commit eafde7a221b981c0458b4620a9c11cb6e8947e9e
Author: Michael Kinney 
Date:   Tue Nov 29 11:28:07 2016 -0800

Vlv2TbltDevicePkg: Add /y flag to generate report files

https://bugzilla.tianocore.org/show_bug.cgi?id=273

Update build script to generate a report file and put
both the report file and the log file in the directory
Vlv2TbltDevicePkg with an EDK2_Vlv2TbltDevicePkg prefix.

Cc: Jiewen Yao 
Cc: David Wei 
Cc: Mang Guo 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael Kinney 
Reviewed-by: Jiewen Yao 

commit e2c3248699ee8455a74af6382ab99fb905e0890c
Author: Michael Kinney 
Date:   Tue Nov 29 11:19:20 2016 -0800

Vlv2TbltDevicePkg: Fix typo in name of nul output file

https://bugzilla.tianocore.org/show_bug.cgi?id=272

Fix typo in script file.  To prevent output from being
shown, then output file should be 'nul', not 'null'.

Cc: Jiewen Yao 
Cc: David Wei 
Cc: Mang Guo 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael Kinney 
Reviewed-by: Jiewen Yao 

commit eee350c067e008a8ea0a3c6548448d410b14a1b9
Author: Michael Kinney 
Date:   Tue Nov 29 10:33:36 2016 -0800

Vlv2TbltDevicePkg: Use 4K aligned PE/COFF sections

Update [BuildOptions] to use of 4K aligned PE/COFF
image sections to support page level protection of
DXE_RUNTIME_DRIVER, SMM_CORE, and DXE_SMM_DRIVER
modules.

Cc: Jiewen Yao 
Cc: David Wei 
Cc: Mang Guo 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael Kinney 
Reviewed-by: Jiewen Yao 

commit 

Re: [Xen-devel] [ARM] Handling CMA pool device nodes in Dom0

[Peng Fan]

On Tue, Nov 29, 2016 at 01:49:51PM +, Julien Grall wrote:
>(CC Stefano)
>
>On 25/11/16 12:19, Iurii Mykhalskyi wrote:
>>Hello!
>
>Hi Iurii,
>
>>
>>I'm working under Renesas Gen3 H3 board with 4GB RAM (Salvator-X)
>>support in Xen mainline.
>>
>>Salvator-X has several  CMA pool nodes, for example:
>>
>>1:
>>adsp_reserved: linux,adsp {
>>compatible = "shared-dma-pool";
>>reusable;
>>reg = <0x 0x5700 0x0 0x0100>;
>>};
>>
>>2:
>>linux,cma {
>>compatible = "shared-dma-pool";
>>reusable;
>>reg = <0x 0x5800 0x0 0x1800>;
>>linux,cma-default;
>>};
>>
>>During Dom0 allocation, we can't guarantee, that allocated memory will
>>contain mentioned regions.
>>In second сase, we can actually hardcode mapped region by using separate
>>DTS for Dom0 with changed memory regions.
>>But for first one, this in not an option - this pool is used for audio
>>DSP and its firmware relies on this addresses.
>>
>>What is the correct way to solve this situation?
>>Does Xen has some mechanism to handle such cases?

How about using "alloc-ranges"?

Regards,
Peng.

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

Re: [Xen-devel] ARM: Request backport for Xen 4.6 and Xen 4.5

[Jan Beulich]

>>> On 01.12.16 at 18:58,  wrote:
> While testing XSA-201, I noticed that Xen 4.5 and 4.5 were not able to 
> boot on the Foundation Model [1].
> 
> The Foundation Model is a free model provided by ARM for ARMv8 which is 
> supported by Xen since the beginning.
> 
> I am able to Xen boot after applying the patch:
> 
> eb6fe7a "arm64: fix incorrect memory region size in TCR_EL2"
> 
> So would it be possible to backport this patch in Xen 4.6 and Xen 4.5?

I'll let Stefano take care of the 4.6 part (if he deems it reasonable),
but I'd like to point out that 4.5 is closed for non-security fixes (see
lists.xenproject.org/archives/html/xen-announce/2016-09/msg4.html).

Jan
--- Begin Message ---
All,

I am pleased to announce the release of Xen 4.5.5. This is
available immediately from its git repository
https://xenbits.xen.org/gitweb/?p=xen.git;a=shortlog;h=refs/heads/stable-4.5 
(tag RELEASE-4.5.5) or from the XenProject download page
https://www.xenproject.org/downloads/xen-archives/xen-45-series/xen-455.html 
(where a list of changes can also be found).

Note that this is the last XenProject coordinated release of the 4.5
stable series. The tree will be switched to security only maintenance
mode after this release.

We recommend all users of the 4.5 stable series to update to this
last point release.

Note regarding 4.5.4: An issue was found late in the release process,
after the various trees were already tagged (in fact, one of those
tags was wrong). We therefore decided to skip version 4.5.4.

Regards, Jan

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

[Xen-devel] [PATCH v2] xen/scsifront: don't request a slot on the ring until request is ready

[Juergen Gross]

Instead of requesting a new slot on the ring to the backend early, do
so only after all has been setup for the request to be sent. This
makes error handling easier as we don't need to undo the request id
allocation and ring slot allocation.

Suggested-by: Jan Beulich 
Signed-off-by: Juergen Gross 
---
Aargh! Need more coffee! Resend with corrected mail address for Martin Petersen
---
 drivers/scsi/xen-scsifront.c | 190 +++
 1 file changed, 84 insertions(+), 106 deletions(-)

diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c
index c01316c..9aa1fe1 100644
--- a/drivers/scsi/xen-scsifront.c
+++ b/drivers/scsi/xen-scsifront.c
@@ -79,10 +79,13 @@
 struct vscsifrnt_shadow {
/* command between backend and frontend */
unsigned char act;
+   uint8_t nr_segments;
uint16_t rqid;
+   uint16_t ref_rqid;
 
unsigned int nr_grants; /* number of grants in gref[] */
struct scsiif_request_segment *sg;  /* scatter/gather elements */
+   struct scsiif_request_segment seg[VSCSIIF_SG_TABLESIZE];
 
/* Do reset or abort function. */
wait_queue_head_t wq_reset; /* reset work queue   */
@@ -172,68 +175,90 @@ static void scsifront_put_rqid(struct vscsifrnt_info 
*info, uint32_t id)
scsifront_wake_up(info);
 }
 
-static struct vscsiif_request *scsifront_pre_req(struct vscsifrnt_info *info)
+static int scsifront_do_request(struct vscsifrnt_info *info,
+   struct vscsifrnt_shadow *shadow)
 {
struct vscsiif_front_ring *ring = &(info->ring);
struct vscsiif_request *ring_req;
+   struct scsi_cmnd *sc = shadow->sc;
uint32_t id;
+   int i, notify;
+
+   if (RING_FULL(>ring))
+   return -EBUSY;
 
id = scsifront_get_rqid(info);  /* use id in response */
if (id >= VSCSIIF_MAX_REQS)
-   return NULL;
+   return -EBUSY;
+
+   info->shadow[id] = shadow;
+   shadow->rqid = id;
 
ring_req = RING_GET_REQUEST(&(info->ring), ring->req_prod_pvt);
-
ring->req_prod_pvt++;
 
-   ring_req->rqid = (uint16_t)id;
+   ring_req->rqid= id;
+   ring_req->act = shadow->act;
+   ring_req->ref_rqid= shadow->ref_rqid;
+   ring_req->nr_segments = shadow->nr_segments;
 
-   return ring_req;
-}
+   ring_req->id  = sc->device->id;
+   ring_req->lun = sc->device->lun;
+   ring_req->channel = sc->device->channel;
+   ring_req->cmd_len = sc->cmd_len;
 
-static void scsifront_do_request(struct vscsifrnt_info *info)
-{
-   struct vscsiif_front_ring *ring = &(info->ring);
-   int notify;
+   BUG_ON(sc->cmd_len > VSCSIIF_MAX_COMMAND_SIZE);
+
+   memcpy(ring_req->cmnd, sc->cmnd, sc->cmd_len);
+
+   ring_req->sc_data_direction   = (uint8_t)sc->sc_data_direction;
+   ring_req->timeout_per_command = sc->request->timeout / HZ;
+
+   for (i = 0; i < (shadow->nr_segments & ~VSCSIIF_SG_GRANT); i++)
+   ring_req->seg[i] = shadow->seg[i];
 
RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(ring, notify);
if (notify)
notify_remote_via_irq(info->irq);
+
+   return 0;
 }
 
-static void scsifront_gnttab_done(struct vscsifrnt_info *info, uint32_t id)
+static void scsifront_gnttab_done(struct vscsifrnt_info *info,
+ struct vscsifrnt_shadow *shadow)
 {
-   struct vscsifrnt_shadow *s = info->shadow[id];
int i;
 
-   if (s->sc->sc_data_direction == DMA_NONE)
+   if (shadow->sc->sc_data_direction == DMA_NONE)
return;
 
-   for (i = 0; i < s->nr_grants; i++) {
-   if (unlikely(gnttab_query_foreign_access(s->gref[i]) != 0)) {
+   for (i = 0; i < shadow->nr_grants; i++) {
+   if (unlikely(gnttab_query_foreign_access(shadow->gref[i]))) {
shost_printk(KERN_ALERT, info->host, KBUILD_MODNAME
 "grant still in use by backend\n");
BUG();
}
-   gnttab_end_foreign_access(s->gref[i], 0, 0UL);
+   gnttab_end_foreign_access(shadow->gref[i], 0, 0UL);
}
 
-   kfree(s->sg);
+   kfree(shadow->sg);
 }
 
 static void scsifront_cdb_cmd_done(struct vscsifrnt_info *info,
   struct vscsiif_response *ring_rsp)
 {
+   struct vscsifrnt_shadow *shadow;
struct scsi_cmnd *sc;
uint32_t id;
uint8_t sense_len;
 
id = ring_rsp->rqid;
-   sc = info->shadow[id]->sc;
+   shadow = info->shadow[id];
+   sc = shadow->sc;
 
BUG_ON(sc == NULL);
 
-   scsifront_gnttab_done(info, id);
+   scsifront_gnttab_done(info, shadow);
scsifront_put_rqid(info, id);
 
sc->result = ring_rsp->rslt;
@@ -366,7 +391,6 @@ static void 

[Xen-devel] [PATCH v2] xen/scsifront: don't request a slot on the ring until request is ready

[Juergen Gross]

Instead of requesting a new slot on the ring to the backend early, do
so only after all has been setup for the request to be sent. This
makes error handling easier as we don't need to undo the request id
allocation and ring slot allocation.

Suggested-by: Jan Beulich 
Signed-off-by: Juergen Gross 
---
Resend with corrected mail address for Martin Peter
---
 drivers/scsi/xen-scsifront.c | 190 +++
 1 file changed, 84 insertions(+), 106 deletions(-)

diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c
index c01316c..9aa1fe1 100644
--- a/drivers/scsi/xen-scsifront.c
+++ b/drivers/scsi/xen-scsifront.c
@@ -79,10 +79,13 @@
 struct vscsifrnt_shadow {
/* command between backend and frontend */
unsigned char act;
+   uint8_t nr_segments;
uint16_t rqid;
+   uint16_t ref_rqid;
 
unsigned int nr_grants; /* number of grants in gref[] */
struct scsiif_request_segment *sg;  /* scatter/gather elements */
+   struct scsiif_request_segment seg[VSCSIIF_SG_TABLESIZE];
 
/* Do reset or abort function. */
wait_queue_head_t wq_reset; /* reset work queue   */
@@ -172,68 +175,90 @@ static void scsifront_put_rqid(struct vscsifrnt_info 
*info, uint32_t id)
scsifront_wake_up(info);
 }
 
-static struct vscsiif_request *scsifront_pre_req(struct vscsifrnt_info *info)
+static int scsifront_do_request(struct vscsifrnt_info *info,
+   struct vscsifrnt_shadow *shadow)
 {
struct vscsiif_front_ring *ring = &(info->ring);
struct vscsiif_request *ring_req;
+   struct scsi_cmnd *sc = shadow->sc;
uint32_t id;
+   int i, notify;
+
+   if (RING_FULL(>ring))
+   return -EBUSY;
 
id = scsifront_get_rqid(info);  /* use id in response */
if (id >= VSCSIIF_MAX_REQS)
-   return NULL;
+   return -EBUSY;
+
+   info->shadow[id] = shadow;
+   shadow->rqid = id;
 
ring_req = RING_GET_REQUEST(&(info->ring), ring->req_prod_pvt);
-
ring->req_prod_pvt++;
 
-   ring_req->rqid = (uint16_t)id;
+   ring_req->rqid= id;
+   ring_req->act = shadow->act;
+   ring_req->ref_rqid= shadow->ref_rqid;
+   ring_req->nr_segments = shadow->nr_segments;
 
-   return ring_req;
-}
+   ring_req->id  = sc->device->id;
+   ring_req->lun = sc->device->lun;
+   ring_req->channel = sc->device->channel;
+   ring_req->cmd_len = sc->cmd_len;
 
-static void scsifront_do_request(struct vscsifrnt_info *info)
-{
-   struct vscsiif_front_ring *ring = &(info->ring);
-   int notify;
+   BUG_ON(sc->cmd_len > VSCSIIF_MAX_COMMAND_SIZE);
+
+   memcpy(ring_req->cmnd, sc->cmnd, sc->cmd_len);
+
+   ring_req->sc_data_direction   = (uint8_t)sc->sc_data_direction;
+   ring_req->timeout_per_command = sc->request->timeout / HZ;
+
+   for (i = 0; i < (shadow->nr_segments & ~VSCSIIF_SG_GRANT); i++)
+   ring_req->seg[i] = shadow->seg[i];
 
RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(ring, notify);
if (notify)
notify_remote_via_irq(info->irq);
+
+   return 0;
 }
 
-static void scsifront_gnttab_done(struct vscsifrnt_info *info, uint32_t id)
+static void scsifront_gnttab_done(struct vscsifrnt_info *info,
+ struct vscsifrnt_shadow *shadow)
 {
-   struct vscsifrnt_shadow *s = info->shadow[id];
int i;
 
-   if (s->sc->sc_data_direction == DMA_NONE)
+   if (shadow->sc->sc_data_direction == DMA_NONE)
return;
 
-   for (i = 0; i < s->nr_grants; i++) {
-   if (unlikely(gnttab_query_foreign_access(s->gref[i]) != 0)) {
+   for (i = 0; i < shadow->nr_grants; i++) {
+   if (unlikely(gnttab_query_foreign_access(shadow->gref[i]))) {
shost_printk(KERN_ALERT, info->host, KBUILD_MODNAME
 "grant still in use by backend\n");
BUG();
}
-   gnttab_end_foreign_access(s->gref[i], 0, 0UL);
+   gnttab_end_foreign_access(shadow->gref[i], 0, 0UL);
}
 
-   kfree(s->sg);
+   kfree(shadow->sg);
 }
 
 static void scsifront_cdb_cmd_done(struct vscsifrnt_info *info,
   struct vscsiif_response *ring_rsp)
 {
+   struct vscsifrnt_shadow *shadow;
struct scsi_cmnd *sc;
uint32_t id;
uint8_t sense_len;
 
id = ring_rsp->rqid;
-   sc = info->shadow[id]->sc;
+   shadow = info->shadow[id];
+   sc = shadow->sc;
 
BUG_ON(sc == NULL);
 
-   scsifront_gnttab_done(info, id);
+   scsifront_gnttab_done(info, shadow);
scsifront_put_rqid(info, id);
 
sc->result = ring_rsp->rslt;
@@ -366,7 +391,6 @@ static void scsifront_finish_all(struct 

[Xen-devel] [PATCH v2] xen/scsifront: don't request a slot on the ring until request is ready

[Juergen Gross]

Instead of requesting a new slot on the ring to the backend early, do
so only after all has been setup for the request to be sent. This
makes error handling easier as we don't need to undo the request id
allocation and ring slot allocation.

Suggested-by: Jan Beulich 
Signed-off-by: Juergen Gross 
---
 drivers/scsi/xen-scsifront.c | 190 +++
 1 file changed, 84 insertions(+), 106 deletions(-)

diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c
index c01316c..9aa1fe1 100644
--- a/drivers/scsi/xen-scsifront.c
+++ b/drivers/scsi/xen-scsifront.c
@@ -79,10 +79,13 @@
 struct vscsifrnt_shadow {
/* command between backend and frontend */
unsigned char act;
+   uint8_t nr_segments;
uint16_t rqid;
+   uint16_t ref_rqid;
 
unsigned int nr_grants; /* number of grants in gref[] */
struct scsiif_request_segment *sg;  /* scatter/gather elements */
+   struct scsiif_request_segment seg[VSCSIIF_SG_TABLESIZE];
 
/* Do reset or abort function. */
wait_queue_head_t wq_reset; /* reset work queue   */
@@ -172,68 +175,90 @@ static void scsifront_put_rqid(struct vscsifrnt_info 
*info, uint32_t id)
scsifront_wake_up(info);
 }
 
-static struct vscsiif_request *scsifront_pre_req(struct vscsifrnt_info *info)
+static int scsifront_do_request(struct vscsifrnt_info *info,
+   struct vscsifrnt_shadow *shadow)
 {
struct vscsiif_front_ring *ring = &(info->ring);
struct vscsiif_request *ring_req;
+   struct scsi_cmnd *sc = shadow->sc;
uint32_t id;
+   int i, notify;
+
+   if (RING_FULL(>ring))
+   return -EBUSY;
 
id = scsifront_get_rqid(info);  /* use id in response */
if (id >= VSCSIIF_MAX_REQS)
-   return NULL;
+   return -EBUSY;
+
+   info->shadow[id] = shadow;
+   shadow->rqid = id;
 
ring_req = RING_GET_REQUEST(&(info->ring), ring->req_prod_pvt);
-
ring->req_prod_pvt++;
 
-   ring_req->rqid = (uint16_t)id;
+   ring_req->rqid= id;
+   ring_req->act = shadow->act;
+   ring_req->ref_rqid= shadow->ref_rqid;
+   ring_req->nr_segments = shadow->nr_segments;
 
-   return ring_req;
-}
+   ring_req->id  = sc->device->id;
+   ring_req->lun = sc->device->lun;
+   ring_req->channel = sc->device->channel;
+   ring_req->cmd_len = sc->cmd_len;
 
-static void scsifront_do_request(struct vscsifrnt_info *info)
-{
-   struct vscsiif_front_ring *ring = &(info->ring);
-   int notify;
+   BUG_ON(sc->cmd_len > VSCSIIF_MAX_COMMAND_SIZE);
+
+   memcpy(ring_req->cmnd, sc->cmnd, sc->cmd_len);
+
+   ring_req->sc_data_direction   = (uint8_t)sc->sc_data_direction;
+   ring_req->timeout_per_command = sc->request->timeout / HZ;
+
+   for (i = 0; i < (shadow->nr_segments & ~VSCSIIF_SG_GRANT); i++)
+   ring_req->seg[i] = shadow->seg[i];
 
RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(ring, notify);
if (notify)
notify_remote_via_irq(info->irq);
+
+   return 0;
 }
 
-static void scsifront_gnttab_done(struct vscsifrnt_info *info, uint32_t id)
+static void scsifront_gnttab_done(struct vscsifrnt_info *info,
+ struct vscsifrnt_shadow *shadow)
 {
-   struct vscsifrnt_shadow *s = info->shadow[id];
int i;
 
-   if (s->sc->sc_data_direction == DMA_NONE)
+   if (shadow->sc->sc_data_direction == DMA_NONE)
return;
 
-   for (i = 0; i < s->nr_grants; i++) {
-   if (unlikely(gnttab_query_foreign_access(s->gref[i]) != 0)) {
+   for (i = 0; i < shadow->nr_grants; i++) {
+   if (unlikely(gnttab_query_foreign_access(shadow->gref[i]))) {
shost_printk(KERN_ALERT, info->host, KBUILD_MODNAME
 "grant still in use by backend\n");
BUG();
}
-   gnttab_end_foreign_access(s->gref[i], 0, 0UL);
+   gnttab_end_foreign_access(shadow->gref[i], 0, 0UL);
}
 
-   kfree(s->sg);
+   kfree(shadow->sg);
 }
 
 static void scsifront_cdb_cmd_done(struct vscsifrnt_info *info,
   struct vscsiif_response *ring_rsp)
 {
+   struct vscsifrnt_shadow *shadow;
struct scsi_cmnd *sc;
uint32_t id;
uint8_t sense_len;
 
id = ring_rsp->rqid;
-   sc = info->shadow[id]->sc;
+   shadow = info->shadow[id];
+   sc = shadow->sc;
 
BUG_ON(sc == NULL);
 
-   scsifront_gnttab_done(info, id);
+   scsifront_gnttab_done(info, shadow);
scsifront_put_rqid(info, id);
 
sc->result = ring_rsp->rslt;
@@ -366,7 +391,6 @@ static void scsifront_finish_all(struct vscsifrnt_info 
*info)
 
 static int map_data_for_request(struct 

[Xen-devel] [xen-unstable baseline-only test] 68145: regressions - FAIL

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 68145 xen-unstable real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/68145/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-libvirt-vhd  9 debian-di-install fail REGR. vs. 68117
 test-amd64-amd64-libvirt  9 debian-installfail REGR. vs. 68117
 test-amd64-amd64-libvirt-pair 15 debian-install/dst_host  fail REGR. vs. 68117
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 20 leak-check/check fail REGR. 
vs. 68117
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 14 guest-saverestore.2 
fail REGR. vs. 68117

Regressions which are regarded as allowable (not blocking):
 test-xtf-amd64-amd64-25 xen-install  fail blocked in 68117
 test-amd64-amd64-i386-pvgrub  9 debian-di-installfail blocked in 68117
 test-xtf-amd64-amd64-5   10 xtf-fep  fail   like 68117
 test-xtf-amd64-amd64-4   10 xtf-fep  fail   like 68117
 test-xtf-amd64-amd64-3   10 xtf-fep  fail   like 68117
 test-xtf-amd64-amd64-1   10 xtf-fep  fail   like 68117
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 68117
 test-amd64-i386-xl-qemut-winxpsp3-vcpus1  9 windows-installfail like 68117
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1  9 windows-installfail like 68117

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-rumprun-amd64  1 build-check(1)   blocked  n/a
 test-amd64-i386-rumprun-i386  1 build-check(1)   blocked  n/a
 build-amd64-rumprun   6 xen-buildfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-armhf-armhf-xl-midway   12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-midway   13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 build-i386-rumprun6 xen-buildfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  a7a578ce6b8634eec30cb8445ea98e18d9b4e9b8
baseline version:
 xen  99a10da1b4fee8ef7a096e5fd3608f6c15932eb0

Last test of basis68117  2016-11-28 22:48:44 Z3 days
Testing same since68145  2016-12-01 14:22:14 Z0 days1 attempts


People who touched revisions under test:
  Dario Faggioli 
  George Dunlap 
  

[Xen-devel] [linux-4.1 test] 102755: regressions - FAIL

[osstest service owner]

flight 102755 linux-4.1 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/102755/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 6 xen-boot fail REGR. vs. 
101737
 test-amd64-i386-freebsd10-amd64  6 xen-boot  fail REGR. vs. 101737
 test-amd64-amd64-xl   6 xen-boot fail REGR. vs. 101737
 test-amd64-amd64-qemuu-nested-intel  6 xen-boot  fail REGR. vs. 101737
 test-amd64-amd64-xl-multivcpu  6 xen-bootfail REGR. vs. 101737
 test-amd64-amd64-xl-pvh-intel  6 xen-bootfail REGR. vs. 101737
 test-amd64-amd64-xl-qemuu-winxpsp3  6 xen-boot   fail REGR. vs. 101737
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm  6 xen-boot fail REGR. vs. 101737
 test-amd64-i386-xl-qemut-win7-amd64  6 xen-boot  fail REGR. vs. 101737
 test-amd64-i386-pair  9 xen-boot/src_hostfail REGR. vs. 101737
 test-amd64-i386-pair 10 xen-boot/dst_hostfail REGR. vs. 101737
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 6 xen-boot fail REGR. vs. 101737
 test-armhf-armhf-libvirt-xsm 14 guest-stop   fail REGR. vs. 101737
 build-armhf-pvops 5 kernel-build   fail in 102733 REGR. vs. 101737

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-libvirt-vhd 9 debian-di-install fail in 102733 pass in 102755
 test-amd64-i386-qemuu-rhel6hvm-intel  6 xen-boot   fail pass in 102733
 test-amd64-amd64-libvirt  6 xen-boot   fail pass in 102733
 test-amd64-i386-xl-raw6 xen-boot   fail pass in 102733
 test-amd64-amd64-xl-xsm  19 guest-start/debian.repeat  fail pass in 102733

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-libvirt 15 guest-start/debian.repeatfail  like 101672
 test-armhf-armhf-xl-rtds 15 guest-start/debian.repeatfail  like 101715
 test-armhf-armhf-xl-cubietruck 15 guest-start/debian.repeat   fail like 101715
 test-armhf-armhf-libvirt 13 saverestore-support-checkfail  like 101737
 test-armhf-armhf-xl-credit2  11 guest-start  fail  like 101737
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 101737
 test-armhf-armhf-xl  15 guest-start/debian.repeatfail  like 101737
 test-armhf-armhf-xl-multivcpu 15 guest-start/debian.repeatfail like 101737
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 101737
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail like 101737
 test-armhf-armhf-libvirt-qcow2  9 debian-di-install   fail like 101737
 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail  like 101737
 test-armhf-armhf-xl-vhd   9 debian-di-installfail  like 101737

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-multivcpu  1 build-check(1)  blocked in 102733 n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked in 102733 n/a
 test-armhf-armhf-libvirt-qcow2  1 build-check(1) blocked in 102733 n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked in 102733 n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked in 102733 n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked in 102733 n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked in 102733 n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1) blocked in 102733 n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked in 102733 n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked in 102733 n/a
 test-armhf-armhf-libvirt-xsm  1 build-check(1)   blocked in 102733 n/a
 test-armhf-armhf-xl-xsm   1 build-check(1)   blocked in 102733 n/a
 test-amd64-amd64-libvirt12 migrate-support-check fail in 102733 never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 12 

Re: [Xen-devel] [PATCH] AMD IOMMU: Support IOAPIC IDs larger than 128

[Suravee Suthikulpanit]

Hi Jan,

On 12/01/2016 06:58 PM, Jan Beulich wrote:

On 01.12.16 at 12:04,  wrote:

Currently, the driver uses the APIC ID to index into the ioapic_sbdf array.
The current MAX_IO_APICS is 128, which causes the driver initialization
to fail on the system with IOAPIC ID >= 128.

Instead, this patch adds APIC ID in the struct ioapic_sbdf,
which is used to match the entry when searching through the array.


Wouldn't it have been a lot simpler to just bump the array size to
256? I'll comment on the rest of the patch anyway ...


Yes, it would, and that's what I originally tried. However, I was 
thinking that it would be unnecessarily waste of space since that would 
affect serveral structures i.e.

* mp_ioapic_routing[MAX_IO_APICS]
* mp_ioapics[MAX_IO_APICS]
* nr_ioapic_entries[MAX_IO_APICS]
* vector_map[MAX_IO_APICS]
* ioapic_sbdf[MAX_IO_APICS]

If you think this is a reasonable change, I can simplify the patch. The 
number 256 should be reasonable for x1APIC since it only supports 8-bit 
APIC ID. However, this might be an issue later on with 32-bit APIC ID w/ 
x2APIC.


Thanks,
Suravee

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

Re: [Xen-devel] Intentional EPT Misconfigurations in xen

[唐伟文]

Hi,

Thanks a lot. I find that if guest writes or reads MSR related to
MTRR which will cause vm exit, xen will just calculate mtrr type and store
it without setting the EPT. Why doesn't xen set the memory type (ie,emt
filed)in entry of EPT when handling vm exit resulting from writing/reading
MSR related to MTRR?

I also find that xen will get the field of corresponding entries properly
set when handling EPT Misconfigurations. But memory type related to these
entries may be changed by guest after xen dose this. So how do we handle
these properly configured EPT entries which don't cause EPT
Misconfigurations any more when the memory type is changed by guest? Thanks




2016-11-28 18:02 GMT+08:00 Andrew Cooper :

> On 28/11/16 08:44, 唐伟文 wrote:
>
> Hi,
>
> I have a question that why dose xen hypervisor set entry of ept as invalid
> (misconfigured) deliberately which will cause VM exit resulting from EPT
> Misconfigurations. http://xenbits.xenproject.org/gitweb/
> ?p=xen.git;a=commit;h=aa9114edd97b292cd89b3616e3f2089471fd2201 I find the
> answer in this website which is about a patch of xen. They say that it is
> necessary to set EPT entry as misconfigured in order to force re-evaluation
> of memory type as necessary.
>
> But, I still don't konw why we need to reevaluate memory type? That is to
> say, why can't we determine memory type during the initialization of EPT. And
> if it is necessary to do this , which entry of EPT should be misconfigured
> intentionally?  Thanks
>
>
> EPT Memory types change at runtime, due to guest actions such as changing
> the MTRRs, changing CR0.CD, etc, or due to toolstack options such as
> enabling logdirty mode for live migration.
>
> ~Andrew
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [linux-3.18 test] 102754: regressions - FAIL

[osstest service owner]

flight 102754 linux-3.18 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/102754/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-multivcpu  6 xen-bootfail REGR. vs. 101675
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 6 xen-boot fail REGR. vs. 
101675
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 6 xen-boot fail REGR. vs. 
101675
 test-amd64-amd64-xl-qemut-debianhvm-amd64  6 xen-bootfail REGR. vs. 101675
 test-amd64-amd64-libvirt-xsm  6 xen-boot fail REGR. vs. 101675
 test-amd64-i386-pair  9 xen-boot/src_hostfail REGR. vs. 101675
 test-amd64-i386-pair 10 xen-boot/dst_hostfail REGR. vs. 101675
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 6 xen-boot fail REGR. vs. 101675
 test-amd64-amd64-libvirt-pair  9 xen-boot/src_host   fail REGR. vs. 101675
 test-amd64-amd64-libvirt-pair 10 xen-boot/dst_host   fail REGR. vs. 101675
 test-amd64-amd64-xl-qemut-win7-amd64  6 xen-boot fail REGR. vs. 101675
 test-amd64-amd64-amd64-pvgrub  6 xen-bootfail REGR. vs. 101675
 test-amd64-i386-libvirt-pair  9 xen-boot/src_hostfail REGR. vs. 101675
 test-amd64-i386-libvirt-pair 10 xen-boot/dst_hostfail REGR. vs. 101675
 test-amd64-amd64-qemuu-nested-intel  6 xen-boot  fail REGR. vs. 101675
 test-amd64-amd64-xl-qemuu-ovmf-amd64  6 xen-boot fail REGR. vs. 101675
 test-amd64-i386-xl-qemut-win7-amd64  6 xen-boot  fail REGR. vs. 101675
 build-i386-pvops  5 kernel-build   fail in 102732 REGR. vs. 101675

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-pair 9 xen-boot/src_host  fail pass in 102732
 test-amd64-amd64-pair10 xen-boot/dst_host  fail pass in 102732
 test-amd64-amd64-xl-xsm   6 xen-boot   fail pass in 102732

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds  9 debian-install  fail in 102732 like 101675
 test-armhf-armhf-libvirt 13 saverestore-support-checkfail  like 101675
 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail  like 101675
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 101675
 test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail  like 101675
 test-armhf-armhf-libvirt-qcow2 12 saverestore-support-check   fail like 101675

Tests which did not succeed, but are not blocking:
 test-amd64-i386-freebsd10-i386  1 build-check(1) blocked in 102732 n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked in 
102732 n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked in 
102732 n/a
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 1 build-check(1) blocked in 
102732 n/a
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 1 build-check(1) blocked in 102732 n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked in 102732 n/a
 test-amd64-i386-xl-qemuu-winxpsp3  1 build-check(1)  blocked in 102732 n/a
 test-amd64-i386-xl-qemut-winxpsp3  1 build-check(1)  blocked in 102732 n/a
 test-amd64-i386-xl-qemut-debianhvm-amd64 1 build-check(1) blocked in 102732 n/a
 test-amd64-i386-qemut-rhel6hvm-intel  1 build-check(1)   blocked in 102732 n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)blocked in 102732 n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked in 102732 n/a
 test-amd64-i386-xl1 build-check(1)   blocked in 102732 n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked in 102732 n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked in 102732 n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)blocked in 102732 n/a
 test-amd64-i386-xl-raw1 build-check(1)   blocked in 102732 n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1) blocked in 102732 n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked in 102732 n/a
 test-amd64-i386-qemuu-rhel6hvm-intel  1 build-check(1)   blocked in 102732 n/a
 test-amd64-i386-xl-qemut-winxpsp3-vcpus1 1 build-check(1) blocked in 102732 n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)blocked in 102732 n/a
 test-amd64-i386-pair  1 build-check(1)   blocked in 102732 n/a
 test-amd64-i386-rumprun-i386  1 build-check(1)   blocked in 102732 n/a
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked 
in 102732 n/a
 test-amd64-i386-xl-qemut-win7-amd64  1 build-check(1)blocked in 102732 n/a
 test-amd64-i386-qemut-rhel6hvm-amd  1 build-check(1) blocked in 102732 n/a
 test-amd64-amd64-rumprun-amd64  6 xen-boot fail never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-intel 14 guest-saverestorefail  never pass
 

Re: [Xen-devel] [DOC v1] Xen transport for 9pfs

[Stefano Stabellini]

On Wed, 30 Nov 2016, David Vrabel wrote:
> On 29/11/16 23:34, Stefano Stabellini wrote:
> > 
> > The producer (the backend for **in**, the frontend for **out**) writes to 
> > the
> > array in the following way:
> > 
> - read memory barrier
> > - read *cons*, *prod* from shared memory
> > - write to array at position *prod* up to *cons*, wrapping around the 
> > circular
> >   buffer when necessary
> - write memory barrier
> > - increase *prod*
> > - notify the other end via evtchn
> > 
> > The consumer (the backend for **out**, the frontend for **in**) reads from 
> > the
> > array in the following way:
> 
> - read memory barrier
> > - read *prod*, *cons* from shared memory
> > - read from array at position *cons* up to *prod*, wrapping around the 
> > circular
> >   buffer when necessary
> > - memory barrier
> > - increase *cons*
> > - notify the other end via evtchn
> 
> Your barriers are wrong (see corrections above).

Thanks for the review. Sorry for not specifying the type of memory
barriers. I agree with some of your suggestions, but I don't understand
why you moved the read memory barrier before reading prod and cons.

Shouldn't it be for a producer:
  - read *cons*, *prod* from shared memory
  - read memory barrier
  - write to array at position *prod* up to *cons*, wrapping around the circular
buffer when necessary
  - write memory barrier
  - increase *prod*
  - notify the other end via evtchn
 
and for a consumer:
  - read *prod*, *cons* from shared memory
  - read memory barrier
  - read from array at position *cons* up to *prod*, wrapping around the 
circular
buffer when necessary
  - general memory barrier
  - increase *cons*
  - notify the other end via evtchn

If this is wrong, could you please explain why?


> I think you should use a private copy of cons/prod in the
> consumer/producer and use this to validate that the shared prod/cons is
> within a sensible range.

With the masking macro provided (MASK_XEN_9PFS_IDX), indices cannot go
out of range:

#define MASK_XEN_9PFS_IDX(idx) ((idx) & (XEN_9PFS_RING_SIZE - 1))

That said, it could be a cheap additional validation.


> You're missing a mechanism to omit unnecessary evtchn notifications
> (like the standard ring protocol has).

Yes, indeed. I'll see what I can do.


> This all looks like a generic "transfer byte stream" mechanism which
> could be usefully made generic and not specific to 9pfs.

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

[Xen-devel] [xen-4.4-testing test] 102751: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

flight 102751 xen-4.4-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/102751/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf-libvirt   3 host-install(3)broken REGR. vs. 102521
 build-armhf3 host-install(3) broken in 102730 REGR. vs. 102521
 test-armhf-armhf-xl-multivcpu 15 guest-start/debian.repeat fail REGR. vs. 
102521

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-multivcpu 11 guest-start fail in 102718 pass in 102751
 test-amd64-i386-xend-qemut-winxpsp3 9 windows-install fail in 102718 pass in 
102751
 test-amd64-amd64-amd64-pvgrub 9 debian-di-install fail in 102730 pass in 102751
 test-amd64-i386-xl-qemuu-debianhvm-amd64 9 debian-hvm-install fail in 102730 
pass in 102751
 test-amd64-i386-qemuu-rhel6hvm-amd 9 redhat-install fail in 102730 pass in 
102751
 test-amd64-i386-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail in 102730 pass 
in 102751
 test-amd64-amd64-i386-pvgrub 9 debian-di-install fail in 102730 pass in 102751
 test-xtf-amd64-amd64-2 20 xtf/test-hvm32-invlpg~shadow fail in 102730 pass in 
102751
 test-amd64-i386-qemut-rhel6hvm-intel 9 redhat-install fail in 102730 pass in 
102751
 test-xtf-amd64-amd64-2 29 xtf/test-hvm32pae-invlpg~shadow fail in 102730 pass 
in 102751
 test-xtf-amd64-amd64-2 40 xtf/test-hvm64-invlpg~shadow fail in 102730 pass in 
102751
 test-amd64-i386-pv  14 guest-saverestore fail in 102730 pass in 102751
 test-amd64-amd64-xl-qemuu-winxpsp3 15 guest-localmigrate/x10 fail in 102730 
pass in 102751
 test-amd64-i386-xl-qemuu-debianhvm-amd64 15 guest-localmigrate/x10 fail pass 
in 102718
 test-amd64-amd64-xl-qemuu-win7-amd64 13 guest-localmigrate fail pass in 102730

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop  fail in 102718 like 102521
 test-xtf-amd64-amd64-4   16 xtf/test-pv32pae-selftestfail  like 102521
 test-xtf-amd64-amd64-2   16 xtf/test-pv32pae-selftestfail  like 102521
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 102521
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail like 102521
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 102521

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked in 102730 n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked in 102730 n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)  blocked in 102730 n/a
 build-armhf-libvirt   1 build-check(1)   blocked in 102730 n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked in 102730 n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked in 102730 n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1) blocked in 102730 n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-amd64-amd64-rumprun-amd64  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-amd64-i386-rumprun-i386  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-qcow2  9 debian-di-install  fail in 102718 never pass
 test-armhf-armhf-libvirt-raw  9 debian-di-installfail in 102718 never pass
 test-armhf-armhf-libvirt 11 guest-start  fail in 102718 never pass
 test-xtf-amd64-amd64-4 51 xtf/test-pv32pae-pv-iopl~hypercall fail in 102730 
never pass
 test-xtf-amd64-amd64-2 51 xtf/test-pv32pae-pv-iopl~hypercall fail in 102730 
never pass
 test-xtf-amd64-amd64-4 52 xtf/test-pv32pae-pv-iopl~vmassist fail in 102730 
never pass
 test-xtf-amd64-amd64-2 52 xtf/test-pv32pae-pv-iopl~vmassist fail in 102730 
never pass
 test-xtf-amd64-amd64-5   10 xtf-fep  fail   never pass
 test-xtf-amd64-amd64-5   16 xtf/test-pv32pae-selftestfail   never pass
 test-xtf-amd64-amd64-5   18 xtf/test-hvm32-cpuid-faulting fail  never pass
 build-i386-rumprun7 xen-buildfail   never pass
 build-amd64-rumprun   7 xen-buildfail   never pass
 test-xtf-amd64-amd64-4   10 xtf-fep  fail   never pass
 test-xtf-amd64-amd64-3   10 xtf-fep  fail   never pass
 test-xtf-amd64-amd64-4   18 xtf/test-hvm32-cpuid-faulting fail  never pass
 test-xtf-amd64-amd64-2   10 xtf-fep  fail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-xtf-amd64-amd64-1   10 xtf-fep  fail   never pass
 test-xtf-amd64-amd64-3   16 xtf/test-pv32pae-selftestfail   never pass
 test-armhf-armhf-xl-vhd   9 debian-di-installfail   never pass
 test-xtf-amd64-amd64-3   18 

Re: [Xen-devel] [DOC v1] Xen transport for 9pfs

[Stefano Stabellini]

On Thu, 1 Dec 2016, Dario Faggioli wrote:
> On Tue, 2016-11-29 at 15:34 -0800, Stefano Stabellini wrote:
> > ### Frontend XenBus Nodes
> > 
> > num-rings
> >  Values: 
> > 
> >  Number of rings. It needs to be lower or equal to max-rings.
> > 
> > port- (port-0, port-1, etc)
> >  Values: 
> > 
> >  The identifier of the Xen event channel used to signal
> > activity
> >  in the ring buffer. One for each ring.
> > 
> > ring-ref- (ring-ref-0, ring-ref-1, etc)
> >
> blkif uses ring-ref%u, rather than ring-ref-%u (i.e., no dash before
> the index). Not a big deal, I guess, but I thought it could be nice to
> be a bit more uniform.

Sure, but in this case each ring-ref-%u is used to map a different ring.
That said, I can make the change.


> >  Values: 
> > 
> >  The Xen grant reference granting permission for the backend
> > to
> >  map a page with information to setup a share ring. One for
> > each
> >  ring.
> > 
> So, this means there can be multiple rings (up to max-rings advertised
> by backend), _each_of_which_ has its own event channel, and
> _each_of_which_ can be a multi-page (in case ring_order > 0) ring... Is
> this correct?

That's right, and the size of each ring can be up to max-ring-page-order.


> If it is, what's the typical envisioned use of these multiple rings, if
> I can ask?

They are used to handle multiple read/write requests in parallel. Let's
assume that we configure the rings to be 8K each. Given that the data is
transmitted over the ring, each ring can hold only one outstanding 4K
write request (there is an header for each write request).

With two 8K rings, we can have two outstanding 4K write requests, each
of them processed in parallel on a different vcpu.

The system is completely configurable in terms of number and size of
rings, so a user can configure it to only export one 4K ring for
example or go as far as several 2MB rings.


> (Maybe it's obvious, I'm still not so familiar with these protocols..
> but I'm trying to improve, and that's why I'm here, so bear with me if
> possible. :-D)
> 
> > ## Ring Setup
> > 
> > The shared page has the following layout:
> > 
> > typedef uint32_t XEN_9PFS_RING_IDX;
> > 
> > struct xen_9pfs_intf {
> > XEN_9PFS_RING_IDX in_cons, in_prod;
> > XEN_9PFS_RING_IDX out_cons, out_prod;
> > 
> > uint32_t ring_order;
> > grant_ref_t ref[];
> > };
> > 
> > /* not actually C compliant (ring_order changes from socket to
> > socket) */
> > struct ring_data {
> > char in[((1 << ring_order) << PAGE_SHIFT) / 2];
> > char out[((1 << ring_order) << PAGE_SHIFT) / 2];
> > };
> >
> Sorry, what does "ring_order changes from socket to socket" mean?

Woops, copy/paste error from PVCalls. I meant "ring_order changes from
ring to ring".


> > The binary layout of `struct xen_9pfs_intf` follows:
> > 
> > 0 4 8 121620
> > +-+-+-+-+-+
> > | in_cons | in_prod |out_cons |out_prod |ring_orde|
> > +-+-+-+-+-+
> > 
> > 202426  4092  4096
> > +-+-+//---+-+
> > |  ref[0] |  ref[1] | |  ref[N] |
> > +-+-+//---+-+
> > 
> > **N.B** For one page, N is maximum 1019 ((4096-20)/4), but given that
> > N
> > needs to be a power of two, actually max N is 512.
> >
> It may again be me being still too naive, but I'd quickly add at least
> another example, with the value of N computed for a multiple pages
> ring. Something like: "For 4 pages (i.e., ring_orfer=2), N is..."

For 4 pages, N is 4. N is the number of pages that make up the ring.

Maybe there is a misunderstanding, let me try to explain it again: each
page shared via xenstore contains information to handle one new ring,
including grant references for the pages making up the multipage ring
itself. I'll repeat: pages shared via xenstore are not used as a ring,
they are used to setup the rings, each page has the info to setup a new
ring.

The structure of these "setup pages" is `struct xen_9pfs_intf`. Each
page is completely separate and independent from the others. Given that
one page is just 4096 bytes, it can contain max 512 grant refs (see
calculation above). So the max size of one multipage ring is 512 pages =
2MB.

Does it make sense?___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 3/3] Significant changes to decision making; some new roles and minor changes

[Stefano Stabellini]

On Thu, 1 Dec 2016, Lars Kurth wrote:
> On 01/12/2016 22:36, "Stefano Stabellini"  wrote:
> 
> >On Thu, 1 Dec 2016, Ian Jackson wrote:
> >> Lars Kurth writes ("Re: [PATCH v5 3/3] Significant changes to decision
> >>making; some new roles and minor changes"):
> >> > Maybe Ian has some views on what is better from a theoretical
> >>viewpoint:
> >> > Voting mechanisms are a bit of a hobby of his
> >> 
> >> The underlying problem here is that the reality is that the Xen
> >> Project's by-far most important subproject is the hypervisor; that it
> >> seems that the governance probably ought to reflect that; but that it
> >> is difficult to do this without special casing it or providing an
> >> objective metric of the hypervisor subproject's size.
> >> 
> >> I don't think it is possible to square this circle.  Our options are:
> >> 
> >> 1. Explicitly recognise the hypervisor subproject as special.
> >>(This could be done by creating a new `superproject' maturity
> >>category, or simply by naming it explicitly.)
> >> 
> >> 2. Do some kind of bodge which tries to reduce the impact of the
> >>potential unknown management practices of other subprojects
> >>(particularly, that they might appoint lots of leaders).
> >> 
> >> 3. Restructure the hypervisor sub-project.
> >> 
> >> The current proposal is (2) and has the virtue of not incentivising a
> >> subproject to appoint lots of leaders simply to get more votes
> >> overall.  But it is still rather weak because it has to treat the
> >> hypervisor subproject as only one amongst many, so hypervisor leaders
> >> are under-powered and fringe leaders over-powered.
> >> 
> >> Another way to deal with this would be to split the hypervisor
> >> subproject (3, above).  For example, we could create subprojects for
> >> some subset of minios, osstest, xtf, various out-of-tree tools,...
> >> (many of which would have only one leadership team member).
> >> 
> >> That would mean that the hypervisor-focused maintainers would get
> >> additional votes via their other "hats".  (They would still get a vote
> >> in the hypervisor subproject, if they have a hypervisor leadership
> >> position too.)
> >> 
> >> This is perhaps less unnatural.  It still leaves fringe leaders
> >> somewhat over-powered: this time, leaders of more-hypervisor-related
> >> (or some such) fringe things, rather than leaders of
> >> less-hypervisor-related fringe things.
> >
> >Istinctively, I don't like the idea of splitting up the hypervisor
> >project in multiple projects.
> >
> >I am no voting expert, but maybe we could consider explicitly weighting
> >each project differently. The advantage is that the mechanism would be
> >obvious rather than implicit. For example "Project A = 10" and "Project
> >B = 6".  In the previous example:
> >
> >project A, weight 6, leadership team size 2, total positive votes 2, 100%
> >project B, weight 10, leadership team size 12, negative votes 8, positive
> >votes 4, 33%
> >Total favor: (100*6 + 33*10) / (6+10) = 58.12 -> fail
> >
> >The problem is how to come up with the numbers in the first place and
> >how to update them when necessary, to reflect changes in maturity, size
> >and activity of a project.
> >
> >For the sake of updating the document and moving forward with the other,
> >more important, changes, could we postpone modifications to project wide
> >changes? Or just separate them out to a different patch so that most
> >people can give their +1 to the other patches?
> 
> Sure: these are fairly independent. I don't want to re-run the vote:
> so I propose to 
> a) just apply the bulk of the changes on the website
>(v3 of governance)
> b) I will split out the remaining ones around global
>Voting and re-send as separate patch (v3.1)
> 
> This is because I don't have enough time before going on winter
> Vacation.
> 
> Is this workable?

+1 from me

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

Re: [Xen-devel] [PATCH v5 3/3] Significant changes to decision making; some new roles and minor changes

[Lars Kurth]

On 01/12/2016 22:36, "Stefano Stabellini"  wrote:

>On Thu, 1 Dec 2016, Ian Jackson wrote:
>> Lars Kurth writes ("Re: [PATCH v5 3/3] Significant changes to decision
>>making; some new roles and minor changes"):
>> > Maybe Ian has some views on what is better from a theoretical
>>viewpoint:
>> > Voting mechanisms are a bit of a hobby of his
>> 
>> The underlying problem here is that the reality is that the Xen
>> Project's by-far most important subproject is the hypervisor; that it
>> seems that the governance probably ought to reflect that; but that it
>> is difficult to do this without special casing it or providing an
>> objective metric of the hypervisor subproject's size.
>> 
>> I don't think it is possible to square this circle.  Our options are:
>> 
>> 1. Explicitly recognise the hypervisor subproject as special.
>>(This could be done by creating a new `superproject' maturity
>>category, or simply by naming it explicitly.)
>> 
>> 2. Do some kind of bodge which tries to reduce the impact of the
>>potential unknown management practices of other subprojects
>>(particularly, that they might appoint lots of leaders).
>> 
>> 3. Restructure the hypervisor sub-project.
>> 
>> The current proposal is (2) and has the virtue of not incentivising a
>> subproject to appoint lots of leaders simply to get more votes
>> overall.  But it is still rather weak because it has to treat the
>> hypervisor subproject as only one amongst many, so hypervisor leaders
>> are under-powered and fringe leaders over-powered.
>> 
>> Another way to deal with this would be to split the hypervisor
>> subproject (3, above).  For example, we could create subprojects for
>> some subset of minios, osstest, xtf, various out-of-tree tools,...
>> (many of which would have only one leadership team member).
>> 
>> That would mean that the hypervisor-focused maintainers would get
>> additional votes via their other "hats".  (They would still get a vote
>> in the hypervisor subproject, if they have a hypervisor leadership
>> position too.)
>> 
>> This is perhaps less unnatural.  It still leaves fringe leaders
>> somewhat over-powered: this time, leaders of more-hypervisor-related
>> (or some such) fringe things, rather than leaders of
>> less-hypervisor-related fringe things.
>
>Istinctively, I don't like the idea of splitting up the hypervisor
>project in multiple projects.
>
>I am no voting expert, but maybe we could consider explicitly weighting
>each project differently. The advantage is that the mechanism would be
>obvious rather than implicit. For example "Project A = 10" and "Project
>B = 6".  In the previous example:
>
>project A, weight 6, leadership team size 2, total positive votes 2, 100%
>project B, weight 10, leadership team size 12, negative votes 8, positive
>votes 4, 33%
>Total favor: (100*6 + 33*10) / (6+10) = 58.12 -> fail
>
>The problem is how to come up with the numbers in the first place and
>how to update them when necessary, to reflect changes in maturity, size
>and activity of a project.
>
>For the sake of updating the document and moving forward with the other,
>more important, changes, could we postpone modifications to project wide
>changes? Or just separate them out to a different patch so that most
>people can give their +1 to the other patches?

Sure: these are fairly independent. I don't want to re-run the vote:
so I propose to 
a) just apply the bulk of the changes on the website
   (v3 of governance)
b) I will split out the remaining ones around global
   Voting and re-send as separate patch (v3.1)

This is because I don't have enough time before going on winter
Vacation.

Is this workable?

Lars

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

Re: [Xen-devel] [PATCH v5 3/3] Significant changes to decision making; some new roles and minor changes

[Stefano Stabellini]

On Thu, 1 Dec 2016, Ian Jackson wrote:
> Lars Kurth writes ("Re: [PATCH v5 3/3] Significant changes to decision 
> making; some new roles and minor changes"):
> > Maybe Ian has some views on what is better from a theoretical viewpoint:
> > Voting mechanisms are a bit of a hobby of his
> 
> The underlying problem here is that the reality is that the Xen
> Project's by-far most important subproject is the hypervisor; that it
> seems that the governance probably ought to reflect that; but that it
> is difficult to do this without special casing it or providing an
> objective metric of the hypervisor subproject's size.
> 
> I don't think it is possible to square this circle.  Our options are:
> 
> 1. Explicitly recognise the hypervisor subproject as special.
>(This could be done by creating a new `superproject' maturity
>category, or simply by naming it explicitly.)
> 
> 2. Do some kind of bodge which tries to reduce the impact of the
>potential unknown management practices of other subprojects
>(particularly, that they might appoint lots of leaders).
> 
> 3. Restructure the hypervisor sub-project.
> 
> The current proposal is (2) and has the virtue of not incentivising a
> subproject to appoint lots of leaders simply to get more votes
> overall.  But it is still rather weak because it has to treat the
> hypervisor subproject as only one amongst many, so hypervisor leaders
> are under-powered and fringe leaders over-powered.
> 
> Another way to deal with this would be to split the hypervisor
> subproject (3, above).  For example, we could create subprojects for
> some subset of minios, osstest, xtf, various out-of-tree tools,...
> (many of which would have only one leadership team member).
> 
> That would mean that the hypervisor-focused maintainers would get
> additional votes via their other "hats".  (They would still get a vote
> in the hypervisor subproject, if they have a hypervisor leadership
> position too.)
> 
> This is perhaps less unnatural.  It still leaves fringe leaders
> somewhat over-powered: this time, leaders of more-hypervisor-related
> (or some such) fringe things, rather than leaders of
> less-hypervisor-related fringe things.

Istinctively, I don't like the idea of splitting up the hypervisor
project in multiple projects.

I am no voting expert, but maybe we could consider explicitly weighting
each project differently. The advantage is that the mechanism would be
obvious rather than implicit. For example "Project A = 10" and "Project
B = 6".  In the previous example:

project A, weight 6, leadership team size 2, total positive votes 2, 100%
project B, weight 10, leadership team size 12, negative votes 8, positive votes 
4, 33%
Total favor: (100*6 + 33*10) / (6+10) = 58.12 -> fail

The problem is how to come up with the numbers in the first place and
how to update them when necessary, to reflect changes in maturity, size
and activity of a project.

For the sake of updating the document and moving forward with the other,
more important, changes, could we postpone modifications to project wide
changes? Or just separate them out to a different patch so that most
people can give their +1 to the other patches?

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

Re: [Xen-devel] ARM: Request backport for Xen 4.6 and Xen 4.5

[Stefano Stabellini]

Done

On Thu, 1 Dec 2016, Julien Grall wrote:
> Hi,
> 
> While testing XSA-201, I noticed that Xen 4.5 and 4.5 were not able to boot on
> the Foundation Model [1].
> 
> The Foundation Model is a free model provided by ARM for ARMv8 which is
> supported by Xen since the beginning.
> 
> I am able to Xen boot after applying the patch:
> 
> eb6fe7a "arm64: fix incorrect memory region size in TCR_EL2"
> 
> So would it be possible to backport this patch in Xen 4.6 and Xen 4.5?
> 
> Cheers,
> 
> [1] https://www.arm.com/products/tools/models/foundation-model.php
> 
> -- 
> Julien Grall
> 

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

Re: [Xen-devel] Xen ARM - Exposing a PL011 to the guest

[Stefano Stabellini]

On Thu, 1 Dec 2016, Christoffer Dall wrote:
> On Wed, Nov 30, 2016 at 02:26:50PM -0800, Stefano Stabellini wrote:
> > On Wed, 30 Nov 2016, Julien Grall wrote:
> > > Hi all,
> > > 
> > > Few months ago, Linaro has published the version 2 of the VM specification
> > > [1].
> > > 
> > > For those who don't know, the specification provides guidelines to 
> > > guarantee a
> > > compliant OS images could run on various hypervisor (e.g Xen, KVM).
> > > 
> > > Looking at the specification, it will require Xen to expose new devices 
> > > to the
> > > guest: pl011, rtc, persistent flash (for UEFI variables).
> > > 
> > > The RTC and persistent will only be used by the UEFI firwmare. The 
> > > firwmare is
> > > custom made for Xen guest and be loaded by the toolstack, so we could
> > > theoretically provide PV drivers for those.
> > > 
> > > This is not the case for the PL011. The guest will be shipped with a
> > > PL011/SBSA UART driver,.This means it will expect to access it through 
> > > MMIO.
> > > 
> > > So we have to emulate a PL011. The question is where? Before suggesting 
> > > some
> > > ideas, the guest/user will expect to be able to interact with the console
> > > through the UART. This means that the UART and xenconsoled needs to
> > > communicate together.
> > > 
> > > I think we can distinct two places where the PL011 could be emulated:
> > > in the hypervisor, or outside the hypervisor.
> > > 
> > > Emulating the UART in the hypervisor means that we take the risk to 
> > > increase
> > > to the attack surface of Xen if there is a bug in the emulation code. The
> > > attack surface could be reduced by emulating the UART in another exception
> > > level (e.g EL1, EL0) but still under the control of the hypervisor. 
> > > Usually
> > > the guest is communicating between with xenconsoled using a ring. For the
> > > first console this could be discovered using hypercall HVMOP_get_param. 
> > > For
> > > the second and onwards, it described in xenstore. I would not worry too 
> > > much
> > > about emulating multiple PL011s, so we could implement the PV frontend in 
> > > Xen.
> > > 
> > > Emulating the UART outside the hypervisor (e.g in DOM0 or special domain)
> > > would require to bring the concept of ioreq server on ARM. Which left the
> > > question where do we emulate the PL011? The best place would be 
> > > xenconsoled.
> > > But I am not sure how would be the security impact here. Does all guest
> > > consoles are emulated within the same daemon?
> > 
> > One xenconsoled instance handles all PV console frontends. However QEMU,
> > not xenconsoled, handles secondary PV consoles and emulated serials. One
> > QEMU instance per VM. The PV console protocol is pretty trivial and not
> > easy to exploit.
> > 
> > Instead of emulating PL011 in xenconsoled we could do it in QEMU (or
> > something like it). But I think we should do something else instead, see
> > below.
> > 
> > 
> > > I would lean towards the first solution if we implement all the security
> > > safety I mentioned. Although, the second solution would be a good move if 
> > > we
> > > decide to implement more devices (e.g RTC, pflash) in the future.
> > > 
> > > Do you have any opinions?
> > 
> > As I have just written in this other email:
> > 
> > http://marc.info/?l=xen-devel=148054285829397
> > 
> > I don't think we should introduce userspace emulators in Dom0 on ARM.
> > The reason is that they are bad both for security and performance. In
> > general I think that the best solution is to introduce emulators in EL1
> > in Xen.
> 
> Slightly hijacking the discussion:
> 
> Why would you run stuff in EL1 as opposed to EL0?
> 
> My understanding of the HCR_EL2.TGE bit is exactly designed to run
> bare-metal applications on top of a hypervisor running in EL2 and has
> the added benefit that I think you can switch to your bare-metal
> application without having to context-switch any EL1 tate, because TGE
> just disabled the whole stage 1 MMU.  Obviously this doesn't give you
> virtual memory, unless you map that using stage 2 tables, but that might
> actually be preferred, and since the whole point is to separate your
> non-privileged hypervisor applications from the hypervisor's memory
> anyhow, it may be fine.
> 
> I don't know enough about Xen's impementation to say whether this is
> doable or not, but I think there are potential advantages in terms of
> performance and using the architecture as designed.

You might be right and it is definitely worth investigating.


> Thinking about this, if you run something in EL1 on top of Xen without
> setting the TGE bit, what happens if your application misbehaves, for
> example causes a page fault?  That would be taken at EL1, right?  So
> will you install an EL1 handler for exceptions that somehow forwards the
> trap into EL2?  Sounds cleaner to me to catch anything weird the
> application does to EL2.

Yes, I think it would be better to handle that kind of faults at EL2.

Re: [Xen-devel] Xen ARM small task (WAS: Re: [Xen Question])

[Stefano Stabellini]

On Thu, 1 Dec 2016, Julien Grall wrote:
> On 01/12/16 02:07, Stefano Stabellini wrote:
> > On Fri, 25 Nov 2016, Julien Grall wrote:
> > > Hi Stefano,
> 
> Hi,
> 
> > > On 23/11/16 19:55, Stefano Stabellini wrote:
> > > > Actually I am thinking that the default values should be in the
> > > > emulators themselves. After all they are the part of the code that knows
> > > > more about vuarts.
> > > 
> > > Can you expand what you mean by emulator? I was never expecting to have a
> > > fully emulated UART exposed to the guest (i.e read/write character
> > > support)
> > > except for a PL011.
> > 
> > Once we start having emulators, it is possible that we'll end up with
> > more than one. For example, we introduce the PL011 now, then in a couple
> > of years somebody wants to add ns16550 because it is the only one that
> > Windows 2019 supports. I am assuming that one way or another they'll run
> > in a low privilege mode (see other recent threads).
> 
> Well, if this Windows is meant to run on SBSA complaint server, it will have
> to support either PL011 or SBSA (a subset of PL011).

I am not thinking about SBSA compliant OSes, that is the easy case :-)


> If we are going to allow more kind of UART? Why don't we have a disk emulator
> in Xen? How about a network emulator? Overall Windows 2019 may not have PV
> drivers for the network and disk.
> 
> I really think we have to draw a line of what we are supporting. The PL011 is
> mandatory by a specification. If the guest is not compliant then it will have
> to use either PV drivers or having a device assigned.
> 
> The addition of a new emulator in upstream would need to be justified. I don't
> want to end up bringing QEMU in Xen.

Nobody wants to bring QEMU into Xen. To be pedantic we would be
bringing QEMU into xen.git, not into "Xen", but we don't want that
either.

Of course, any addition would need to be well justified, but at the same
time, I don't think we can rule out any emulator a priori.  We'll have
to evaluate the issue on a case by case basis. As usual, it is going to
be a trade-off between complexity, maintainability and use cases that it
enables. Everybody dislike QEMU on Xen on x86, but it enabled Windows XP
virtual machines back in the day. I might disagree with the way it was
done, but I wonder what would be of the Xen Project today if those
emullators hadn't been added in 2005.

Of course the fewer emulators, the better. I wouldn't accept an IDE
emulator in Xen on ARM today for example. However sometimes they are
unavoidable, that's why it is important to provide a safe execution
environment for them (meaning: not in Xen at EL2).

Today it is pretty much the same thing to add an emulator in the
hypervisor or in QEMU on x86. Somebody has to maintain the code no
matter where it lives and provide security support for it. Every QEMU
vulnerability ends up becoming a Xen vulnerability. In all honesty, it
is better to introduce them in QEMU so that at least the few people that
use stubdoms are less affected. In the future it is going to be similar
to introduce new emulators in xen.git at EL0/1 or in QEMU running
unprivileged in Dom0. This is to say that having emulators out of Xen
(or out of xen.git) is not necessarily an improvement if they are still
able to do exactly the same things, such as mapping any random page in
memory.


> > > The current vuart (see xen/arch/arm/vuart.c) is very simple but require
> > > someone to configure it. For DOM0, this is configured by the serial
> > > driver.
> > > For guest we need someone doing the same.
> > 
> > I understand. For clarity, I'll call "PL011 emulator" the one that will
> > end up being used for DomUs, which might be based on, or different from,
> > xen/arch/arm/vuart.c. It doesn't exist yet.
> > 
> > The PL011 emulator should have default values for everything. Some of
> > these values could be configured by libxl, but none should be required
> > to be configured by libxl. The last thing we want is to disseminate
> > numbers and addresses in libxl. One of these parameters could be the
> > MMIO address, but it is just an example, we don't necessarily need to
> > support changing it. It could be a decent feature to have but I don't
> > think is important if we'll support configurable memory layouts soon.
> 
> This is what I have been suggested from the beginning :).
> 
> But in the case of baremetal application, we want to be able to do logging
> only (i.e not reading). They will have a driver for the host UART. It would be
> pointless and potentially difficult to emulate a full UART here. This is where
> vuart come in place (see the use case mentioned by Edgar).

I was suggesting to kill two birds with one stone and just do PL011 for
DomUs (disabled by default, of course). Instead, you would keep the two
use cases separate? PL011 for VM spec guests and vuart for baremetal
apps? That's an option, but we would still need to feed back the logs
to xenconsoled.

How would you export the vuart in the VM 

[Xen-devel] [libvirt test] 102750: tolerable all pass - PUSHED

[osstest service owner]

flight 102750 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/102750/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail  like 102726
 test-armhf-armhf-libvirt 13 saverestore-support-checkfail  like 102726
 test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail  like 102726
 test-armhf-armhf-libvirt-qcow2 12 saverestore-support-check   fail like 102726

Tests which did not succeed, but are not blocking:
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass

version targeted for testing:
 libvirt  655429a0d4a51e7deb533e1d17c1a41360ad2965
baseline version:
 libvirt  bb738f9fcdc3967903a6ff78111dfa989f61d04d

Last test of basis   102726  2016-11-30 04:24:03 Z1 days
Testing same since   102750  2016-12-01 04:20:44 Z0 days1 attempts


People who touched revisions under test:
  Boris Fiuczynski 
  Christian Ehrhardt 
  Eric Farman 
  Guido Günther 
  Jiri Denemark 
  Ján Tomko 
  Laine Stump 
  Martin Kletzander 
  Nikos Mavrogiannopoulos 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-libvirt-xsm pass
 test-armhf-armhf-libvirt-xsm pass
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-libvirt pass
 test-armhf-armhf-libvirt pass
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-i386-libvirt-pair pass
 test-armhf-armhf-libvirt-qcow2   pass
 test-armhf-armhf-libvirt-raw pass
 test-amd64-amd64-libvirt-vhd pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

+ branch=libvirt
+ revision=655429a0d4a51e7deb533e1d17c1a41360ad2965
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z 

Re: [Xen-devel] [RFD] OP-TEE (and probably other TEEs) support

[Stefano Stabellini]

On Thu, 1 Dec 2016, Volodymyr Babchuk wrote:
> > - TEE may run in parallel of the guest OS, this means that we have
> > to make sure the page will never be removed by the guest OS (see
> > XENMEM_decrease_reservation hypercall).
> Hmmm... I don't know how XEN handles guest memory in details. Can we
> somehow pin pages, so they can't be removed until client unregisters
> shared memory buffer?
> In new OP-TEE shmem design there will be call to register shared
> memory. Client will pass list of pages in this call and OP-TEE will
> map them in its address space. At this moment we need to pin them in
> hypervisor, so guest can't get rid of them, until "unregister shared
> memory" call made. Is this possible?

Yes, pages can be pinned, but we need to cover it in the design doc and
the code.

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

Re: [Xen-devel] [RFD] OP-TEE (and probably other TEEs) support

[Stefano Stabellini]

On Thu, 1 Dec 2016, Volodymyr Babchuk wrote:
> Hello Julien,
> 
> 
> 
> On 1 December 2016 at 16:24, Julien Grall  wrote:
> > Hi Stefano,
> >
> >
> > On 30/11/16 21:53, Stefano Stabellini wrote:
> >>
> >> On Mon, 28 Nov 2016, Julien Grall wrote:
> >
> > If not, then it might be worth to consider a 3rd solution where the TEE
> > SMC
> > calls are forwarded to a specific domain handling the SMC on behalf of
> > the
> > guests. This would allow to upgrade the TEE layer without having to
> > upgrade
> > the hypervisor.
> 
>  Yes, this is good idea. How this can look? I imagine following flow:
>  Hypervisor traps SMC, uses event channel to pass request to Dom0. Some
>  userspace daemon receives it, maps pages with request data, alters is
>  (e.g. by replacing IPAs with PAs), sends request to hypervisor to
>  issue real SMC, then alters response and only then returns data back
>  to guest.
> >>>
> >>>
> >>> The event channel is only a way to notify (similar to an interrupt), you
> >>> would
> >>> need a shared memory page between the hypervisor and the client to
> >>> communicate
> >>> the SMC information.
> >>>
> >>> I was thinking to get advantage of the VM event API for trapping the SMC.
> >>> But
> >>> I am not sure if it is the best solution here. Stefano, do you have any
> >>> opinions here?
> >>>
>  I can see only one benefit there - this code will be not in
>  hypervisor. And there are number of drawbacks:
> 
>  Stability: if this userspace demon will crash or get killed by, say,
>  OOM, we will lose information about all opened sessions, mapped shared
>  buffers, etc.That would be complete disaster.
> >>>
> >>>
> >>> I disagree on your statement, you would gain in isolation. If your
> >>> userspace
> >>> crashes (because of an emulation bug), you will only loose access to TEE
> >>> for a
> >>> bit. If the hypervisor crashes (because of an emulation bug), then you
> >>> take
> >>> down the platform. I agree that you lose information when the userspace
> >>> app is
> >>> crashing but your platform is still up. Isn't it the most important?
> >>>
> >>> Note that I think it would be "fairly easy" to implement code to reset
> >>> everything or having a backup on the side.
> >>>
>  Performance: how big will be latency introduced by switching between
>  hypervisor, dom0 SVC and USR modes? I have seen use case where TEE is
>  part of video playback pipe (it decodes DRM media).
>  There also can be questions about security, but Dom0 in any case can
>  access any memory from any guest.
> >>>
> >>>
> >>> But those concerns would be the same in the hypervisor, right? If your
> >>> emulation is buggy then a guest would get access to all the memory.
> >>>
>  But I really like the idea, because I don't want to mess with
>  hypervisor when I don't need to. So, how do you think, how it will
>  affect performance?
> >>>
> >>>
> >>> I can't tell here. I would recommend you to try a quick prototype (e.g
> >>> receiving and sending SMC) and see what would be the overhead.
> >>>
> >>> When I wrote my previous e-mail, I mentioned "specific domain", because I
> >>> don't think it is strictly necessary to forward the SMC to DOM0. If you
> >>> are
> >>> concern about overloading DOM0, you could have a separate service domain
> >>> that
> >>> would handle TEE for you. You could have your "custom OS" handling TEE
> >>> request
> >>> directly in kernel space (i.e SVC).
> >>>
> >>> This would be up to the developer of this TEE-layer to decide what to do.
> >>
> >>
> >> Thanks Julien from bringing me into the discussion. These are my
> >> thoughts on the matter.
> >>
> >>
> >> Running emulators in Dom0 (AKA QEMU on x86) has always meant giving them
> >> full Dom0 privileges so far. I don't think that is acceptable. There is
> >> work undergoing on the x86 side of things to fix the situation, see:
> >>
> >>
> >> http://marc.info/?i=1479489244-2201-1-git-send-email-paul.durrant%40citrix.com
> >>
> >> But if the past is any indication of future development speed, we are
> >> still a couple of Xen releases away at least from having unprivileged
> >> emulators in Dom0 on x86. By unprivileged, I mean that they are not able
> >> to map any random page in memory, but just the ones belonging to the
> >> virtual machine that they are serving. Until then, having an emulator in
> >> userspace Dom0 is just as bad as having it in the hypervisor from a
> >> security standpoint.
> >>
> >> I would only consider this option, if we mandate from the start, in the
> >> design doc and implementations, that the emulators need to be
> >> unprivileged on ARM. This would likely require a new set of hypercalls
> >> and possibly Linux privcmds. And even then, this solution would still
> >> present a series of problems:
> >>
> >> - latency
> >> - scalability
> >> - validation against the root of trust
> >> - certifications 

[Xen-devel] [rumprun baseline test] 102749: all pass

[osstest service owner]

"Old" tested version had not actually been tested; therefore in this
flight we test it, rather than a new candidate.  The baseline, if
any, is the most recent actually tested revision.

flight 102749 rumprun real [real]
http://logs.test-lab.xenproject.org/osstest/logs/102749/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 rumprun  39a97f37a85e44c69b662f6b97b688fbe892603b
baseline version:
 rumprun  39a97f37a85e44c69b662f6b97b688fbe892603b

Last test of basis   102749  2016-12-01 04:20:22 Z0 days
Testing same since0  1970-01-01 00:00:00 Z 17136 days0 attempts

jobs:
 build-amd64  pass
 build-i386   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 build-amd64-rumprun  pass
 build-i386-rumprun   pass
 test-amd64-amd64-rumprun-amd64   pass
 test-amd64-i386-rumprun-i386 pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Published tested tree is already up to date.


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

Re: [Xen-devel] [PATCH RFC] tools/xenlight: Create xenlight Makefile

[Ronald Rojas]

On Wed, Nov 30, 2016 at 12:30:04PM +1100, George Dunlap wrote:
> On Tue, Nov 29, 2016 at 4:18 AM, Ronald Rojas  wrote:
> > Created basic Makfele for the Golang xenlight
> > project so that the project is built and
> > installed. A template template is alo added.
> 
> Thanks Ronald!  Not a bad first submission, but a lot of things that
> need tightening up.
> 
> First, the normal style of most commit messages is more direct;
> usually in the form "Do X" (like a recipe), or in the form "We do Y".
> 
> So to translate the comment above, I'd probably say something like this:
> 
> "Create a basic Makefile to build and install libxenlight Golang
> bindings.  Also add a template."  (Or, as an example of the second
> form, "We also add a template so that we have something to build.")

Alright, I can change the commit message. I'm still getting used to
how to write patches but I'll keep it in mind of next time. 

> 
> But in the final patch, we'll probably be introducing all the libxl
> golang bindings in one go (not first the makefile, then the bindings,
> ), so it probably makes sense to have your mock-up start with that
> assumption, then explain that it's still a work in progress.
> 
> The best way to do this is to put comments for the reviewers under a
> line with three dashes ("---") in the commit message.
> 
> So something like this:
> 
> 8<
> 
> tools: Introduce xenlight golang bindings
> 
> Create tools/golang/xenlight and hook it into the main tools Makefile.
> 
> Signed-off-by: John Smith 
> 
> ---
> 
> Eventually this patch will contain the actual bindings package; for
> now just include a stub package.
> 
> To do:
> - Make a real package
> - Have configure detect golang bindings properly
> 
> -->8

Understood :). I will do this in the next iteration of the patch

> 
> >  tools/Makefile| 15 +++
> >  tools/golang/xenlight/Makefile| 29 +
> >  tools/golang/xenlight/xenlight.go |  7 +++
> >  3 files changed, 51 insertions(+)
> >  create mode 100644 tools/golang/xenlight/Makefile
> >  create mode 100644 tools/golang/xenlight/xenlight.go
> >
> > diff --git a/tools/Makefile b/tools/Makefile
> > index 71515b4..b89f06b 100644
> > --- a/tools/Makefile
> > +++ b/tools/Makefile
> > @@ -11,6 +11,7 @@ SUBDIRS-y += misc
> >  SUBDIRS-y += examples
> >  SUBDIRS-y += hotplug
> >  SUBDIRS-y += xentrace
> > +SUBDIRS-y += golang/xenlight
> 
> As Wei said, you should follow the python model, and put another
> Makefile in tools/golang.

Sorry but I don't really I don't really understand how the python
Makefile works. In particular I don't really understand how the 
build rule and setup.py works( maybe because I haven't written 
any python). Would you mind giving me some intuition on how it 
works?

> 
> And you should add a comment here saying that you plan to use
> autotools to do this eventually:
> 
> # FIXME: Have this controlled by a configure variable

Understood.

> 
> >  SUBDIRS-$(CONFIG_XCUTILS) += xcutils
> >  SUBDIRS-$(CONFIG_X86) += firmware
> >  SUBDIRS-y += console
> > @@ -311,6 +312,20 @@ subdir-install-debugger/gdbsx: .phony
> >  subdir-all-debugger/gdbsx: .phony
> > $(MAKE) -C debugger/gdbsx all
> >
> > +subdir-all-golang/xenlight: .phony
> > +   $(MAKE) -C golang/xenlight all
> > +
> > +subdir-clean-golang/xenlight: .phony
> > +   $(MAKE) -C golang/xenlight clean
> > +
> > +subdir-install-golang/xenlight: .phony
> > +   $(MAKE) -C golang/xenlight install
> > +
> > +subdir-build-golang/xenlight: .phony
> > +   $(MAKE) -C golang/xenlight build
> > +
> > +subdir-distclean-golang/xenlight: .phony
> > +   $(MAKE) -C golang/xenlight distclean
> >
> >  subdir-clean-debugger/kdd subdir-distclean-debugger/kdd: .phony
> > $(MAKE) -C debugger/kdd clean
> > diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
> > new file mode 100644
> > index 000..c723c1d
> > --- /dev/null
> > +++ b/tools/golang/xenlight/Makefile
> > @@ -0,0 +1,29 @@
> > +XEN_ROOT=$(CURDIR)/../../..
> > +include $(XEN_ROOT)/tools/Rules.mk
> > +
> > +BINARY = xenlightgo
> > +GO = go
> > +
> > +.PHONY: all
> > +all: build
> > +
> > +.PHONY: build
> > +build: xenlight
> > +
> > +.PHONY: install
> > +install: build
> > +   [ -f $(BINARY) ] || $(INSTALL_PROG) xenlight.go $(DESTDIR)$(bindir)
> 
> Is there a reason you decided to make this a binary (and to install it
> as a binary), rather than making a stub package?
I'm not sure what you mean here. Doesn't this install the go file xenlight.go, 
not a binary file? 
Or do you want me to create a package and install that instead package? 
Sorry, I'm a little lost here.
> 
> Thanks,
>  -George
Thanks, 
Ronald 

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

Re: [Xen-devel] [PATCH v4 10/24] x86/emul: Always use fault semantics for software events

[Boris Ostrovsky]

On 12/01/2016 11:55 AM, Andrew Cooper wrote:
> The common case is already using fault semantics out of x86_emulate(), as that
> is how VT-x/SVM expects to inject the event (given suitable hardware support).
>
> However, x86_emulate() returning X86EMUL_EXCEPTION and also completing a
> register writeback is problematic for callers.
>
> Switch the logic to always using fault semantics, and leave svm_inject_trap()
> to fix up %eip if necessary.
>
> Signed-off-by: Andrew Cooper 

Reviewed-by:  Boris Ostrovsky 


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

Re: [Xen-devel] [PATCH 1/2] x86: Make E820_X_MAX unconditionally larger than E820MAX

[Alex Thorlton]

On Wed, Nov 30, 2016 at 07:21:48AM +0100, Ingo Molnar wrote:
> 
> * Alex Thorlton  wrote:
> 
> > It's really not necessary to limit E820_X_MAX to 128 in the non-EFI
> > case.  This commit drops E820_X_MAX's dependency on CONFIG_EFI, so that
> > E820_X_MAX is always at least slightly larger than E820MAX.
> > 
> > The real motivation behind this is actually to prevent some issues in
> > the Xen kernel, where the XENMEM_machine_memory_map hypercall can
> > produce an e820 map larger than 128 entries, even on systems where the
> > original e820 table was quite a bit smaller than that, depending on how
> > many IOAPICs are installed on the system.
> > 
> > Signed-off-by: Alex Thorlton 
> > Suggested-by: Ingo Molnar 
> > Cc: Russ Anderson 
> > Cc: David Vrabel 
> > Cc: Ingo Molnar 
> > Cc: Juergen Gross 
> > Cc: Thomas Gleixner 
> > Cc: "H. Peter Anvin" 
> > Cc: Denys Vlasenko 
> > Cc: Boris Ostrovsky 
> > Cc: x...@kernel.org
> > Cc: xen-de...@lists.xenproject.org
> > ---
> >  arch/x86/include/asm/e820.h | 10 ++
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/e820.h b/arch/x86/include/asm/e820.h
> > index 476b574..aa00d33 100644
> > --- a/arch/x86/include/asm/e820.h
> > +++ b/arch/x86/include/asm/e820.h
> > @@ -1,13 +1,15 @@
> >  #ifndef _ASM_X86_E820_H
> >  #define _ASM_X86_E820_H
> >  
> > -#ifdef CONFIG_EFI
> > +/*
> > + * We need to make sure that E820_X_MAX is defined
> > + * before we include uapi/asm/e820.h
> > + */
> >  #include 
> >  #define E820_X_MAX (E820MAX + 3 * MAX_NUMNODES)
> 
> What we need an explanation for in the comment is what does this stand for 
> (what 
> does the 'X' mean?), and what is the magic 3*MAX_NUMNODES about?

I'm not actually certain why 3*MAX_NUMNODES was chosen as the max size
for this table, but it was written by somebody here at SGI, so I'm sure
I can find out :)

As for the 'X,' I'm assuming that's meant to imply that this is the
maxmimum size for the 'eXtended' table, but that could be made more
clear in the comment as well.

I'll come up with a better comment for this and submit a v3.

Thanks, Ingo!

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

[Xen-devel] [xen-4.7-testing test] 102744: tolerable FAIL - PUSHED

[osstest service owner]

flight 102744 xen-4.7-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/102744/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-i386-xl-raw   10 guest-start  fail in 102725 pass in 102744
 test-armhf-armhf-xl-vhd  9 debian-di-install fail in 102725 pass in 102744
 test-amd64-i386-rumprun-i386 16 rumprun-demo-xenstorels/xenstorels.repeat fail 
in 102725 pass in 102744
 test-armhf-armhf-libvirt-raw  9 debian-di-install  fail pass in 102725

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-libvirt-raw 12 saverestore-support-check fail in 102725 like 
102536
 test-armhf-armhf-xl-rtds 15 guest-start/debian.repeatfail  like 102518
 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail  like 102536
 test-armhf-armhf-libvirt 13 saverestore-support-checkfail  like 102536
 test-armhf-armhf-libvirt-qcow2 12 saverestore-support-check   fail like 102536
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 102536
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 102536

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt-raw 11 migrate-support-check fail in 102725 never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass

version targeted for testing:
 xen  e144f21309b1f637919433caf734600da34962ec
baseline version:
 xen  206fc7084dfaf05c55fd9de650f93a7ef9fe0722

Last test of basis   102536  2016-11-22 20:45:38 Z8 days
Failing since102711  2016-11-29 15:46:49 Z2 days3 attempts
Testing same since   102725  2016-11-30 03:06:03 Z1 days2 attempts


People who touched revisions under test:
  Ian Jackson 
  Stefano Stabellini 
  Wei Chen 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64-xtf  pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   

[Xen-devel] [ovmf test] 102745: all pass - PUSHED

[osstest service owner]

flight 102745 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/102745/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf eae631bc687c3cbfab01632ce3bc21783d22b590
baseline version:
 ovmf e148e6e9625f8a0054f131bacba4e5c9a21a4377

Last test of basis   102727  2016-11-30 04:36:01 Z1 days
Testing same since   102745  2016-12-01 00:13:56 Z0 days1 attempts


People who touched revisions under test:
  Ard Biesheuvel 
  Feng Tian 
  Jan Dabros 
  Jeff Fan 
  Jiewen Yao 
  Laszlo Ersek 
  Marcin Wojtas 
  Michael Kinney 
  Richard Thomaiyar 
  Ryan Harkin 
  Thomaiyar, Richard Marian 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

+ branch=ovmf
+ revision=eae631bc687c3cbfab01632ce3bc21783d22b590
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push ovmf 
eae631bc687c3cbfab01632ce3bc21783d22b590
+ branch=ovmf
+ revision=eae631bc687c3cbfab01632ce3bc21783d22b590
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=ovmf
+ xenbranch=xen-unstable
+ '[' xovmf = xlinux ']'
+ linuxbranch=
+ '[' x = x ']'
+ qemuubranch=qemu-upstream-unstable
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable
+ prevxenbranch=xen-4.7-testing
+ '[' xeae631bc687c3cbfab01632ce3bc21783d22b590 = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/xtf.git
++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git
++ : git://xenbits.xen.org/xtf.git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git
++ : git://git.seabios.org/seabios.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/seabios.git
++ : 

[Xen-devel] ARM: Request backport for Xen 4.6 and Xen 4.5

[Julien Grall]

Hi,

While testing XSA-201, I noticed that Xen 4.5 and 4.5 were not able to 
boot on the Foundation Model [1].


The Foundation Model is a free model provided by ARM for ARMv8 which is 
supported by Xen since the beginning.


I am able to Xen boot after applying the patch:

eb6fe7a "arm64: fix incorrect memory region size in TCR_EL2"

So would it be possible to backport this patch in Xen 4.6 and Xen 4.5?

Cheers,

[1] https://www.arm.com/products/tools/models/foundation-model.php

--
Julien Grall

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

Re: [Xen-devel] Kernel Panics on Xen ARM64 for Domain0 and Guest

[Julien Grall]

On 28/11/16 15:29, t...@kernel.org wrote:

Hello,


Hello,


On Mon, Nov 28, 2016 at 11:59:15AM +, Julien Grall wrote:

commit 3ca45a46f8af8c4a92dd8a08eac57787242d5021
percpu: ensure the requested alignment is power of two


It would have been useful to specify the tree used. In this case,
this commit comes from linux-next.


I'm surprised this actually triggered.


diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index f193414..4986dc0 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -372,8 +372,7 @@ static int __init xen_guest_init(void)
 * for secondary CPUs as they are brought up.
 * For uniformity we use VCPUOP_register_vcpu_info even on cpu0.
 */
-   xen_vcpu_info = __alloc_percpu(sizeof(struct vcpu_info),
-  sizeof(struct vcpu_info));
+   xen_vcpu_info = alloc_percpu(struct vcpu_info);
if (xen_vcpu_info == NULL)
return -ENOMEM;


Yes, this looks correct.  Can you please cc stable too?  percpu
allocator never supported alignments which aren't power of two and has
always behaved incorrectly with alignments which aren't power of two.


I will send the patch soon with stable CCed.

Regards,

--
Julien Grall

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

[Xen-devel] [ANNOUNCEMENT] Xen 4.8 RC8

[Ian Jackson]

Hi.

Xen 4.8.0 RC8 is tagged.  You can check it out from xen.git, where
there is a signed tag:

  git://xenbits.xen.org/xen.git 4.8.0-rc8

For you convenience, please find tarball and signature at:

  https://downloads.xenproject.org/release/xen/4.8.0-rc8/

Please send bug reports and test reports to
xen-de...@lists.xenproject.org. When sending reports, please CC
relevant maintainers and the Release Manager, wei.l...@citrix.com

Enjoy!

Ian.

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

Re: [Xen-devel] [PATCH v4 07/15] pvh/acpi: Install handlers for ACPI-related PVH IO accesses

[Boris Ostrovsky]

On 12/01/2016 11:32 AM, Jan Beulich wrote:
 On 29.11.16 at 16:33,  wrote:
>> +void hvm_acpi_init(struct domain *d)
>> +{
>> +if ( has_acpi_dm_ff(d) )
>> +return;
>> +
>> +register_portio_handler(d, XEN_ACPI_CPU_MAP,
>> +XEN_ACPI_CPU_MAP_LEN, acpi_guest_access);
>> +register_portio_handler(d, ACPI_GPE0_BLK_ADDRESS_V1,
>> +sizeof (d->arch.hvm_domain.acpi.gpe0_sts) +
>> +sizeof (d->arch.hvm_domain.acpi.gpe0_en),
> Keyword or not, we don't put spaces between sizeof and the
> opening parenthesis.
>
>> +acpi_guest_access);
>> +register_portio_handler(d, ACPI_PM1A_EVT_BLK_ADDRESS_V1,
>> +sizeof (d->arch.hvm_domain.acpi.pm1a_sts) +
>> +sizeof (d->arch.hvm_domain.acpi.pm1a_en),
>> +acpi_guest_access);
> Does it really result in most legible / maintainable code (in
> subsequent patches) if all three use the same handler?

I can split them into 2 --- one for pm1a/gpe0 and the other for CPU map.
They are indeed handled differently.


>
>> --- a/xen/include/public/arch-x86/hvm/save.h
>> +++ b/xen/include/public/arch-x86/hvm/save.h
>> @@ -532,6 +532,8 @@ struct hvm_hw_acpi {
>>  uint32_t tmr_val;   /* PM_TMR_BLK.TMR_VAL: 32bit free-running counter */
>>  uint16_t pm1a_sts;  /* PM1a_EVT_BLK.PM1a_STS: status register */
>>  uint16_t pm1a_en;   /* PM1a_EVT_BLK.PM1a_EN: enable register */
>> +uint16_t gpe0_sts;
>> +uint16_t gpe0_en;
>>  };
> Don't you need to create compat handling for the case where a
> smaller struct arrives during migration? Or do we zero extend
> structures nowadays without extra code being needed (assuming
> zero extension is what we want in that case in the first place)?

I thought that the fact that new fields are added at the end would make
this safe. And I assumed we will always read into a newly-allocated
hvm_domain, so those registers would be zero.

But being more explicit about this is probably better, I'll add
DECLARE_HVM_SAVE_TYPE_COMPAT.

Andrew, will this interfere with your hypervisor migration v2 plan?


-boris


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

[Xen-devel] [PATCH v4 18/24] x86/shadow: Avoid raising faults behind the emulators back

[Andrew Cooper]

Use x86_emul_{hw_exception,pagefault}() rather than
{pv,hvm}_inject_page_fault() and hvm_inject_hw_exception() to cause raised
faults to be known to the emulator.  This requires altering the callers of
x86_emulate() to properly re-inject the event.

Signed-off-by: Andrew Cooper 
Acked-by: Tim Deegan 
---
CC: Jan Beulich 

v4:
 * Fix the commit message following v3's changes.
 * s/is_hvm_vcpu/has_hvm_container_domain/
 * Adjust description of permitted exceptions.
 * Disallow #GP/#SS with non-zero error codes.
v3:
 * Split out #DB handling to an earlier part of the series
 * Don't inject #GP faults for unexpected events, but do reenter the guest.
v2:
 * New
---
 xen/arch/x86/mm/shadow/common.c | 13 ++---
 xen/arch/x86/mm/shadow/multi.c  | 40 +---
 2 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index f07803b..e509cc1 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -162,8 +162,9 @@ static int hvm_translate_linear_addr(
 
 if ( !okay )
 {
-hvm_inject_hw_exception(
-(seg == x86_seg_ss) ? TRAP_stack_error : TRAP_gp_fault, 0);
+x86_emul_hw_exception(
+(seg == x86_seg_ss) ? TRAP_stack_error : TRAP_gp_fault,
+0, _ctxt->ctxt);
 return X86EMUL_EXCEPTION;
 }
 
@@ -323,7 +324,7 @@ pv_emulate_read(enum x86_segment seg,
 
 if ( (rc = copy_from_user(p_data, (void *)offset, bytes)) != 0 )
 {
-pv_inject_page_fault(0, offset + bytes - rc); /* Read fault. */
+x86_emul_pagefault(0, offset + bytes - rc, ctxt); /* Read fault. */
 return X86EMUL_EXCEPTION;
 }
 
@@ -1720,10 +1721,8 @@ static mfn_t emulate_gva_to_mfn(struct vcpu *v, unsigned 
long vaddr,
 gfn = paging_get_hostmode(v)->gva_to_gfn(v, NULL, vaddr, );
 if ( gfn == gfn_x(INVALID_GFN) )
 {
-if ( is_hvm_vcpu(v) )
-hvm_inject_page_fault(pfec, vaddr);
-else
-pv_inject_page_fault(pfec, vaddr);
+x86_emul_pagefault(pfec, vaddr, _ctxt->ctxt);
+
 return _mfn(BAD_GVA_TO_GFN);
 }
 
diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
index 7d9081b..fc18d65 100644
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -3373,18 +3373,36 @@ static int sh_page_fault(struct vcpu *v,
 
 r = x86_emulate(_ctxt.ctxt, emul_ops);
 
-/*
- * The previous lack of inject_{sw,hw}*() hooks caused exceptions raised
- * by the emulator itself to become X86EMUL_UNHANDLEABLE.  Such exceptions
- * now set event_pending instead.  Exceptions raised behind the back of
- * the emulator don't yet set event_pending.
- *
- * For now, cause such cases to return to the X86EMUL_UNHANDLEABLE path,
- * for no functional change from before.  Future patches will fix this
- * properly.
- */
 if ( r == X86EMUL_EXCEPTION && emul_ctxt.ctxt.event_pending )
-r = X86EMUL_UNHANDLEABLE;
+{
+/*
+ * This emulation covers writes to shadow pagetables.  We tolerate #PF
+ * (from accesses spanning pages, concurrent paging updated from
+ * vcpus, etc) and #GP[0]/#SS[0] (from segmentation errors).  Anything
+ * else is an emulation bug, or a guest playing with the instruction
+ * stream under Xen's feet.
+ */
+if ( emul_ctxt.ctxt.event.type == X86_EVENTTYPE_HW_EXCEPTION &&
+ ((emul_ctxt.ctxt.event.vector == TRAP_page_fault) ||
+  (((emul_ctxt.ctxt.event.vector == TRAP_gp_fault) ||
+(emul_ctxt.ctxt.event.vector == TRAP_stack_error)) &&
+   emul_ctxt.ctxt.event.error_code == 0)) )
+{
+if ( has_hvm_container_domain(d) )
+hvm_inject_event(_ctxt.ctxt.event);
+else
+pv_inject_event(_ctxt.ctxt.event);
+
+goto emulate_done;
+}
+else
+{
+SHADOW_PRINTK(
+"Unexpected event (type %u, vector %#x) from emulation\n",
+emul_ctxt.ctxt.event.type, emul_ctxt.ctxt.event.vector);
+r = X86EMUL_UNHANDLEABLE;
+}
+}
 
 /*
  * NB. We do not unshadow on X86EMUL_EXCEPTION. It's not clear that it
-- 
2.1.4


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

[Xen-devel] [PATCH v4 17/24] x86/pv: Avoid raising faults behind the emulators back

[Andrew Cooper]

Use x86_emul_pagefault() rather than pv_inject_page_fault() to cause raised
pagefaults to be known to the emulator.  This requires altering the callers of
x86_emulate() to properly re-inject the event.

Signed-off-by: Andrew Cooper 
Acked-by: Tim Deegan 
Reviewed-by: Jan Beulich 
---
v4:
 * Tweak comment wording.
 * Drop default cases.
v3:
 * Split out #DB handling to an earlier part of the series
 * Don't raise #GP faults for unexpected events, but do return back to the
   guest.
v2:
 * New
---
 xen/arch/x86/mm.c | 101 ++
 1 file changed, 63 insertions(+), 38 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 5d59479..14552a1 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5136,7 +5136,7 @@ static int ptwr_emulated_read(
 if ( !__addr_ok(addr) ||
  (rc = __copy_from_user(p_data, (void *)addr, bytes)) )
 {
-pv_inject_page_fault(0, addr + bytes - rc); /* Read fault. */
+x86_emul_pagefault(0, addr + bytes - rc, ctxt);  /* Read fault. */
 return X86EMUL_EXCEPTION;
 }
 
@@ -5177,8 +5177,9 @@ static int ptwr_emulated_update(
 addr &= ~(sizeof(paddr_t)-1);
 if ( (rc = copy_from_user(, (void *)addr, sizeof(paddr_t))) != 0 )
 {
-pv_inject_page_fault(0, /* Read fault. */
- addr + sizeof(paddr_t) - rc);
+x86_emul_pagefault(0, /* Read fault. */
+   addr + sizeof(paddr_t) - rc,
+   _ctxt->ctxt);
 return X86EMUL_EXCEPTION;
 }
 /* Mask out bits provided by caller. */
@@ -5379,27 +5380,38 @@ int ptwr_do_page_fault(struct vcpu *v, unsigned long 
addr,
 page_unlock(page);
 put_page(page);
 
-/*
- * The previous lack of inject_{sw,hw}*() hooks caused exceptions raised
- * by the emulator itself to become X86EMUL_UNHANDLEABLE.  Such exceptions
- * now set event_pending instead.  Exceptions raised behind the back of
- * the emulator don't yet set event_pending.
- *
- * For now, cause such cases to return to the X86EMUL_UNHANDLEABLE path,
- * for no functional change from before.  Future patches will fix this
- * properly.
- */
-if ( rc == X86EMUL_EXCEPTION && ptwr_ctxt.ctxt.event_pending )
-rc = X86EMUL_UNHANDLEABLE;
+/* More strict than x86_emulate_wrapper(), as this is now true for PV. */
+ASSERT(ptwr_ctxt.ctxt.event_pending == (rc == X86EMUL_EXCEPTION));
 
-if ( rc == X86EMUL_UNHANDLEABLE )
-goto bail;
+switch ( rc )
+{
+case X86EMUL_EXCEPTION:
+/*
+ * This emulation only covers writes to pagetables which are marked
+ * read-only by Xen.  We tolerate #PF (in case a concurrent pagetable
+ * update has succeeded on a different vcpu).  Anything else is an
+ * emulation bug, or a guest playing with the instruction stream under
+ * Xen's feet.
+ */
+if ( ptwr_ctxt.ctxt.event.type == X86_EVENTTYPE_HW_EXCEPTION &&
+ ptwr_ctxt.ctxt.event.vector == TRAP_page_fault )
+pv_inject_event(_ctxt.ctxt.event);
+else
+gdprintk(XENLOG_WARNING,
+ "Unexpected event (type %u, vector %#x) from emulation\n",
+ ptwr_ctxt.ctxt.event.type, ptwr_ctxt.ctxt.event.vector);
 
-if ( ptwr_ctxt.ctxt.retire.singlestep )
-pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+/* Fallthrough */
+case X86EMUL_OKAY:
 
-perfc_incr(ptwr_emulations);
-return EXCRET_fault_fixed;
+if ( ptwr_ctxt.ctxt.retire.singlestep )
+pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+
+/* Fallthrough */
+case X86EMUL_RETRY:
+perfc_incr(ptwr_emulations);
+return EXCRET_fault_fixed;
+}
 
  bail:
 return 0;
@@ -5519,26 +5531,39 @@ int mmio_ro_do_page_fault(struct vcpu *v, unsigned long 
addr,
 else
 rc = x86_emulate(, _ro_emulate_ops);
 
-/*
- * The previous lack of inject_{sw,hw}*() hooks caused exceptions raised
- * by the emulator itself to become X86EMUL_UNHANDLEABLE.  Such exceptions
- * now set event_pending instead.  Exceptions raised behind the back of
- * the emulator don't yet set event_pending.
- *
- * For now, cause such cases to return to the X86EMUL_UNHANDLEABLE path,
- * for no functional change from before.  Future patches will fix this
- * properly.
- */
-if ( rc == X86EMUL_EXCEPTION && ctxt.event_pending )
-rc = X86EMUL_UNHANDLEABLE;
+/* More strict than x86_emulate_wrapper(), as this is now true for PV. */
+ASSERT(ctxt.event_pending == (rc == X86EMUL_EXCEPTION));
 
-if ( rc == X86EMUL_UNHANDLEABLE )
-return 0;
+switch ( rc )
+{
+case X86EMUL_EXCEPTION:
+/*
+ * This emulation only covers 

[Xen-devel] [PATCH v4 11/24] x86/emul: Implement singlestep as a retire flag

[Andrew Cooper]

The behaviour of singlestep is to raise #DB after the instruction has been
completed, but implementing it with inject_hw_exception() causes x86_emulate()
to return X86EMUL_EXCEPTION, despite succesfully completing execution of the
instruction, including register writeback.

Instead, use a retire flag to indicate singlestep, which causes x86_emulate()
to return X86EMUL_OKAY.

Update all callers of x86_emulate() to use the new retire flag.  This fixes
the behaviour of singlestep for shadow pagetable updates and mmcfg/mmio_ro
intercepts, which previously discarded the exception.

With this change, all uses of X86EMUL_EXCEPTION from x86_emulate() are
believed to have strictly fault semantics.

Signed-off-by: Andrew Cooper 
Reviewed-by: Paul Durrant 
Acked-by: Tim Deegan 
---
CC: Jan Beulich 

v4:
 * s/is_hvm_vcpu/has_hvm_container_domain/
 * Adjust comments and entry condition into the PAE second-half case.
v3:
 * New
---
 xen/arch/x86/hvm/emulate.c |  3 +++
 xen/arch/x86/mm.c  | 11 ++-
 xen/arch/x86/mm/shadow/multi.c | 35 ++
 xen/arch/x86/x86_emulate/x86_emulate.c |  9 -
 xen/arch/x86/x86_emulate/x86_emulate.h |  6 ++
 5 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index fe62500..91c79fa 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -1788,6 +1788,9 @@ static int _hvm_emulate_one(struct hvm_emulate_ctxt 
*hvmemul_ctxt,
 if ( rc != X86EMUL_OKAY )
 return rc;
 
+if ( hvmemul_ctxt->ctxt.retire.singlestep )
+hvm_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+
 new_intr_shadow = hvmemul_ctxt->intr_shadow;
 
 /* MOV-SS instruction toggles MOV-SS shadow, else we just clear it. */
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index b7c7122..231c7bf 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5382,6 +5382,9 @@ int ptwr_do_page_fault(struct vcpu *v, unsigned long addr,
 if ( rc == X86EMUL_UNHANDLEABLE )
 goto bail;
 
+if ( ptwr_ctxt.ctxt.retire.singlestep )
+pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+
 perfc_incr(ptwr_emulations);
 return EXCRET_fault_fixed;
 
@@ -5503,7 +5506,13 @@ int mmio_ro_do_page_fault(struct vcpu *v, unsigned long 
addr,
 else
 rc = x86_emulate(, _ro_emulate_ops);
 
-return rc != X86EMUL_UNHANDLEABLE ? EXCRET_fault_fixed : 0;
+if ( rc == X86EMUL_UNHANDLEABLE )
+return 0;
+
+if ( ctxt.retire.singlestep )
+pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+
+return EXCRET_fault_fixed;
 }
 
 void *alloc_xen_pagetable(void)
diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
index 9ee48a8..eac2330 100644
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -3422,18 +3422,36 @@ static int sh_page_fault(struct vcpu *v,
 v->arch.paging.last_write_emul_ok = 0;
 #endif
 
+if ( emul_ctxt.ctxt.retire.singlestep )
+{
+if ( has_hvm_container_domain(d) )
+hvm_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+else
+pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+}
+
 #if GUEST_PAGING_LEVELS == 3 /* PAE guest */
-if ( r == X86EMUL_OKAY ) {
+/*
+ * If there are no pending actions, emulate up to four extra instructions
+ * in the hope of catching the "second half" of a 64-bit pagetable write.
+ */
+if ( r == X86EMUL_OKAY && !emul_ctxt.ctxt.retire.raw )
+{
 int i, emulation_count=0;
 this_cpu(trace_emulate_initial_va) = va;
-/* Emulate up to four extra instructions in the hope of catching
- * the "second half" of a 64-bit pagetable write. */
+
 for ( i = 0 ; i < 4 ; i++ )
 {
 shadow_continue_emulation(_ctxt, regs);
 v->arch.paging.last_write_was_pt = 0;
 r = x86_emulate(_ctxt.ctxt, emul_ops);
-if ( r == X86EMUL_OKAY )
+
+/*
+ * Only continue the search for the second half if there are no
+ * exceptions or pending actions.  Otherwise, give up and re-enter
+ * the guest.
+ */
+if ( r == X86EMUL_OKAY && !emul_ctxt.ctxt.retire.raw )
 {
 emulation_count++;
 if ( v->arch.paging.last_write_was_pt )
@@ -3449,6 +3467,15 @@ static int sh_page_fault(struct vcpu *v,
 {
 perfc_incr(shadow_em_ex_fail);
 TRACE_SHADOW_PATH_FLAG(TRCE_SFLAG_EMULATION_LAST_FAILED);
+
+if ( emul_ctxt.ctxt.retire.singlestep )
+{
+if ( has_hvm_container_domain(d) )
+hvm_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+else
+  

[Xen-devel] [PATCH v4 10/24] x86/emul: Always use fault semantics for software events

[Andrew Cooper]

The common case is already using fault semantics out of x86_emulate(), as that
is how VT-x/SVM expects to inject the event (given suitable hardware support).

However, x86_emulate() returning X86EMUL_EXCEPTION and also completing a
register writeback is problematic for callers.

Switch the logic to always using fault semantics, and leave svm_inject_trap()
to fix up %eip if necessary.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Tim Deegan 
CC: Boris Ostrovsky 
CC: Suravee Suthikulpanit 

v4:
 * s/eip/rip/
 * Zero the high bits of regs->rip and vmcb->nextrip outside of 64bit mode.
 * Spelling fixes
v3:
 * New
---
 xen/arch/x86/hvm/svm/svm.c | 60 ++
 xen/arch/x86/x86_emulate/x86_emulate.c |  2 --
 xen/arch/x86/x86_emulate/x86_emulate.h |  8 -
 3 files changed, 46 insertions(+), 24 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 912d871..8c4f3c7 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1209,7 +1209,7 @@ static void svm_inject_event(const struct x86_event 
*event)
 struct vmcb_struct *vmcb = curr->arch.hvm_svm.vmcb;
 eventinj_t eventinj = vmcb->eventinj;
 struct x86_event _event = *event;
-const struct cpu_user_regs *regs = guest_cpu_user_regs();
+struct cpu_user_regs *regs = guest_cpu_user_regs();
 
 switch ( _event.vector )
 {
@@ -1242,44 +1242,52 @@ static void svm_inject_event(const struct x86_event 
*event)
 eventinj.fields.v = 1;
 eventinj.fields.vector = _event.vector;
 
-/* Refer to AMD Vol 2: System Programming, 15.20 Event Injection. */
+/*
+ * Refer to AMD Vol 2: System Programming, 15.20 Event Injection.
+ *
+ * On hardware lacking NextRIP support, and all hardware in the case of
+ * icebp, software events with trap semantics need emulating, so %rip in
+ * the trap frame points after the instruction.
+ *
+ * The x86 emulator (if requested by the x86_swint_emulate_* choice) will
+ * have performed checks such as presence/dpl/etc and believes that the
+ * event injection will succeed without faulting.
+ *
+ * The x86 emulator will always provide fault semantics for software
+ * events, with _trap.insn_len set appropriately.  If the injection
+ * requires emulation, move %rip forwards at this point.
+ */
 switch ( _event.type )
 {
 case X86_EVENTTYPE_SW_INTERRUPT: /* int $n */
-/*
- * Software interrupts (type 4) cannot be properly injected if the
- * processor doesn't support NextRIP.  Without NextRIP, the emulator
- * will have performed DPL and presence checks for us, and will have
- * moved eip forward if appropriate.
- */
 if ( cpu_has_svm_nrips )
-vmcb->nextrip = regs->eip + _event.insn_len;
+vmcb->nextrip = regs->rip + _event.insn_len;
+else
+regs->rip += _event.insn_len;
 eventinj.fields.type = X86_EVENTTYPE_SW_INTERRUPT;
 break;
 
 case X86_EVENTTYPE_PRI_SW_EXCEPTION: /* icebp */
 /*
- * icebp's injection must always be emulated.  Software injection help
- * in x86_emulate has moved eip forward, but NextRIP (if used) still
- * needs setting or execution will resume from 0.
+ * icebp's injection must always be emulated, as hardware does not
+ * special case HW_EXCEPTION with vector 1 (#DB) as having trap
+ * semantics.
  */
+regs->rip += _event.insn_len;
 if ( cpu_has_svm_nrips )
-vmcb->nextrip = regs->eip;
+vmcb->nextrip = regs->rip;
 eventinj.fields.type = X86_EVENTTYPE_HW_EXCEPTION;
 break;
 
 case X86_EVENTTYPE_SW_EXCEPTION: /* int3, into */
 /*
- * The AMD manual states that .type=3 (HW exception), .vector=3 or 4,
- * will perform DPL checks.  Experimentally, DPL and presence checks
- * are indeed performed, even without NextRIP support.
- *
- * However without NextRIP support, the event injection still needs
- * fully emulating to get the correct eip in the trap frame, yet get
- * the correct faulting eip should a fault occur.
+ * Hardware special cases HW_EXCEPTION with vectors 3 and 4 as having
+ * trap semantics, and will perform DPL checks.
  */
 if ( cpu_has_svm_nrips )
-vmcb->nextrip = regs->eip + _event.insn_len;
+vmcb->nextrip = regs->rip + _event.insn_len;
+else
+regs->rip += _event.insn_len;
 eventinj.fields.type = X86_EVENTTYPE_HW_EXCEPTION;
 break;
 
@@ -1290,6 +1298,16 @@ static void svm_inject_event(const struct x86_event 
*event)
 break;
 }
 
+/*
+ * If injecting an event outside of 64bit mode, zero 

[Xen-devel] [PATCH v4 for-4.9 00/24] [SUBSET] XSA-191 followup

[Andrew Cooper]

This is the quantity of changes required to fix some edgecases in XSA-191
which were ultimately chosen not to go out in the security fix.  The main
purpose of this series is to fix emulation sufficiently to allow the final
patch to avoid opencoding all of the segmenation logic.

Because this is a very long series and there are not many changes from v3, I
am only sending out the patches which have changes, to avoid spamming the
list.

Andrew Cooper (24):
  x86/shadow: Fix #PFs from emulated writes crossing a page boundary
  x86/emul: Drop X86EMUL_CMPXCHG_FAILED
  x86/emul: Simplfy emulation state setup
  x86/emul: Rename hvm_trap to x86_event and move it into the emulation 
infrastructure
  x86/emul: Rename HVM_DELIVER_NO_ERROR_CODE to X86_EVENT_NO_EC
  x86/pv: Implement pv_inject_{event,page_fault,hw_exception}()
  x86/emul: Clean up the naming of the retire union
  x86/emul: Correct the behaviour of pop %ss and interrupt shadowing
  x86/emul: Provide a wrapper to x86_emulate() to ASSERT() certain behaviour
  x86/emul: Always use fault semantics for software events
  x86/emul: Implement singlestep as a retire flag
  x86/emul: Remove opencoded exception generation
  x86/emul: Rework emulator event injection
  x86/vmx: Use hvm_{get,set}_segment_register() rather than 
vmx_{get,set}_segment_register()
  x86/hvm: Reposition the modification of raw segment data from the VMCB/VMCS
  x86/emul: Avoid raising faults behind the emulators back
  x86/pv: Avoid raising faults behind the emulators back
  x86/shadow: Avoid raising faults behind the emulators back
  x86/hvm: Extend the hvm_copy_*() API with a pagefault_info pointer
  x86/hvm: Reimplement hvm_copy_*_nofault() in terms of no pagefault_info
  x86/hvm: Rename hvm_copy_*_guest_virt() to hvm_copy_*_guest_linear()
  x86/hvm: Avoid __hvm_copy() raising #PF behind the emulators back
  x86/emul: Prepare to allow use of system segments for memory references
  x86/emul: Use system-segment relative memory accesses

 tools/tests/x86_emulator/test_x86_emulator.c |   1 +
 tools/tests/x86_emulator/x86_emulate.c   |   4 +-
 xen/arch/x86/hvm/emulate.c   | 147 ---
 xen/arch/x86/hvm/hvm.c   | 370 +++
 xen/arch/x86/hvm/io.c|   4 +-
 xen/arch/x86/hvm/nestedhvm.c |   2 +-
 xen/arch/x86/hvm/svm/nestedsvm.c |  13 +-
 xen/arch/x86/hvm/svm/svm.c   | 156 +--
 xen/arch/x86/hvm/vmx/intr.c  |   2 +-
 xen/arch/x86/hvm/vmx/realmode.c  |  16 +-
 xen/arch/x86/hvm/vmx/vmx.c   | 109 
 xen/arch/x86/hvm/vmx/vvmx.c  |  44 ++--
 xen/arch/x86/mm.c|  91 ++-
 xen/arch/x86/mm/shadow/common.c  |  40 +--
 xen/arch/x86/mm/shadow/multi.c   |  71 -
 xen/arch/x86/traps.c | 147 ++-
 xen/arch/x86/x86_emulate/x86_emulate.c   | 355 ++---
 xen/arch/x86/x86_emulate/x86_emulate.h   | 221 +---
 xen/include/asm-x86/desc.h   |   6 +
 xen/include/asm-x86/domain.h |  26 ++
 xen/include/asm-x86/hvm/emulate.h|   3 -
 xen/include/asm-x86/hvm/hvm.h|  86 +++
 xen/include/asm-x86/hvm/support.h|  42 ++-
 xen/include/asm-x86/hvm/svm/nestedsvm.h  |   6 +-
 xen/include/asm-x86/hvm/vcpu.h   |   2 +-
 xen/include/asm-x86/hvm/vmx/vmx.h|   2 -
 xen/include/asm-x86/hvm/vmx/vvmx.h   |   4 +-
 xen/include/asm-x86/mm.h |   1 -
 28 files changed, 1209 insertions(+), 762 deletions(-)

-- 
2.1.4


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

[Xen-devel] [PATCH v4 09/24] x86/emul: Provide a wrapper to x86_emulate() to ASSERT() certain behaviour

[Andrew Cooper]

In debug builds, confirm that some properties of x86_emulate()'s behaviour
actually hold.  The first property, fixed in a previous change, is that retire
flags are only ever set in the X86EMUL_OKAY case.

While adjusting the userspace test harness to cope with ASSERT() in
x86_emulate.h, fix a build problem introduced in c/s 122dd9575c7 "x86emul:
in_longmode() should not ignore ->read_msr() errors" by providing an
implementation of likely()/unlikely().

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 

v4:
 * Shuffle #ifdefary
 * Correct the documentation for x86_emulate()
 * Use true/false in likely/unlikely definitions
v3:
 * New
---
 tools/tests/x86_emulator/test_x86_emulator.c |  1 +
 tools/tests/x86_emulator/x86_emulate.c   |  4 +++-
 xen/arch/x86/x86_emulate/x86_emulate.c   |  3 +++
 xen/arch/x86/x86_emulate/x86_emulate.h   | 27 ++-
 4 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/tools/tests/x86_emulator/test_x86_emulator.c 
b/tools/tests/x86_emulator/test_x86_emulator.c
index f255fef..b54fd11 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -1,3 +1,4 @@
+#include 
 #include 
 #include 
 #include 
diff --git a/tools/tests/x86_emulator/x86_emulate.c 
b/tools/tests/x86_emulator/x86_emulate.c
index c46b7fc..897b9ab 100644
--- a/tools/tests/x86_emulator/x86_emulate.c
+++ b/tools/tests/x86_emulator/x86_emulate.c
@@ -16,7 +16,6 @@ typedef bool bool_t;
 #define EFER_LMA   (1 << 10)
 
 #define BUG() abort()
-#define ASSERT assert
 #define ASSERT_UNREACHABLE() assert(!__LINE__)
 
 #if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
@@ -50,4 +49,7 @@ typedef bool bool_t;
 #define __init
 #define __maybe_unused __attribute__((__unused__))
 
+#define likely(x) __builtin_expect(!!(x), true)
+#define unlikely(x)   __builtin_expect(!!(x), false)
+
 #include "x86_emulate/x86_emulate.c"
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c 
b/xen/arch/x86/x86_emulate/x86_emulate.c
index bfcc05d..3e602da 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -2404,6 +2404,9 @@ x86_decode(
 #undef insn_fetch_bytes
 #undef insn_fetch_type
 
+/* Undo DEBUG wrapper. */
+#undef x86_emulate
+
 int
 x86_emulate(
 struct x86_emulate_ctxt *ctxt,
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h 
b/xen/arch/x86/x86_emulate/x86_emulate.h
index ef39601..60f9105 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -23,6 +23,10 @@
 #ifndef __X86_EMULATE_H__
 #define __X86_EMULATE_H__
 
+#if !defined(__XEN__) && !defined(ASSERT)
+#define ASSERT assert
+#endif
+
 #define MAX_INST_LEN 15
 
 struct x86_emulate_ctxt;
@@ -546,13 +550,34 @@ struct x86_emulate_stub {
 
 /*
  * x86_emulate: Emulate an instruction.
- * Returns -1 on failure, 0 on success.
+ * Returns X86EMUL_* constants.
  */
 int
 x86_emulate(
 struct x86_emulate_ctxt *ctxt,
 const struct x86_emulate_ops *ops);
 
+#ifndef NDEBUG
+/*
+ * In debug builds, wrap x86_emulate() with some assertions about its expected
+ * behaviour.
+ */
+static inline int x86_emulate_wrapper(
+struct x86_emulate_ctxt *ctxt,
+const struct x86_emulate_ops *ops)
+{
+int rc = x86_emulate(ctxt, ops);
+
+/* Retire flags should only be set for successful instruction emulation. */
+if ( rc != X86EMUL_OKAY )
+ASSERT(ctxt->retire.raw == 0);
+
+return rc;
+}
+
+#define x86_emulate x86_emulate_wrapper
+#endif
+
 /*
  * Given the 'reg' portion of a ModRM byte, and a register block, return a
  * pointer into the block that addresses the relevant register.
-- 
2.1.4


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

Re: [Xen-devel] [PATCH v4 01/15] x86/pmtimer: Move ACPI registers from PMTState to hvm_domain

[Boris Ostrovsky]

On 12/01/2016 11:29 AM, Andrew Cooper wrote:
> On 01/12/16 16:28, Boris Ostrovsky wrote:
>> On 12/01/2016 10:52 AM, Jan Beulich wrote:
 --- a/xen/include/public/arch-x86/hvm/save.h
 +++ b/xen/include/public/arch-x86/hvm/save.h
 @@ -525,16 +525,16 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
  
  
  /*
 - * PM timer
 + * ACPI registers
   */
  
 -struct hvm_hw_pmtimer {
 +struct hvm_hw_acpi {
  uint32_t tmr_val;   /* PM_TMR_BLK.TMR_VAL: 32bit free-running counter 
 */
  uint16_t pm1a_sts;  /* PM1a_EVT_BLK.PM1a_STS: status register */
  uint16_t pm1a_en;   /* PM1a_EVT_BLK.PM1a_EN: enable register */
  };
  
 -DECLARE_HVM_SAVE_TYPE(PMTIMER, 13, struct hvm_hw_pmtimer);
 +DECLARE_HVM_SAVE_TYPE(ACPI, 13, struct hvm_hw_acpi);
>>> However much I appreciate this switch to a better name, I'm not
>>> convinced we can actually do this as easily: There's no
>>> __XEN_TOOLS__ guard anywhere in this file, and hence everything
>>> here is part of the stable ABI. I'm afraid you minimally will have to
>>> add interface version guards, retaining the old naming for old
>>> consumers.
>> Right, I haven't though about out-of-tree users. Should new fields
>> (added in patch 7) also be guarded?
> Be aware that my Hypervisor migration v2 plans (which follow the CPUID
> plans) will remove all of this (as it should never have gotten into the
> ABI to start with), and replace it with something looking suspiciously
> like the other migration v2 stream formats.
>
> If you can get away without changing names for now, probably best to
> just leave comment.

OK, I can leave it as pmtimer then.

-boris

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

Re: [Xen-devel] [PATCH v4 06/15] domctl: Add XEN_DOMCTL_acpi_access

[Boris Ostrovsky]

On 12/01/2016 11:06 AM, Jan Beulich wrote:
>
>> +++ b/xen/include/public/domctl.h
>> @@ -1144,6 +1144,29 @@ struct xen_domctl_psr_cat_op {
>>  typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
>>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
>>  
>> +/* ACPI Generic Address Structure */
>> +typedef struct gas {
> xen_acpi_gas
>
>> +#define XEN_ACPI_SYSTEM_MEMORY 0
>> +#define XEN_ACPI_SYSTEM_IO 1
>> +uint8_tspace_id;   /* Address space */
>> +uint8_tbit_width;  /* Size in bits of given register */
>> +uint8_tbit_offset; /* Bit offset within the register */
>> +uint8_taccess_width;   /* Minimum Access size (ACPI 3.0) */
>> +uint64_t   address;/* 64-bit address of register */
> uint64_aligned_t with explicit padding added ahead of it.
>
> And then there's the question of what uses of this will look like:
> I'm not convinced we need to stick to the exact ACPI layout
> here, unless you expect (or could imagine) for the tool stack to
> hold GAS structures coming from elsewhere in its hands. If we
> don't follow the layout as strictly, we could namely widen
> bit_width (and maybe bit_offset) to allow for larger transfers
> in one go. And in such a relaxed model I don't think we'd need
> access_width at all as a field.

There is indeed no current need to use actual ACPI GAS layout but then
it's not GAS, really, and should be named something else.

-boris


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

Re: [Xen-devel] [PATCH v4 07/15] pvh/acpi: Install handlers for ACPI-related PVH IO accesses

[Jan Beulich]

>>> On 29.11.16 at 16:33,  wrote:
> +void hvm_acpi_init(struct domain *d)
> +{
> +if ( has_acpi_dm_ff(d) )
> +return;
> +
> +register_portio_handler(d, XEN_ACPI_CPU_MAP,
> +XEN_ACPI_CPU_MAP_LEN, acpi_guest_access);
> +register_portio_handler(d, ACPI_GPE0_BLK_ADDRESS_V1,
> +sizeof (d->arch.hvm_domain.acpi.gpe0_sts) +
> +sizeof (d->arch.hvm_domain.acpi.gpe0_en),

Keyword or not, we don't put spaces between sizeof and the
opening parenthesis.

> +acpi_guest_access);
> +register_portio_handler(d, ACPI_PM1A_EVT_BLK_ADDRESS_V1,
> +sizeof (d->arch.hvm_domain.acpi.pm1a_sts) +
> +sizeof (d->arch.hvm_domain.acpi.pm1a_en),
> +acpi_guest_access);

Does it really result in most legible / maintainable code (in
subsequent patches) if all three use the same handler?

> --- a/xen/include/public/arch-x86/hvm/save.h
> +++ b/xen/include/public/arch-x86/hvm/save.h
> @@ -532,6 +532,8 @@ struct hvm_hw_acpi {
>  uint32_t tmr_val;   /* PM_TMR_BLK.TMR_VAL: 32bit free-running counter */
>  uint16_t pm1a_sts;  /* PM1a_EVT_BLK.PM1a_STS: status register */
>  uint16_t pm1a_en;   /* PM1a_EVT_BLK.PM1a_EN: enable register */
> +uint16_t gpe0_sts;
> +uint16_t gpe0_en;
>  };

Don't you need to create compat handling for the case where a
smaller struct arrives during migration? Or do we zero extend
structures nowadays without extra code being needed (assuming
zero extension is what we want in that case in the first place)?

Also the same remark regarding this not being __XEN_TOOLS__
guarded applies here.

Jan


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

Re: [Xen-devel] [PATCH v4 01/15] x86/pmtimer: Move ACPI registers from PMTState to hvm_domain

[Andrew Cooper]

On 01/12/16 16:28, Boris Ostrovsky wrote:
> On 12/01/2016 10:52 AM, Jan Beulich wrote:
>>> --- a/xen/include/public/arch-x86/hvm/save.h
>>> +++ b/xen/include/public/arch-x86/hvm/save.h
>>> @@ -525,16 +525,16 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
>>>  
>>>  
>>>  /*
>>> - * PM timer
>>> + * ACPI registers
>>>   */
>>>  
>>> -struct hvm_hw_pmtimer {
>>> +struct hvm_hw_acpi {
>>>  uint32_t tmr_val;   /* PM_TMR_BLK.TMR_VAL: 32bit free-running counter 
>>> */
>>>  uint16_t pm1a_sts;  /* PM1a_EVT_BLK.PM1a_STS: status register */
>>>  uint16_t pm1a_en;   /* PM1a_EVT_BLK.PM1a_EN: enable register */
>>>  };
>>>  
>>> -DECLARE_HVM_SAVE_TYPE(PMTIMER, 13, struct hvm_hw_pmtimer);
>>> +DECLARE_HVM_SAVE_TYPE(ACPI, 13, struct hvm_hw_acpi);
>> However much I appreciate this switch to a better name, I'm not
>> convinced we can actually do this as easily: There's no
>> __XEN_TOOLS__ guard anywhere in this file, and hence everything
>> here is part of the stable ABI. I'm afraid you minimally will have to
>> add interface version guards, retaining the old naming for old
>> consumers.
>
> Right, I haven't though about out-of-tree users. Should new fields
> (added in patch 7) also be guarded?

Be aware that my Hypervisor migration v2 plans (which follow the CPUID
plans) will remove all of this (as it should never have gotten into the
ABI to start with), and replace it with something looking suspiciously
like the other migration v2 stream formats.

If you can get away without changing names for now, probably best to
just leave comment.

~Andrew

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

Re: [Xen-devel] [PATCH v5 3/3] Significant changes to decision making; some new roles and minor changes

[Ian Jackson]

Lars Kurth writes ("Re: [PATCH v5 3/3] Significant changes to decision making; 
some new roles and minor changes"):
> Maybe Ian has some views on what is better from a theoretical viewpoint:
> Voting mechanisms are a bit of a hobby of his

The underlying problem here is that the reality is that the Xen
Project's by-far most important subproject is the hypervisor; that it
seems that the governance probably ought to reflect that; but that it
is difficult to do this without special casing it or providing an
objective metric of the hypervisor subproject's size.

I don't think it is possible to square this circle.  Our options are:

1. Explicitly recognise the hypervisor subproject as special.
   (This could be done by creating a new `superproject' maturity
   category, or simply by naming it explicitly.)

2. Do some kind of bodge which tries to reduce the impact of the
   potential unknown management practices of other subprojects
   (particularly, that they might appoint lots of leaders).

3. Restructure the hypervisor sub-project.

The current proposal is (2) and has the virtue of not incentivising a
subproject to appoint lots of leaders simply to get more votes
overall.  But it is still rather weak because it has to treat the
hypervisor subproject as only one amongst many, so hypervisor leaders
are under-powered and fringe leaders over-powered.

Another way to deal with this would be to split the hypervisor
subproject (3, above).  For example, we could create subprojects for
some subset of minios, osstest, xtf, various out-of-tree tools,...
(many of which would have only one leadership team member).

That would mean that the hypervisor-focused maintainers would get
additional votes via their other "hats".  (They would still get a vote
in the hypervisor subproject, if they have a hypervisor leadership
position too.)

This is perhaps less unnatural.  It still leaves fringe leaders
somewhat over-powered: this time, leaders of more-hypervisor-related
(or some such) fringe things, rather than leaders of
less-hypervisor-related fringe things.

> Another potential issue with the model above is that people could be in
> several leadership teams (not something we have today). So maybe we need
> to state that they can only vote once and need to chose for which team
> they vote. This opens up the possibility of tactical voting.

This is a bad idea for the reason you say.  If someone gets two votes
in this way, so be it.

Ian.

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

Re: [Xen-devel] [PATCH v4 05/15] acpi/x86: Define ACPI IO registers for PVH guests

[Boris Ostrovsky]

On 12/01/2016 10:57 AM, Jan Beulich wrote:
 On 29.11.16 at 16:33,  wrote:
>> --- a/xen/include/public/arch-x86/xen.h
>> +++ b/xen/include/public/arch-x86/xen.h
>> @@ -291,6 +291,13 @@ struct xen_arch_domainconfig {
>>   XEN_X86_EMU_PIT)
>>  uint32_t emulation_flags;
>>  };
>> +
>> +/* Location of online VCPU bitmap. */
>> +#define XEN_ACPI_CPU_MAP 0xaf00
>> +#define XEN_ACPI_CPU_MAP_LEN ((HVM_MAX_VCPUS + 7) / 8)
>> +
>> +/* GPE0 bit set during CPU hotplug */
>> +#define XEN_GPE0_CPUHP_BIT   2
> Here I'm unsure - isn't this an ACPI specific register? If so, the name
> would better be XEN_ACPI_GPE0_CPUHP_BIT.
>
> With that (or with a good reason why the current name is better)

No, it should have "ACPI".

-boris


> Reviewed-by: Jan Beulich 
> but the patch needs re-basing afaict.
>
> Jan
>


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

Re: [Xen-devel] [PATCH v4 01/15] x86/pmtimer: Move ACPI registers from PMTState to hvm_domain

[Boris Ostrovsky]

On 12/01/2016 10:52 AM, Jan Beulich wrote:
>> --- a/xen/include/public/arch-x86/hvm/save.h
>> +++ b/xen/include/public/arch-x86/hvm/save.h
>> @@ -525,16 +525,16 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
>>  
>>  
>>  /*
>> - * PM timer
>> + * ACPI registers
>>   */
>>  
>> -struct hvm_hw_pmtimer {
>> +struct hvm_hw_acpi {
>>  uint32_t tmr_val;   /* PM_TMR_BLK.TMR_VAL: 32bit free-running counter */
>>  uint16_t pm1a_sts;  /* PM1a_EVT_BLK.PM1a_STS: status register */
>>  uint16_t pm1a_en;   /* PM1a_EVT_BLK.PM1a_EN: enable register */
>>  };
>>  
>> -DECLARE_HVM_SAVE_TYPE(PMTIMER, 13, struct hvm_hw_pmtimer);
>> +DECLARE_HVM_SAVE_TYPE(ACPI, 13, struct hvm_hw_acpi);
> However much I appreciate this switch to a better name, I'm not
> convinced we can actually do this as easily: There's no
> __XEN_TOOLS__ guard anywhere in this file, and hence everything
> here is part of the stable ABI. I'm afraid you minimally will have to
> add interface version guards, retaining the old naming for old
> consumers.


Right, I haven't though about out-of-tree users. Should new fields
(added in patch 7) also be guarded?

-boris


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

[Xen-devel] [xen-4.6-testing test] 102741: tolerable FAIL - PUSHED

[osstest service owner]

flight 102741 xen-4.6-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/102741/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-i386-freebsd10-amd64 10 guest-start   fail in 102723 pass in 102741
 test-amd64-i386-xl-raw9 debian-di-install  fail pass in 102723
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 15 
guest-localmigrate/x10 fail pass in 102723
 test-amd64-i386-xl-qemuu-debianhvm-amd64 9 debian-hvm-install fail pass in 
102723
 test-armhf-armhf-xl-multivcpu  9 debian-installfail pass in 102723
 test-armhf-armhf-xl-cubietruck 16 guest-start.2fail pass in 102723
 test-amd64-amd64-xl-qemuu-winxpsp3 15 guest-localmigrate/x10 fail pass in 
102723

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-libvirt 13 saverestore-support-checkfail  like 102712
 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail  like 102712
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 102712
 test-armhf-armhf-libvirt-qcow2 12 saverestore-support-check   fail like 102712
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 102712
 test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail  like 102712
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 102712
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail like 102712
 test-armhf-armhf-xl-rtds 15 guest-start/debian.repeatfail  like 102712

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-multivcpu 12 migrate-support-check fail in 102723 never 
pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-check fail in 102723 
never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  22f70a3edd2f7f798391a381433da350e2272871
baseline version:
 xen  0ba95621b8988ad5ceb76b43e76be404fd798f7b

Last test of basis   102712  2016-11-29 15:46:48 Z2 days
Testing same since   102723  2016-11-30 01:15:21 Z1 days2 attempts


People who touched revisions under test:
  Ian Jackson 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64-xtf  pass
 build-amd64  pass
 build-armhf  pass
 build-i386  

Re: [Xen-devel] [PATCH v10 06/13] efi: create new early memory allocator

[Daniel Kiper]

On Thu, Dec 01, 2016 at 09:08:45AM -0700, Jan Beulich wrote:
> >>> On 01.12.16 at 16:41,  wrote:
> > On Thu, Dec 01, 2016 at 06:13:34AM -0700, Jan Beulich wrote:
> >> >>> On 30.11.16 at 14:04,  wrote:
> >> > --- a/xen/common/efi/boot.c
> >> > +++ b/xen/common/efi/boot.c
> >> > @@ -98,6 +98,52 @@ static CHAR16 __initdata newline[] = L"\r\n";
> >> >  #define PrintStr(s) StdOut->OutputString(StdOut, s)
> >> >  #define PrintErr(s) StdErr->OutputString(StdErr, s)
> >> >
> >> > +#ifndef CONFIG_ARM
> >> > +
> >> > +/*
> >> > + * TODO: Enable EFI boot allocator on ARM.
> >> > + * This code can be common for x86 and ARM.
> >> > + * Things TODO on ARM before enabling ebmalloc:
> >> > + *   - estimate required EBMALLOC_SIZE value,
> >> > + *   - where (in which section) ebmalloc_mem[] should live; if in 
> >> > .bss.page_aligned
> >> > + * then whole BSS zeroing have to be disabled in 
> >> > xen/arch/arm/arm64/head.S;
> >> > + * though BSS should be initialized somehow before use of variables 
> >> > living there,
> >> > + *   - call free_ebmalloc_unused_mem() sowehere in init code.
> >> > + */
> >> > +
> >> > +#define EBMALLOC_SIZE   MB(1)
> >>
> >> The previous communication with Julien ended in it being acceptable
> >> to him for this to be zero for ARM for now, eliminating (or at least
> >> reducing) the #ifndef CONFIG_ARM guarded region(s).
> >
> > That would be nice. Sadly it does not solve problem because ebmalloc()
> > and free_ebmalloc_unused_mem() are static and have to be #ifdef around
> > them too. Otherwise compiler complains because there are no callers for
> > both functions on ARM.
>
> How about attaching __maybe_unused to these two functions?

If you are OK with it I can do it.

Daniel

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

Re: [Xen-devel] [PATCH v10 06/13] efi: create new early memory allocator

[Jan Beulich]

>>> On 01.12.16 at 16:41,  wrote:
> On Thu, Dec 01, 2016 at 06:13:34AM -0700, Jan Beulich wrote:
>> >>> On 30.11.16 at 14:04,  wrote:
>> > --- a/xen/common/efi/boot.c
>> > +++ b/xen/common/efi/boot.c
>> > @@ -98,6 +98,52 @@ static CHAR16 __initdata newline[] = L"\r\n";
>> >  #define PrintStr(s) StdOut->OutputString(StdOut, s)
>> >  #define PrintErr(s) StdErr->OutputString(StdErr, s)
>> >
>> > +#ifndef CONFIG_ARM
>> > +
>> > +/*
>> > + * TODO: Enable EFI boot allocator on ARM.
>> > + * This code can be common for x86 and ARM.
>> > + * Things TODO on ARM before enabling ebmalloc:
>> > + *   - estimate required EBMALLOC_SIZE value,
>> > + *   - where (in which section) ebmalloc_mem[] should live; if in 
>> > .bss.page_aligned
>> > + * then whole BSS zeroing have to be disabled in 
>> > xen/arch/arm/arm64/head.S;
>> > + * though BSS should be initialized somehow before use of variables 
>> > living there,
>> > + *   - call free_ebmalloc_unused_mem() sowehere in init code.
>> > + */
>> > +
>> > +#define EBMALLOC_SIZE MB(1)
>>
>> The previous communication with Julien ended in it being acceptable
>> to him for this to be zero for ARM for now, eliminating (or at least
>> reducing) the #ifndef CONFIG_ARM guarded region(s).
> 
> That would be nice. Sadly it does not solve problem because ebmalloc()
> and free_ebmalloc_unused_mem() are static and have to be #ifdef around
> them too. Otherwise compiler complains because there are no callers for
> both functions on ARM.

How about attaching __maybe_unused to these two functions?

Jan


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

Re: [Xen-devel] [PATCH v4 06/15] domctl: Add XEN_DOMCTL_acpi_access

[Jan Beulich]

>>> On 29.11.16 at 16:33,  wrote:
> --- /dev/null
> +++ b/xen/arch/x86/hvm/acpi.c
> @@ -0,0 +1,24 @@
> +/* acpi.c: ACPI access handling
> + *
> + * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
> + */
> +#include 
> +#include 
> +#include 
> +
> +
> +int hvm_acpi_domctl_access(struct domain *d, uint8_t rw, gas_t *gas,

I guess the gas pointer can be const?

> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -1144,6 +1144,29 @@ struct xen_domctl_psr_cat_op {
>  typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
>  
> +/* ACPI Generic Address Structure */
> +typedef struct gas {

xen_acpi_gas

> +#define XEN_ACPI_SYSTEM_MEMORY 0
> +#define XEN_ACPI_SYSTEM_IO 1
> +uint8_tspace_id;   /* Address space */
> +uint8_tbit_width;  /* Size in bits of given register */
> +uint8_tbit_offset; /* Bit offset within the register */
> +uint8_taccess_width;   /* Minimum Access size (ACPI 3.0) */
> +uint64_t   address;/* 64-bit address of register */

uint64_aligned_t with explicit padding added ahead of it.

And then there's the question of what uses of this will look like:
I'm not convinced we need to stick to the exact ACPI layout
here, unless you expect (or could imagine) for the tool stack to
hold GAS structures coming from elsewhere in its hands. If we
don't follow the layout as strictly, we could namely widen
bit_width (and maybe bit_offset) to allow for larger transfers
in one go. And in such a relaxed model I don't think we'd need
access_width at all as a field.

> +} gas_t;

xen_acpi_gas_t

> +
> +struct xen_domctl_acpi_access {
> +gas_t  gas;/* IN: Register being accessed */
> +
> +#define XEN_DOMCTL_ACPI_READ   0
> +#define XEN_DOMCTL_ACPI_WRITE  1
> +uint8_trw; /* IN: Read or write */
> +
> +XEN_GUEST_HANDLE_64(uint8) val;/* IN/OUT: data */

I'd prefer if we made this void, as there's no type associated with
the data really. And please add explicit padding again.

Jan


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

Re: [Xen-devel] [PATCH v4 05/15] acpi/x86: Define ACPI IO registers for PVH guests

[Jan Beulich]

>>> On 29.11.16 at 16:33,  wrote:
> --- a/xen/include/public/arch-x86/xen.h
> +++ b/xen/include/public/arch-x86/xen.h
> @@ -291,6 +291,13 @@ struct xen_arch_domainconfig {
>   XEN_X86_EMU_PIT)
>  uint32_t emulation_flags;
>  };
> +
> +/* Location of online VCPU bitmap. */
> +#define XEN_ACPI_CPU_MAP 0xaf00
> +#define XEN_ACPI_CPU_MAP_LEN ((HVM_MAX_VCPUS + 7) / 8)
> +
> +/* GPE0 bit set during CPU hotplug */
> +#define XEN_GPE0_CPUHP_BIT   2

Here I'm unsure - isn't this an ACPI specific register? If so, the name
would better be XEN_ACPI_GPE0_CPUHP_BIT.

With that (or with a good reason why the current name is better)
Reviewed-by: Jan Beulich 
but the patch needs re-basing afaict.

Jan


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

Re: [Xen-devel] [PATCH v4 01/15] x86/pmtimer: Move ACPI registers from PMTState to hvm_domain

[Jan Beulich]

>>> On 29.11.16 at 16:33,  wrote:
> @@ -108,12 +111,12 @@ static void pmt_update_time(PMTState *s)
>  s->last_gtime = curr_gtime;
>  
>  /* Update timer value atomically wrt lock-free reads in handle_pmt_io(). 
> */
> -*(volatile uint32_t *)>pm.tmr_val = tmr_val;
> +*(volatile uint32_t *)>tmr_val = tmr_val;

Please use write_atomic() instead of retaining this casting.

> @@ -197,7 +202,7 @@ static int handle_evt_io(
>  }
>  else /* p->dir == IOREQ_READ */
>  {
> -data = s->pm.pm1a_sts | (((uint32_t) s->pm.pm1a_en) << 16);
> +data = acpi->pm1a_sts | (((uint32_t) acpi->pm1a_en) << 16);

Please drop the stray blank and parentheses.

> @@ -237,16 +243,17 @@ static int handle_pmt_io(
>   * updated value with a lock-free atomic read.
>   */
>  spin_barrier(>lock);
> -*val = read_atomic(>pm.tmr_val);
> +*val = read_atomic(&(acpi->tmr_val));

Please don't add unnecessary parentheses.

> @@ -261,21 +268,21 @@ static int pmtimer_save(struct domain *d, 
> hvm_domain_context_t *h)
>  x = (((s->vcpu->arch.hvm_vcpu.guest_time ?: hvm_get_guest_time(s->vcpu)) 
> -
>s->last_gtime) * s->scale) >> 32;
>  if ( x < 1UL<<31 )
> -s->pm.tmr_val += x;
> -if ( (s->pm.tmr_val & TMR_VAL_MSB) != msb )
> -s->pm.pm1a_sts |= TMR_STS;
> + acpi->tmr_val += x;

Hard tab.

> --- a/xen/include/public/arch-x86/hvm/save.h
> +++ b/xen/include/public/arch-x86/hvm/save.h
> @@ -525,16 +525,16 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
>  
>  
>  /*
> - * PM timer
> + * ACPI registers
>   */
>  
> -struct hvm_hw_pmtimer {
> +struct hvm_hw_acpi {
>  uint32_t tmr_val;   /* PM_TMR_BLK.TMR_VAL: 32bit free-running counter */
>  uint16_t pm1a_sts;  /* PM1a_EVT_BLK.PM1a_STS: status register */
>  uint16_t pm1a_en;   /* PM1a_EVT_BLK.PM1a_EN: enable register */
>  };
>  
> -DECLARE_HVM_SAVE_TYPE(PMTIMER, 13, struct hvm_hw_pmtimer);
> +DECLARE_HVM_SAVE_TYPE(ACPI, 13, struct hvm_hw_acpi);

However much I appreciate this switch to a better name, I'm not
convinced we can actually do this as easily: There's no
__XEN_TOOLS__ guard anywhere in this file, and hence everything
here is part of the stable ABI. I'm afraid you minimally will have to
add interface version guards, retaining the old naming for old
consumers.

Jan


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

Re: [Xen-devel] Xen ARM - Exposing a PL011 to the guest

[Julien Grall]

Hi Christoffer,

On 30/11/16 16:24, Christoffer Dall wrote:

On Wed, Nov 30, 2016 at 03:29:32PM +, Julien Grall wrote:

Hi all,

Few months ago, Linaro has published the version 2 of the VM
specification [1].

For those who don't know, the specification provides guidelines to
guarantee a compliant OS images could run on various hypervisor (e.g
Xen, KVM).

Looking at the specification, it will require Xen to expose new
devices to the guest: pl011, rtc, persistent flash (for UEFI
variables).

The RTC and persistent will only be used by the UEFI firwmare.


Why would a guest booting without UEFI not want to use the RTC directly?


Sorry, I had only the VM spec in mind. Of course, an RTC could be used 
for guest booting without UEFI.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v10 06/13] efi: create new early memory allocator

[Daniel Kiper]

On Thu, Dec 01, 2016 at 06:13:34AM -0700, Jan Beulich wrote:
> >>> On 30.11.16 at 14:04,  wrote:
> > --- a/xen/common/efi/boot.c
> > +++ b/xen/common/efi/boot.c
> > @@ -98,6 +98,52 @@ static CHAR16 __initdata newline[] = L"\r\n";
> >  #define PrintStr(s) StdOut->OutputString(StdOut, s)
> >  #define PrintErr(s) StdErr->OutputString(StdErr, s)
> >
> > +#ifndef CONFIG_ARM
> > +
> > +/*
> > + * TODO: Enable EFI boot allocator on ARM.
> > + * This code can be common for x86 and ARM.
> > + * Things TODO on ARM before enabling ebmalloc:
> > + *   - estimate required EBMALLOC_SIZE value,
> > + *   - where (in which section) ebmalloc_mem[] should live; if in 
> > .bss.page_aligned
> > + * then whole BSS zeroing have to be disabled in 
> > xen/arch/arm/arm64/head.S;
> > + * though BSS should be initialized somehow before use of variables 
> > living there,
> > + *   - call free_ebmalloc_unused_mem() sowehere in init code.
> > + */
> > +
> > +#define EBMALLOC_SIZE  MB(1)
>
> The previous communication with Julien ended in it being acceptable
> to him for this to be zero for ARM for now, eliminating (or at least
> reducing) the #ifndef CONFIG_ARM guarded region(s).

That would be nice. Sadly it does not solve problem because ebmalloc()
and free_ebmalloc_unused_mem() are static and have to be #ifdef around
them too. Otherwise compiler complains because there are no callers for
both functions on ARM. So, I think that proposed solution is the simplest one.
I understand that you would like to reduce number of changes needed to
introduce EFI boot allocator on ARM. Though I am not sure how we can
simplify this step further.

Daniel

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

Re: [Xen-devel] Xen ARM small task (WAS: Re: [Xen Question])

[Julien Grall]

On 01/12/16 02:07, Stefano Stabellini wrote:

On Fri, 25 Nov 2016, Julien Grall wrote:

Hi Stefano,


Hi,


On 23/11/16 19:55, Stefano Stabellini wrote:

Actually I am thinking that the default values should be in the
emulators themselves. After all they are the part of the code that knows
more about vuarts.


Can you expand what you mean by emulator? I was never expecting to have a
fully emulated UART exposed to the guest (i.e read/write character support)
except for a PL011.


Once we start having emulators, it is possible that we'll end up with
more than one. For example, we introduce the PL011 now, then in a couple
of years somebody wants to add ns16550 because it is the only one that
Windows 2019 supports. I am assuming that one way or another they'll run
in a low privilege mode (see other recent threads).


Well, if this Windows is meant to run on SBSA complaint server, it will 
have to support either PL011 or SBSA (a subset of PL011).


If we are going to allow more kind of UART? Why don't we have a disk 
emulator in Xen? How about a network emulator? Overall Windows 2019 may 
not have PV drivers for the network and disk.


I really think we have to draw a line of what we are supporting. The 
PL011 is mandatory by a specification. If the guest is not compliant 
then it will have to use either PV drivers or having a device assigned.


The addition of a new emulator in upstream would need to be justified. I 
don't want to end up bringing QEMU in Xen.



The current vuart (see xen/arch/arm/vuart.c) is very simple but require
someone to configure it. For DOM0, this is configured by the serial driver.
For guest we need someone doing the same.


I understand. For clarity, I'll call "PL011 emulator" the one that will
end up being used for DomUs, which might be based on, or different from,
xen/arch/arm/vuart.c. It doesn't exist yet.

The PL011 emulator should have default values for everything. Some of
these values could be configured by libxl, but none should be required
to be configured by libxl. The last thing we want is to disseminate
numbers and addresses in libxl. One of these parameters could be the
MMIO address, but it is just an example, we don't necessarily need to
support changing it. It could be a decent feature to have but I don't
think is important if we'll support configurable memory layouts soon.


This is what I have been suggested from the beginning :).

But in the case of baremetal application, we want to be able to do 
logging only (i.e not reading). They will have a driver for the host 
UART. It would be pointless and potentially difficult to emulate a full 
UART here. This is where vuart come in place (see the use case mentioned 
by Edgar).



So the toolstack would pass down all the info provided by the users to
Xen. Xen would start the appropriate emulator, initializing anything not
specifically configured by libxl to default values. No need for long
lists of defaults in libxl.



If so, you will end up people asking to implement each of their UART
(8250,
exynos, pl011...) in the toolstack. A user would have to pay attention
whether
this model is supported or not by their toolstack.


It is up to the maintainers to decide how many and which vuart should be
configurable. libxl would have the capability of listing supported models
of vuarts. Today libxl already does that for nics and vgas.


As we discussed recently, the goal of exposing the vuart is to let the guest
write data not read without having to bring a full PV drivers.

Supporting multiple fully emulated UARTs would be very similarly to
incorporate piece of QEMU code within Xen. I think we both well know what it
means in term of security.

We have to emulate a PL011 because this is part of the VM spec. If you think
that more kind of UART have to be emulated, then I would like to see real use
case as nobody stepped up for that on the ML so far.


Unfortunately we have to expect that the number of requests for
emulators will only increase going forward. We need to have a proper low
privilege mode to run them in, to avoid security issues in Xen.


See my answer above.





Lastly, the pl011 emulation needs to be easily enabled by any user without
requiring a knowledge on the guest memory layout (which is not stable
BTW). By
default the layout is static, so what's the point to let the user
configuring
it?


This is my reasoning: people that request a vuart explicitly in the VM
config file are people that are configuring an embedded system with
non-Linux OSes because all others should be able to use the PV console
effectively.

In that case, to setup the system with the minimum amount of
configuration and efforts, they might want to emulate one of the UARTs
supported by their non-Linux OSes. The PL011 is pretty widespread, so it
could be a good choice.
Additionally they know the memory layout of all their VMs, so they can
easily pick an unused address and configure it both in the VM config
file and in their non-Linux OS.


Their 

Re: [Xen-devel] [RFD] OP-TEE (and probably other TEEs) support

[Volodymyr Babchuk]

Hi Julien

On 1 December 2016 at 17:19, Julien Grall  wrote:
> On 29/11/16 19:19, Volodymyr Babchuk wrote:
>>
>> Hi Julien,
>
>
> Hi Volodymyr,
>
>
>>
>>
>> On 29 November 2016 at 20:55, Julien Grall  wrote:
>>>
>>> Hi Volodymyr,
>>>
>>> On 29/11/16 17:40, Volodymyr Babchuk wrote:


 On 29 November 2016 at 18:02, Julien Grall  wrote:
>
>
> On 29/11/16 15:27, Volodymyr Babchuk wrote:
>>
>>
>> On 28 November 2016 at 22:10, Julien Grall 
>> wrote:
>>>
>>>
>>> On 28/11/16 18:09, Volodymyr Babchuk wrote:


 On 28 November 2016 at 18:14, Julien Grall 
 wrote:
>
>
> On 24/11/16 21:10, Volodymyr Babchuk wrote:
>
>
> I don't follow your point here. Why would the SMC handler need to map
> the
> guest memory?


 Because this is how parameters are passed. We can pass some parameters
 in registers, but for example in OP-TEE registers hold only address of
 command buffer. In this command buffer there are actual parameters.
 Some of those parameters can be references to another memory objects.
 So, to translate IPAs to PAs we need to map this command buffer,
 analyze it and so on.
>>>
>>>
>>>
>>> So the SMC issued will contain a PA of a page belonging to the guest or
>>> Xen?
>>
>> It will be guest page. But all references to other pages will have
>> real PAs, so TEE can work with them.
>>
>> Lets dive into example: hypervisor traps SMC, mediation layer (see
>> below) can see that there was INVOKE_COMMAND request. There are
>> address of command buffer in register pair (r1, r2). Mediation layer
>> changes address in this register pair to real PA of the command
>> buffer. Then it maps specified page and checks parameters. One of
>> parameters have type MEMREF, so mediation layer has to change IPA of
>> specified buffer to PA. Then it issues real SMC call.
>> After return from SMC it inspects registers and buffer again and
>> replaces memory references back.
>
>
> I was about to ask whether SMC call have some kind of metadata to know the
> parameter, but you answered it on another mail. So I will follow-up there.
Yes, it looked like question to me, so I answered there.

> Regarding the rest, you said that the buffer passed to the real TEE will be
> baked into guest memory. There are few problems with that you don't seem to
> address in this design document:
> - The buffer may be contiguous in the IPA space but discontinuous in
> PA space. This is because Xen may not be able to allocate all the memory for
> the guest contiguously in PA space. So how do you plan to handle buffer
> greater than a Xen page granularity (i.e 4K)
Yep. Currently I'm finishing to rework memory handling in OP-TEE. All
memory-related requests will work with pages lists. That should
eliminate this problem.

> - Can all type memory could be passed to TEE (e.g foreign page,
> grant, mmio...)? I suspect not.
Currently I plan to map only user pages or pages allocated by
__get_free_pages(). Right now I can't image why we need to support
other memory types.

> - TEE may run in parallel of the guest OS, this means that we have
> to make sure the page will never be removed by the guest OS (see
> XENMEM_decrease_reservation hypercall).
Hmmm... I don't know how XEN handles guest memory in details. Can we
somehow pin pages, so they can't be removed until client unregisters
shared memory buffer?
In new OP-TEE shmem design there will be call to register shared
memory. Client will pass list of pages in this call and OP-TEE will
map them in its address space. At this moment we need to pin them in
hypervisor, so guest can't get rid of them, until "unregister shared
memory" call made. Is this possible?

> - The IPA -> PA translation can be slow as this would need to be
> done in software (see p2m_lookup). Is there any upper limit of the number of
> buffer and indirection available?
They are limited by virtual address space in OP-TEE. Currently it is
about 16M for shared memory.
There will be at least one IPA->PA translation per std SMC call (to
find command buffer). More translations if this is "register shared
memory" call.
In other call types OP-TEE uses shared memory cookie (reference)
instead of addresses and resolves this cookie to actual address on
OP-TEE core side. This should minimize count of IPA->PA translations.

-- 
WBR Volodymyr Babchuk aka lorc [+380976646013]
mailto: vlad.babc...@gmail.com

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

Re: [Xen-devel] [PATCH 2/8] x86/head: Refactor 32-bit pgtable setup

[Boris Ostrovsky]

On 10/31/2016 08:33 AM, Boris Ostrovsky wrote:
>
>
> On 10/14/2016 02:05 PM, Boris Ostrovsky wrote:
>> From: Matt Fleming 
>>
>> The new Xen PVH entry point requires page tables to be setup by the
>> kernel since it is entered with paging disabled.
>>
>> Pull the common code out of head_32.S and into pgtable_32.S so that
>> setup_pgtable_32 can be invoked from both the new Xen entry point and
>> the existing startup_32 code.
>
>
> Ping to x86 maintainers.

Pinging again.

I will be re-sending this series at some point (it has been delayed by
some hypervisor changes that will be needed) but I'd like to hear from
x86 maintainers whether this will be acceptable before I post this again.

Thanks.
-boris

>
> Peter, you had questions about this patch. Did I answer them?
>
> -boris
>
>
>>
>> Cc: Boris Ostrovsky 
>> Cc: Thomas Gleixner 
>> Cc: Ingo Molnar 
>> Cc: "H. Peter Anvin" 
>> Cc: x...@kernel.org
>> Signed-off-by: Matt Fleming 
>> ---
>>  arch/x86/Makefile|   2 +
>>  arch/x86/kernel/Makefile |   2 +
>>  arch/x86/kernel/head_32.S| 168
>> +
>>  arch/x86/kernel/pgtable_32.S | 196
>> +++
>>  4 files changed, 201 insertions(+), 167 deletions(-)
>>  create mode 100644 arch/x86/kernel/pgtable_32.S
>>
>> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
>> index 2d44933..67cc771 100644
>> --- a/arch/x86/Makefile
>> +++ b/arch/x86/Makefile
>> @@ -204,6 +204,8 @@ head-y += arch/x86/kernel/head$(BITS).o
>>  head-y += arch/x86/kernel/ebda.o
>>  head-y += arch/x86/kernel/platform-quirks.o
>>
>> +head-$(CONFIG_X86_32) += arch/x86/kernel/pgtable_32.o
>> +
>>  libs-y  += arch/x86/lib/
>>
>>  # See arch/x86/Kbuild for content of core part of the kernel
>> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
>> index 4dd5d50..eae85a5 100644
>> --- a/arch/x86/kernel/Makefile
>> +++ b/arch/x86/kernel/Makefile
>> @@ -8,6 +8,8 @@ extra-y+= ebda.o
>>  extra-y+= platform-quirks.o
>>  extra-y+= vmlinux.lds
>>
>> +extra-$(CONFIG_X86_32) += pgtable_32.o
>> +
>>  CPPFLAGS_vmlinux.lds += -U$(UTS_MACHINE)
>>
>>  ifdef CONFIG_FUNCTION_TRACER
>> diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
>> index 5f40126..0db066e 100644
>> --- a/arch/x86/kernel/head_32.S
>> +++ b/arch/x86/kernel/head_32.S
>> @@ -41,51 +41,6 @@
>>  #define X86_VENDOR_IDnew_cpu_data+CPUINFO_x86_vendor_id
>>
>>  /*
>> - * This is how much memory in addition to the memory covered up to
>> - * and including _end we need mapped initially.
>> - * We need:
>> - * (KERNEL_IMAGE_SIZE/4096) / 1024 pages (worst case, non PAE)
>> - * (KERNEL_IMAGE_SIZE/4096) / 512 + 4 pages (worst case for PAE)
>> - *
>> - * Modulo rounding, each megabyte assigned here requires a kilobyte of
>> - * memory, which is currently unreclaimed.
>> - *
>> - * This should be a multiple of a page.
>> - *
>> - * KERNEL_IMAGE_SIZE should be greater than pa(_end)
>> - * and small than max_low_pfn, otherwise will waste some page table
>> entries
>> - */
>> -
>> -#if PTRS_PER_PMD > 1
>> -#define PAGE_TABLE_SIZE(pages) (((pages) / PTRS_PER_PMD) +
>> PTRS_PER_PGD)
>> -#else
>> -#define PAGE_TABLE_SIZE(pages) ((pages) / PTRS_PER_PGD)
>> -#endif
>> -
>> -/*
>> - * Number of possible pages in the lowmem region.
>> - *
>> - * We shift 2 by 31 instead of 1 by 32 to the left in order to avoid a
>> - * gas warning about overflowing shift count when gas has been compiled
>> - * with only a host target support using a 32-bit type for internal
>> - * representation.
>> - */
>> -LOWMEM_PAGES = (((2<<31) - __PAGE_OFFSET) >> PAGE_SHIFT)
>> -
>> -/* Enough space to fit pagetables for the low memory linear map */
>> -MAPPING_BEYOND_END = PAGE_TABLE_SIZE(LOWMEM_PAGES) << PAGE_SHIFT
>> -
>> -/*
>> - * Worst-case size of the kernel mapping we need to make:
>> - * a relocatable kernel can live anywhere in lowmem, so we need to
>> be able
>> - * to map all of lowmem.
>> - */
>> -KERNEL_PAGES = LOWMEM_PAGES
>> -
>> -INIT_MAP_SIZE = PAGE_TABLE_SIZE(KERNEL_PAGES) * PAGE_SIZE
>> -RESERVE_BRK(pagetables, INIT_MAP_SIZE)
>> -
>> -/*
>>   * 32-bit kernel entrypoint; only used by the boot CPU.  On entry,
>>   * %esi points to the real-mode code as a 32-bit pointer.
>>   * CS and DS must be 4 GB flat segments, but we don't depend on
>> @@ -157,92 +112,7 @@ ENTRY(startup_32)
>>  call load_ucode_bsp
>>  #endif
>>
>> -/*
>> - * Initialize page tables.  This creates a PDE and a set of page
>> - * tables, which are located immediately beyond __brk_base.  The
>> variable
>> - * _brk_end is set up to point to the first "safe" location.
>> - * Mappings are created both at virtual address 0 (identity mapping)
>> - * and PAGE_OFFSET for up to _end.
>> - */
>> -#ifdef CONFIG_X86_PAE
>> -
>> -/*
>> - * In PAE mode initial_page_table is statically defined to 

Re: [Xen-devel] [PATCH] tools/libacpi: set FADT boot flag to notify lack of VGA for PVHv2 guests

[Jan Beulich]

>>> On 01.12.16 at 16:08,  wrote:
> On Thu, Dec 01, 2016 at 12:40:01PM +, Roger Pau Monne wrote:
>> --- a/tools/libacpi/acpi2_0.h
>> +++ b/tools/libacpi/acpi2_0.h
>> @@ -229,6 +229,7 @@ struct acpi_20_fadt {
>>   */
>>  #define ACPI_LEGACY_DEVICES (1 << 0)
>>  #define ACPI_8042   (1 << 1)
>> +#define ACPI_FADT_NO_VGA(1 << 2)
> 
> Hm, I'm wondering whether I should drop the "_FADT_" here, other flags don't 
> have it already.

No, the other two should gain _FADT (see actbl.h).

Jan


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

Re: [Xen-devel] [RFD] OP-TEE (and probably other TEEs) support

[Julien Grall]

On 29/11/16 19:19, Volodymyr Babchuk wrote:

Hi Julien,


Hi Volodymyr,




On 29 November 2016 at 20:55, Julien Grall  wrote:

Hi Volodymyr,

On 29/11/16 17:40, Volodymyr Babchuk wrote:


On 29 November 2016 at 18:02, Julien Grall  wrote:


On 29/11/16 15:27, Volodymyr Babchuk wrote:


On 28 November 2016 at 22:10, Julien Grall  wrote:


On 28/11/16 18:09, Volodymyr Babchuk wrote:


On 28 November 2016 at 18:14, Julien Grall 
wrote:


On 24/11/16 21:10, Volodymyr Babchuk wrote:


I don't follow your point here. Why would the SMC handler need to map the
guest memory?


Because this is how parameters are passed. We can pass some parameters
in registers, but for example in OP-TEE registers hold only address of
command buffer. In this command buffer there are actual parameters.
Some of those parameters can be references to another memory objects.
So, to translate IPAs to PAs we need to map this command buffer,
analyze it and so on.



So the SMC issued will contain a PA of a page belonging to the guest or Xen?

It will be guest page. But all references to other pages will have
real PAs, so TEE can work with them.

Lets dive into example: hypervisor traps SMC, mediation layer (see
below) can see that there was INVOKE_COMMAND request. There are
address of command buffer in register pair (r1, r2). Mediation layer
changes address in this register pair to real PA of the command
buffer. Then it maps specified page and checks parameters. One of
parameters have type MEMREF, so mediation layer has to change IPA of
specified buffer to PA. Then it issues real SMC call.
After return from SMC it inspects registers and buffer again and
replaces memory references back.


I was about to ask whether SMC call have some kind of metadata to know 
the parameter, but you answered it on another mail. So I will follow-up 
there.


Regarding the rest, you said that the buffer passed to the real TEE will 
be baked into guest memory. There are few problems with that you don't 
seem to address in this design document:
	- The buffer may be contiguous in the IPA space but discontinuous in PA 
space. This is because Xen may not be able to allocate all the memory 
for the guest contiguously in PA space. So how do you plan to handle 
buffer greater than a Xen page granularity (i.e 4K)
	- Can all type memory could be passed to TEE (e.g foreign page, grant, 
mmio...)? I suspect not.
	- TEE may run in parallel of the guest OS, this means that we have to 
make sure the page will never be removed by the guest OS (see 
XENMEM_decrease_reservation hypercall).
- The IPA -> PA translation can be slow as this would need to 
be done in software (see p2m_lookup). Is there any upper limit of the 
number of buffer and indirection available?


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH] tools/libacpi: set FADT boot flag to notify lack of VGA for PVHv2 guests

[Roger Pau Monne]

On Thu, Dec 01, 2016 at 12:40:01PM +, Roger Pau Monne wrote:
> --- a/tools/libacpi/acpi2_0.h
> +++ b/tools/libacpi/acpi2_0.h
> @@ -229,6 +229,7 @@ struct acpi_20_fadt {
>   */
>  #define ACPI_LEGACY_DEVICES (1 << 0)
>  #define ACPI_8042   (1 << 1)
> +#define ACPI_FADT_NO_VGA(1 << 2)

Hm, I'm wondering whether I should drop the "_FADT_" here, other flags don't 
have it already.

Roger.

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

Re: [Xen-devel] [RFD] OP-TEE (and probably other TEEs) support

[Volodymyr Babchuk]

Hello Julien,



On 1 December 2016 at 16:24, Julien Grall  wrote:
> Hi Stefano,
>
>
> On 30/11/16 21:53, Stefano Stabellini wrote:
>>
>> On Mon, 28 Nov 2016, Julien Grall wrote:
>
> If not, then it might be worth to consider a 3rd solution where the TEE
> SMC
> calls are forwarded to a specific domain handling the SMC on behalf of
> the
> guests. This would allow to upgrade the TEE layer without having to
> upgrade
> the hypervisor.

 Yes, this is good idea. How this can look? I imagine following flow:
 Hypervisor traps SMC, uses event channel to pass request to Dom0. Some
 userspace daemon receives it, maps pages with request data, alters is
 (e.g. by replacing IPAs with PAs), sends request to hypervisor to
 issue real SMC, then alters response and only then returns data back
 to guest.
>>>
>>>
>>> The event channel is only a way to notify (similar to an interrupt), you
>>> would
>>> need a shared memory page between the hypervisor and the client to
>>> communicate
>>> the SMC information.
>>>
>>> I was thinking to get advantage of the VM event API for trapping the SMC.
>>> But
>>> I am not sure if it is the best solution here. Stefano, do you have any
>>> opinions here?
>>>
 I can see only one benefit there - this code will be not in
 hypervisor. And there are number of drawbacks:

 Stability: if this userspace demon will crash or get killed by, say,
 OOM, we will lose information about all opened sessions, mapped shared
 buffers, etc.That would be complete disaster.
>>>
>>>
>>> I disagree on your statement, you would gain in isolation. If your
>>> userspace
>>> crashes (because of an emulation bug), you will only loose access to TEE
>>> for a
>>> bit. If the hypervisor crashes (because of an emulation bug), then you
>>> take
>>> down the platform. I agree that you lose information when the userspace
>>> app is
>>> crashing but your platform is still up. Isn't it the most important?
>>>
>>> Note that I think it would be "fairly easy" to implement code to reset
>>> everything or having a backup on the side.
>>>
 Performance: how big will be latency introduced by switching between
 hypervisor, dom0 SVC and USR modes? I have seen use case where TEE is
 part of video playback pipe (it decodes DRM media).
 There also can be questions about security, but Dom0 in any case can
 access any memory from any guest.
>>>
>>>
>>> But those concerns would be the same in the hypervisor, right? If your
>>> emulation is buggy then a guest would get access to all the memory.
>>>
 But I really like the idea, because I don't want to mess with
 hypervisor when I don't need to. So, how do you think, how it will
 affect performance?
>>>
>>>
>>> I can't tell here. I would recommend you to try a quick prototype (e.g
>>> receiving and sending SMC) and see what would be the overhead.
>>>
>>> When I wrote my previous e-mail, I mentioned "specific domain", because I
>>> don't think it is strictly necessary to forward the SMC to DOM0. If you
>>> are
>>> concern about overloading DOM0, you could have a separate service domain
>>> that
>>> would handle TEE for you. You could have your "custom OS" handling TEE
>>> request
>>> directly in kernel space (i.e SVC).
>>>
>>> This would be up to the developer of this TEE-layer to decide what to do.
>>
>>
>> Thanks Julien from bringing me into the discussion. These are my
>> thoughts on the matter.
>>
>>
>> Running emulators in Dom0 (AKA QEMU on x86) has always meant giving them
>> full Dom0 privileges so far. I don't think that is acceptable. There is
>> work undergoing on the x86 side of things to fix the situation, see:
>>
>>
>> http://marc.info/?i=1479489244-2201-1-git-send-email-paul.durrant%40citrix.com
>>
>> But if the past is any indication of future development speed, we are
>> still a couple of Xen releases away at least from having unprivileged
>> emulators in Dom0 on x86. By unprivileged, I mean that they are not able
>> to map any random page in memory, but just the ones belonging to the
>> virtual machine that they are serving. Until then, having an emulator in
>> userspace Dom0 is just as bad as having it in the hypervisor from a
>> security standpoint.
>>
>> I would only consider this option, if we mandate from the start, in the
>> design doc and implementations, that the emulators need to be
>> unprivileged on ARM. This would likely require a new set of hypercalls
>> and possibly Linux privcmds. And even then, this solution would still
>> present a series of problems:
>>
>> - latency
>> - scalability
>> - validation against the root of trust
>> - certifications (because they are part of Dom0 and nobody can certify
>>   that)
>>
>>
>> The other option that traditionally is proposed is using stubdoms.
>> Specialized little VMs to run emulators, each VM runs one emulator
>> instance. They are far better from a security standpoint, and could be

Re: [Xen-devel] Kernel panic on Xen virtualisation in Debian

[Sander Eikelenboom]

Thursday, December 1, 2016, 2:59:36 PM, you wrote:

> On 01.12.2016 14:26, Wei Liu wrote:

>> This is still the same kernel log that was sent some time ago.
>> So, if you have built Xen with debug=y, could you try to set Xen log
>> level to the highest and capture "xl dmesg" when guest crashes?

> It's not the guest that crashes, it's dom0. So when the host crashes, 
> I'm not able to issue any commands anymore.

>> But I think this is increasingly likely to be a Linux kernel issue
>> because you've tried multiple versions of xen. Maybe it is time to try
>> different versions of Dom0 kernels (sorry if you've tried that, I can't
>> remember all the details over so many moons).

> Yes, indeed I have tried different kernels, but I can't remember details 
> as well... ;/


Hi Ingo,

Have you tried without enabling "ndisc" (QoS) and "ipv6" ?
They are both present in your log and i assume you are using a bridged network 
config ?
You wouldn't be the first to stumble over a more generic kernel network bug
while using Xen, due to less well tested combinations.
So it's worth testing if plain ipv4 and no QoS works.

--
Sander
 


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

Re: [Xen-devel] [PATCH v2 00/35] libxl LOG*D functions

[Wei Liu]

On Fri, Nov 18, 2016 at 03:59:59PM +, Wei Liu wrote:
> On Thu, Nov 17, 2016 at 06:35:42PM +0100, Cédric Bosdonnat wrote:
> > Hey all,
> > 
> > Here is v2 addressing Wei's comments on patch #1. The 3 libxl.c
> > patches haven't been merged, but the commit message of the first
> > one has been slightly rewritten to help understanding the reason
> > of the split.
> > 
> 
> Series:
> 
> Acked-by: Wei Liu 

Can you provide me with a git branch that contains these patches and
have my acks folded in?

Wei.

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

Re: [Xen-devel] [RFD] OP-TEE (and probably other TEEs) support

[Julien Grall]

Hi Stefano,

On 30/11/16 21:53, Stefano Stabellini wrote:

On Mon, 28 Nov 2016, Julien Grall wrote:

If not, then it might be worth to consider a 3rd solution where the TEE
SMC
calls are forwarded to a specific domain handling the SMC on behalf of the
guests. This would allow to upgrade the TEE layer without having to
upgrade
the hypervisor.

Yes, this is good idea. How this can look? I imagine following flow:
Hypervisor traps SMC, uses event channel to pass request to Dom0. Some
userspace daemon receives it, maps pages with request data, alters is
(e.g. by replacing IPAs with PAs), sends request to hypervisor to
issue real SMC, then alters response and only then returns data back
to guest.


The event channel is only a way to notify (similar to an interrupt), you would
need a shared memory page between the hypervisor and the client to communicate
the SMC information.

I was thinking to get advantage of the VM event API for trapping the SMC. But
I am not sure if it is the best solution here. Stefano, do you have any
opinions here?


I can see only one benefit there - this code will be not in
hypervisor. And there are number of drawbacks:

Stability: if this userspace demon will crash or get killed by, say,
OOM, we will lose information about all opened sessions, mapped shared
buffers, etc.That would be complete disaster.


I disagree on your statement, you would gain in isolation. If your userspace
crashes (because of an emulation bug), you will only loose access to TEE for a
bit. If the hypervisor crashes (because of an emulation bug), then you take
down the platform. I agree that you lose information when the userspace app is
crashing but your platform is still up. Isn't it the most important?

Note that I think it would be "fairly easy" to implement code to reset
everything or having a backup on the side.


Performance: how big will be latency introduced by switching between
hypervisor, dom0 SVC and USR modes? I have seen use case where TEE is
part of video playback pipe (it decodes DRM media).
There also can be questions about security, but Dom0 in any case can
access any memory from any guest.


But those concerns would be the same in the hypervisor, right? If your
emulation is buggy then a guest would get access to all the memory.


But I really like the idea, because I don't want to mess with
hypervisor when I don't need to. So, how do you think, how it will
affect performance?


I can't tell here. I would recommend you to try a quick prototype (e.g
receiving and sending SMC) and see what would be the overhead.

When I wrote my previous e-mail, I mentioned "specific domain", because I
don't think it is strictly necessary to forward the SMC to DOM0. If you are
concern about overloading DOM0, you could have a separate service domain that
would handle TEE for you. You could have your "custom OS" handling TEE request
directly in kernel space (i.e SVC).

This would be up to the developer of this TEE-layer to decide what to do.


Thanks Julien from bringing me into the discussion. These are my
thoughts on the matter.


Running emulators in Dom0 (AKA QEMU on x86) has always meant giving them
full Dom0 privileges so far. I don't think that is acceptable. There is
work undergoing on the x86 side of things to fix the situation, see:

http://marc.info/?i=1479489244-2201-1-git-send-email-paul.durrant%40citrix.com

But if the past is any indication of future development speed, we are
still a couple of Xen releases away at least from having unprivileged
emulators in Dom0 on x86. By unprivileged, I mean that they are not able
to map any random page in memory, but just the ones belonging to the
virtual machine that they are serving. Until then, having an emulator in
userspace Dom0 is just as bad as having it in the hypervisor from a
security standpoint.

I would only consider this option, if we mandate from the start, in the
design doc and implementations, that the emulators need to be
unprivileged on ARM. This would likely require a new set of hypercalls
and possibly Linux privcmds. And even then, this solution would still
present a series of problems:

- latency
- scalability
- validation against the root of trust
- certifications (because they are part of Dom0 and nobody can certify
  that)


The other option that traditionally is proposed is using stubdoms.
Specialized little VMs to run emulators, each VM runs one emulator
instance. They are far better from a security standpoint, and could be
certifiable. They might still pose problems from a root of trust point
of view. However the real issue with stubdoms, is just that being
treated as VMs they show up in "xl list", they introduce latency, they
consume a lot of memory, etc. Also dealing with Mini-OS can be unfunny.
I think that this option is only a little better than the previous
option, but it is still not great.


This brings us to the third and last option. Introducing the emulators
in the hypervisor. This is acceptable only if they are run in a lower

[Xen-devel] [qemu-mainline baseline-only test] 68131: tolerable FAIL

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 68131 qemu-mainline real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/68131/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-qemuu-nested-intel 16 debian-hvm-install/l1/l2 fail like 68099
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1  9 windows-installfail like 68099

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-midway   12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-midway   13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail never pass
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail never pass

version targeted for testing:
 qemuu1cd56fd2e14f67ead2f0458b4ae052f19865c41c
baseline version:
 qemuu00227fefd2059464cd2f59aed29944874c630e2f

Last test of basis68099  2016-11-25 13:18:37 Z6 days
Testing same since68131  2016-11-30 18:20:36 Z0 days1 attempts


People who touched revisions under test:
  Adrian Bunk 
  Alberto Garcia 
  Alistair Francis 
  Benjamin Herrenschmidt 
  Bobby Bingham 
  Changlimin 
  David Gibson 
  Dr. David Alan Gilbert 
  Eduardo Habkost 
  Eric Blake 
  Fam Zheng 
  Francis Deslauriers 
  Greg Kurz 
  Guenter Roeck 
  Hervé Poussineau 
  Jan Beulich 
  Jose Ricardo Ziviani 
  Kevin Wolf 
  Laurent Vivier 
  Li Qiang 
  Mark Cave-Ayland 
  Max Reitz 
  Michael Roth 
  Olaf Hering 
  Paolo Bonzini 
  Peter Maydell 
  Peter Xu 
  Richard Henderson 
  Stefan Hajnoczi 
  Stefano Stabellini 
  Thomas Huth 
  Vladimir Svoboda 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm   

Re: [Xen-devel] Kernel panic on Xen virtualisation in Debian

[Wei Liu]

On Thu, Dec 01, 2016 at 02:59:36PM +0100, Ingo Jürgensmann wrote:
> On 01.12.2016 14:26, Wei Liu wrote:
> 
> >This is still the same kernel log that was sent some time ago.
> >So, if you have built Xen with debug=y, could you try to set Xen log
> >level to the highest and capture "xl dmesg" when guest crashes?
> 
> It's not the guest that crashes, it's dom0. So when the host crashes, I'm
> not able to issue any commands anymore.
> 

Oh, sorry for speaking non-sense. In that case you will need to setup a
serial console to capture output on the fly.

Wei.

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

[Xen-devel] [xen-unstable test] 102735: tolerable FAIL - PUSHED

[osstest service owner]

flight 102735 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/102735/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail  like 102704
 test-armhf-armhf-libvirt-qcow2 12 saverestore-support-check   fail like 102704
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail like 102704
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 102704
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 102704
 test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail  like 102704
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 102704
 test-armhf-armhf-libvirt 13 saverestore-support-checkfail  like 102704
 test-amd64-amd64-xl-rtds  9 debian-install   fail  like 102704

Tests which did not succeed, but are not blocking:
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  a7a578ce6b8634eec30cb8445ea98e18d9b4e9b8
baseline version:
 xen  99a10da1b4fee8ef7a096e5fd3608f6c15932eb0

Last test of basis   102704  2016-11-29 02:01:05 Z2 days
Failing since102720  2016-11-29 20:16:44 Z1 days2 attempts
Testing same since   102735  2016-11-30 13:39:22 Z1 days1 attempts


People who touched revisions under test:
  Dario Faggioli 
  George Dunlap 
  Ian Jackson 
  Julien Grall 
  Stefano Stabellini 
  Wei Chen 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64-xtf  pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-oldkern  pass
 build-i386-oldkern   pass
 

Re: [Xen-devel] Kernel panic on Xen virtualisation in Debian

[Ingo Jürgensmann]

On 01.12.2016 14:26, Wei Liu wrote:


This is still the same kernel log that was sent some time ago.
So, if you have built Xen with debug=y, could you try to set Xen log
level to the highest and capture "xl dmesg" when guest crashes?


It's not the guest that crashes, it's dom0. So when the host crashes, 
I'm not able to issue any commands anymore.



But I think this is increasingly likely to be a Linux kernel issue
because you've tried multiple versions of xen. Maybe it is time to try
different versions of Dom0 kernels (sorry if you've tried that, I can't
remember all the details over so many moons).


Yes, indeed I have tried different kernels, but I can't remember details 
as well... ;/


--
Ciao...  //http://blog.windfluechter.net
  Ingo \X/ XMPP: i...@jabber.windfluechter.net


gpg pubkey:  http://www.juergensmann.de/ij_public_key.asc

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

Re: [Xen-devel] [PATCH] tools/libacpi: set FADT boot flag to notify lack of VGA for PVHv2 guests

[Boris Ostrovsky]

On 12/01/2016 08:29 AM, Roger Pau Monne wrote:

On Thu, Dec 01, 2016 at 06:09:09AM -0700, Jan Beulich wrote:

On 01.12.16 at 13:40,  wrote:

PVHv2 guests don't have any VGA card, and as so it must be notified in the
FADT.

Signed-off-by: Roger Pau Monné 


Reviewed-by: Jan Beulich 

Looking at this ...


--- a/tools/libacpi/acpi2_0.h
+++ b/tools/libacpi/acpi2_0.h
@@ -229,6 +229,7 @@ struct acpi_20_fadt {
  */
 #define ACPI_LEGACY_DEVICES (1 << 0)
 #define ACPI_8042   (1 << 1)
+#define ACPI_FADT_NO_VGA(1 << 2)


... though makes me wonder: Don't we need a similar patch then
for ACPI_FADT_NO_CMOS_RTC, the setting of which then would
need to be in sync with XEN_X86_EMU_RTC?



This will also eliminate the need in Linux to explicitly disable RTC for 
PVH guests. With ACPI_FADT_NO_CMOS_RTC this will be taken care of by 
generic ACPI boot code.


-boris




Right... and we also need to remove ACPI_8042, which is set by default. Will
prepare two more patches to fix this.

Roger.



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

Re: [Xen-devel] [COVERITY ACCESS] for Embedded/Automotive team

[Artem Mygaiev]

On 30.11.16 21:21, Andrew Cooper wrote:
> On 29/11/16 15:09, Artem Mygaiev wrote:
>> Hi Julien
>>
>> On 29.11.16 16:27, Julien Grall wrote:
>>> Hi Artem,
>>>
>>> On 29/11/16 14:21, Artem Mygaiev wrote:
 Lars, the project is approved by Coverity. Scan has found some issues in
 xen/arch/arm on master, part of them are false positives.
>>> Perfect. It would be interesting to know the list of issues so we can
>>> categorize them (i.e are they security issue) and address them.
>> Let me clean up the build scripts a bit and I will send you invite to
>> Coverity Scan project
> Can I get an invite as well please?
>
> ~Andrew
Sent

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

Re: [Xen-devel] [PATCH] libxl: invert xc and domain model resume calls in xc_domain_resume()

[Wei Liu]

On Tue, Nov 29, 2016 at 11:15:36AM -0800, Stefano Stabellini wrote:
> On Tue, 29 Nov 2016, Juergen Gross wrote:
> > On 29/11/16 08:34, Wei Liu wrote:
> > > On Mon, Nov 28, 2016 at 02:53:57PM +0100, Cédric Bosdonnat wrote:
> > >> Resume is sometimes silently failing for HVM guests. Getting the
> > >> xc_domain_resume() and libxl__domain_resume_device_model() in the
> > >> reverse order than what is in the suspend code fixes the problem.
> > >>
> > >> Signed-off-by: Cédric Bosdonnat 
> > >  
> > > I think it would be nice to explain why reversing the order fixes the
> > > problem for you. My guess is because device model needs to be ready when
> > > the guest runs, but I'm not fully convinced by this explanation --
> > > guests should just be trapped in the hypervisor waiting for device model
> > > to come up.
> > 
> > I'm not completely sure this is true. qemu is in "stopped" state, so it
> > might be any emulation requests are just silently dropped. In any case
> > it is just weird to stop qemu in suspend case only after suspending the
> > domain, but let it continue _after_ resuming the domain. So I'd rather
> > expect an explanation (not from Cedric) why this should be okay in case
> > the patch isn't accepted.
> 
> Calling xc_domain_resume before libxl__domain_resume_device_model seems
> wrong to me. For example in libxl_domain_unpause we call
> libxl__domain_resume_device_model, then xc_domain_unpause. We should get
> the DM ready before resuming the VM, right?
> 

Yes, I would think so, too.

I'm inclined to accept this patch. At the end of the day, even if QEMU
doesn't drop requests now, it doesn't mean it will never drop requests
in the future.

Wei.

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

Re: [Xen-devel] [PATCH] tools/libacpi: set FADT boot flag to notify lack of VGA for PVHv2 guests

[Roger Pau Monne]

On Thu, Dec 01, 2016 at 06:09:09AM -0700, Jan Beulich wrote:
> >>> On 01.12.16 at 13:40,  wrote:
> > PVHv2 guests don't have any VGA card, and as so it must be notified in the 
> > FADT.
> > 
> > Signed-off-by: Roger Pau Monné 
> 
> Reviewed-by: Jan Beulich 
> 
> Looking at this ...
> 
> > --- a/tools/libacpi/acpi2_0.h
> > +++ b/tools/libacpi/acpi2_0.h
> > @@ -229,6 +229,7 @@ struct acpi_20_fadt {
> >   */
> >  #define ACPI_LEGACY_DEVICES (1 << 0)
> >  #define ACPI_8042   (1 << 1)
> > +#define ACPI_FADT_NO_VGA(1 << 2)
> 
> ... though makes me wonder: Don't we need a similar patch then
> for ACPI_FADT_NO_CMOS_RTC, the setting of which then would
> need to be in sync with XEN_X86_EMU_RTC?

Right... and we also need to remove ACPI_8042, which is set by default. Will 
prepare two more patches to fix this.

Roger.

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

Re: [Xen-devel] [PATCH v3 17/24] x86/pv: Avoid raising faults behind the emulators back

[Jan Beulich]

>>> On 01.12.16 at 14:12,  wrote:
> On 01/12/16 12:57, Jan Beulich wrote:
> On 30.11.16 at 14:50,  wrote:
>>> @@ -5379,30 +5380,41 @@ int ptwr_do_page_fault(struct vcpu *v, unsigned 
>>> long addr,
>>>  page_unlock(page);
>>>  put_page(page);
>>>  
>>> -/*
>>> - * The previous lack of inject_{sw,hw}*() hooks caused exceptions 
>>> raised
>>> - * by the emulator itself to become X86EMUL_UNHANDLEABLE.  Such 
>>> exceptions
>>> - * now set event_pending instead.  Exceptions raised behind the back of
>>> - * the emulator don't yet set event_pending.
>>> - *
>>> - * For now, cause such cases to return to the X86EMUL_UNHANDLEABLE 
>>> path,
>>> - * for no functional change from before.  Future patches will fix this
>>> - * properly.
>>> - */
>>> -if ( rc == X86EMUL_EXCEPTION && ptwr_ctxt.ctxt.event_pending )
>>> -rc = X86EMUL_UNHANDLEABLE;
>>> +/* More strict than x86_emulate_wrapper(), as this is now true for PV. 
>>> */
>>> +ASSERT(ptwr_ctxt.ctxt.event_pending == (rc == X86EMUL_EXCEPTION));
>>>  
>>> -if ( rc == X86EMUL_UNHANDLEABLE )
>>> -goto bail;
>>> +switch ( rc )
>>> +{
>>> +case X86EMUL_EXCEPTION:
>>> +/*
>>> + * This emulation only covers writes to pagetables which marked
>>> + * read-only by Xen.  We tolerate #PF (from hitting an adjacent 
>>> page).
>> Why "adjacent"? Aiui the only legitimate #PF here can be from a
>> page having got paged out by the guest by the time here, and that
>> could be either the page table page itself, or the page(s) holding
>> the instruction (which normally aren't adjacent at all).
> 
> This is less clear-cut than the subsequent case, as non-aligned accesses
> are disallowed, so simply misaligning a write across the page boundary
> won't result in the emulator raising #PF.

I don't understand - what does this have to do with possibly getting
#PF from fetching the insn?

> One issue I have decided to defer is the behaviour of UNHANDLEABLE in
> this case.  If the #PF we are servicing is caused by a misaligned write
> to a l1e, instead of explicitly re-injecting #PF, we let the logic
> continue searching for all other cases which may have caused a #PF.
> 
> It would be better to explicitly disallow the access by re-raising #PF
> than returning UNHANDLEABLE, as it skips the rest of the pagefault handler.

At the point of the check we don't know yet whether we're dealing
with a page table, so we need to give other handlers a chance.

>>> + * Anything else is an emulation bug, or a guest playing with the
>>> + * instruction stream under Xen's feet.
>>> + */
>>> +if ( ptwr_ctxt.ctxt.event.type == X86_EVENTTYPE_HW_EXCEPTION &&
>>> + ptwr_ctxt.ctxt.event.vector == TRAP_page_fault )
>>> +pv_inject_event(_ctxt.ctxt.event);
>>> +else
>>> +gdprintk(XENLOG_WARNING,
>>> + "Unexpected event (type %u, vector %#x) from 
>>> emulation\n",
>>> + ptwr_ctxt.ctxt.event.type, 
>>> ptwr_ctxt.ctxt.event.vector);
>>> +
>>> +/* Fallthrough */
>>> +case X86EMUL_OKAY:
>>>  
>>> -if ( ptwr_ctxt.ctxt.retire.singlestep )
>>> -pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
>>> +if ( ptwr_ctxt.ctxt.retire.singlestep )
>>> +pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
>>>  
>>> -perfc_incr(ptwr_emulations);
>>> -return EXCRET_fault_fixed;
>>> +/* Fallthrough */
>>> +case X86EMUL_RETRY:
>>> +perfc_incr(ptwr_emulations);
>>> +return EXCRET_fault_fixed;
>>>  
>>>   bail:
>>> -return 0;
>>> +default:
>>> +return 0;
>>> +}
>>>  }
>> I think omitting the default label would not only make the patch
>> slightly smaller, but also result in the bail label look less misplaced.
> 
> The default label is needed to cover the UNHANDLEABLE case.

Certainly not - it can just fall out of the switch statement, reaching
the pre-existing "bail" label. I could see your point if rc was of an
enum type ...

Jan

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

Re: [Xen-devel] Kernel panic on Xen virtualisation in Debian

[Wei Liu]

On Tue, Nov 29, 2016 at 09:20:55PM +0100, Ingo Jürgensmann wrote:
> Am 29.11.2016 um 10:08 schrieb Wei Liu :
> >> http://paste.debian.net/895464/
> > Entry not found -- maybe it expired... Sorry.
> 
> Here it is: 
> 

This is still the same kernel log that was sent some time ago.

So, if you have built Xen with debug=y, could you try to set Xen log
level to the highest and capture "xl dmesg" when guest crashes?

But I think this is increasingly likely to be a Linux kernel issue
because you've tried multiple versions of xen. Maybe it is time to try
different versions of Dom0 kernels (sorry if you've tried that, I can't
remember all the details over so many moons).

Wei.

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

Re: [Xen-devel] [PATCH 2/3] Add Xen colo support for qemu-upstream colo codes

[Wei Liu]

On Wed, Nov 30, 2016 at 05:47:51PM +0800, Zhang Chen wrote:
> Because of qemu codes update, we update Xen colo block codes
> 
> Signed-off-by: Zhang Chen 

COLO being an experimental feature means that you can change it at will,
so for both patches:

Acked-by: Wei Liu 

But, this sort of patch does make me wonder how QEMU handles backward
compatibility for command line options...

> ---
>  tools/libxl/libxl_colo_qdisk.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxl/libxl_colo_qdisk.c b/tools/libxl/libxl_colo_qdisk.c
> index d271d1f..300bff2 100644
> --- a/tools/libxl/libxl_colo_qdisk.c
> +++ b/tools/libxl/libxl_colo_qdisk.c
> @@ -169,9 +169,9 @@ static void colo_qdisk_save_preresume(libxl__egc *egc,
>  /* qmp command doesn't support the driver "nbd" */
>  node = GCSPRINTF("colo_node%d",
>   libxl__device_disk_dev_number(disk->vdev, NULL, NULL));
> -cmd = GCSPRINTF("drive_add buddy driver=replication,mode=primary,"
> +cmd = GCSPRINTF("drive_add -n buddy driver=replication,mode=primary,"
>  "file.driver=nbd,file.host=%s,file.port=%d,"
> -"file.export=%s,node-name=%s,if=none",
> +"file.export=%s,node-name=%s",
>  host, port, export_name, node);
>  ret = libxl__qmp_hmp(gc, domid, cmd, NULL);
>  if (ret)
> -- 
> 2.7.4
> 
> 
> 

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

Re: [Xen-devel] [PATCH 1/3] Don't create default ioreq server

[Wei Liu]

On Wed, Nov 30, 2016 at 05:47:50PM +0800, Zhang Chen wrote:
> The ioreq server make colo run failed.
> 
> Signed-off-by: Zhang Chen 
> Signed-off-by: Wen Congyang 
> ---
>  xen/arch/x86/hvm/hvm.c | 11 ---
>  1 file changed, 11 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 25dc759..8522852 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -5339,17 +5339,6 @@ static int hvmop_get_param(
>  case HVM_PARAM_IOREQ_PFN:
>  case HVM_PARAM_BUFIOREQ_PFN:
>  case HVM_PARAM_BUFIOREQ_EVTCHN:
> -{
> -domid_t domid;
> -
> -/* May need to create server. */
> -domid = d->arch.hvm_domain.params[HVM_PARAM_DM_DOMAIN];
> -rc = hvm_create_ioreq_server(d, domid, 1,
> - HVM_IOREQSRV_BUFIOREQ_LEGACY, NULL);
> -if ( rc != 0 && rc != -EEXIST )
> -goto out;
> -}
> -/*FALLTHRU*/

What is broken by ioreq server? I don't think you can change the code
like this because this is definitely going to be wrong for other use
cases -- try create a guest without COLO.

If you can be more specific about what is broken in COLO we might be
able to devise a fix for you.

>  default:
>  a.value = d->arch.hvm_domain.params[a.index];
>  break;
> -- 
> 2.7.4
> 
> 
> 

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

Re: [Xen-devel] [PATCH v3 18/24] x86/shadow: Avoid raising faults behind the emulators back

[Andrew Cooper]

On 01/12/16 13:00, Jan Beulich wrote:
 On 30.11.16 at 14:50,  wrote:
>> --- a/xen/arch/x86/mm/shadow/multi.c
>> +++ b/xen/arch/x86/mm/shadow/multi.c
>> @@ -3373,18 +3373,35 @@ static int sh_page_fault(struct vcpu *v,
>>  
>>  r = x86_emulate(_ctxt.ctxt, emul_ops);
>>  
>> -/*
>> - * The previous lack of inject_{sw,hw}*() hooks caused exceptions raised
>> - * by the emulator itself to become X86EMUL_UNHANDLEABLE.  Such 
>> exceptions
>> - * now set event_pending instead.  Exceptions raised behind the back of
>> - * the emulator don't yet set event_pending.
>> - *
>> - * For now, cause such cases to return to the X86EMUL_UNHANDLEABLE path,
>> - * for no functional change from before.  Future patches will fix this
>> - * properly.
>> - */
>>  if ( r == X86EMUL_EXCEPTION && emul_ctxt.ctxt.event_pending )
>> -r = X86EMUL_UNHANDLEABLE;
>> +{
>> +/*
>> + * This emulation covers writes to shadow pagetables.  We tolerate 
>> #PF
>> + * (from hitting adjacent pages) and #GP/#SS (from segmentation
>> + * errors).  Anything else is an emulation bug, or a guest playing
>> + * with the instruction stream under Xen's feet.
>> + */
> Same comment here regarding "adjacent".

In this case, the answer is different.  A misaligned write across the
end of a shadow pagetable may legitimately trigger a #PF.

>
>> +if ( emul_ctxt.ctxt.event.type == X86_EVENTTYPE_HW_EXCEPTION &&
>> + (emul_ctxt.ctxt.event.vector < 32) &&
>> + ((1u << emul_ctxt.ctxt.event.vector) &
>> +  ((1u << TRAP_stack_error) | (1u << TRAP_gp_fault) |
>> +   (1u << TRAP_page_fault))) )
> May I suggest to also demand an error code of zero for #GP/#SS?

Ok.

>
>> +{
>> +if ( is_hvm_vcpu(v) )
> has_hvm_container_domain()?

Very good point.  Will fix.

~Andrew

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

Re: [Xen-devel] [PATCH v10 06/13] efi: create new early memory allocator

[Jan Beulich]

>>> On 30.11.16 at 14:04,  wrote:
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -98,6 +98,52 @@ static CHAR16 __initdata newline[] = L"\r\n";
>  #define PrintStr(s) StdOut->OutputString(StdOut, s)
>  #define PrintErr(s) StdErr->OutputString(StdErr, s)
>  
> +#ifndef CONFIG_ARM
> +
> +/*
> + * TODO: Enable EFI boot allocator on ARM.
> + * This code can be common for x86 and ARM.
> + * Things TODO on ARM before enabling ebmalloc:
> + *   - estimate required EBMALLOC_SIZE value,
> + *   - where (in which section) ebmalloc_mem[] should live; if in 
> .bss.page_aligned
> + * then whole BSS zeroing have to be disabled in 
> xen/arch/arm/arm64/head.S;
> + * though BSS should be initialized somehow before use of variables 
> living there,
> + *   - call free_ebmalloc_unused_mem() sowehere in init code.
> + */
> +
> +#define EBMALLOC_SIZEMB(1)

The previous communication with Julien ended in it being acceptable
to him for this to be zero for ARM for now, eliminating (or at least
reducing) the #ifndef CONFIG_ARM guarded region(s).

Jan


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

Re: [Xen-devel] [PATCH v3 17/24] x86/pv: Avoid raising faults behind the emulators back

[Andrew Cooper]

On 01/12/16 12:57, Jan Beulich wrote:
 On 30.11.16 at 14:50,  wrote:
>> @@ -5379,30 +5380,41 @@ int ptwr_do_page_fault(struct vcpu *v, unsigned long 
>> addr,
>>  page_unlock(page);
>>  put_page(page);
>>  
>> -/*
>> - * The previous lack of inject_{sw,hw}*() hooks caused exceptions raised
>> - * by the emulator itself to become X86EMUL_UNHANDLEABLE.  Such 
>> exceptions
>> - * now set event_pending instead.  Exceptions raised behind the back of
>> - * the emulator don't yet set event_pending.
>> - *
>> - * For now, cause such cases to return to the X86EMUL_UNHANDLEABLE path,
>> - * for no functional change from before.  Future patches will fix this
>> - * properly.
>> - */
>> -if ( rc == X86EMUL_EXCEPTION && ptwr_ctxt.ctxt.event_pending )
>> -rc = X86EMUL_UNHANDLEABLE;
>> +/* More strict than x86_emulate_wrapper(), as this is now true for PV. 
>> */
>> +ASSERT(ptwr_ctxt.ctxt.event_pending == (rc == X86EMUL_EXCEPTION));
>>  
>> -if ( rc == X86EMUL_UNHANDLEABLE )
>> -goto bail;
>> +switch ( rc )
>> +{
>> +case X86EMUL_EXCEPTION:
>> +/*
>> + * This emulation only covers writes to pagetables which marked
> It looks to me as if either the "which" wants to be dropped, or "are" /
> "have been" be inserted after it.

I meant "which are".

>
>> + * read-only by Xen.  We tolerate #PF (from hitting an adjacent 
>> page).
> Why "adjacent"? Aiui the only legitimate #PF here can be from a
> page having got paged out by the guest by the time here, and that
> could be either the page table page itself, or the page(s) holding
> the instruction (which normally aren't adjacent at all).

This is less clear-cut than the subsequent case, as non-aligned accesses
are disallowed, so simply misaligning a write across the page boundary
won't result in the emulator raising #PF.

One issue I have decided to defer is the behaviour of UNHANDLEABLE in
this case.  If the #PF we are servicing is caused by a misaligned write
to a l1e, instead of explicitly re-injecting #PF, we let the logic
continue searching for all other cases which may have caused a #PF.

It would be better to explicitly disallow the access by re-raising #PF
than returning UNHANDLEABLE, as it skips the rest of the pagefault handler.

>
>> + * Anything else is an emulation bug, or a guest playing with the
>> + * instruction stream under Xen's feet.
>> + */
>> +if ( ptwr_ctxt.ctxt.event.type == X86_EVENTTYPE_HW_EXCEPTION &&
>> + ptwr_ctxt.ctxt.event.vector == TRAP_page_fault )
>> +pv_inject_event(_ctxt.ctxt.event);
>> +else
>> +gdprintk(XENLOG_WARNING,
>> + "Unexpected event (type %u, vector %#x) from 
>> emulation\n",
>> + ptwr_ctxt.ctxt.event.type, 
>> ptwr_ctxt.ctxt.event.vector);
>> +
>> +/* Fallthrough */
>> +case X86EMUL_OKAY:
>>  
>> -if ( ptwr_ctxt.ctxt.retire.singlestep )
>> -pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
>> +if ( ptwr_ctxt.ctxt.retire.singlestep )
>> +pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
>>  
>> -perfc_incr(ptwr_emulations);
>> -return EXCRET_fault_fixed;
>> +/* Fallthrough */
>> +case X86EMUL_RETRY:
>> +perfc_incr(ptwr_emulations);
>> +return EXCRET_fault_fixed;
>>  
>>   bail:
>> -return 0;
>> +default:
>> +return 0;
>> +}
>>  }
> I think omitting the default label would not only make the patch
> slightly smaller, but also result in the bail label look less misplaced.

The default label is needed to cover the UNHANDLEABLE case.

~Andrew

>
> With at least the comment aspect above taken care of,
> Reviewed-by: Jan Beulich 
>
> Jan
>


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

Re: [Xen-devel] [PATCH] tools/libacpi: set FADT boot flag to notify lack of VGA for PVHv2 guests

[Jan Beulich]

>>> On 01.12.16 at 13:40,  wrote:
> PVHv2 guests don't have any VGA card, and as so it must be notified in the 
> FADT.
> 
> Signed-off-by: Roger Pau Monné 

Reviewed-by: Jan Beulich 

Looking at this ...

> --- a/tools/libacpi/acpi2_0.h
> +++ b/tools/libacpi/acpi2_0.h
> @@ -229,6 +229,7 @@ struct acpi_20_fadt {
>   */
>  #define ACPI_LEGACY_DEVICES (1 << 0)
>  #define ACPI_8042   (1 << 1)
> +#define ACPI_FADT_NO_VGA(1 << 2)

... though makes me wonder: Don't we need a similar patch then
for ACPI_FADT_NO_CMOS_RTC, the setting of which then would
need to be in sync with XEN_X86_EMU_RTC?

Jan

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

Re: [Xen-devel] [PATCH v3 18/24] x86/shadow: Avoid raising faults behind the emulators back

[Jan Beulich]

>>> On 30.11.16 at 14:50,  wrote:
> --- a/xen/arch/x86/mm/shadow/multi.c
> +++ b/xen/arch/x86/mm/shadow/multi.c
> @@ -3373,18 +3373,35 @@ static int sh_page_fault(struct vcpu *v,
>  
>  r = x86_emulate(_ctxt.ctxt, emul_ops);
>  
> -/*
> - * The previous lack of inject_{sw,hw}*() hooks caused exceptions raised
> - * by the emulator itself to become X86EMUL_UNHANDLEABLE.  Such 
> exceptions
> - * now set event_pending instead.  Exceptions raised behind the back of
> - * the emulator don't yet set event_pending.
> - *
> - * For now, cause such cases to return to the X86EMUL_UNHANDLEABLE path,
> - * for no functional change from before.  Future patches will fix this
> - * properly.
> - */
>  if ( r == X86EMUL_EXCEPTION && emul_ctxt.ctxt.event_pending )
> -r = X86EMUL_UNHANDLEABLE;
> +{
> +/*
> + * This emulation covers writes to shadow pagetables.  We tolerate 
> #PF
> + * (from hitting adjacent pages) and #GP/#SS (from segmentation
> + * errors).  Anything else is an emulation bug, or a guest playing
> + * with the instruction stream under Xen's feet.
> + */

Same comment here regarding "adjacent".

> +if ( emul_ctxt.ctxt.event.type == X86_EVENTTYPE_HW_EXCEPTION &&
> + (emul_ctxt.ctxt.event.vector < 32) &&
> + ((1u << emul_ctxt.ctxt.event.vector) &
> +  ((1u << TRAP_stack_error) | (1u << TRAP_gp_fault) |
> +   (1u << TRAP_page_fault))) )

May I suggest to also demand an error code of zero for #GP/#SS?

> +{
> +if ( is_hvm_vcpu(v) )

has_hvm_container_domain()?

Jan


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

Re: [Xen-devel] [PATCH v3 17/24] x86/pv: Avoid raising faults behind the emulators back

[Jan Beulich]

>>> On 30.11.16 at 14:50,  wrote:
> @@ -5379,30 +5380,41 @@ int ptwr_do_page_fault(struct vcpu *v, unsigned long 
> addr,
>  page_unlock(page);
>  put_page(page);
>  
> -/*
> - * The previous lack of inject_{sw,hw}*() hooks caused exceptions raised
> - * by the emulator itself to become X86EMUL_UNHANDLEABLE.  Such 
> exceptions
> - * now set event_pending instead.  Exceptions raised behind the back of
> - * the emulator don't yet set event_pending.
> - *
> - * For now, cause such cases to return to the X86EMUL_UNHANDLEABLE path,
> - * for no functional change from before.  Future patches will fix this
> - * properly.
> - */
> -if ( rc == X86EMUL_EXCEPTION && ptwr_ctxt.ctxt.event_pending )
> -rc = X86EMUL_UNHANDLEABLE;
> +/* More strict than x86_emulate_wrapper(), as this is now true for PV. */
> +ASSERT(ptwr_ctxt.ctxt.event_pending == (rc == X86EMUL_EXCEPTION));
>  
> -if ( rc == X86EMUL_UNHANDLEABLE )
> -goto bail;
> +switch ( rc )
> +{
> +case X86EMUL_EXCEPTION:
> +/*
> + * This emulation only covers writes to pagetables which marked

It looks to me as if either the "which" wants to be dropped, or "are" /
"have been" be inserted after it.

> + * read-only by Xen.  We tolerate #PF (from hitting an adjacent 
> page).

Why "adjacent"? Aiui the only legitimate #PF here can be from a
page having got paged out by the guest by the time here, and that
could be either the page table page itself, or the page(s) holding
the instruction (which normally aren't adjacent at all).

> + * Anything else is an emulation bug, or a guest playing with the
> + * instruction stream under Xen's feet.
> + */
> +if ( ptwr_ctxt.ctxt.event.type == X86_EVENTTYPE_HW_EXCEPTION &&
> + ptwr_ctxt.ctxt.event.vector == TRAP_page_fault )
> +pv_inject_event(_ctxt.ctxt.event);
> +else
> +gdprintk(XENLOG_WARNING,
> + "Unexpected event (type %u, vector %#x) from 
> emulation\n",
> + ptwr_ctxt.ctxt.event.type, ptwr_ctxt.ctxt.event.vector);
> +
> +/* Fallthrough */
> +case X86EMUL_OKAY:
>  
> -if ( ptwr_ctxt.ctxt.retire.singlestep )
> -pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
> +if ( ptwr_ctxt.ctxt.retire.singlestep )
> +pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
>  
> -perfc_incr(ptwr_emulations);
> -return EXCRET_fault_fixed;
> +/* Fallthrough */
> +case X86EMUL_RETRY:
> +perfc_incr(ptwr_emulations);
> +return EXCRET_fault_fixed;
>  
>   bail:
> -return 0;
> +default:
> +return 0;
> +}
>  }

I think omitting the default label would not only make the patch
slightly smaller, but also result in the bail label look less misplaced.

With at least the comment aspect above taken care of,
Reviewed-by: Jan Beulich 

Jan


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

Re: [Xen-devel] [PATCH v5 3/3] Significant changes to decision making; some new roles and minor changes

[Lars Kurth]

On 01/12/2016 09:52, "Lars Kurth"  wrote:

>On 30/11/2016 23:27, "Stefano Stabellini"  wrote:
>
>>On Wed, 23 Nov 2016, Lars Kurth wrote:
>>>
>>>
>>
>>This is basically the same voting mechanism described under "Leadership
>>Team
>>Decisions", counted per project, then averaged, isn't?
>
>That is correct.
>
>>It worries me that it could lead to very different results depending on
>>the project leadership team sizes.
>>
>>For example, let's say that only 2 projects reach the quorum:
>>project A, leadership team size 2, total positive votes 2, 100%
>>project B, leadership team size 12, negative votes 8, positive votes 4,
>>33%
>>Total favor 66.5% -> pass (or very close to)
>
>The issue that prompted this change was in effect created by the number of
>committers in different mature projects (aka, the fact that XAPI has 12 -
>14 - I have to verify the correct number, as some people in the XAPI
>committer list don't work on XAPI any more). Where according to the
>current scheme, projects with large leadership teams can in effect use
>their larger voting block to get their opinion through.
>
>One way of maybe addressing this, would be to be more specific about the
>minimum size of a Leadership team (see "Projects without functional
>Project Leadership Team"). I think a team needs to have at least 3 members
>to be functional. Another way to add an extra check may be to add a
>specific requirement to Graduation Review which checks that the Leadership
>team is of an appropriate size for the size of the project (although we
>may have to be specific on what an appropriate size is).
>
>In reality, we don't have a problem with this today, as the leadership
>teams for the two mature projects (XAPI and Hypervisor) are actually
>large. We have
>* 7 for the Hypervisor
>* 12 for XAPI (although this is probably to big, but in reality
>participation tends to be low)
>
>The two projects which could qualify for maturity in the coming year are
>Win PV drivers (3 leadership team members) and MirageOS (probably should
>have a similar size to the Hypervisor Team).
>
>Also, it is worthwhile pointing out, that Global Decisions should
>practically hardly ever be needed. Only in the following situations
>1) Creating, graduating, completing/archiving of sub-projects
>2) Some changes to this document (goals, principles, project wide decision
>making and project governance): if we apply the new rules, only this
>change would need a global decision (as we added a principle and changed
>local decision making). And this would be the first one, we had since
>introducing the governance 5 years ago
>3) Namespace issues: aka naming conventions for lists, ... - which
>primarily would be bike-shed issues. But again we only used this once
>4) Boundary issues: aka making local per-subproject policies and
>conventions global
>
>>However I don't have a concrete suggestion on how to improve this. Given
>>that any project could appoint any number of people in their leadership
>>teams, I am not sure that accounting for the size of the teams would
>>make things much better. On the other hand the number of people in the
>>leadership team should represent the size of the project somewhat, so it
>>could make sense to account for the votes proportionally.
>>
>>Any opinions?
>
>The only other way I can think of is to weight a project's vote by some
>level of activity (e.g. proportion of contributions averaged over 3
>years). But that would become complicated.
>
>Another way may be to add an extra bucket which contains all projects. In
>the example above
>
>project A, leadership team size 2, total positive votes 2, 100% (pass)
>project B, leadership team size 12, negative votes 8, positive votes 4,
>33% (fail)
>ALL (which is like the popular vote): size 14, negative votes 8, positive
>votes 6, 42% (fail)
>Average 58% (or very close to) -> fail  ... which does change this example
>
>
>Or some sort of rule, which requires that the popular and aggregated votes
>have to be within a certain percentage of each other, otherwise the vote
>does not count and has to be repeated

I thought a bit more about this.

Another way to look at it, which may be simpler, is to require that the
"popular vote" 
A) Has a minimum requirement of 1/2 of the votes in favour.
B) Or possibly better that there is a simple majority in the popular vote

In this example, the total number of leadership team members across both
teams is 14: the total number of votes in favour for the proposal is 6 and
8 against. So it would fail on a quorum requirement.

Let's just look at this scenario in different ways: aka make it closer

A: 2/2 in favour (100%) pass
B: 5/12 in favour (41.666%) fail
ALL: 7/14 in favour (50%) pass quorum, but no majority, fail 2/3 vote

Average (A+B) = 70.8% pass, pass on quorum
Average (A+B+ALL) = 63.888% (fail on 2/3 vote)

I didn't look at the maths, but it looks to me that Average (A+B+ALL)
would be quite similar to 

[Xen-devel] [PATCH] tools/libacpi: set FADT boot flag to notify lack of VGA for PVHv2 guests

[Roger Pau Monne]

PVHv2 guests don't have any VGA card, and as so it must be notified in the FADT.

Signed-off-by: Roger Pau Monné 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Ian Jackson 
Cc: Wei Liu 
Cc: boris.ostrov...@oracle.com
Cc: konrad.w...@oracle.com
---
 tools/firmware/hvmloader/util.c | 3 ++-
 tools/libacpi/acpi2_0.h | 1 +
 tools/libacpi/build.c   | 2 ++
 tools/libacpi/libacpi.h | 1 +
 4 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
index 6e0cfe7..3e192bf 100644
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -948,7 +948,8 @@ void hvmloader_acpi_build_tables(struct acpi_config *config,
 if ( !strncmp(xenstore_read("platform/acpi_s4", "1"), "1", 1)  )
 config->table_flags |= ACPI_HAS_SSDT_S4;
 
-config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC | ACPI_HAS_WAET);
+config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC | ACPI_HAS_WAET |
+ACPI_HAS_VGA);
 
 config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS;
 
diff --git a/tools/libacpi/acpi2_0.h b/tools/libacpi/acpi2_0.h
index 775eb7a..aeb95a7 100644
--- a/tools/libacpi/acpi2_0.h
+++ b/tools/libacpi/acpi2_0.h
@@ -229,6 +229,7 @@ struct acpi_20_fadt {
  */
 #define ACPI_LEGACY_DEVICES (1 << 0)
 #define ACPI_8042   (1 << 1)
+#define ACPI_FADT_NO_VGA(1 << 2)
 
 /*
  * FADT Fixed Feature Flags.
diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
index 47dae01..8799e2c 100644
--- a/tools/libacpi/build.c
+++ b/tools/libacpi/build.c
@@ -579,6 +579,8 @@ int acpi_build_tables(struct acpi_ctxt *ctxt, struct 
acpi_config *config)
 fadt->x_dsdt = ctxt->mem_ops.v2p(ctxt, dsdt);
 fadt->firmware_ctrl   = ctxt->mem_ops.v2p(ctxt, facs);
 fadt->x_firmware_ctrl = ctxt->mem_ops.v2p(ctxt, facs);
+if ( !(config->table_flags & ACPI_HAS_VGA) )
+fadt->iapc_boot_arch |= ACPI_FADT_NO_VGA;
 set_checksum(fadt,
  offsetof(struct acpi_header, checksum),
  sizeof(struct acpi_20_fadt));
diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
index 1d388f9..d7ea6e1 100644
--- a/tools/libacpi/libacpi.h
+++ b/tools/libacpi/libacpi.h
@@ -30,6 +30,7 @@
 #define ACPI_HAS_TCPA(1<<7)
 #define ACPI_HAS_IOAPIC  (1<<8)
 #define ACPI_HAS_WAET(1<<9)
+#define ACPI_HAS_VGA (1<<10)
 
 struct xen_vmemrange;
 struct acpi_numa {
-- 
2.9.3 (Apple Git-75)


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

Re: [Xen-devel] [PATCH v3 13/24] x86/emul: Rework emulator event injection

[Jan Beulich]

>>> On 30.11.16 at 14:50,  wrote:
> The emulator needs to gain an understanding of interrupts and exceptions
> generated by its actions.
> 
> Move hvm_emulate_ctxt.{exn_pending,trap} into struct x86_emulate_ctxt so they
> are visible to the emulator.  This removes the need for the
> inject_{hw_exception,sw_interrupt}() hooks, which are dropped and replaced
> with x86_emul_{hw_exception,software_event,reset_event}() instead.
> 
> For exceptions raised by x86_emulate() itself (rather than its callbacks), the
> shadow pagetable and PV uses of x86_emulate() previously failed with
> X86EMUL_UNHANDLEABLE due to the lack of inject_*() hooks.
> 
> This behaviour has changed, and such cases will now return X86EMUL_EXCEPTION
> with event_pending set.  Until the callers of x86_emulate() have been updated
> to inject events back into the guest, divert the event_pending case back  into
> the X86EMUL_UNHANDLEABLE path to maintain the same guest-visible behaviour.
> 
> No overall functional change.
> 
> Signed-off-by: Andrew Cooper 
> Reviewed-by: Boris Ostrovsky 
> Reviewed-by: Kevin Tian 

Reviewed-by: Jan Beulich 


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

[Xen-devel] IO APIC interrupt stuck with irr=1 (was: Re: [Xen-users] xen hypervisor does not like my Dom0 LVM partition: I/O Errors)

[Jan Beulich]

>>> On 01.12.16 at 11:29,  wrote:
>> (XEN) Enabling APIC mode:  Flat.  Using 2 I/O APICs
>> (XEN) [VT-D]  RMRR address range bf7da000..bf7d9fff not in reserved memory;
>> need "iommu_inclusive_mapping=1"?
>> (XEN) [VT-D]  RMRR (bf7da000, bf7d9fff) is incorrect
>> (XEN) Failed to parse ACPI DMAR.  Disabling VT-d.

Do things work better with this worked around (as suggested by the
message)?

>> (XEN) IRQ 20 Vec 41:
>> (XEN)   Apic 0x00, Pin 20: vec=29 delivery=LoPri dest=L status=1
>> polarity=1 irr=1 trig=L mask=0 dest_id:8
> 
> So this IO APIC vector seems to be stuck with irr=1, I've assumed that Xen 
> would 
> ack the interrupt if a certain timeout has passed and the guest has not done 
> it, 
> but I could be mistaken.

Interrupts in IRR can't be acked, they first need to propagate to
ISR (in LAPIC terms). Hence we'd need to know the state of the
corresponding ISR bit (and for completeness also the IRR one) in
the LAPIC. I would suspect that the ISR bit is also set, and _that_
would then indicate we may have missed issuing an EOI. But it
might also be that some interrupt at a higher priority (larger
vector number) is not disappearing from ISR, effectively masking
the relatively low numbered vector here.

Also, are you positive about the IRR bit here being _permanently_
set, rather than just at the point this one sample was taken?

> I've also seen similar issues on some boxes, this seems 
> to always happen on boxes with more than one IO APIC. In the past I've solved 
> it 
> by setting ioapic_ack=old, but that doesn't seem to work for his case.

Not so here (for the last so many years), so I wonder whether it
matters what Dom0 kernel is in use.

>> And here are the messages to prove there was a lost interrupt:
>> 
>> 11/30/16 5:09 PMjaga-Desktopkernel[10056.569371] ata2: lost
>> interrupt (Status 0x58)
>> 11/30/16 5:09 PMjaga-Desktopkernel[10056.569402] ata3: lost
>> interrupt (Status 0x58)
>> 11/30/16 6:00 PMjaga-Desktopkernel[0.187813] DMAR-IR: This
>> system BIOS has enabled interrupt remapping
>> 11/30/16 6:00 PMjaga-Desktopkernel[0.187813] interrupt
>> remapping is being disabled.  Please

These two messages are suspicious: The kernel should keep its hands
off any IOMMU things when running under Xen.

Jan


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

Re: [Xen-devel] [PATCH v3 11/24] x86/emul: Implement singlestep as a retire flag

[Jan Beulich]

>>> On 01.12.16 at 12:23,  wrote:
> On 01/12/16 11:16, Jan Beulich wrote:
> On 30.11.16 at 14:50,  wrote:
>>> @@ -3422,6 +3422,16 @@ static int sh_page_fault(struct vcpu *v,
>>>  v->arch.paging.last_write_emul_ok = 0;
>>>  #endif
>>>  
>>> +if ( emul_ctxt.ctxt.retire.singlestep )
>>> +{
>>> +if ( is_hvm_vcpu(v) )
>>> +hvm_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
>>> +else
>>> +pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
>>> +
>>> +goto emulate_done;
>> This results in skipping the PAE special code (which I think is intended)
> 
> Correct
> 
>> but also the trace_shadow_emulate(), which I don't think is wanted.
> 
> Hmm.  It is only the PAE case we want to skip.  Perhaps changing the PAE
> entry condition to
> 
> diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
> index c45d260..28ff945 100644
> --- a/xen/arch/x86/mm/shadow/multi.c
> +++ b/xen/arch/x86/mm/shadow/multi.c
> @@ -3480,7 +3480,7 @@ static int sh_page_fault(struct vcpu *v,
>  }
>  
>  #if GUEST_PAGING_LEVELS == 3 /* PAE guest */
> -if ( r == X86EMUL_OKAY ) {
> +if ( r == X86EMUL_OKAY && !emul_ctxt.ctxt.retire.raw ) {
>  int i, emulation_count=0;
>  this_cpu(trace_emulate_initial_va) = va;
>  /* Emulate up to four extra instructions in the hope of catching
> 
> would be better, along with suitable comments and style fixes?

Yes I think so (and I see Tim has said so too).

>>> @@ -5415,11 +5414,11 @@ x86_emulate(
>>>  if ( !mode_64bit() )
>>>  _regs.eip = (uint32_t)_regs.eip;
>>>  
>>> -*ctxt->regs = _regs;
>>> +/* Was singestepping active at the start of this instruction? */
>>> +if ( (rc == X86EMUL_OKAY) && (ctxt->regs->eflags & EFLG_TF) )
>>> +ctxt->retire.singlestep = true;
>> Don't we need to avoid doing this when mov_ss is set? Or does the
>> hardware perhaps do the necessary deferring for us?
> 
> I am currently reading up about this in the manual.

Tell me if you find anything. All I have is my memory of good old
DOS days, where I recall single stepping over %ss loads always
surprised me (over time of course with a fading level of surprise)
in taking two steps. The only thing I can't tell for sure is whether
this maybe was a cute feature of the debugger (recognizing the
%ss load instructions).

Jan


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

Re: [Xen-devel] [PATCH] AMD IOMMU: Support IOAPIC IDs larger than 128

[Jan Beulich]

>>> On 01.12.16 at 12:04,  wrote:
> Currently, the driver uses the APIC ID to index into the ioapic_sbdf array.
> The current MAX_IO_APICS is 128, which causes the driver initialization
> to fail on the system with IOAPIC ID >= 128.
> 
> Instead, this patch adds APIC ID in the struct ioapic_sbdf,
> which is used to match the entry when searching through the array.

Wouldn't it have been a lot simpler to just bump the array size to
256? I'll comment on the rest of the patch anyway ...

> Also, this patch removes the use of ioapic_cmdline bit-map, which is
> used to track the ivrs_ioapic options via command line.
> Instead, it introduces the cmd flag in the struct ioapic_cmdline,

... in struct ioapic_sbdf, afaics.

Note that one of the reasons not to do it this way from the
beginning was that ioapic_sbdf[] is permanent, whereas
ioapic_cmdline is __initdata.

>  static void __init parse_ivrs_ioapic(char *str)
>  {
>  const char *s = str;
>  unsigned long id;
>  unsigned int seg, bus, dev, func;
> +int idx;
>  
>  ASSERT(*s == '[');
>  id = simple_strtoul(s + 1, , 0);
> -if ( id >= ARRAY_SIZE(ioapic_sbdf) || *s != ']' || *++s != '=' )
> +
> +if ( *s != ']' || *++s != '=' )
> +return;
> +
> +idx = ioapic_id_to_index(id);
> +/* If the entry exist, just ignore the option. */
> +if ( idx >= 0 )
>  return;

This is a change in behavior, which I don't think we want: So far later
command line options were allowed to override earlier ones.

> @@ -714,43 +724,36 @@ static u16 __init parse_ivhd_device_special(
>   * consistency here --- whether entry's IOAPIC ID is valid and
>   * whether there are conflicting/duplicated entries.
>   */
> -apic = find_first_bit(ioapic_cmdline, ARRAY_SIZE(ioapic_sbdf));
> -while ( apic < ARRAY_SIZE(ioapic_sbdf) )
> +for ( apic = 0 ; apic < nr_ioapic_sbdf; apic++ )
>  {
>  if ( ioapic_sbdf[apic].bdf == bdf &&
>   ioapic_sbdf[apic].seg == seg )
>  break;
> -apic = find_next_bit(ioapic_cmdline, ARRAY_SIZE(ioapic_sbdf),
> - apic + 1);
>  }
> -if ( apic < ARRAY_SIZE(ioapic_sbdf) )
> +if ( apic < nr_ioapic_sbdf )
>  {
>  AMD_IOMMU_DEBUG("IVHD: Command line override present for IO-APIC 
> %#x"
>  "(IVRS: %#x devID %04x:%02x:%02x.%u)\n",
> -apic, special->handle, seg, PCI_BUS(bdf),
> -PCI_SLOT(bdf), PCI_FUNC(bdf));
> +ioapic_sbdf[apic].id, special->handle, seg,
> +PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf));
>  break;

Note the comment visible as context at the beginning of this hunk:
How can you now tell apart duplicate entries from ones specified
on the command line?

> @@ -1028,15 +1036,15 @@ static int __init parse_ivrs_table(struct 
> acpi_table_header *table)
>  if ( !nr_ioapic_entries[apic] )
>  continue;
>  
> -if ( !ioapic_sbdf[IO_APIC_ID(apic)].seg &&
> +if ( !ioapic_sbdf[apic].seg &&

Can apic really be used as array index here? Don't you need to look
up the index via ioapic_id_to_index()? Or otherwise wouldn't it make
sense to use the same array slots for a particular IO-APIC in both
nr_ioapic_entries[] and ioapic_sbdf[], instead of allocating them via
get_next_ioapic_bdf_index()?

> +int ioapic_id_to_index(unsigned int apic_id)
> +{
> +int idx;
> +
> +if ( !nr_ioapic_sbdf )
> +return -EINVAL;
> +
> +for ( idx = 0 ; idx < nr_ioapic_sbdf; idx++ )

Due to the use as array index I think idx should be unsigned int, no
matter that it's also used as return value of the function. As the
function would only ever return -EINVAL as error indicator, it may
even be worth to consider it having an unsigned return value, with
e.g. MAX_IO_APICS being the error indicator, to avoid using signed
array indexes elsewhere.

> +int get_next_ioapic_bdf_index(void)

__init

> +{
> +if ( nr_ioapic_sbdf < MAX_IO_APICS )
> + return nr_ioapic_sbdf++;

Stray hard tab.

> --- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> @@ -104,8 +104,14 @@ int amd_setup_hpet_msi(struct msi_desc *msi_desc);
>  extern struct ioapic_sbdf {
>  u16 bdf, seg;
>  u16 *pin_2_idx;
> +u8 id;
> +bool cmd;

I'd prefer if you used cmdline. Please fit both fields into the 4-byte
hole ahead of the pointer.

Jan

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

Re: [Xen-devel] [PATCH v3 17/24] x86/pv: Avoid raising faults behind the emulators back

[Tim Deegan]

At 13:50 + on 30 Nov (1480513834), Andrew Cooper wrote:
> Use x86_emul_pagefault() rather than pv_inject_page_fault() to cause raised
> pagefaults to be known to the emulator.  This requires altering the callers of
> x86_emulate() to properly re-inject the event.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Tim Deegan 

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

Re: [Xen-devel] [PATCH v3 18/24] x86/shadow: Avoid raising faults behind the emulators back

[Andrew Cooper]

On 01/12/16 11:39, Tim Deegan wrote:
> At 13:50 + on 30 Nov (1480513835), Andrew Cooper wrote:
>> Use x86_emul_{hw_exception,pagefault}() rather than
>> {pv,hvm}_inject_page_fault() and hvm_inject_hw_exception() to cause raised
>> faults to be known to the emulator.  This requires altering the callers of
>> x86_emulate() to properly re-inject the event.
>>
>> While fixing this, fix the singlestep behaviour.  Previously, an otherwise
>> successful emulation would fail if singlestepping was active, as the emulator
>> couldn't raise #DB.  This is unreasonable from the point of view of the 
>> guest.
>>
>> We therefore tolerate #PF/#GP/SS and #DB being raised by the emulator, but
>> reject anything else as unexpected.
>>
>> Signed-off-by: Andrew Cooper 
> Please update the patch description to remove the bits about
> singlestepping and #DB. With that,
>
> Acked-by: Tim Deegan 

Oh of course.  I managed to do that on the previous patch, but not this
one.  Sorry and thanks.

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

Re: [Xen-devel] [PATCH v3 18/24] x86/shadow: Avoid raising faults behind the emulators back

[Tim Deegan]

At 13:50 + on 30 Nov (1480513835), Andrew Cooper wrote:
> Use x86_emul_{hw_exception,pagefault}() rather than
> {pv,hvm}_inject_page_fault() and hvm_inject_hw_exception() to cause raised
> faults to be known to the emulator.  This requires altering the callers of
> x86_emulate() to properly re-inject the event.
> 
> While fixing this, fix the singlestep behaviour.  Previously, an otherwise
> successful emulation would fail if singlestepping was active, as the emulator
> couldn't raise #DB.  This is unreasonable from the point of view of the guest.
> 
> We therefore tolerate #PF/#GP/SS and #DB being raised by the emulator, but
> reject anything else as unexpected.
> 
> Signed-off-by: Andrew Cooper 

Please update the patch description to remove the bits about
singlestepping and #DB. With that,

Acked-by: Tim Deegan 

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

Re: [Xen-devel] [PATCH v3 13/24] x86/emul: Rework emulator event injection

[Tim Deegan]

At 13:50 + on 30 Nov (1480513830), Andrew Cooper wrote:
> The emulator needs to gain an understanding of interrupts and exceptions
> generated by its actions.
> 
> Move hvm_emulate_ctxt.{exn_pending,trap} into struct x86_emulate_ctxt so they
> are visible to the emulator.  This removes the need for the
> inject_{hw_exception,sw_interrupt}() hooks, which are dropped and replaced
> with x86_emul_{hw_exception,software_event,reset_event}() instead.
> 
> For exceptions raised by x86_emulate() itself (rather than its callbacks), the
> shadow pagetable and PV uses of x86_emulate() previously failed with
> X86EMUL_UNHANDLEABLE due to the lack of inject_*() hooks.
> 
> This behaviour has changed, and such cases will now return X86EMUL_EXCEPTION
> with event_pending set.  Until the callers of x86_emulate() have been updated
> to inject events back into the guest, divert the event_pending case back into
> the X86EMUL_UNHANDLEABLE path to maintain the same guest-visible behaviour.
> 
> No overall functional change.
> 
> Signed-off-by: Andrew Cooper 
> Reviewed-by: Boris Ostrovsky 
> Reviewed-by: Kevin Tian 

Acked-by: Tim Deegan 

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

Re: [Xen-devel] [PATCH v3 11/24] x86/emul: Implement singlestep as a retire flag

[Tim Deegan]

At 11:23 + on 01 Dec (1480591394), Andrew Cooper wrote:
> Hmm.  It is only the PAE case we want to skip.  Perhaps changing the PAE
> entry condition to
> 
> diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
> index c45d260..28ff945 100644
> --- a/xen/arch/x86/mm/shadow/multi.c
> +++ b/xen/arch/x86/mm/shadow/multi.c
> @@ -3480,7 +3480,7 @@ static int sh_page_fault(struct vcpu *v,
>  }
>  
>  #if GUEST_PAGING_LEVELS == 3 /* PAE guest */
> -if ( r == X86EMUL_OKAY ) {
> +if ( r == X86EMUL_OKAY && !emul_ctxt.ctxt.retire.raw ) {
>  int i, emulation_count=0;
>  this_cpu(trace_emulate_initial_va) = va;
>  /* Emulate up to four extra instructions in the hope of catching
> 
> would be better, along with suitable comments and style fixes?

That would be OK by me, and with that change,

Acked-by: Tim Deegan 

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

[Xen-devel] [xtf test] 102740: all pass - PUSHED

[osstest service owner]

flight 102740 xtf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/102740/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 xtf  1f021c88130b4d2d818ba4f269b3929339c00a88
baseline version:
 xtf  36431397ac902e791e16a016ade3413bd6b63069

Last test of basis   102488  2016-11-21 17:48:17 Z9 days
Testing same since   102740  2016-11-30 19:45:46 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 

jobs:
 build-amd64-xtf  pass
 build-amd64  pass
 build-amd64-pvopspass
 test-xtf-amd64-amd64-1   pass
 test-xtf-amd64-amd64-2   pass
 test-xtf-amd64-amd64-3   pass
 test-xtf-amd64-amd64-4   pass
 test-xtf-amd64-amd64-5   pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

+ branch=xtf
+ revision=1f021c88130b4d2d818ba4f269b3929339c00a88
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xtf 
1f021c88130b4d2d818ba4f269b3929339c00a88
+ branch=xtf
+ revision=1f021c88130b4d2d818ba4f269b3929339c00a88
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=xtf
+ xenbranch=xen-unstable
+ '[' xxtf = xlinux ']'
+ linuxbranch=
+ '[' x = x ']'
+ qemuubranch=qemu-upstream-unstable
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable
+ prevxenbranch=xen-4.7-testing
+ '[' x1f021c88130b4d2d818ba4f269b3929339c00a88 = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/xtf.git
++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git
++ : git://xenbits.xen.org/xtf.git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git
++ : git://git.seabios.org/seabios.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/seabios.git
++ : git://xenbits.xen.org/osstest/seabios.git
++ : https://github.com/tianocore/edk2.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/linux-firmware.git
++ : osst...@xenbits.xen.org:/home/osstest/ext/linux-firmware.git
++ : git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
++ : osst...@xenbits.xen.org:/home/xen/git/linux-pvops.git
++ : git://xenbits.xen.org/linux-pvops.git
++ : tested/linux-3.14
++ : tested/linux-arm-xen
++ '[' 

Re: [Xen-devel] [PATCH v4 03/14] xen/x86: allow calling {shadow/hap}_set_allocation with the idle domain

[Tim Deegan]

At 16:49 + on 30 Nov (1480524579), Roger Pau Monne wrote:
> ... and using the "preempted" parameter. Introduce a new helper that can
> be used from both hypercall or idle vcpu context (ie: during Dom0
> creation) in order to check if preemption is needed. If such preemption
> happens, the caller should then call process_pending_softirqs in order to
> drain the pending softirqs, and then call *_set_allocation again to continue
> with it's execution.
> 
> This allows us to call *_set_allocation() when building domain 0.
> 
> While there also document hypercall_preempt_check and add an assert to
> local_events_need_delivery in order to be sure it's not called by the idle
> domain, which doesn't receive any events (and that in turn
> hypercall_preempt_check is also not called by the idle domain).
> 
> Signed-off-by: Roger Pau Monné 
> Acked-by: George Dunlap 

Acked-by: Tim Deegan 

Thanks,

Tim.

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

Re: [Xen-devel] [PATCH v3 11/24] x86/emul: Implement singlestep as a retire flag

[Andrew Cooper]

On 01/12/16 11:16, Jan Beulich wrote:
 On 30.11.16 at 14:50,  wrote:
>> The behaviour of singlestep is to raise #DB after the instruction has been
>> completed, but implementing it with inject_hw_exception() causes 
>> x86_emulate()
>> to return X86EMUL_EXCEPTION, despite succesfully completing execution of the
>> instruction, including register writeback.
> Nice, I think that'll help simplify the privop patch a bit.
>
>> --- a/xen/arch/x86/mm/shadow/multi.c
>> +++ b/xen/arch/x86/mm/shadow/multi.c
>> @@ -3422,6 +3422,16 @@ static int sh_page_fault(struct vcpu *v,
>>  v->arch.paging.last_write_emul_ok = 0;
>>  #endif
>>  
>> +if ( emul_ctxt.ctxt.retire.singlestep )
>> +{
>> +if ( is_hvm_vcpu(v) )
>> +hvm_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
>> +else
>> +pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
>> +
>> +goto emulate_done;
> This results in skipping the PAE special code (which I think is intended)

Correct

> but also the trace_shadow_emulate(), which I don't think is wanted.

Hmm.  It is only the PAE case we want to skip.  Perhaps changing the PAE
entry condition to

diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
index c45d260..28ff945 100644
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -3480,7 +3480,7 @@ static int sh_page_fault(struct vcpu *v,
 }
 
 #if GUEST_PAGING_LEVELS == 3 /* PAE guest */
-if ( r == X86EMUL_OKAY ) {
+if ( r == X86EMUL_OKAY && !emul_ctxt.ctxt.retire.raw ) {
 int i, emulation_count=0;
 this_cpu(trace_emulate_initial_va) = va;
 /* Emulate up to four extra instructions in the hope of catching

would be better, along with suitable comments and style fixes?

>
>> @@ -3433,7 +3443,7 @@ static int sh_page_fault(struct vcpu *v,
>>  shadow_continue_emulation(_ctxt, regs);
>>  v->arch.paging.last_write_was_pt = 0;
>>  r = x86_emulate(_ctxt.ctxt, emul_ops);
>> -if ( r == X86EMUL_OKAY )
>> +if ( r == X86EMUL_OKAY && !emul_ctxt.ctxt.retire.raw )
> I think this wants to have a comment attached explaining why
> a blanket check of all current and future retire flags here is the
> right thing (or benign).

Ok.

>
>> @@ -3449,6 +3459,15 @@ static int sh_page_fault(struct vcpu *v,
>>  {
>>  perfc_incr(shadow_em_ex_fail);
>>  TRACE_SHADOW_PATH_FLAG(TRCE_SFLAG_EMULATION_LAST_FAILED);
>> +
>> +if ( emul_ctxt.ctxt.retire.singlestep )
>> +{
>> +if ( is_hvm_vcpu(v) )
>> +hvm_inject_hw_exception(TRAP_debug, 
>> X86_EVENT_NO_EC);
>> +else
>> +pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
>> +}
>> +
>>  break; /* Don't emulate again if we failed! */
> This comment is now slightly stale.

"failed to find the second half of the write".  In combination with a
suitable comment in the hunk above, this should be fine as is.

>
>> @@ -5415,11 +5414,11 @@ x86_emulate(
>>  if ( !mode_64bit() )
>>  _regs.eip = (uint32_t)_regs.eip;
>>  
>> -*ctxt->regs = _regs;
>> +/* Was singestepping active at the start of this instruction? */
>> +if ( (rc == X86EMUL_OKAY) && (ctxt->regs->eflags & EFLG_TF) )
>> +ctxt->retire.singlestep = true;
> Don't we need to avoid doing this when mov_ss is set? Or does the
> hardware perhaps do the necessary deferring for us?

I am currently reading up about this in the manual.  I need more coffee.

~Andrew

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

Re: [Xen-devel] [PATCH v3 09/24] x86/emul: Provide a wrapper to x86_emulate() to ASSERT() certain behaviour

[Jan Beulich]

>>> On 01.12.16 at 11:58,  wrote:
> On 01/12/16 10:40, Jan Beulich wrote:
>>
>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>>> @@ -2404,6 +2404,11 @@ x86_decode(
>>>  #undef insn_fetch_bytes
>>>  #undef insn_fetch_type
>>>  
>>> +/* Undo DEBUG wrapper. */
>>> +#ifdef x86_emulate
>>> +#undef x86_emulate
>>> +#endif
>> I don't see the need for the #ifdef here.
> 
> It will break the non-debug build if removed, as x86_emulate wouldn't be
> a define.

#undef for something that isn't a #define is well defined to do
nothing.

Jan


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