Re: [Xen-devel] Ping#2: [PATCH] x86emul: keep compiler from using {x, y, z}mm registers itself

[Julien Grall]

Hi,

On 11/21/2017 01:29 PM, Andrew Cooper wrote:

On 21/11/17 13:26, Jan Beulich wrote:

On 06.11.17 at 16:04, <george.dun...@citrix.com> wrote:

On 11/06/2017 11:59 AM, Jan Beulich wrote:

On 16.10.17 at 14:42,  wrote:

On 16.10.17 at 14:37, <andrew.coop...@citrix.com> wrote:

On 16/10/17 13:32, Jan Beulich wrote:

Since the emulator acts on the live hardware registers, we need to
prevent the compiler from using them e.g. for inlined memcpy() /
memset() (as gcc7 does). We can't, however, set this from the command
line, as otherwise the 64-bit build would face issues with functions
returning floating point values and being declared in standard headers.

As the pragma isn't available prior to gcc6, we need to invoke it
conditionally. Luckily up to gcc6 we haven't seen generated code access
SIMD registers beyond what our asm()s do.

Reported-by: George Dunlap <george.dun...@citrix.com>
Signed-off-by: Jan Beulich <jbeul...@suse.com>
---
While this doesn't affect core functionality, I think it would still be
nice for it to be allowed in for 4.10.

Agreed.

Has this been tested with Clang?

Sorry, no - still haven't got around to set up a suitable Clang
locally.


  It stands a good chance of being
compatible, but we may need an && !defined(__clang__) included.

Should non-gcc silently ignore "#pragma GCC ..." it doesn't
recognize, or not define __GNUC__ in the first place if it isn't
sufficiently compatible? I.e. if anything I'd expect we need
"#elif defined(__clang__)" to achieve the same for Clang by
some different pragma (if such exists).

Not having received any reply so far, I'm wondering whether
being able to build the test harness with clang is more
important than for it to work correctly when built with gcc. I
can't predict when I would get around to set up a suitable
clang on my dev systems.

I agree with the argument you make above.  On the unlikely chance
there's a problem Travis should catch it, and someone who actually has a
clang setup can help sort it out.

I'm still lacking an ack, before it being sensible to check with Julien
whether this is still fine to go in at this late stage.


I would be happy with that.

Release-acked-by: Julien Grall <julien.gr...@linaro.org>

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2] sync CPU state upon final domain destruction

[Julien Grall]

Hi,

On 11/22/2017 01:00 PM, Jan Beulich wrote:

On 22.11.17 at 13:39, <jbeul...@suse.com> wrote:

See the code comment being added for why we need this.

This is being placed here to balance between the desire to prevent
future similar issues (the risk of which would grow if it was put
further down the call stack, e.g. in vmx_vcpu_destroy()) and the
intention to limit the performance impact (otherwise it could also go
into rcu_do_batch(), paralleling the use in do_tasklet_work()).

Reported-by: Igor Druzhinin <igor.druzhi...@citrix.com>
Signed-off-by: Jan Beulich <jbeul...@suse.com>


I'm sorry, Julien, I did forget to Cc you (for 4.10 inclusion).


Release-acked-by: Julien Grall <julien.gr...@linaro.org>

Cheers,




---
v2: Move from vmx_vcpu_destroy() to complete_domain_destroy().

--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -794,6 +794,14 @@ static void complete_domain_destroy(stru
  struct vcpu *v;
  int i;
  
+/*

+ * Flush all state for the vCPU previously having run on the current CPU.
+ * This is in particular relevant for x86 HVM ones on VMX, so that this
+ * flushing of state won't happen from the TLB flush IPI handler behind
+ * the back of a vmx_vmcs_enter() / vmx_vmcs_exit() section.
+ */
+sync_local_execstate();
+
  for ( i = d->max_vcpus - 1; i >= 0; i-- )
  {
  if ( (v = d->vcpu[i]) == NULL )




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



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

Re: [Xen-devel] [Draft Design v3] ACPI/IORT Support in Xen.

[Julien Grall]

i similar to linux iort_node_map_rid be mapped to
query_streamid

6. IORT Generation
---
It is proposed to have a common helper library to generate IORT for dom0/U.
Note: it is desired to have IORT generation code sharing between toolstack
and Xen.

a. For Dom0
  rid_deviceId_map can be used directly to generate dom0 IORT table.
  Exclusions of nodes is still open for suggestions.

b. For DomU
  Minimal structure is discussed in section 4. It will be further discussed
  in the context of PCI PT design.

7. Implementation Phases
-
a. IORT Parsing and RID Query
b. IORT Generation for Dom0
c. IORT Generation for DomU.

8. References:
-
[0] https://www.mail-archive.com/xen-devel@lists.xen.org/msg121667.html
[1] ARM DEN0049C: 
http://infocenter.arm.com/help/topic/com.arm.doc.den0049c/DEN0049C_IO_Remapping_Table.pdf 


[2] https://www.mail-archive.com/xen-devel@lists.xen.org/msg123082.html
[3] NA.
[4] https://www.mail-archive.com/xen-devel@lists.xen.org/msg123434.html
[5] https://www.mail-archive.com/xen-devel@lists.xen.org/msg123080.html
[6] 
https://github.com/mjaggi-cavium/xen-wip/commit/df006d64bdbb5c8344de5a710da8bf64c9e8edd5 


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 06/16] SUPPORT.md: Add scalability features

[Julien Grall]

Hi George,

On 11/21/2017 04:43 PM, George Dunlap wrote:

On 11/16/2017 03:19 PM, Julien Grall wrote:

On 13/11/17 15:41, George Dunlap wrote:

Signed-off-by: George Dunlap <george.dun...@citrix.com>
---
CC: Ian Jackson <ian.jack...@citrix.com>
CC: Wei Liu <wei.l...@citrix.com>
CC: Andrew Cooper <andrew.coop...@citrix.com>
CC: Jan Beulich <jbeul...@suse.com>
CC: Stefano Stabellini <sstabell...@kernel.org>
CC: Konrad Wilk <konrad.w...@oracle.com>
CC: Tim Deegan <t...@xen.org>
CC: Julien Grall <julien.gr...@arm.com>
---
   SUPPORT.md | 21 +
   1 file changed, 21 insertions(+)

diff --git a/SUPPORT.md b/SUPPORT.md
index c884fac7f5..a8c56d13dd 100644
--- a/SUPPORT.md
+++ b/SUPPORT.md
@@ -195,6 +195,27 @@ on embedded platforms.
     Enables NUMA aware scheduling in Xen
   +## Scalability
+
+### 1GB/2MB super page support
+
+    Status, x86 HVM/PVH: : Supported
+    Status, ARM: Supported
+
+NB that this refers to the ability of guests
+to have higher-level page table entries point directly to memory,
+improving TLB performance.
+This is independent of the ARM "page granularity" feature (see below).


I am not entirely sure about this paragraph for Arm. I understood this
section as support for stage-2 page-table (aka EPT on x86) but the
paragraph lead me to believe to it is for guest.

The size of super pages of guests will depend on the page granularity
used by itself and the format of the page-table (e.g LPAE vs short
descriptor). We have no control on that.

What we have control is the size of mapping used for stage-2 page-table.


Stepping back from the document for a minute: would it make sense to use
"hardware assisted paging" (HAP) for Intel EPT, AMD RVI (previously
NPT), and ARM stage-2 pagetables?  HAP was already a general term used
to describe the two x86 technologies; and I think the description makes
sense, because if we didn't have hardware-assisted stage 2 pagetables
we'd need Xen-provided shadow pagetables.


I think using the term "hardware assisted paging" should be fine to 
refer the 3 technologies.




Back to the question at hand, there are four different things:

1. Whether Xen itself uses superpage mappings for its virtual address
space.  (Not sure if Xen does this or not.)


Xen is trying to use superpage mappings for itself whenever it is possible.



2. Whether Xen uses superpage mappings for HAP.  Xen uses this on x86
when hardware support is -- I take it Xen does this on ARM as well?


The size of superpages supported will depend on the page-table format 
(short-descriptor vs LPAE) and the granularity used.


Supersection (16MB) for short-descriptor is optional but mandatory when 
the processor support LPAE. LPAE is mandatory with virtualization. So 
all size of superpages are supported.


Note that stage-2 page-tables can only use LPAE page-table.

I would also rather avoid to mention any superpage size for Arm in 
SUPPORT.MD as there are a lot.


Short-descriptor is always using 4KB granularity supports 16MB, 1MB, 64KB

LPAE supports 4KB, 16KB, 64KB granularities. Each of them having 
different size of superpage.




3. Whether Xen provides the *interface* for a guest to use L2 or L3
superpages (for 4k page granularity, 2MiB or 1GiB respectively) in its
own pagetables.  I *think* HAP on x86 provides the interface whenever
the underlying hardware does.  I assume it's the same for ARM?  In the
case of shadow mode, we only provide the interface for 2MiB pagetables.


See above. We have no way to control that in the guest.



4. Whether a guest using L2 or L3 superpages will actually have
superpages, or whether it's "only emulated".  As Jan said, for shadow
pagetables on x86, the underlying pagetables still only have 4k pages,
so the guest will get no benefit from using L2 superpages in its
pagetables (either in terms of reduced memory reads on a tlb miss, or in
terms of larger effectiveness of each TLB entry).

#3 and #4 are probably the most pertinent to users, with #2 being next
on the list, and #1 being least.

Does that make sense?


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 04/16] SUPPORT.md: Add core ARM features

[Julien Grall]

Hi George,

On 11/21/2017 10:45 AM, George Dunlap wrote:

On 11/21/2017 08:11 AM, Jan Beulich wrote:

On 13.11.17 at 16:41, <george.dun...@citrix.com> wrote:

+### ARM/SMMUv1
+
+Status: Supported
+
+### ARM/SMMUv2
+
+Status: Supported


Do these belong here, when IOMMU isn't part of the corresponding
x86 patch?


Since there was recently a time when these weren't supported, I think
it's useful to have them in here.  (Julien, let me know if you think
otherwise.)


I think it is useful to keep them. There are other IOMMUs existing on 
Arm (e.g SMMUv3, IPMMU-VMSA) that we don't yet support in Xen.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] Next Xen Arm Community call - Wednesday 22nd November

[Julien Grall]

Hi all,

Quick reminder, the call will be tomorrow (Wednesday 22nd) at 5pm GMT.

The details to join the call are:

Call+44 1223 406065 (Local dial in)
and enter the access code below followed by # key.
Participant code: 4915191

Mobile Auto Dial:
VoIP: voip://+441223406065;4915191#
iOS devices: +44 1223 406065,4915191 and press #
Other devices: +44 1223 406065x4915191#

Additional Calling Information:

UK +44 1142828002
US CA +1 4085761502
US TX +1 5123141073
JP +81 453455355
DE +49 8945604050
NO +47 73187518
SE +46 46313131
FR +33 497235101
TW +886 35657119
HU +36 13275600
IE +353 91337900

Toll Free

UK 0800 1412084
US +1 8668801148
CN +86 4006782367
IN 0008009868365
IN +918049282778
TW 08000 22065
HU 0680981587
IE 1800800022
KF +972732558877

Cheers,

On 16 November 2017 at 11:54, Julien Grall <julien.gr...@linaro.org> wrote:
> Hi all,
>
> Apologies I was meant to organize the call earlier.
>
> I would suggest to have the next community call on Wednesday 22nd November
> 5pm GMT. Does it sound good?
>
> Do you have any specific topic you would like to discuss?
>
> Cheers,
>
> --
> Julien Grall

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

Re: [Xen-devel] [PATCH for-4.10] x86/hvm: Don't corrupt the HVM context stream when writing the MSR record

[Julien Grall]

Hi,

On 11/16/2017 10:45 PM, Andrew Cooper wrote:

Ever since it was introduced in c/s bd1f0b45ff, hvm_save_cpu_msrs() has had a
bug whereby it corrupts the HVM context stream if some, but fewer than the
maximum number of MSRs are written.

_hvm_init_entry() creates an hvm_save_descriptor with length for
msr_count_max, but in the case that we write fewer than max, h->cur only moves
forward by the amount of space used, causing the subsequent
hvm_save_descriptor to be written within the bounds of the previous one.

To resolve this, reduce the length reported by the descriptor to match the
actual number of bytes used.

A typical failure on the destination side looks like:

 (XEN) HVM4 restore: CPU_MSR 0
 (XEN) HVM4.0 restore: not enough data left to read 56 MSR bytes
 (XEN) HVM4 restore: failed to load entry 20/0

Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
---
CC: Jan Beulich <jbeul...@suse.com>
CC: Wei Liu <wei.l...@citrix.com>
CC: Julien Grall <julien.gr...@arm.com>

This wants backporting to all stable trees, so should also be considered for
inclusion into 4.10 at this point.


Release-acked-by: Julien Grall <julien.gr...@linaro.org>

Cheers,


---
  xen/arch/x86/hvm/hvm.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 0af498a..c5e8467 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1330,6 +1330,7 @@ static int hvm_save_cpu_msrs(struct domain *d, 
hvm_domain_context_t *h)
  
  for_each_vcpu ( d, v )

  {
+struct hvm_save_descriptor *d = _p(>data[h->cur]);
  struct hvm_msr *ctxt;
  unsigned int i;
  
@@ -1348,8 +1349,13 @@ static int hvm_save_cpu_msrs(struct domain *d, hvm_domain_context_t *h)

  ctxt->msr[i]._rsvd = 0;
  
  if ( ctxt->count )

+{
+/* Rewrite length to indicate how much space we actually used. */
+d->length = HVM_CPU_MSR_SIZE(ctxt->count);
  h->cur += HVM_CPU_MSR_SIZE(ctxt->count);
+}
  else
+/* or rewind and remove the descriptor from the stream. */
  h->cur -= sizeof(struct hvm_save_descriptor);
  }
  



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

Re: [Xen-devel] [PATCH for-4.10] tools/libxc: Fix restoration of PV MSRs after migrate

[Julien Grall]

Hi,

On 11/16/2017 09:13 PM, Andrew Cooper wrote:

There are two bugs in process_vcpu_msrs() which clearly demonstrate that I
didn't test this bit of Migration v2 very well when writing it...

vcpu->msrsz is always expected to be a multiple of xen_domctl_vcpu_msr_t
records in a spec-compliant stream, so the modulo yields 0 for the msr_count,
rather than the actual number sent in the stream.

Passing 0 for the msr_count causes the hypercall to exit early, and hides the
fact that the guest handle is inserted into the wrong field in the domctl
union.

The reason that these bugs have gone unnoticed for so long is that the only
MSRs passed like this for PV guests are the AMD DBGEXT MSRs, which only exist
in fairly modern hardware, and whose use doesn't appear to be implemented in
any contemporary PV guests.

Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
---
CC: Jan Beulich <jbeul...@suse.com>
CC: Ian Jackson <ian.jack...@eu.citrix.com>
CC: Wei Liu <wei.l...@citrix.com>
CC: Julien Grall <julien.gr...@arm.com>

This wants backporting to all stable trees, so should also be considered for
inclusion into 4.10 at this point.


Release-acked-by: Julien Grall <julien.gr...@linaro.org>

Cheers,


---
  tools/libxc/xc_sr_restore_x86_pv.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/libxc/xc_sr_restore_x86_pv.c 
b/tools/libxc/xc_sr_restore_x86_pv.c
index 50e25c1..ed0fd0e 100644
--- a/tools/libxc/xc_sr_restore_x86_pv.c
+++ b/tools/libxc/xc_sr_restore_x86_pv.c
@@ -455,8 +455,8 @@ static int process_vcpu_msrs(struct xc_sr_context *ctx,
  domctl.cmd = XEN_DOMCTL_set_vcpu_msrs;
  domctl.domain = ctx->domid;
  domctl.u.vcpu_msrs.vcpu = vcpuid;
-domctl.u.vcpu_msrs.msr_count = vcpu->msrsz % sizeof(xen_domctl_vcpu_msr_t);
-set_xen_guest_handle(domctl.u.vcpuextstate.buffer, buffer);
+domctl.u.vcpu_msrs.msr_count = vcpu->msrsz / sizeof(xen_domctl_vcpu_msr_t);
+set_xen_guest_handle(domctl.u.vcpu_msrs.msrs, buffer);
  
  memcpy(buffer, vcpu->msr, vcpu->msrsz);
  



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

Re: [Xen-devel] Next Xen Arm Community call - Wednesday 22nd November

[Julien Grall]

Answering to myself.

On 16 November 2017 at 11:54, Julien Grall <julien.gr...@linaro.org> wrote:
> Hi all,
>
> Apologies I was meant to organize the call earlier.
>
> I would suggest to have the next community call on Wednesday 22nd November
> 5pm GMT. Does it sound good?
>
> Do you have any specific topic you would like to discuss?

I would like to discuss about Power Saving when using Xen (e.g
suspend, CPUFreq, idling).

I will send the details of the call tomorrow.

Cheers,

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

Re: [Xen-devel] [RFC v2 7/7] xen/iommu: smmu-v3: Add Xen specific code to enable the ported driver

[Julien Grall]

Hi,

On 20/11/17 15:19, Robin Murphy wrote:

On 20/11/17 14:25, Julien Grall wrote:
[...]



+    else {
   cpu_relax();


Hmmm I now see why you added cpu_relax() at the top. Well, on Xen 
cpu_relax is just a barrier. On Linux it is used to yield.


And that bit is worrying me. The Linux code will allow context 
switching to another tasks if the code is taking too much time.


Xen is not preemptible, so is it fine?
This is used when consuming the command queue and could be a 
potential performance issue if the queue is large. (This is never the 
case).

I am wondering if we should define a yeild in long run?


As I said before, Xen is not preemptible. In this particular case, 
there are spinlock taken by the callers (e.g any function assigning 
device). So yield would just make it worst.


The arguments here don't make much sense - the "yield" instruction has 
nothing to do with software-level concepts of preemption. It is a hint 
to SMT *hardware* that this logical processor is doing nothing useful in 
the short term, so it might be a good idea to let other logical 
processor(s) have priority over shared execution resources if applicable.


Oh, sorry I thought this could also be used by the software to switch 
between thread. Please disregard my comment then.




Until SMT CPUs become commonly available, though, it's somewhat of a 
moot point and mostly just a future-proofing consideration.


Robin.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC v2 7/7] xen/iommu: smmu-v3: Add Xen specific code to enable the ported driver

[Julien Grall]

Hi Sameer,

On 19/11/17 07:45, Goel, Sameer wrote:

On 10/12/2017 10:36 AM, Julien Grall wrote:

+
+typedef paddr_t phys_addr_t;
+typedef paddr_t dma_addr_t;
+
+/* Alias to Xen device tree helpers */
+#define device_node dt_device_node
+#define of_phandle_args dt_phandle_args
+#define of_device_id dt_device_match
+#define of_match_node dt_match_node
+#define of_property_read_u32(np, pname, out) (!dt_property_read_u32(np, pname, 
out))
+#define of_property_read_bool dt_property_read_bool
+#define of_parse_phandle_with_args dt_parse_phandle_with_args
+#define mutex spinlock_t
+#define mutex_init spin_lock_init
+#define mutex_lock spin_lock
+#define mutex_unlock spin_unlock


mutex and spinlock are not the same. The former is sleeping whilst the later is 
not.

Can you please explain why this is fine and possibly add that in a comment?


Mutex is used to protect the access to smmu device internal data structure when 
setting up the s2 config and installing stes for a given device in Linux. The 
ste programming  operation can be competitively long but in the current 
testing, I did not see this blocking for too long. I will put in a comment.


Well, I don't think that this is a justification. You tested on one 
platform and does not explain how you perform them.


If I understand correctly, that mutex is only used when assigning 
device. So it might be ok to switch to spinlock. But that's not because 
the operation is not too long, it just because it would be only perform 
by the toolstack (domctl) and will not be issued by guest.





+
+/* Xen: Helpers to get device MMIO and IRQs */
+struct resource {
+    u64 addr;
+    u64 size;
+    unsigned int type;
+};


Likely we want a compat header for defining Linux helpers. This would avoid 
replicating it everywhere.

Agreed.


That should be



+
+#define resource_size(res) ((res)->size)
+
+#define platform_device device
+
+#define IORESOURCE_MEM 0
+#define IORESOURCE_IRQ 1
+
+static struct resource *platform_get_resource(struct platform_device *pdev,
+  unsigned int type,
+  unsigned int num)
+{
+    /*
+ * The resource is only used between 2 calls of platform_get_resource.
+ * It's quite ugly but it's avoid to add too much code in the part
+ * imported from Linux
+ */
+    static struct resource res;
+    struct acpi_iort_node *iort_node;
+    struct acpi_iort_smmu_v3 *node_smmu_data;
+    int ret = 0;
+
+    res.type = type;
+
+    switch (type) {
+    case IORESOURCE_MEM:
+    if (pdev->type == DEV_ACPI) {
+    ret = 1;
+    iort_node = pdev->acpi_node;
+    node_smmu_data =
+    (struct acpi_iort_smmu_v3 *)iort_node->node_data;
+
+    if (node_smmu_data != NULL) {
+    res.addr = node_smmu_data->base_address;
+    res.size = SZ_128K;
+    ret = 0;
+    }
+    } else {
+    ret = dt_device_get_address(dev_to_dt(pdev), num,
+    , );
+    }
+
+    return ((ret) ? NULL : );
+
+    case IORESOURCE_IRQ:
+    ret = platform_get_irq(dev_to_dt(pdev), num);


No IRQ for ACPI?

For IRQs the code calls platform_get_irq_byname. So, the IORESOURCE_IRQ 
implementation is not needed at all. (DT or ACPI)


Please document it then.

[...]




+    udelay(sleep_us); \
+    } \
+    (cond) ? 0 : -ETIMEDOUT; \
+})
+
+#define readl_relaxed_poll_timeout(addr, val, cond, delay_us, timeout_us) \
+    readx_poll_timeout(readl_relaxed, addr, val, cond, delay_us, timeout_us)
+
+/* Xen: Helpers for IRQ functions */
+#define request_irq(irq, func, flags, name, dev) request_irq(irq, flags, func, 
name, dev)
+#define free_irq release_irq
+
+enum irqreturn {
+    IRQ_NONE    = (0 << 0),
+    IRQ_HANDLED    = (1 << 0),
+};
+
+typedef enum irqreturn irqreturn_t;
+
+/* Device logger functions */
+#define dev_print(dev, lvl, fmt, ...)    \
+ printk(lvl "smmu: " fmt, ## __VA_ARGS__)
+
+#define dev_dbg(dev, fmt, ...) dev_print(dev, XENLOG_DEBUG, fmt, ## 
__VA_ARGS__)
+#define dev_notice(dev, fmt, ...) dev_print(dev, XENLOG_INFO, fmt, ## 
__VA_ARGS__)
+#define dev_warn(dev, fmt, ...) dev_print(dev, XENLOG_WARNING, fmt, ## 
__VA_ARGS__)
+#define dev_err(dev, fmt, ...) dev_print(dev, XENLOG_ERR, fmt, ## __VA_ARGS__)
+#define dev_info(dev, fmt, ...) dev_print(dev, XENLOG_INFO, fmt, ## 
__VA_ARGS__)
+
+#define dev_err_ratelimited(dev, fmt, ...)    \
+ dev_print(dev, XENLOG_ERR, fmt, ## __VA_ARGS__)
+
+#define dev_name(dev) dt_node_full_name(dev_to_dt(dev))
+
+/* Alias to Xen allocation helpers */
+#define kfree xfree
+#define kmalloc(size, flags)    _xmalloc(size, sizeof(void *))
+#define kzalloc(size, flags)    _xzalloc(size, sizeof(void *))
+#define devm_kzalloc(dev, size, flags)    _xzalloc(size, sizeof(void *))
+#define kmalloc_array(size, n, flags)    _xmalloc_array(size, sizeof(void *), 
n)
+
+/* Com

Re: [Xen-devel] Unable to create guest PV domain on OMAP5432

[Julien Grall]

Hello,

On 15 November 2017 at 11:34, Jayadev Kumaran  wrote:
>>> What defconfig are you based on? Do you have a device-tree support
>>> enabled?
> I use omap2plus_defconfig . Yes , device tree support is there and the dts
> file used is omap5-uevm.dts

Some options in omap2plus_defconfig might upset the kernel such as
CONFIG_ARM_APPENDED_DTB.

>
>>> But it did not get a command line to setup console on hvc0, or the kernel
>>> crashed in earliest stages.
>
> Is there a way to debug which one of the above two possibilities has lead to
> the issue ? As I'm using the dom0 kernel image for my guest, is it still
> possible that it could be a kernel crash since it has already booted up for
> dom0.

From a previous e-mail I see you are using 3.15. Is there any particular reason
to not use at least a stable kernel if not a new one?

Cheers,

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

Re: [Xen-devel] Unable to create guest PV domain on OMAP5432

[Julien Grall]

On 17 November 2017 at 12:15, Volodymyr Babchuk  wrote:
> Hi Jayadev,
>
> On 17 November 2017 at 13:53, Andrii Anisov  wrote:
>>>
>>> Is there a way to debug which one of the above two possibilities has lead
>>> to the issue ?
>>
>> Four years ago I did it in a following way:
>> - wire up to UART2 pins on an expansion connector (this sheet might be
>> useful for you: http://www.ti.com/lit/ug/swcu130/swcu130.pdf)
>> - assign UART2 IOMEM ranges to DomU
>> - enable in domu kernel earlyprintk and patch it to output to omap UART2
>>
>> Nowadays assigning UART2 IOMEM ranges might be a bit more complex due to XEN
>> evolution.
>>
>> Another way is to use JTAG which understands virtualization, and allows you
>> to debug DomU.
> I want to add some debugging techniques:
>  - By tapping CTRL+A three times to switch to XEN console. Then `d`
> will show you CPU register states. You will be able to see where your
> DomU executes right now. This can help along with addr2line, objdump
> and other tools.
>
>  - You can modify traps.c in XEN to crash domain when SMC instruction
> is trapped. Then you can put SMC invocation into various parts of
> kernel code, to see if it reaches that place. This can help on early
> stages, when console is not available.

No need for modifying the Xen. Xen already provides debug facilities when
CONFIG_DEBUG=y.

You can have a look at do_debug_trap. The ones you likely want are:

  - hvc 0x will show the state of the vCPU
  - hvc 0xfffe will print the PC

Cheers,

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

Re: [Xen-devel] Unable to create guest PV domain on OMAP5432

[Julien Grall]

On 8 November 2017 at 14:52, Konrad Rzeszutek Wilk
 wrote:
> On Wed, Nov 08, 2017 at 10:47:20AM +0530, Jayadev Kumaran wrote:
>> Hello all,
>>
>> I'm trying to implement Xen hypervisor support on OMAP5432.I have followed
>> the steps as in
>> https://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions/OMAP5432_uEVM
>> for the initial setup. I'm able to see the domain 0 successfully up.
>>
>>
>>
>>
>>
>>
>>
>> *root@omap5-evm:~# /etc/init.d/xencommons startStarting
>> /usr/local/sbin/xenstored...Setting domain 0 name, domid and JSON
>> config...Done setting up Dom0Starting xenconsoled...Starting QEMU as disk
>> backend for dom0*
>>
>>
>>
>> *root@omap5-evm:~# xl listNameID
>> Mem VCPUs  State   Time(s)Domain-0
>> 0   512 2 r-  11.5*
>
> What does 'xl info' say? B/c:
> ..
>> Expanding d4 grant table from 0 to 1 frames(XEN) memory.c:238:d0v0 Could
>> not allocate order=18 extent: id=4 memflags=0xc0 (0 of 1)libxl: error:
>
> .. looks like it could not allocate memory?

This message is not important. The toolstack will always try to use
very big mapping first and then
scale down if it is not possible to allocate.

In this case, the platform does not have 1G contiguous, and will
fallback to 2M mapping.

Cheers,

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

Re: [Xen-devel] [RFC PATCH 00/31] CPUFreq on ARM

[Julien Grall]

Hi,

First of all, thank you Oleksandr for starting a thread around CPUFreq 
support.


On 11/16/2017 05:04 PM, Andre Przywara wrote:

On 16/11/17 14:57, Oleksandr Tyshchenko wrote:

On Wed, Nov 15, 2017 at 4:28 PM, Andre Przywara
<andre.przyw...@linaro.org> wrote:
Anyway, I think we should go step-by-step.
If community agreed that CPUFreq feature in Xen on ARM was needed and
SCPI/SCMI based approach
was the right thing to do in general I would stick to next taking into
the account Andre's suggestions
regarding some guest input:

1. Xen do have CPUFreq logic. It measures CPUs utilization by itself.
2. In addition it can collect OPP change requests from the guests:
   - There are some politics describing which guest is allowed to send
OPP change request.
   - Of course, involved guests have CPUFreq enabled. All we need is
these OPP change requests don't lead to
 any physical changes and be picked up by Xen. Here we could use
Andre's idea here (SCPI CPUFreq + SMC mailbox with hvc method).
3. Xen makes a decision based on the whole system status it measures
periodically and guests input (OPP change requests) if present.
4. Xen actually issues command to change the CPU frequency (sends OPP
change request to SCP).

How does it sound?


0. Decide whether CPUFreq justifies 1.-4. in the first place. That
sounds like a lot of work and code, so we should be sure it's worth it.

I wonder if you could provide some input, ideally measurements on the
actual power savings CPUFreq provides.

Does the wish to have CPUFreq purely come from some "tick-the-box"
exercise? As in: We have it on native Linux, so we need it in Xen?

What power savings can we expect from CPUFreq? Can those possible
savings be transferred into a virtualized environment at all? And do
those saving justify all the extra code in Xen?

I think those questions need to be answered first, then we can discuss
about the implementation details.


I am going to throw a bit more ideas. From the discussion, it look like 
to me the story is around power saving when using Xen. Am I right?


Have you explored some other possibility to save power? I am asking 
that, because the topic is fairly new with Xen.


Once area where power could be saved is the idle loop (see idle_loop in 
arch/arm/domain.c). At the momment only WFI is used. It would be 
possible to go in deeper low-power state by using PSCI.


Similarly, the virtual PSCI implementation for suspend is quite simple. 
You could potentially use those information to decide what to do with 
the pCPU (suspend, turning off...).


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [libvirt test] 116216: regressions - FAIL

[Julien Grall]

Hi all,

On 16 November 2017 at 09:40, osstest service owner
<osstest-ad...@xenproject.org> wrote:
> flight 116216 libvirt real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/116216/
>
> Regressions :-(
>
> Tests which did not succeed and are blocking,
> including tests which could not be run:
>  build-i386-libvirt6 libvirt-buildfail REGR. vs. 
> 115476
>  build-armhf-libvirt   6 libvirt-buildfail REGR. vs. 
> 115476

This is failing with:

xenconfig/xen_xl.c: In function 'xenFormatXLVnuma':
xenconfig/xen_xl.c:1264:5: error: format '%ld' expects argument of
type 'long int', but argument 3 has type 'size_t' [-Werror=format=]
 virBufferAsprintf(, "pnode=%ld", node);
 ^
xenconfig/xen_xl.c:1268:5: error: format '%ld' expects argument of
type 'long int', but argument 3 has type 'size_t' [-Werror=format=]
 virBufferAsprintf(, "size=%ld", nodeSize);
 ^
cc1: all warnings being treated as errors

I think this was introduced with 03d0959af "xenconfig: add domxml
conversions for xen-xl".

Cheers,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH 14/16] SUPPORT.md: Add statement on PCI passthrough

[Julien Grall]

Hi George,

On 13/11/17 15:41, George Dunlap wrote:

Signed-off-by: George Dunlap <george.dun...@citrix.com>
---
CC: Ian Jackson <ian.jack...@citrix.com>
CC: Wei Liu <wei.l...@citrix.com>
CC: Andrew Cooper <andrew.coop...@citrix.com>
CC: Jan Beulich <jbeul...@suse.com>
CC: Stefano Stabellini <sstabell...@kernel.org>
CC: Konrad Wilk <konrad.w...@oracle.com>
CC: Tim Deegan <t...@xen.org>
CC: Rich Persaud <pers...@gmail.com>
CC: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
CC: Christopher Clark <christopher.w.cl...@gmail.com>
CC: James McKenzie <james.mcken...@bromium.com>
---
  SUPPORT.md | 33 -
  1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/SUPPORT.md b/SUPPORT.md
index 3e352198ce..a8388f3dc5 100644
--- a/SUPPORT.md
+++ b/SUPPORT.md
@@ -454,9 +454,23 @@ there is currently no xl support.
  
  ## Security
  
+### Driver Domains

+
+Status: Supported, with caveats
+
+"Driver domains" means allowing non-Domain 0 domains
+with access to physical devices to act as back-ends.
+
+See the appropriate "Device Passthrough" section
+for more information about security support.
+
  ### Device Model Stub Domains
  
-Status: Supported

+Status: Supported, with caveats
+
+Vulnerabilities of a device model stub domain
+to a hostile driver domain (either compromised or untrusted)
+are excluded from security support.
  
  ### KCONFIG Expert
  
@@ -522,6 +536,23 @@ Virtual Performance Management Unit for HVM guests

  Disabled by default (enable with hypervisor command line option).
  This feature is not security supported: see 
http://xenbits.xen.org/xsa/advisory-163.html
  
+### x86/PCI Device Passthrough

+
+Status: Supported, with caveats
+
+Only systems using IOMMUs will be supported.
+
+Not compatible with migration, altp2m, introspection, memory sharing, or 
memory paging.
+
+Because of hardware limitations
+(affecting any operating system or hypervisor),
+it is generally not safe to use this feature
+to expose a physical device to completely untrusted guests.
+However, this feature can still confer significant security benefit
+when used to remove drivers and backends from domain 0
+(i.e., Driver Domains).
+See docs/PCI-IOMMU-bugs.txt for more information.


Where can I find this file? Is it in staging?

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 09/16] SUPPORT.md: Add ARM-specific virtual hardware

[Julien Grall]

Hi George,

On 13/11/17 15:41, George Dunlap wrote:

Signed-off-by: George Dunlap <george.dun...@citrix.com>
---
Do we need to add anything more here?

And do we need to include ARM ACPI for guests?

CC: Ian Jackson <ian.jack...@citrix.com>
CC: Wei Liu <wei.l...@citrix.com>
CC: Andrew Cooper <andrew.coop...@citrix.com>
CC: Jan Beulich <jbeul...@suse.com>
CC: Stefano Stabellini <sstabell...@kernel.org>
CC: Konrad Wilk <konrad.w...@oracle.com>
CC: Tim Deegan <t...@xen.org>
CC: Julien Grall <julien.gr...@arm.com>
---
  SUPPORT.md | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/SUPPORT.md b/SUPPORT.md
index b95ee0ebe7..8235336c41 100644
--- a/SUPPORT.md
+++ b/SUPPORT.md
@@ -412,6 +412,16 @@ Virtual Performance Management Unit for HVM guests
  Disabled by default (enable with hypervisor command line option).
  This feature is not security supported: see 
http://xenbits.xen.org/xsa/advisory-163.html
  
+### ARM/Non-PCI device passthrough

+
+Status: Supported


Sorry I didn't notice that until now. I am not comfortable to say 
"Supported" without any caveats.


As with PCI device passthrough, you at least need an IOMMU present on 
the platform. Sadly, it does not mean all DMA-capable devices on that 
platform will be protected by the IOMMU. This is also assuming, the 
IOMMU do sane things.


There are potentially other problem coming up with MSI support. But I 
haven't yet fully thought about it.



+
+### ARM: 16K and 64K page granularity in guests
+
+Status: Supported, with caveats
+
+No support for QEMU backends in a 16K or 64K domain.
+
  ## Virtual Hardware, QEMU
  
  These are devices available in HVM mode using a qemu devicemodel (the default).




Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 09/16] SUPPORT.md: Add ARM-specific virtual hardware

[Julien Grall]

Hi George,

On 13/11/17 15:41, George Dunlap wrote:

Signed-off-by: George Dunlap <george.dun...@citrix.com>
---
Do we need to add anything more here?

And do we need to include ARM ACPI for guests?


I don't have any opinion here. However, if we decide to include, then we 
should also include Device-Tree.




CC: Ian Jackson <ian.jack...@citrix.com>
CC: Wei Liu <wei.l...@citrix.com>
CC: Andrew Cooper <andrew.coop...@citrix.com>
CC: Jan Beulich <jbeul...@suse.com>
CC: Stefano Stabellini <sstabell...@kernel.org>
CC: Konrad Wilk <konrad.w...@oracle.com>
CC: Tim Deegan <t...@xen.org>
CC: Julien Grall <julien.gr...@arm.com>
---
  SUPPORT.md | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/SUPPORT.md b/SUPPORT.md
index b95ee0ebe7..8235336c41 100644
--- a/SUPPORT.md
+++ b/SUPPORT.md
@@ -412,6 +412,16 @@ Virtual Performance Management Unit for HVM guests
  Disabled by default (enable with hypervisor command line option).
  This feature is not security supported: see 
http://xenbits.xen.org/xsa/advisory-163.html
  
+### ARM/Non-PCI device passthrough

+
+Status: Supported
+
+### ARM: 16K and 64K page granularity in guests
+
+Status: Supported, with caveats
+
+No support for QEMU backends in a 16K or 64K domain.
+
  ## Virtual Hardware, QEMU
  
  These are devices available in HVM mode using a qemu devicemodel (the default).




--
Julien Grall

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

Re: [Xen-devel] [PATCH 06/16] SUPPORT.md: Add scalability features

[Julien Grall]

Hi George,

On 13/11/17 15:41, George Dunlap wrote:

Superpage support and PVHVM.

Signed-off-by: George Dunlap <george.dun...@citrix.com>
---
CC: Ian Jackson <ian.jack...@citrix.com>
CC: Wei Liu <wei.l...@citrix.com>
CC: Andrew Cooper <andrew.coop...@citrix.com>
CC: Jan Beulich <jbeul...@suse.com>
CC: Stefano Stabellini <sstabell...@kernel.org>
CC: Konrad Wilk <konrad.w...@oracle.com>
CC: Tim Deegan <t...@xen.org>
CC: Julien Grall <julien.gr...@arm.com>
---
  SUPPORT.md | 21 +
  1 file changed, 21 insertions(+)

diff --git a/SUPPORT.md b/SUPPORT.md
index c884fac7f5..a8c56d13dd 100644
--- a/SUPPORT.md
+++ b/SUPPORT.md
@@ -195,6 +195,27 @@ on embedded platforms.
  
  Enables NUMA aware scheduling in Xen
  
+## Scalability

+
+### 1GB/2MB super page support
+
+Status, x86 HVM/PVH: : Supported
+Status, ARM: Supported
+
+NB that this refers to the ability of guests
+to have higher-level page table entries point directly to memory,
+improving TLB performance.
+This is independent of the ARM "page granularity" feature (see below).


I am not entirely sure about this paragraph for Arm. I understood this 
section as support for stage-2 page-table (aka EPT on x86) but the 
paragraph lead me to believe to it is for guest.


The size of super pages of guests will depend on the page granularity 
used by itself and the format of the page-table (e.g LPAE vs short 
descriptor). We have no control on that.


What we have control is the size of mapping used for stage-2 page-table.


+
+### x86/PVHVM
+
+Status: Supported
+
+This is a useful label for a set of hypervisor features
+which add paravirtualized functionality to HVM guests
+for improved performance and scalability.
+This includes exposing event channels to HVM guests.
+
  # Format and definitions
  
  This file contains prose, and machine-readable fragments.




Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH for-4.10] tools: xentoolcore_restrict_all: Do deregistration before close

[Julien Grall]

Hi Ian,

On 14/11/17 14:57, Ian Jackson wrote:

Julien Grall writes ("Re: [PATCH] tools: xentoolcore_restrict_all: Do deregistration 
before close"):

I think this is 4.10 material, xentoolcore was introduced in this
release and it would be good to have it right from now. I want to
confirm that you are both happy with that?


Yes, absolutely.  Sorry, I forgot the for-4.10 tag in the Subject.


Release-acked-by: Julien Grall <julien.gr...@linaro.org>

Cheers,


--
Julien Grall

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

Re: [Xen-devel] [PATCH for-4.10 v2] x86/hvm: Fix altp2m_vcpu_enable_notify error handling

[Julien Grall]

Hi,

On 15/11/17 14:16, Andrew Cooper wrote:

On 15/11/17 14:10, Jan Beulich wrote:

On 15.11.17 at 14:47, <a...@bitdefender.com> wrote:

The altp2m_vcpu_enable_notify subop handler might skip calling
rcu_unlock_domain() after rcu_lock_current_domain().  Albeit since both
rcu functions are no-ops when run on the current domain, this doesn't
really have repercussions.

The second change is adding a missing break that would have potentially
enabled #VE for the current domain even if it had intended to enable it
for another one (not a supported functionality).

Thanks, much better.


Signed-off-by: Adrian Pop <a...@bitdefender.com>
Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>

Reviewed-by: Jan Beulich <jbeul...@suse.com>


FOAD, Requesting a release ack for this change.


Release-acked-by: Julien Grall <julien.gr...@linaro.org>

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 03/12] ARM: VGIC: remove gic_clear_pending_irqs()

[Julien Grall]

Hi Stefano,

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

On Fri, 10 Nov 2017, Andre Przywara wrote:

Hi,

On 26/10/17 01:14, Stefano Stabellini wrote:

On Thu, 19 Oct 2017, Andre Przywara wrote:

gic_clear_pending_irqs() was not only misnamed, but also misplaced, as
a function solely dealing with the GIC emulation should not live in gic.c.
Move the functionality of this function into its only caller in vgic.c

Signed-off-by: Andre Przywara <andre.przyw...@arm.com>


The reason why gic_clear_pending_irqs is in gic.c is that lr_mask and
lr_pending are considered part of the gic driver (gic.c). On the other
end, inflight is part of the vgic.

As an example, the idea is that the code outside of gic.c (for example
vgic.c) shouldn't have to know, or have to care, whether a given IRQ is
in the lr_pending queue or actually in a LR register.


I can understand that the lr_pending queue *should* be logical
continuation of the LR registers, something like spill-over LRs.
Though I wasn't aware of this before ;-)
So I can see that from a *logical* point of view it looks like it
belongs to the hardware part of the GIC (more specifically gic-vgic.c),
which deals with the actual LRs. But I guess this is somewhat of a grey
area.

BUT:
This is a design choice of the VGIC, and one which the KVM VGIC design
for instance does *not* share. Also my earlier Xen VGIC rework patches
got rid of this as well (because dealing with two lists is too complicated).
Also, the name is misleading: gic_clear_pending_irqs() does not hint at
all that this is dealing with the GIC emulation, I think it should read
vgic_vcpu_clear_pending_irqs().
And as it accesses VGIC specific data structures only, I don't think it
belongs to gic.c, really.
So I could live with moving it into the new gic-vgic.c, let me see if
that works.

The need for this patch didn't come out of the blue, I actually need it
to be able to reuse gic.c with *any* other VGIC implementation. And this
applies to both a VGIC rework and the KVM VGIC port.
These lr_queue and lr_pending queues are really an implementation detail
of the existing *VGIC*, and, more importantly: they refer to the struct
pending_irq, which is definitely a VGIC detail.

The rabbit to follow in this series is to strictly split the usage of
struct pending_irq from the hardware GIC driver. The KVM VGIC does not
have a "struct pending_irq", so we can't have anything mentioning that
in code that should survive a KVM VGIC port.
So short of replacing gic.c at all, moving everything mentioning
pending_irq out of gic.c is the only option.


Could you at least retain gic_clear_pending_irqs as a separate function?

pending_irq is clearly separate from anything vgic and doesn't belong
there. Nonetheless, I can live with moving gic_clear_pending_irqs to
vgic.c to make future development easier, but at least let's keep
gic_clear_pending_irqs as is.


Why do you think pending_irq does not belong to vgic? pending_irq is 
defined by vgic.h and contain the state of the interrupt for the virtual 
GIC.


However, looking at gic_clear_pending_irqs. It is only called by 
vgic_clear_pending_irqs which is then only called by do_common_cpu_on.


The commit message adding this code (see 803d6c993a "xen/arm: clear 
pending irq queues on do_psci_cpu_on") says: "Also when (re)activating a 
vcpu, clear the vgic and gic irq queues: we don't want to inject any 
irqs that couldn't be handled by the vcpu right before going offline."


Even on real hardware you may have pending interrupt at boot. This is 
taken care by the OS when initializing the GIC CPU interface. So what is 
that function trying to solve?


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC] [Draft Design v2] ACPI/IORT Support in Xen.

[Julien Grall]

On 16/11/17 12:39, Manish Jaggi wrote:



On 11/16/2017 5:23 PM, Julien Grall wrote:

Hi Manish,

On 16/11/17 11:46, Manish Jaggi wrote:



On 11/16/2017 5:07 PM, Julien Grall wrote:



On 16/11/17 07:39, Manish Jaggi wrote:

On 11/14/2017 6:53 PM, Julien Grall wrote:

3. IORT for Dom0
-
IORT for Dom0 is based on host iort. Few nodes could be removed 
or modified.

  For instance
- Host SMMU nodes should not be present as Xen should only touch it.
- platform nodes (named components) may be controlled by xen 
command line.


I am not sure where does this example come from? As I said, there 
are no plan to support Platform Device passthrough with ACPI. A 
better example here would removing PMCG.


It came from review comments on my previous IORT SMMU hiding patch. 
Andre suggested that Platform Nodes are needed.


After some brainstorming with Julien we found two problems:
1) This only covers RC nodes, but not "named components" (platform
devices), which we will need. ...

From: 
https://www.mail-archive.com/xen-devel@lists.xen.org/msg123434.html


I think you misunderstood my comment here... What I call "device 
passthrough" is giving access to a device to a domain other than the 
Hardware Domain


There are no plan for supporting platform device-passthrough on ACPI 
and I don't understand why you would like to control that using the 
command line.


What Andre was saying is your series was not covering the "named 
components" for the Hardware Domain.


The section 3 is IORT for Dom0, where I mentioned that  some platform 
devices can be hidden from dom0.
So your comment on Platform device Passthrough might not be valid 
then as it is for domU's only.


Regarding the visibility of a platform device for dom0, I took cue 
from your comment below


Where did I ever mention the command line solution? Please stop trying 
to put words in my mouth.


There are other reason than passthrough to hide device from the 
Hardware Domain.



Lets put some clarity on the below items specifically for dom0
a. can platform devices can be part of dom0 IORT ?


Yes. Otherwise, how would you be able to use MSI with platform device?

b. If (a) yes, then how to decide on a finer grain the visibility of 
platform devices for Dom0

     Update ACPI tables to remove the device?
c. Is fine grain visibility of platform device for dom0 to be covered in 
my current patchset


No.

Cheers,

--
Julien Grall

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

[Xen-devel] Next Xen Arm Community call - Wednesday 22nd November

[Julien Grall]

Hi all,

Apologies I was meant to organize the call earlier.

I would suggest to have the next community call on Wednesday 22nd 
November 5pm GMT. Does it sound good?


Do you have any specific topic you would like to discuss?

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC] [Draft Design v2] ACPI/IORT Support in Xen.

[Julien Grall]

Hi Manish,

On 16/11/17 11:46, Manish Jaggi wrote:



On 11/16/2017 5:07 PM, Julien Grall wrote:



On 16/11/17 07:39, Manish Jaggi wrote:

On 11/14/2017 6:53 PM, Julien Grall wrote:

3. IORT for Dom0
-
IORT for Dom0 is based on host iort. Few nodes could be removed or 
modified.

  For instance
- Host SMMU nodes should not be present as Xen should only touch it.
- platform nodes (named components) may be controlled by xen 
command line.


I am not sure where does this example come from? As I said, there 
are no plan to support Platform Device passthrough with ACPI. A 
better example here would removing PMCG.


It came from review comments on my previous IORT SMMU hiding patch. 
Andre suggested that Platform Nodes are needed.


After some brainstorming with Julien we found two problems:
1) This only covers RC nodes, but not "named components" (platform
devices), which we will need. ...

From: 
https://www.mail-archive.com/xen-devel@lists.xen.org/msg123434.html


I think you misunderstood my comment here... What I call "device 
passthrough" is giving access to a device to a domain other than the 
Hardware Domain


There are no plan for supporting platform device-passthrough on ACPI 
and I don't understand why you would like to control that using the 
command line.


What Andre was saying is your series was not covering the "named 
components" for the Hardware Domain.


The section 3 is IORT for Dom0, where I mentioned that  some platform 
devices can be hidden from dom0.
So your comment on Platform device Passthrough might not be valid then 
as it is for domU's only.


Regarding the visibility of a platform device for dom0, I took cue from 
your comment below


Where did I ever mention the command line solution? Please stop trying 
to put words in my mouth.


There are other reason than passthrough to hide device from the Hardware 
Domain.




This has two benefits:

...
3) We could decide in a finer grain which devices (e.g platform 
device)Dom0 can see.

From: https://www.mail-archive.com/xen-devel@lists.xen.org/msg124534.html



Cheers,





--
Julien Grall

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

[Xen-devel] Xen 4.10 RC5

[Julien Grall]

Hi all,

Xen 4.10 RC5 is tagged. You can check that out from xen.git:

  git://xenbits.xen.org/xen.git 4.10.0-rc5

For your convenience there is also a tarball at:
https://downloads.xenproject.org/release/xen/4.10.0-rc5/xen-4.10.0-rc5.tar.gz

And the signature is at:
https://downloads.xenproject.org/release/xen/4.10.0-rc5/xen-4.10.0-rc5.tar.gz.sig

Please send bug reports and test reports to
xen-de...@lists.xenproject.org. When sending bug reports, please CC
relevant maintainers and me (julien.gr...@linaro.org).

Thanks,

--
Julien Grall

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

Re: [Xen-devel] [RFC] [Draft Design v2] ACPI/IORT Support in Xen.

[Julien Grall]

On 16/11/17 07:39, Manish Jaggi wrote:

On 11/14/2017 6:53 PM, Julien Grall wrote:

3. IORT for Dom0
-
IORT for Dom0 is based on host iort. Few nodes could be removed or 
modified.

  For instance
- Host SMMU nodes should not be present as Xen should only touch it.
- platform nodes (named components) may be controlled by xen command 
line.


I am not sure where does this example come from? As I said, there are 
no plan to support Platform Device passthrough with ACPI. A better 
example here would removing PMCG.


It came from review comments on my previous IORT SMMU hiding patch. 
Andre suggested that Platform Nodes are needed.


After some brainstorming with Julien we found two problems:
1) This only covers RC nodes, but not "named components" (platform
devices), which we will need. ...

From: https://www.mail-archive.com/xen-devel@lists.xen.org/msg123434.html


I think you misunderstood my comment here... What I call "device 
passthrough" is giving access to a device to a domain other than the 
Hardware Domain


There are no plan for supporting platform device-passthrough on ACPI and 
I don't understand why you would like to control that using the command 
line.


What Andre was saying is your series was not covering the "named 
components" for the Hardware Domain.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH for-4.10 2/2] xen/arm: p2m: Add more debug in get_page_from_gva

[Julien Grall]

On 11/16/2017 01:36 AM, Stefano Stabellini wrote:

On Wed, 15 Nov 2017, Julien Grall wrote:

The function get_page_from_gva is used by copy_*_guest helpers to
translate a guest virtual address to a machine physical address and take
reference on the page.

There are a couple of errors path that will return the same value making

^ paths


difficult to know the exact error. Add more debug in each error patch

^ it difficult



only for debug-build.

This should help narrowing down the intermittent failure with the
hypercall GNTTABOP_copy (see [1]).

[1] https://lists.xen.org/archives/html/xen-devel/2017-11/msg00942.html

Signed-off-by: Julien Grall <julien.gr...@linaro.org>


Acked-by: Stefano Stabellini <sstabell...@kernel.org>

fixed on commit


I am not sure why this was merged given Andrew gave some comments...





---
  xen/arch/arm/p2m.c | 13 +
  1 file changed, 13 insertions(+)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index f6b3d8e421..417609ede2 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1428,16 +1428,29 @@ struct page_info *get_page_from_gva(struct vcpu *v, 
vaddr_t va,
  par = gvirt_to_maddr(va, , flags);
  
  if ( par )

+{
+dprintk(XENLOG_G_DEBUG,
+"%pv: gvirt_to_maddr failed va=%#"PRIvaddr" flags=0x%lx 
par=%#"PRIx64"\n",
+v, va, flags, par);
  goto err;
+}
  
  if ( !mfn_valid(maddr_to_mfn(maddr)) )

+{
+dprintk(XENLOG_G_DEBUG, "%pv: Invalid MFN %#"PRI_mfn"\n",
+v, mfn_x(maddr_to_mfn(maddr)));
  goto err;
+}
  
  page = mfn_to_page(maddr_to_mfn(maddr));

  ASSERT(page);
  
  if ( unlikely(!get_page(page, d)) )

+{
+dprintk(XENLOG_G_DEBUG, "%pv: Failing to acquire the MFN 
%#"PRI_mfn"\n",
+v, mfn_x(maddr_to_mfn(maddr)));
  page = NULL;
+}
  
  err:

  if ( !page && p2m->mem_access_enabled )


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

Re: [Xen-devel] [PATCH for-4.10 2/2] xen/arm: p2m: Add more debug in get_page_from_gva

[Julien Grall]

Hi Andrew,

On 11/15/2017 07:43 PM, Andrew Cooper wrote:

On 15/11/17 19:34, Julien Grall wrote:

The function get_page_from_gva is used by copy_*_guest helpers to
translate a guest virtual address to a machine physical address and take
reference on the page.

There are a couple of errors path that will return the same value making
difficult to know the exact error. Add more debug in each error patch
only for debug-build.

This should help narrowing down the intermittent failure with the
hypercall GNTTABOP_copy (see [1]).

[1] https://lists.xen.org/archives/html/xen-devel/2017-11/msg00942.html

Signed-off-by: Julien Grall <julien.gr...@linaro.org>
---
  xen/arch/arm/p2m.c | 13 +
  1 file changed, 13 insertions(+)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index f6b3d8e421..417609ede2 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1428,16 +1428,29 @@ struct page_info *get_page_from_gva(struct vcpu *v, 
vaddr_t va,
  par = gvirt_to_maddr(va, , flags);
  
  if ( par )

+{
+dprintk(XENLOG_G_DEBUG,
+"%pv: gvirt_to_maddr failed va=%#"PRIvaddr" flags=0x%lx 
par=%#"PRIx64"\n",
+v, va, flags, par);


Given the long round-trip time on debugging output, how about trying to
dump the guest and/or second stage table walk?


I thought about it, however at the moment dump_s1_guest_walk() is very 
minimal and would be add much value here. Thought, Now that we have code 
to do first-stage walk (see guest_walk_tables), we might be able to get 
a better dump here. Thought I am not sure it would be 4.10 material.


However, I think we could try to translate the guest VA to a guest PA 
using hardware instruction and then do the second-stage walk using 
dump_p2m_lookup.


Let me have a look.

Cheers,

--
Julien Grall

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

[Xen-devel] [PATCH for-4.10 0/2] xen/arm: Add more debug in get_page_from_gva

[Julien Grall]

Hi all,

It looks like get_page_from_gva intermittenly fails on the Arndale (see [1])
leading to Dom0 crashing.

At the moment it is a bit hard to know why given the hypervisor does not
provide much information.

This series add more debug in get_page_from_gva to hopefully narrow down
the issue.

I think the 2 patches are good candidate for Xen 4.10 as this may help fixing
a dom0 crash on the Arndale.

Cheers,

[1] https://lists.xen.org/archives/html/xen-devel/2017-11/msg00942.html

Julien Grall (2):
  xen/arm: mm: Change the return value of gvirt_to_maddr
  xen/arm: p2m: Add more debug in get_page_from_gva

 xen/arch/arm/domain_build.c |  8 
 xen/arch/arm/kernel.c   |  8 
 xen/arch/arm/p2m.c  | 19 ---
 xen/include/asm-arm/mm.h|  9 +++--
 4 files changed, 31 insertions(+), 13 deletions(-)

-- 
2.11.0


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

[Xen-devel] [PATCH for-4.10 1/2] xen/arm: mm: Change the return value of gvirt_to_maddr

[Julien Grall]

Currently, gvirt_to_maddr return -EFAULT when the translation failed.
It might be useful to return the PAR_EL1 (Physical Address Register)
in such a case to get a better idea of the reason.

So modify the return value to use 0 on sucess or return the PAR on
failure.

The callers are modified to reflect the change of the return value.

Note that with the change in gvirt_to_maddr, ma needs to be initialized
to avoid GCC been confused (i.e value may be unitialized) with the new
construction.

Signed-off-by: Julien Grall <julien.gr...@linaro.org>
---
 xen/arch/arm/domain_build.c | 8 
 xen/arch/arm/kernel.c   | 8 
 xen/arch/arm/p2m.c  | 6 +++---
 xen/include/asm-arm/mm.h| 9 +++--
 4 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index bf29299707..c74f4dd69d 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2002,15 +2002,15 @@ static void initrd_load(struct kernel_info *kinfo)
 
 for ( offs = 0; offs < len; )
 {
-int rc;
-paddr_t s, l, ma;
+uint64_t par;
+paddr_t s, l, ma = 0;
 void *dst;
 
 s = offs & ~PAGE_MASK;
 l = min(PAGE_SIZE - s, len);
 
-rc = gvirt_to_maddr(load_addr + offs, , GV2M_WRITE);
-if ( rc )
+par = gvirt_to_maddr(load_addr + offs, , GV2M_WRITE);
+if ( par )
 {
 panic("Unable to translate guest address");
 return;
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index c2755a9ab9..a6c6413712 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -167,15 +167,15 @@ static void kernel_zimage_load(struct kernel_info *info)
paddr, load_addr, load_addr + len);
 for ( offs = 0; offs < len; )
 {
-int rc;
-paddr_t s, l, ma;
+uint64_t par;
+paddr_t s, l, ma = 0;
 void *dst;
 
 s = offs & ~PAGE_MASK;
 l = min(PAGE_SIZE - s, len);
 
-rc = gvirt_to_maddr(load_addr + offs, , GV2M_WRITE);
-if ( rc )
+par = gvirt_to_maddr(load_addr + offs, , GV2M_WRITE);
+if ( par )
 {
 panic("Unable to map translate guest address");
 return;
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 68b488997d..f6b3d8e421 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1414,7 +1414,7 @@ struct page_info *get_page_from_gva(struct vcpu *v, 
vaddr_t va,
 struct p2m_domain *p2m = p2m_get_hostp2m(d);
 struct page_info *page = NULL;
 paddr_t maddr = 0;
-int rc;
+uint64_t par;
 
 /*
  * XXX: To support a different vCPU, we would need to load the
@@ -1425,9 +1425,9 @@ struct page_info *get_page_from_gva(struct vcpu *v, 
vaddr_t va,
 
 p2m_read_lock(p2m);
 
-rc = gvirt_to_maddr(va, , flags);
+par = gvirt_to_maddr(va, , flags);
 
-if ( rc )
+if ( par )
 goto err;
 
 if ( !mfn_valid(maddr_to_mfn(maddr)) )
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index cd6dfb54b9..ad2f2a43dc 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -266,11 +266,16 @@ static inline void *maddr_to_virt(paddr_t ma)
 }
 #endif
 
-static inline int gvirt_to_maddr(vaddr_t va, paddr_t *pa, unsigned int flags)
+/*
+ * Translate a guest virtual address to a machine address.
+ * Return the fault information if the translation has failed else 0.
+ */
+static inline uint64_t gvirt_to_maddr(vaddr_t va, paddr_t *pa,
+  unsigned int flags)
 {
 uint64_t par = gva_to_ma_par(va, flags);
 if ( par & PAR_F )
-return -EFAULT;
+return par;
 *pa = (par & PADDR_MASK & PAGE_MASK) | ((unsigned long) va & ~PAGE_MASK);
 return 0;
 }
-- 
2.11.0


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

[Xen-devel] [PATCH for-4.10 2/2] xen/arm: p2m: Add more debug in get_page_from_gva

[Julien Grall]

The function get_page_from_gva is used by copy_*_guest helpers to
translate a guest virtual address to a machine physical address and take
reference on the page.

There are a couple of errors path that will return the same value making
difficult to know the exact error. Add more debug in each error patch
only for debug-build.

This should help narrowing down the intermittent failure with the
hypercall GNTTABOP_copy (see [1]).

[1] https://lists.xen.org/archives/html/xen-devel/2017-11/msg00942.html

Signed-off-by: Julien Grall <julien.gr...@linaro.org>
---
 xen/arch/arm/p2m.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index f6b3d8e421..417609ede2 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1428,16 +1428,29 @@ struct page_info *get_page_from_gva(struct vcpu *v, 
vaddr_t va,
 par = gvirt_to_maddr(va, , flags);
 
 if ( par )
+{
+dprintk(XENLOG_G_DEBUG,
+"%pv: gvirt_to_maddr failed va=%#"PRIvaddr" flags=0x%lx 
par=%#"PRIx64"\n",
+v, va, flags, par);
 goto err;
+}
 
 if ( !mfn_valid(maddr_to_mfn(maddr)) )
+{
+dprintk(XENLOG_G_DEBUG, "%pv: Invalid MFN %#"PRI_mfn"\n",
+v, mfn_x(maddr_to_mfn(maddr)));
 goto err;
+}
 
 page = mfn_to_page(maddr_to_mfn(maddr));
 ASSERT(page);
 
 if ( unlikely(!get_page(page, d)) )
+{
+dprintk(XENLOG_G_DEBUG, "%pv: Failing to acquire the MFN 
%#"PRI_mfn"\n",
+v, mfn_x(maddr_to_mfn(maddr)));
 page = NULL;
+}
 
 err:
 if ( !page && p2m->mem_access_enabled )
-- 
2.11.0


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

Re: [Xen-devel] [xen-unstable test] 116178: regressions - FAIL

[Julien Grall]

000

Nov 15 05:24:04.755233 [ 2156.804759] 1fa0:  c037d88c c1201fa8 c12387b1 
   c1000c5c

Nov 15 05:24:04.763183 [ 2156.813005] 1fc0:    c100068c 
 c10abe40 c1318ed4 c120301c

Nov 15 05:24:04.771237 [ 2156.821250] 1fe0: c10abe3c c12087e0 6020406a 410fc0f4 
 6020807c  

Nov 15 05:24:04.779267 [ 2156.829511] [] (gnttab_batch_copy) from 
[] (xenvif_tx_action+0x80/0x738)

Nov 15 05:24:04.787225 [ 2156.838010] [] (xenvif_tx_action) from 
[] (xenvif_poll+0x28/0x64)

Nov 15 05:24:04.795188 [ 2156.845908] [] (xenvif_poll) from 
[] (net_rx_action+0x1e4/0x2d8)

Nov 15 05:24:04.803191 [ 2156.853717] [] (net_rx_action) from 
[] (__do_softirq+0xfc/0x218)

Nov 15 05:24:04.811217 [ 2156.861528] [] (__do_softirq) from 
[] (irq_exit+0xe4/0x140)

Nov 15 05:24:04.819157 [ 2156.868908] [] (irq_exit) from [] 
(__handle_domain_irq+0x60/0xb4)

Nov 15 05:24:04.827160 [ 2156.876806] [] (__handle_domain_irq) from 
[] (gic_handle_irq+0x48/0x8c)

Nov 15 05:24:04.835169 [ 2156.885223] [] (gic_handle_irq) from 
[] (__irq_svc+0x6c/0x90)

Nov 15 05:24:04.843205 [ 2156.892771] Exception stack(0xc1201f50 to 0xc1201f98)

Nov 15 05:24:04.843240 [ 2156.897894] 1f40: 
0001  0001 c031c520

Nov 15 05:24:04.859140 [ 2156.906141] 1f60: c120 c1203034 c1203098 0001 
  c11151a8 c12030a0

Nov 15 05:24:04.867283 [ 2156.914387] 1f80: 192b8000 c1201fa0 c030928c c0309290 
6013 

Nov 15 05:24:04.867324 [ 2156.921078] [] (__irq_svc) from 
[] (arch_cpu_idle+0x38/0x3c)

Nov 15 05:24:04.875243 [ 2156.928542] [] (arch_cpu_idle) from 
[] (cpu_startup_entry+0x194/0x218)

Nov 15 05:24:04.883200 [ 2156.936874] [] (cpu_startup_entry) from 
[] (start_kernel+0x380/0x38c)

Nov 15 05:24:04.891277 [ 2156.945116] Code: e1c432b4 eae0 e7f001f2 e8bd80f8 
(e7f001f2) 

Nov 15 05:24:04.899168 [ 2156.951298] ---[ end trace 766604e7ecb29bdc ]---

Nov 15 05:24:04.907166 [ 2156.955961] Kernel panic - not syncing: Fatal 
exception in interrupt

Nov 15 05:24:04.915143 [ 2156.962415] CPU1: stopping

Nov 15 05:24:04.915174 [ 2156.965166] CPU: 1 PID: 0 Comm: swapper/1 Tainted: G  
D 4.9.20+ #1

Nov 15 05:24:04.923290 [ 2156.972453] Hardware name: SAMSUNG EXYNOS (Flattened 
Device Tree)

Nov 15 05:24:04.931162 [ 2156.978631] [] (unwind_backtrace) from 
[] (show_stack+0x10/0x14)

Nov 15 05:24:04.939192 [ 2156.986436] [] (show_stack) from 
[] (dump_stack+0x98/0xac)

Nov 15 05:24:04.939219 [ 2156.993729] [] (dump_stack) from 
[] (handle_IPI+0x174/0x194)

Nov 15 05:24:04.947171 [ 2157.001192] [] (handle_IPI) from 
[] (gic_handle_irq+0x88/0x8c)

Nov 15 05:24:04.955151 [ 2157.008829] [] (gic_handle_irq) from 
[] (__irq_svc+0x6c/0x90)

Nov 15 05:24:04.963147 [ 2157.016376] Exception stack(0xd98dbf88 to 0xd98dbfd0)

Nov 15 05:24:04.971150 [ 2157.021597] bf80:   0001  
0001 c031c520 d98da000 c1203034

Nov 15 05:24:04.979206 [ 2157.029850] bfa0: c1203098 0002   
c11151a8 c12030a0 192c6000 d98dbfd8

Nov 15 05:24:04.987142 [ 2157.038093] bfc0: c030928c c0309290 600f0013 

Nov 15 05:24:04.995142 [ 2157.043118] [] (__irq_svc) from 
[] (arch_cpu_idle+0x38/0x3c)

Nov 15 05:24:05.003148 [ 2157.050584] [] (arch_cpu_idle) from 
[] (cpu_startup_entry+0x194/0x218)

Nov 15 05:24:05.011218 [ 2157.058913] [] (cpu_startup_entry) from 
[<60301c4c>] (0x60301c4c)

Nov 15 05:24:05.011257 [ 2157.066095] ---[ end Kernel panic - not syncing: 
Fatal exception in interrupt


-- 
Julien Grall

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

Re: [Xen-devel] [RFC v2 5/7] acpi:arm64: Add support for parsing IORT table

[Julien Grall]

Hi Sameer,

On 11/15/2017 01:27 AM, Goel, Sameer wrote:



On 11/8/2017 7:41 AM, Manish Jaggi wrote:

Hi Sameer

On 9/21/2017 6:07 AM, Sameer Goel wrote:

Add support for parsing IORT table to initialize SMMU devices.
* The code for creating an SMMU device has been modified, so that the SMMU
device can be initialized.
* The NAMED NODE code has been commented out as this will need DOM0 kernel
support.
* ITS code has been included but it has not been tested.

Signed-off-by: Sameer Goel <sg...@codeaurora.org>

Followup of the discussions we had on iort parsing and querying streamID and 
deviceId based on RID.
I have extended your patchset with a patch that provides an alternative
way of parsing iort into maps : {rid-streamid}, {rid-deviceID)
which can directly be looked up for searching streamId for a rid. This
will remove the need to traverse iort table again.

The test patch just describes the proposed flow and how the parsing and
query code might fit in. I have not tested it.
The code only compiles.

https://github.com/mjaggi-cavium/xen-wip/commit/df006d64bdbb5c8344de5a710da8bf64c9e8edd5
(This repo has all 7 of your patches + test code patch merged.

Note: The commit text of the patch describes the basic flow /assumptions / 
usage of functions.
Please see the code along with the v2 design draft.
[RFC] [Draft Design v2] ACPI/IORT Support in Xen.
https://lists.xen.org/archives/html/xen-devel/2017-11/msg00512.html

I seek your advice on this. Please provide your feedback.

I responded back on the other thread. I think we are fixing something that is 
not broken. I will try to post a couple of new RFCs and let's discuss this with 
incremental changes on the mailing list.


That other thread was a separate mailing list. For the benefits of the 
rest of the community it would be nice if you can post the summary here.


However, nobody said the code was broken. IORT will be used in various 
place in Xen and Manish is looking whether we can parse only once and 
for all the IORT. I think it is latest design is promising for that.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH for-4.10] libs/evtchn: Remove active handler on clean-up or failure

[Julien Grall]

Hi Wei,

On 14/11/17 13:53, Wei Liu wrote:

On Tue, Nov 14, 2017 at 12:14:14PM +, Julien Grall wrote:

Hi,

On 14/11/17 11:51, Ian Jackson wrote:

Ross Lagerwall writes ("Re: [PATCH for-4.10] libs/evtchn: Remove active handler on 
clean-up or failure"):

On 11/10/2017 05:10 PM, Julien Grall wrote:

Commit 89d55473ed16543044a31d1e0d4660cf5a3f49df "xentoolcore_restrict_all:
Implement for libxenevtchn" added a call to register allowing to
restrict the event channel.

However, the call to deregister the handler was not performed if open
failed or when closing the event channel. This will result to corrupt
the list of handlers and potentially crash the application later one.


Sorry for not spotting this during review.
The fix is correct as far as it goes, so:

Acked-by: Ian Jackson <ian.jack...@eu.citrix.com>


The call to xentoolcore_deregister_active_handle is done at the same
place as for the grants. But I am not convinced this is thread safe as
there are potential race between close the event channel and restict
handler. Do we care about that?

...

However, I think it should call xentoolcore__deregister_active_handle()
_before_ calling osdep_evtchn_close() to avoid trying to restrict a
closed fd or some other fd that happens to have the same number.


You are right.  But this slightly weakens the guarantee provided by
xentoolcore_restrict_all.


I think all the other libs need to be fixed as well, unless there was a
reason it was done this way.


I will send a further patch.  In the meantime I suggest we apply
Julien's fix.


I am going to leave the decision to you and Wei. It feels a bit odd to
release-ack my patch :).


We can only commit patches that are both acked and release-acked. The
latter gives RM control over when the patch should be applied.
Sometimes it is better to wait until something else happens (like
getting the tree to a stable state).

That's how I used release-ack anyway.


I feel a bit odd to release-ack my patch and usually for Arm patches 
deferred to Stefano the decision whether the patch is suitable for the 
release.




For this particular patch, my interpretation of what you just said
is you've given us release-ack and we can apply this patch anytime. I
will commit it soon.


Thanks! I hope it will fixed some osstest failure.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH] tools: xentoolcore_restrict_all: Do deregistration before close

[Julien Grall]

Hi,

On 14/11/17 14:02, Wei Liu wrote:

On Tue, Nov 14, 2017 at 12:15:42PM +, Ian Jackson wrote:

Closing the fd before unhooking it from the list runs the risk that a
concurrent thread calls xentoolcore_restrict_all will operate on the
old fd value, which might refer to a new fd by then.  So we need to do
it in the other order.

Sadly this weakens the guarantee provided by xentoolcore_restrict_all
slight, but not (I think) in a problematic way.  It would be possible


slightly


to implement the previous guarantee, but it would involve replacing
all of the close() calls in all of the individual osdep parts of all
of the individual libraries with calls to a new function which does
dup2("/dev/null", thing->fd);
pthread_mutex_lock(_lock);
thing->fd = -1;
pthread_mutex_unlock(_lock);
close(fd);
which would be terribly tedious.

Signed-off-by: Ian Jackson <ian.jack...@eu.citrix.com>


Acked-by: Wei Liu <wei.l...@citrix.com>


I think this is 4.10 material, xentoolcore was introduced in this 
release and it would be good to have it right from now. I want to 
confirm that you are both happy with that?


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC] [Draft Design v2] ACPI/IORT Support in Xen.

[Julien Grall]

ased on the patchset in [5]. I have added a reference code on
top of it which does IORT parsing as described in this section. The code
is available at [6].

The commit also describes the code flow and assumptions.


Which assumptions? I can't see any in the commit message.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v3 for-4.10 1/2] x86/mm: fix potential race conditions in map_pages_to_xen().

[Julien Grall]

Hi,

On 14/11/17 08:20, Jan Beulich wrote:

On 14.11.17 at 07:53, <yu.c.zh...@linux.intel.com> wrote:

From: Min He <min...@intel.com>

In map_pages_to_xen(), a L2 page table entry may be reset to point to
a superpage, and its corresponding L1 page table need be freed in such
scenario, when these L1 page table entries are mapping to consecutive
page frames and having the same mapping flags.

However, variable `pl1e` is not protected by the lock before L1 page table
is enumerated. A race condition may happen if this code path is invoked
simultaneously on different CPUs.

For example, `pl1e` value on CPU0 may hold an obsolete value, pointing
to a page which has just been freed on CPU1. Besides, before this page
is reused, it will still be holding the old PTEs, referencing consecutive
page frames. Consequently the `free_xen_pagetable(l2e_to_l1e(ol2e))` will
be triggered on CPU0, resulting the unexpected free of a normal page.

This patch fixes the above problem by protecting the `pl1e` with the lock.

Also, there're other potential race conditions. For instance, the L2/L3
entry may be modified concurrently on different CPUs, by routines such as
map_pages_to_xen(), modify_xen_mappings() etc. To fix this, this patch will
check the _PAGE_PRESENT and _PAGE_PSE flags, after the spinlock is obtained,
for the corresponding L2/L3 entry.

Signed-off-by: Min He <min...@intel.com>
Signed-off-by: Yi Zhang <yi.z.zh...@intel.com>
Signed-off-by: Yu Zhang <yu.c.zh...@linux.intel.com>


Reviewed-by: Jan Beulich <jbeul...@suse.com>


Please try to have a cover letter in the future when you have multiple 
patches. This will make easier to give comments/release-ack for the all 
the patches. Anyway for the 2 patches:


Release-acked-by: Julien Grall <julien.gr...@linaro.org>

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH for-4.10] libs/evtchn: Remove active handler on clean-up or failure

[Julien Grall]

Hi,

On 14/11/17 11:51, Ian Jackson wrote:

Ross Lagerwall writes ("Re: [PATCH for-4.10] libs/evtchn: Remove active handler on 
clean-up or failure"):

On 11/10/2017 05:10 PM, Julien Grall wrote:

Commit 89d55473ed16543044a31d1e0d4660cf5a3f49df "xentoolcore_restrict_all:
Implement for libxenevtchn" added a call to register allowing to
restrict the event channel.

However, the call to deregister the handler was not performed if open
failed or when closing the event channel. This will result to corrupt
the list of handlers and potentially crash the application later one.


Sorry for not spotting this during review.
The fix is correct as far as it goes, so:

Acked-by: Ian Jackson <ian.jack...@eu.citrix.com>


The call to xentoolcore_deregister_active_handle is done at the same
place as for the grants. But I am not convinced this is thread safe as
there are potential race between close the event channel and restict
handler. Do we care about that?

...

However, I think it should call xentoolcore__deregister_active_handle()
_before_ calling osdep_evtchn_close() to avoid trying to restrict a
closed fd or some other fd that happens to have the same number.


You are right.  But this slightly weakens the guarantee provided by
xentoolcore_restrict_all.


I think all the other libs need to be fixed as well, unless there was a
reason it was done this way.


I will send a further patch.  In the meantime I suggest we apply
Julien's fix.


I am going to leave the decision to you and Wei. It feels a bit odd to 
release-ack my patch :).


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 1/2 v2] xen: Add support for initializing 16550 UART using ACPI

[Julien Grall]

Hi Bhupinder,

On 11/09/2017 10:19 AM, Bhupinder Thakur wrote:

Currently, Xen supports only DT based initialization of 16550 UART.
This patch adds support for initializing 16550 UART using ACPI SPCR table.

This patch also makes the uart initialization code common between DT and
ACPI based initialization.


Can you please have one patch to refactor the code and one to add ACPI 
support? This will be easier to review.




Signed-off-by: Bhupinder Thakur <bhupinder.tha...@linaro.org>
---
TBD:
There was one review comment from Julien about how the uart->io_size is being
calculated. Currently, I am calulating the io_size based on address of the last
UART register.

pci_uart_config also calcualates the uart->io_size like this:

uart->io_size = max(8U << param->reg_shift,
  param->uart_offset);

I am not sure whether we can use similar logic for calculating uart->io_size.

Changes since v1:
- Reused common code between DT and ACPI based initializations

CC: Andrew Cooper <andrew.coop...@citrix.com>
CC: George Dunlap <george.dun...@eu.citrix.com>
CC: Ian Jackson <ian.jack...@eu.citrix.com>
CC: Jan Beulich <jbeul...@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
CC: Stefano Stabellini <sstabell...@kernel.org>
CC: Tim Deegan <t...@xen.org>
CC: Wei Liu <wei.l...@citrix.com>
CC: Julien Grall <julien.gr...@arm.com>

  xen/drivers/char/ns16550.c  | 132 
  xen/include/xen/8250-uart.h |   1 +
  2 files changed, 121 insertions(+), 12 deletions(-)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index e0f8199..cf42fce 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -1463,18 +1463,13 @@ void __init ns16550_init(int index, struct 
ns16550_defaults *defaults)
  }
  
  #ifdef CONFIG_HAS_DEVICE_TREE

-static int __init ns16550_uart_dt_init(struct dt_device_node *dev,
-   const void *data)
+static int ns16550_init_dt(struct ns16550 *uart,
+   const struct dt_device_node *dev)
  {
-struct ns16550 *uart;
-int res;
+int res = 0;
  u32 reg_shift, reg_width;
  u64 io_size;
  
-uart = _com[0];

-
-ns16550_init_common(uart);
-
  uart->baud  = BAUD_AUTO;
  uart->data_bits = 8;
  uart->parity= UART_PARITY_NONE;
@@ -1510,18 +1505,103 @@ static int __init ns16550_uart_dt_init(struct 
dt_device_node *dev,
  
  uart->dw_usr_bsy = dt_device_is_compatible(dev, "snps,dw-apb-uart");
  
+return res;

+}
+#else
+static int ns16550_init_dt(struct ns16550 *uart,
+   const struct dt_device_node *dev)
+{
+return -EINVAL;
+}
+#endif
+
+#ifdef CONFIG_ACPI
+#include 
+static int ns16550_init_acpi(struct ns16550 *uart,
+ const void *data)
+{
+struct acpi_table_spcr *spcr = NULL;
+int status = 0;
+
+status = acpi_get_table(ACPI_SIG_SPCR, 0,
+(struct acpi_table_header **));
+
+if ( ACPI_FAILURE(status) )
+{
+printk("ns16550: Failed to get SPCR table\n");
+return -EINVAL;
+}
+
+uart->baud  = BAUD_AUTO;
+uart->data_bits = 8;
+uart->parity= spcr->parity;
+uart->stop_bits = spcr->stop_bits;
+uart->io_base = spcr->serial_port.address;
+uart->irq = spcr->interrupt;
+uart->reg_width = spcr->serial_port.bit_width / 8;
+uart->reg_shift = 0;
+uart->io_size = UART_MAX_REG << uart->reg_shift;
+
+irq_set_type(spcr->interrupt, spcr->interrupt_type);
+
+return 0;
+}
+#else
+static int ns16550_init_acpi(struct ns16550 *uart,
+ const void *data)
+{
+return -EINVAL;
+}
+#endif
+
+static int ns16550_uart_init(struct ns16550 **puart,
+ const void *data, bool acpi)
+{
+struct ns16550 *uart = _com[0];
+
+*puart = uart;
+
+ns16550_init_common(uart);
+
+return ( acpi ) ? ns16550_init_acpi(uart, data)
+: ns16550_init_dt(uart, data);
+}


This function does not look very useful but getting _com[0].
I do agree that we need it is nice to have common code, but I think you 
went too far here.


There are no need for 3 separate functions and 2 functions for each 
firmware.


I think duplicating the code of ns16550_uart_init for ACPI and DT is 
fine. You could then create a function that is a merge vuart_init and 
register_init.


This would also limit the number of #ifdef within this code.


+
+static void ns16550_vuart_init(struct ns16550 *uart)
+{
+#ifdef CONFIG_ARM
  uart->vuart.base_addr = uart->io_base;
  uart->vuart.size = uart->io_size;
-uart->vuart.data_off = UART_THR <reg_shift;
-uart->vuart.status_off = UART_LSR<reg_shift;
-uart->

Re: [Xen-devel] [PATCH v2] x86/hvm: do not register hpet mmio during s3 cycle

[Julien Grall]

Hi Jan,

On 11/09/2017 02:45 PM, Jan Beulich wrote:

On 09.11.17 at 15:42, <julien.gr...@linaro.org> wrote:

Hi,

On 09/11/17 08:55, Jan Beulich wrote:

On 08.11.17 at 20:46, <chanud...@ainfosec.com> wrote:

Do it once at domain creation (hpet_init).

Sleep -> Resume cycles will end up crashing an HVM guest with hpet as
the sequence during resume takes the path:
-> hvm_s3_suspend
-> hpet_reset
  -> hpet_deinit
  -> hpet_init
-> register_mmio_handler
  -> hvm_next_io_handler

register_mmio_handler will use a new io handler each time, until
eventually it reaches NR_IO_HANDLERS, then hvm_next_io_handler calls
domain_crash.

Signed-off-by: Eric Chanudet <chanud...@ainfosec.com>

---
v2:
* make hpet_reinit static inline (one call site in this file)


Perhaps my prior reply was ambiguous: By "inlining" I meant
literally inlining it (i.e. dropping the standalone function
altogether). Static functions outside of header files should not
normally be marked "inline" explicitly - it should be the compiler
to make that decision.

As doing the adjustment it relatively simple, I wouldn't mind
doing so while committing, saving another round trip. With
that adjustment (or at the very least with the "inline" dropped)
Reviewed-by: Jan Beulich <jbeul...@suse.com>


What would be the risk to get this patch in Xen 4.10?


Close to none, I would say. Of course, if there really was
something wrong with the code restructuring to fix the bug,
basically all HVM guests would be hosed HPET-wise.


On that basis:

Release-acked-by: Julien Grall <julien.gr...@linaro.org>

Cheers,

--
Julien Grall

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

Re: [Xen-devel] Ping: [PATCH] x86emul: keep compiler from using {x, y, z}mm registers itself

[Julien Grall]

Hi,

On 11/06/2017 03:04 PM, George Dunlap wrote:

On 11/06/2017 11:59 AM, Jan Beulich wrote:

On 16.10.17 at 14:42,  wrote:

On 16.10.17 at 14:37, <andrew.coop...@citrix.com> wrote:

On 16/10/17 13:32, Jan Beulich wrote:

Since the emulator acts on the live hardware registers, we need to
prevent the compiler from using them e.g. for inlined memcpy() /
memset() (as gcc7 does). We can't, however, set this from the command
line, as otherwise the 64-bit build would face issues with functions
returning floating point values and being declared in standard headers.

As the pragma isn't available prior to gcc6, we need to invoke it
conditionally. Luckily up to gcc6 we haven't seen generated code access
SIMD registers beyond what our asm()s do.

Reported-by: George Dunlap <george.dun...@citrix.com>
Signed-off-by: Jan Beulich <jbeul...@suse.com>
---
While this doesn't affect core functionality, I think it would still be
nice for it to be allowed in for 4.10.


Agreed.

Has this been tested with Clang?


Sorry, no - still haven't got around to set up a suitable Clang
locally.


  It stands a good chance of being
compatible, but we may need an && !defined(__clang__) included.


Should non-gcc silently ignore "#pragma GCC ..." it doesn't
recognize, or not define __GNUC__ in the first place if it isn't
sufficiently compatible? I.e. if anything I'd expect we need
"#elif defined(__clang__)" to achieve the same for Clang by
some different pragma (if such exists).


Not having received any reply so far, I'm wondering whether
being able to build the test harness with clang is more
important than for it to work correctly when built with gcc. I
can't predict when I would get around to set up a suitable
clang on my dev systems.


I agree with the argument you make above.  On the unlikely chance
there's a problem Travis should catch it, and someone who actually has a
clang setup can help sort it out.


I am not entirely sure whether this count for a ack or not?

I was waiting an Acked-by/Reviewed-by before consider the Release-Acked-by.

--
Julien Grall

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

Re: [Xen-devel] [PATCH for-4.10] common/spinlock: Improve the output from check_lock() if it trips

[Julien Grall]

Hi Jan,

On 11/06/2017 11:09 AM, Jan Beulich wrote:

On 31.10.17 at 11:49, <andrew.coop...@citrix.com> wrote:

--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -44,7 +44,13 @@ static void check_lock(struct lock_debug *debug)
  if ( unlikely(debug->irq_safe != irq_safe) )
  {
  int seen = cmpxchg(>irq_safe, -1, irq_safe);
-BUG_ON(seen == !irq_safe);
+
+if ( seen == !irq_safe )
+{
+printk("CHECKLOCK FAILURE: prev irqsafe: %d, curr irqsafe %d\n",
+   seen, irq_safe);
+BUG();


This really should use XENLOG_ERR imo, so that the message won't
be lost if warnings are rate limited.


The patch was already merged. I guess a follow-up could be done for Xen 
4.10.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH for-4.10] tools/xenstored: Check number of strings passed to do_control()

[Julien Grall]

Hi,

Apologies for the late answer, I missed the e-mail in my inbox.

On 10/27/2017 05:37 PM, Ian Jackson wrote:

Pawel Wieczorkiewicz writes ("[PATCH] tools/xenstored: Check number of strings 
passed to do_control()"):

It is possible to send a zero-string message body to xenstore's
XS_CONTROL handling function. Then the number of strings is used
for an array allocation. This leads to a crash in strcmp() in a
CONTROL sub-command invocation loop.
The output of xs_count_string() should be verified and all 0 or
negative values should be rejected with an EINVAL. At least the
sub-command name must be specified.

The xenstore crash can only be triggered from within dom0 (there
is a check in do_control() rejecting all non-dom0 requests with
an EACCES).


Acked-by: Ian Jackson <ian.jack...@eu.citrix.com>

(Added the for-4.10 tag to the Subject.)


Release-acked-by: Julien Grall <julien.gr...@linaro.org>

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 1/4 v3 for-4.10] libxl: Fix the bug introduced in commit "libxl: use correct type modifier for vuart_gfn"

[Julien Grall]

Hi Wei,

Sorry I missed that e-mail.

On 10/31/2017 10:07 AM, Wei Liu wrote:

Change the tag to for-4.10.

Julien, this is needed to fix vuart emulation.


To confirm, only patch #1 is candidate for Xen 4.10, right? The rest 
will be queued for Xen 4.11?


For patch #1:

Release-acked-by: Julien Grall <julien.gr...@linaro.org>

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [qemu-upstream-unstable test] 116118: FAIL

[Julien Grall]

Hi,

On 11/13/2017 11:53 AM, Wei Liu wrote:

On Mon, Nov 13, 2017 at 11:52:12AM +, Julien Grall wrote:

Hi,

On 11/13/2017 06:44 AM, osstest service owner wrote:

flight 116118 qemu-upstream-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/116118/

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
   build-armhf-pvopsbroken  in 
115713
   build-armhf-pvops  4 host-install(4) broken in 115713 REGR. vs. 
114457


Looking at the test result, build-armhf-pvops seems to have passed nicely
the past few weeks but one time.

So I am not sure why we are blocking here. Mostly the  one.


host-install failure is probably not related to change in code. It is
trying to install a host to do test. In this case, to build kernel.


They have been failing with that for quite a while now. It looks like a 
force push would be justified here. Any opinions?


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [qemu-upstream-unstable test] 116118: FAIL

[Julien Grall]

  test-amd64-i386-libvirt-pair pass
  test-amd64-amd64-amd64-pvgrubpass
  test-amd64-amd64-i386-pvgrub pass
  test-amd64-amd64-pygrub  pass
  test-amd64-i386-libvirt-qcow2fail
  test-amd64-amd64-xl-qcow2pass
  test-armhf-armhf-libvirt-raw pass
  test-amd64-i386-xl-raw   pass
  test-amd64-amd64-xl-rtds pass
  test-armhf-armhf-xl-rtds fail
  test-amd64-amd64-libvirt-vhd pass
  test-armhf-armhf-xl-vhd  fail



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

broken-job build-armhf-pvops broken

Not pushing.


commit b79708a8ed1b3d18bee67baeaf33b3fa529493e2
Author: Anthony PERARD <anthony.per...@citrix.com>
Date:   Tue Oct 10 11:24:18 2017 +0100

 ui/gtk: Fix deprecation of vte_terminal_copy_clipboard
 
 vte_terminal_copy_clipboard() is deprecated in VTE 0.50.
 
 Signed-off-by: Anthony PERARD <anthony.per...@citrix.com>

 Reviewed-by: Daniel P. Berrange <berra...@redhat.com>
 Signed-off-by: Michael Tokarev <m...@tls.msk.ru>
 (cherry picked from commit 70857ad6212276dcda364e36b30258222bdb31bc)

commit d957c46ff18d0d71809fd04fdc78700feacdc902
Author: Roger Pau Monne <roger@citrix.com>
Date:   Thu Aug 24 16:07:03 2017 +0100

 xen/pt: allow QEMU to request MSI unmasking at bind time
 
 When a MSI interrupt is bound to a guest using

 xc_domain_update_msi_irq (XEN_DOMCTL_bind_pt_irq) the interrupt is
 left masked by default.
 
 This causes problems with guests that first configure interrupts and

 clean the per-entry MSIX table mask bit and afterwards enable MSIX
 globally. In such scenario the Xen internal msixtbl handlers would not
 detect the unmasking of MSIX entries because vectors are not yet
 registered since MSIX is not enabled, and vectors would be left
 masked.
 
 Introduce a new flag in the gflags field to signal Xen whether a MSI

 interrupt should be unmasked after being bound.
 
 This also requires to track the mask register for MSI interrupts, so

 QEMU can also notify to Xen whether the MSI interrupt should be bound
 masked or unmasked
 
 Signed-off-by: Roger Pau Monné <roger@citrix.com>

 Reviewed-by: Jan Beulich <jbeul...@suse.com>
 Reported-by: Andreas Kinzler <h...@posteo.de>
 Reviewed-by: Stefano Stabellini <sstabell...@kernel.org>
 Signed-off-by: Stefano Stabellini <sstabell...@kernel.org>
 (cherry picked from commit a8036336609d2e184fc3543a4c439c0ba7d7f3a2)
 Release-acked-by: Julien Grall <julien.gr...@linaro.org>



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



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

[Xen-devel] Xen 4.10 RC4

[Julien Grall]

Hi all,

Xen 4.10 RC4 is tagged. You can check that out from xen.git:

  git://xenbits.xen.org/xen.git 4.10.0-rc4

For your convenience there is also a tarball at:
https://downloads.xenproject.org/release/xen/4.10.0-rc4/xen-4.10.0-rc4.tar.gz

And the signature is at:
https://downloads.xenproject.org/release/xen/4.10.0-rc4/xen-4.10.0-rc4.tar.gz.sig

Please send bug reports and test reports to
xen-de...@lists.xenproject.org. When sending bug reports, please CC
relevant maintainers and me (julien.gr...@linaro.org).

Thanks,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 1/2] x86/mm: fix a potential race condition in map_pages_to_xen().

[Julien Grall]

Hi,

On 11/13/2017 11:06 AM, Jan Beulich wrote:

On 13.11.17 at 11:34,  wrote:

Our debug showed the concerned page->count_info was already(and
unexpectedly)
cleared in free_xenheap_pages(), and the call trace should be like this:

free_xenheap_pages()
  ^
  |
free_xen_pagetable()
  ^
  |
map_pages_to_xen()
  ^
  |
update_xen_mappings()
  ^
  |
get_page_from_l1e()
  ^
  |
mod_l1_entry()
  ^
  |
do_mmu_update()


This ...


Is above description convincing enough? :-)


... is indeed enough for me to suggest to Julien that we take both
patches (once they're ready). But it's his decision in the end.


I will wait the series to be ready before giving my release-ack.

Cheers,



Jan


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



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

[Xen-devel] [PATCH for-4.10] libs/evtchn: Remove active handler on clean-up or failure

[Julien Grall]

Commit 89d55473ed16543044a31d1e0d4660cf5a3f49df "xentoolcore_restrict_all:
Implement for libxenevtchn" added a call to register allowing to
restrict the event channel.

However, the call to deregister the handler was not performed if open
failed or when closing the event channel. This will result to corrupt
the list of handlers and potentially crash the application later one.

Fix it by calling xentoolcore_deregister_active_handle on failure and
closure.

Signed-off-by: Julien Grall <julien.gr...@linaro.org>

---

This patch is fixing a bug introduced after the code freeze by
"xentoolcore_restrict_all: Implement for libxenevtchn".

The call to xentoolcore_deregister_active_handle is done at the same
place as for the grants. But I am not convinced this is thread safe as
there are potential race between close the event channel and restict
handler. Do we care about that?
---
 tools/libs/evtchn/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/libs/evtchn/core.c b/tools/libs/evtchn/core.c
index 14b7549a6b..2dba58bf00 100644
--- a/tools/libs/evtchn/core.c
+++ b/tools/libs/evtchn/core.c
@@ -56,6 +56,7 @@ xenevtchn_handle *xenevtchn_open(xentoollog_logger *logger, 
unsigned open_flags)
 
 err:
 osdep_evtchn_close(xce);
+xentoolcore__deregister_active_handle(>tc_ah);
 xtl_logger_destroy(xce->logger_tofree);
 free(xce);
 return NULL;
@@ -69,6 +70,7 @@ int xenevtchn_close(xenevtchn_handle *xce)
 return 0;
 
 rc = osdep_evtchn_close(xce);
+xentoolcore__deregister_active_handle(>tc_ah);
 xtl_logger_destroy(xce->logger_tofree);
 free(xce);
 return rc;
-- 
2.11.0


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

Re: [Xen-devel] [PATCH v3 for-next 0/4] xen: Convert __page_to_mfn and _mfn_to_page to use typesafe MFN

[Julien Grall]

Hi Jan,

On 09/11/17 15:47, Jan Beulich wrote:

On 09.11.17 at 16:39, <julien.gr...@linaro.org> wrote:

On 09/11/17 15:36, Jan Beulich wrote:

On 09.11.17 at 16:20, <julien.gr...@linaro.org> wrote:

I had a look at the files that needs to convert. It seems there are few
files with page_to_mfn/mfn_to_page re-defined but no callers:
- arch/x86/mm/hap/nested_hap.c
- arch/x86/mm/p2m-pt.c
- arch/x86/pv/traps.c
- arch/x86/pv/mm.c
- arch/x86/pv/iret.c

Those can be fixed now. That leave the following files:
- arch/x86/mm/p2m-ept.c
In that file, the override prevents all the caller to use the
construction mfn_to_page(_mfn(...)) as it mostly deals with hardware.


I'm not clear what you're trying to tell me here. The file has a total
for four mfn_to_page() uses - if overrides don't help (and perhaps
regardless of if they do), I think it wouldn't be very difficult to simply
change the four places. And note that plain staging has no override
there right now.

Because the plain staging still has page_to_mfn() not using typesafe...
You would need to override it even I follow your suggestion.

What I meant is you would replace the 4 occurrences by
mfn_to_page(_mfn(...)). If you are happy with that, then fine.


Oh, sure, that's a fine intermediate state, which we have all over
the place right now. It's clear that it'll take a while to fully carry
through typesafeness to everywhere.


Would you be fine with other part of Xen too? If so, I can have a go at 
removing completely __page_to_mfn/__mfn_to_page.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v3 for-next 0/4] xen: Convert __page_to_mfn and _mfn_to_page to use typesafe MFN

[Julien Grall]

Hi,

On 09/11/17 15:36, Jan Beulich wrote:

On 09.11.17 at 16:20, <julien.gr...@linaro.org> wrote:

I had a look at the files that needs to convert. It seems there are few
files with page_to_mfn/mfn_to_page re-defined but no callers:
- arch/x86/mm/hap/nested_hap.c
- arch/x86/mm/p2m-pt.c
- arch/x86/pv/traps.c
- arch/x86/pv/mm.c
- arch/x86/pv/iret.c

Those can be fixed now. That leave the following files:
- arch/x86/mm/p2m-ept.c
In that file, the override prevents all the caller to use the
construction mfn_to_page(_mfn(...)) as it mostly deals with hardware.


I'm not clear what you're trying to tell me here. The file has a total
for four mfn_to_page() uses - if overrides don't help (and perhaps
regardless of if they do), I think it wouldn't be very difficult to simply
change the four places. And note that plain staging has no override
there right now.
Because the plain staging still has page_to_mfn() not using typesafe... 
You would need to override it even I follow your suggestion.


What I meant is you would replace the 4 occurrences by 
mfn_to_page(_mfn(...)). If you are happy with that, then fine.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v3 for-next 0/4] xen: Convert __page_to_mfn and _mfn_to_page to use typesafe MFN

[Julien Grall]

Hi Jan,

On 06/11/17 12:44, Jan Beulich wrote:

On 06.11.17 at 13:16, <julien.gr...@linaro.org> wrote:




On 06/11/17 12:11, Jan Beulich wrote:

On 06.11.17 at 12:47, <julien.gr...@linaro.org> wrote:

Hi Jan,

On 06/11/17 11:37, Jan Beulich wrote:

On 01.11.17 at 15:03, <julien.gr...@linaro.org> wrote:

Most of the users of page_to_mfn and mfn_to_page are either overriding
the macros to make them work with mfn_t or use mfn_x/_mfn becaue the rest
of the function use mfn_t.

So I think it is time to make __page_to_mfn and __mfn_to_page using typesafe
MFN.


I have to admit that I still find the overall goal confusing: Afaict
the double-underscore-prefixed versions exist only to allow
easily overriding the non-prefixed ones. Hence the first and
foremost goal ought to be to convert everyone to using the
non-prefixed versions. Files wanting to avoid the typed forms
could then continue to use / be switched to the prefixed ones.

What you're doing here is producing a mess: The prefixed
versions should never have been touched in the first place.
And iirc this was discussed before, with the suggestion to use
overrides (for the non-prefixed versions) to limit overall patch
size.


At the end of the discussion in the previous version, you were happy
with the modification done here (see [1]).


  From the angle looked at it back then I indeed was, but I'm looking
at this from a slightly different angle now with the reply above.


Overall, I think this is an improvement compare to what we have today.
Because we enforce the use of MFN typesafe by default. The developer
would have to override the helpers if he wants to to use the
non-typesafe version.

With your suggestion here, you would just keep the override around even
when they are not necessary. They will have to be dropped at some point,
so why not now?


Why would we keep the overrides in place once no longer needed?
All I'm asking for is that the double-underscore prefixed macros be
left alone, and the non-prefixed versions be converted to be
type-safe by default (with the option to override). That'll allow the
prefixed variants to go way altogether once all code was switched
to typesafe, no longer requiring any overrides.


If you left the double-underscore alone, then you can't convert headers
using them to typesafe. This is because they can't use the non-prefixed
version as they may be override.

So what you are suggesting here is will avoid converting those headers
until someone step up and finish to convert all the source to MFN
typesafe. Personally, I find quite silly to have to delay that...


Hmm, I see your point, but if we went the route you suggest,
what would be the steps to reach the ultimate result I've
described (the prefixed variants gone)? I seems to me that
this would require touching a lot of code a second time that is
being touched now, or is going to be touched as further
conversion happens.


We would indeed need to modify the headers to use 
page_to_mfn/mfn_to_page instead of __page_to_mfn/__mfn_to_page. Those 
headers are:

- asm-arm/mm.h
- xen/domain_page.h
- asm-x86/mm.h
- asm-x86/page.h
- asm-x86/p2m.h

I had a look at the files that needs to convert. It seems there are few 
files with page_to_mfn/mfn_to_page re-defined but no callers:

- arch/x86/mm/hap/nested_hap.c
- arch/x86/mm/p2m-pt.c
- arch/x86/pv/traps.c
- arch/x86/pv/mm.c
- arch/x86/pv/iret.c

Those can be fixed now. That leave the following files:
- arch/x86/mm/p2m-ept.c
		In that file, the override prevents all the caller to use the 
construction mfn_to_page(_mfn(...)) as it mostly deals with hardware.

- arch/x86/x86_64/mm.c
This is could be done without too much trouble.
- arch/x86/pv/dom0_build.c
It would need a bit of rework to get the code MFN typesafe in a 
neat way.
- drivers/passthrough/amd/iommu_map.c
It would need a bit of rework to get the code MFN typesafe in a 
neat way.
- common/trace.c
		I think it should be ok. Though I am bit surprised to see uint32_t 
been used for MFN...

- common/memory.c
It would need a bit of rework to get the code MFN typesafe in a 
neat way.
- common/page_alloc.c
This is could be done without too much trouble.
- common/grant_table.c
It would need a bit of rework to get the code MFN typesafe in a 
neat way.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH for-4.10] libevtchn: fix build on non-Linux hosts

[Julien Grall]

Hi,

On 08/11/17 12:56, Wei Liu wrote:

On Wed, Nov 08, 2017 at 12:52:57PM +, Roger Pau Monne wrote:

Non-Linux hosts (where osdep_evtchn_restrict is not yet supported)
made use of errno without including errno.h, fix this by including the
header.

Signed-off-by: Roger Pau Monné <roger@citrix.com>


Acked-by: Wei Liu <wei.l...@citrix.com>


Release-acked-by: Julien Grall <julien.gr...@linaro.org>

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2] x86/hvm: do not register hpet mmio during s3 cycle

[Julien Grall]

Hi,

On 09/11/17 08:55, Jan Beulich wrote:

On 08.11.17 at 20:46, <chanud...@ainfosec.com> wrote:

Do it once at domain creation (hpet_init).

Sleep -> Resume cycles will end up crashing an HVM guest with hpet as
the sequence during resume takes the path:
-> hvm_s3_suspend
   -> hpet_reset
 -> hpet_deinit
 -> hpet_init
   -> register_mmio_handler
 -> hvm_next_io_handler

register_mmio_handler will use a new io handler each time, until
eventually it reaches NR_IO_HANDLERS, then hvm_next_io_handler calls
domain_crash.

Signed-off-by: Eric Chanudet <chanud...@ainfosec.com>

---
v2:
   * make hpet_reinit static inline (one call site in this file)


Perhaps my prior reply was ambiguous: By "inlining" I meant
literally inlining it (i.e. dropping the standalone function
altogether). Static functions outside of header files should not
normally be marked "inline" explicitly - it should be the compiler
to make that decision.

As doing the adjustment it relatively simple, I wouldn't mind
doing so while committing, saving another round trip. With
that adjustment (or at the very least with the "inline" dropped)
Reviewed-by: Jan Beulich <jbeul...@suse.com>


What would be the risk to get this patch in Xen 4.10?

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [BUG] win2008 guest cannot get ip through sriov

[Julien Grall]

On 09/11/17 14:36, Julien Grall wrote:

Hi Roger,

On 09/11/17 09:27, Roger Pau Monné wrote:

On Thu, Nov 09, 2017 at 12:22:49AM +, Hao, Xudong wrote:
Qemu-xen didn't have commit a80363, so I report this issue to ask 
for sync up
with qemu upstream. Last mail I mean I usually used Qemu Xen tree to 
do test,

and found out this issue.

Before requesting the backport, have you tested whether it fixes 
your issues?




Yes, Luwei (Cced) tested it with pass result.


In which case requesting a backport would be in place on the grounds
that's a bug fix.

The patch in question fixes a bug mostly seen when doing
PCI-passthrough of devices that support MSI-X interrupts to Windows
guest (and Windows attempts to setup the interrupts using MSI-X). It
doesn't manifest on Linux or FreeBSD because these OSes use a
different dance to setup MSI-X interrupts, and thus are not affected.
It's likely that other OSes with different MSI-X implementations are
able to trigger that bug. The result of not having the patch is that
interrupts of passed through devices will stay masked, thus
preventing the device from working (unless polling mode only is
used).

Pros:
  - It fixes a bug.
  - The patch is not very big or intrusive.

Cons:
  - It hasn't been tested as it's not in qemu-xen.
  - Only Windows is affected by the bug AFAIK.

IMHO, iff the backport is accepted it should be performed ASAP, so
that the patch can be properly tested before the release.


Thank you for the detailed explanation :). On a previous e-mail Stefano 
were happy with the backport. So:


Release-acked-by: Julien Grall <julien.gr...@linaro.org>


Note that I think we would need to update xen also to point to the 
commit with this backport included.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [BUG] win2008 guest cannot get ip through sriov

[Julien Grall]

Hi Roger,

On 09/11/17 09:27, Roger Pau Monné wrote:

On Thu, Nov 09, 2017 at 12:22:49AM +, Hao, Xudong wrote:

Qemu-xen didn't have commit a80363, so I report this issue to ask for sync up
with qemu upstream. Last mail I mean I usually used Qemu Xen tree to do test,
and found out this issue.

Before requesting the backport, have you tested whether it fixes your issues?



Yes, Luwei (Cced) tested it with pass result.


In which case requesting a backport would be in place on the grounds
that's a bug fix.

The patch in question fixes a bug mostly seen when doing
PCI-passthrough of devices that support MSI-X interrupts to Windows
guest (and Windows attempts to setup the interrupts using MSI-X). It
doesn't manifest on Linux or FreeBSD because these OSes use a
different dance to setup MSI-X interrupts, and thus are not affected.
It's likely that other OSes with different MSI-X implementations are
able to trigger that bug. The result of not having the patch is that
interrupts of passed through devices will stay masked, thus
preventing the device from working (unless polling mode only is
used).

Pros:
  - It fixes a bug.
  - The patch is not very big or intrusive.

Cons:
  - It hasn't been tested as it's not in qemu-xen.
  - Only Windows is affected by the bug AFAIK.

IMHO, iff the backport is accepted it should be performed ASAP, so
that the patch can be properly tested before the release.


Thank you for the detailed explanation :). On a previous e-mail Stefano 
were happy with the backport. So:


Release-acked-by: Julien Grall <julien.gr...@linaro.org>

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH for-4.10] x86/cpuid: Minor fixups missed from previous work

[Julien Grall]

Hi,

On 03/11/17 17:35, Andrew Cooper wrote:

  * Add more feature names to ./xen-cpuid
  * Vertically align the magic comments in cpufeatureset.h

Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
---
CC: Jan Beulich <jbeul...@suse.com>
CC: Ian Jackson <ian.jack...@eu.citrix.com>
CC: Wei Liu <wei.l...@citrix.com>
CC: Julien Grall <julien.gr...@arm.com>

This is a nice-to-have for Xen 4.10.  It is very low risk, as the functional
changes are restricted to debug utilities only.


Release-acked-by: Julien Grall <julien.gr...@linaro.org>

Cheers,


---
  tools/misc/xen-cpuid.c  | 15 ++-
  xen/include/public/arch-x86/cpufeatureset.h |  4 ++--
  2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
index 106be0f..0831f75 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -95,7 +95,7 @@ static const char *str_7b0[32] =
  [ 0] = "fsgsbase", [ 1] = "tsc-adj",
  [ 2] = "sgx",  [ 3] = "bmi1",
  [ 4] = "hle",  [ 5] = "avx2",
-[ 6] = "REZ",  [ 7] = "smep",
+[ 6] = "fdp_exn",  [ 7] = "smep",
  [ 8] = "bmi2", [ 9] = "erms",
  [10] = "invpcid",  [11] = "rtm",
  [12] = "pqm",  [13] = "depfpp",
@@ -121,23 +121,28 @@ static const char *str_Da1[32] =
  static const char *str_7c0[32] =
  {
  [ 0] = "prechwt1", [ 1] = "avx512vbmi",
-[ 2] = "REZ",  [ 3] = "pku",
+[ 2] = "umip", [ 3] = "pku",
  [ 4] = "ospke",
  
  [5 ... 13] = "REZ",
  
  [14] = "avx512_vpopcntdq",
  
-[15 ... 31] = "REZ",

+[15 ... 21] = "REZ",
+
+[22] = "rdpid",
+
+[23 ... 31] = "REZ",
  };
  
  static const char *str_e7d[32] =

  {
  [0 ... 7] = "REZ",
  
-[ 8] = "itsc",

+[ 8] = "itsc", [ 9] = "REZ",
+[10] = "efro",
  
-[9 ... 31] = "REZ",

+[11 ... 31] = "REZ",
  };
  
  static const char *str_e8b[32] =

diff --git a/xen/include/public/arch-x86/cpufeatureset.h 
b/xen/include/public/arch-x86/cpufeatureset.h
index 0ee3ea3..be6da8e 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -239,8 +239,8 @@ XEN_CPUFEATURE(EFRO,  7*32+10) /*   APERF/MPERF 
Read Only interface */
  XEN_CPUFEATURE(CLZERO,8*32+ 0) /*A  CLZERO instruction */
  
  /* Intel-defined CPU features, CPUID level 0x0007:0.edx, word 9 */

-XEN_CPUFEATURE(AVX512_4VNNIW, 9*32+ 2) /*A AVX512 Neural Network Instructions 
*/
-XEN_CPUFEATURE(AVX512_4FMAPS, 9*32+ 3) /*A AVX512 Multiply Accumulation Single 
Precision */
+XEN_CPUFEATURE(AVX512_4VNNIW, 9*32+ 2) /*A  AVX512 Neural Network Instructions 
*/
+XEN_CPUFEATURE(AVX512_4FMAPS, 9*32+ 3) /*A  AVX512 Multiply Accumulation 
Single Precision */
  
  #endif /* XEN_CPUFEATURE */
  



--
Julien Grall

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

Re: [Xen-devel] [PATCH for-4.10] gcov: return EOPNOTSUPP for unimplemented gcov domctl

[Julien Grall]

Hi,

On 07/11/17 14:20, Jan Beulich wrote:

On 07.11.17 at 13:31, <roger@citrix.com> wrote:

ENOSYS should only be used by unimplemented top-level syscalls. Use
EOPNOTSUPP instead.

Signed-off-by: Roger Pau Monné <roger@citrix.com>
Reported-by: Jan Beulich <jbeul...@suse.com>


Reviewed-by: Jan Beulich <jbeul...@suse.com>


Release-acked-by: Julien Grall <julien.gr...@linaro.org>

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [BUG] win2008 guest cannot get ip through sriov

[Julien Grall]

Hi Anthony,

On 08/11/17 12:13, Anthony PERARD wrote:

On Thu, Nov 02, 2017 at 01:49:54PM +, Julien Grall wrote:

On 27/10/17 21:16, Stefano Stabellini wrote:

On Fri, 27 Oct 2017, Julien Grall wrote:

On 27/10/17 08:27, Hao, Xudong wrote:

This bug exist much long time, there are many discussion last year but not a
solution then. I call out it now because there is a fix in qemu upstream:
commit a8036336609d2e184fc3543a4c439c0ba7d7f3a2
Author: Roger Pau Monne <roger@citrix.com>
Date:   Thu Aug 24 16:07:03 2017 +0100

   xen/pt: allow QEMU to request MSI unmasking at bind time

The fix is not in qemu-xen tree yet, when will qemu-xen sync this fix? Is it
possible to catch Xen 4.10's qemu-xen?


I will let Stefano and Anthony providing feedback before giving a release-ack
here.


Yes, I think we should backport the commit as it fixes a genuine bug.
The backport is not risk-free but it only affects PCI Passthrough. Also
the commit has been in QEMU for 2 months now.


Does anyone actually tested it with QEMU Xen tree?


Yes.  I've tested qemu-xen with this patch and PCI passthrough still
works.

Can I get a release-ack?


I'd still like an answer on Roger's question whether this patch fixes 
the issue they met.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [xen-unstable bisection] complete test-amd64-amd64-i386-pvgrub

[Julien Grall]

Hi Wei,

On 08/11/17 11:36, Wei Liu wrote:

On Tue, Nov 07, 2017 at 05:24:32PM +, Julien Grall wrote:

Hi Wei,

On 07/11/17 15:13, Wei Liu wrote:

On Tue, Nov 07, 2017 at 03:09:07PM +, Julien Grall wrote:

Hi Wei,

On 06/11/17 14:55, Wei Liu wrote:

On Mon, Nov 06, 2017 at 01:47:56PM +, osstest service owner wrote:

branch xen-unstable
xenbranch xen-unstable
job test-amd64-amd64-i386-pvgrub
testid guest-start

Tree: linux git://xenbits.xen.org/linux-pvops.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git

*** Found and reproduced problem changeset ***

 Bug is in tree:  xen git://xenbits.xen.org/xen.git
 Bug introduced:  f48b5449dabc770acdde6d25cfbd265cfb71034d
 Bug not present: 86cf189a957129ea1ad6468fe9a0887b9e2819f3
 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/115612/


 commit f48b5449dabc770acdde6d25cfbd265cfb71034d
 Author: Wei Liu <wei.l...@citrix.com>
 Date:   Thu Oct 12 20:19:07 2017 +0100
 tools/dombuilder: Switch to using gfn terminology for console and 
xenstore rings
 The sole use of xc_dom_translated() and xc_dom_p2m() outside of the 
domain
 builder is for libxl_dom() to translate the console and xenstore pfns 
back
 into useful values.  PV guest pfns are only interesting to the domain 
builder,
 and gfns are the address space used by all other hypercalls.
 Renaming the fields in xc_dom_image is deliberate, as it will cause
 out-of-tree users of the dombuilder to notice the different semantics.
 Correct the terminology throughout xc_dom_gnttab{_hvm,}_seed(), which 
are all
 using gfns despite the existing variable names.
 Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
 Reviewed-by: Roger Pau Monn?? <roger@citrix.com>
 Acked-by: Wei Liu <wei.l...@citrix.com>
     Tested-by: Julien Grall <julien.gr...@arm.com>
     Release-acked-by: Julien Grall <julien.gr...@linaro.org>
 [ wei: fix stubdom build ]
 Signed-off-by: Wei Liu <wei.l...@citrix.com>


This has broken pvgrub. The problem is more than just the name of the
variables. I have reverted this and its successor patch.


It looks like osstest is still broken after the patches you reverted (see
[1] and [2]).

AFAICT, the only series between the two flights is the dombuilder, there are
2 patches not reverted.

Do you have an idea of what's going on?

Cheers,

[1] http://logs.test-lab.xenproject.org/osstest/logs/115624/
[2]
https://lists.xenproject.org/archives/html/xen-devel/2017-11/msg00391.html



test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail REGR. vs.  
115526


Looking at the osstest result today, this one seem to be intermittent as 
it passed during the night but failed this morning.



test-armhf-armhf-xl-vhd 15 guest-start/debian.repeat fail REGR. vs.  115526


The log for the xl-vhd contains ([1])

libxl: error: libxl_bootloader.c:283:bootloader_local_detached_cb: Domain 
11:unable to detach locally attached disk
libxl: error: libxl_create.c:1246:domcreate_rebuild_done: Domain 11:cannot 
(re-)build domain: -3
libxl: debug: libxl_domain.c:1138:devices_destroy_cb: Domain 11:Forked pid 5103 
for destroy of domain
libxl: debug: libxl_create.c:1683:do_domain_create: Domain 0:ao 0x5d6e8: 
inprogress: poller=0x56ad8, flags=i
libxl: debug: libxl_event.c:1869:libxl__ao_complete: ao 0x5d6e8: complete, rc=-3
libxl: debug: libxl_event.c:1838:libxl__ao__destroy: ao 0x5d6e8: destroy
libxl: debug: libxl_domain.c:868:libxl_domain_destroy: Domain 11:ao 0x5a170: 
create: how=(nil) callback=(nil) poller=0x56ad8
libxl: error: libxl_domain.c:1000:libxl__destroy_domid: Domain 11:Non-existant 
domain
libxl: error: libxl_domain.c:959:domain_destroy_callback: Domain 11:Unable to 
destroy guest
libxl: error: libxl_domain.c:886:domain_destroy_cb: Domain 11:Destruction of 
domain failed
libxl: debug: libxl_event.c:1869:libxl__ao_complete: ao 0x5a170: complete, 
rc=-21
libxl: debug: libxl_domain.c:877:libxl_domain_destroy: Domain 11:ao 0x5a170: 
inprogress: poller=0x56ad8, flags=ic
libxl: debug: libxl_event.c:1838:libxl__ao__destroy: ao 0x5a170: destroy

It is in guest repeat and has succeed few times before.

Looking at the success/failure ([2]), the same configuration passed on the 
Arndale
(see 115580) but fails reliably on the cubietruck.



The same test failed on Arndale as well in 115314 and 115526, with the
same error messages.

http://logs.test-lab.xenproject.org/osstest/logs/115526/test-armhf-armhf-xl-vhd/16.ts-guest-start.log

So the failure isn't related to Andrew's series.


My guess would be the disk is not detached by the previous guest in time.
Now the question is why? I am not familiar with this area, any ideas?



I don

Re: [Xen-devel] Bringing up OSS test framework on moonshot(aarch64) systems

[Julien Grall]

Hi Ian,

On 08/11/17 11:39, Ian Jackson wrote:

Bhupinder Thakur writes ("Bringing up OSS test framework on moonshot(aarch64) 
systems"):

I am trying to bring up OSS test framework on a couple of moonshot
systems which are accessible to me remotely.


I'm not familiar with the referent of "moonshot" in this context.  IME
"moonshot" is a project name chosen multiple times, for different
projects, by people who want to give an impression that the project is
ambitious.


In that context, this is an moonshot brand from HP. It is a 4.3U that 
accepts up to 45 cartridges (see [1]).


They have x86 cartridges but also provides Arm64 one based on X-Gene 1 
(APM).


Bhupinder is looking at bring up Osstest on the Arm64 cartridges.

Cheers,

[1] 
https://www.anandtech.com/show/8357/exploring-the-low-end-and-micro-server-platforms/2


--
Julien Grall

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

Re: [Xen-devel] [xen-unstable bisection] complete test-amd64-amd64-i386-pvgrub

[Julien Grall]

Hi Wei,

On 07/11/17 15:13, Wei Liu wrote:
> On Tue, Nov 07, 2017 at 03:09:07PM +0000, Julien Grall wrote:
>> Hi Wei,
>>
>> On 06/11/17 14:55, Wei Liu wrote:
>>> On Mon, Nov 06, 2017 at 01:47:56PM +, osstest service owner wrote:
>>>> branch xen-unstable
>>>> xenbranch xen-unstable
>>>> job test-amd64-amd64-i386-pvgrub
>>>> testid guest-start
>>>>
>>>> Tree: linux git://xenbits.xen.org/linux-pvops.git
>>>> Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
>>>> Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
>>>> Tree: qemuu git://xenbits.xen.org/qemu-xen.git
>>>> Tree: xen git://xenbits.xen.org/xen.git
>>>>
>>>> *** Found and reproduced problem changeset ***
>>>>
>>>> Bug is in tree:  xen git://xenbits.xen.org/xen.git
>>>> Bug introduced:  f48b5449dabc770acdde6d25cfbd265cfb71034d
>>>> Bug not present: 86cf189a957129ea1ad6468fe9a0887b9e2819f3
>>>> Last fail repro: 
>>>> http://logs.test-lab.xenproject.org/osstest/logs/115612/
>>>>
>>>>
>>>> commit f48b5449dabc770acdde6d25cfbd265cfb71034d
>>>> Author: Wei Liu <wei.l...@citrix.com>
>>>> Date:   Thu Oct 12 20:19:07 2017 +0100
>>>> tools/dombuilder: Switch to using gfn terminology for console and 
>>>> xenstore rings
>>>> The sole use of xc_dom_translated() and xc_dom_p2m() outside of 
>>>> the domain
>>>> builder is for libxl_dom() to translate the console and xenstore 
>>>> pfns back
>>>> into useful values.  PV guest pfns are only interesting to the 
>>>> domain builder,
>>>> and gfns are the address space used by all other hypercalls.
>>>> Renaming the fields in xc_dom_image is deliberate, as it will cause
>>>> out-of-tree users of the dombuilder to notice the different 
>>>> semantics.
>>>> Correct the terminology throughout xc_dom_gnttab{_hvm,}_seed(), 
>>>> which are all
>>>> using gfns despite the existing variable names.
>>>> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
>>>> Reviewed-by: Roger Pau Monn?? <roger@citrix.com>
>>>> Acked-by: Wei Liu <wei.l...@citrix.com>
>>>> Tested-by: Julien Grall <julien.gr...@arm.com>
>>>> Release-acked-by: Julien Grall <julien.gr...@linaro.org>
>>>> [ wei: fix stubdom build ]
>>>> Signed-off-by: Wei Liu <wei.l...@citrix.com>
>>>
>>> This has broken pvgrub. The problem is more than just the name of the
>>> variables. I have reverted this and its successor patch.
>>
>> It looks like osstest is still broken after the patches you reverted (see
>> [1] and [2]).
>>
>> AFAICT, the only series between the two flights is the dombuilder, there are
>> 2 patches not reverted.
>>
>> Do you have an idea of what's going on?
>>
>> Cheers,
>>
>> [1] http://logs.test-lab.xenproject.org/osstest/logs/115624/
>> [2]
>> https://lists.xenproject.org/archives/html/xen-devel/2017-11/msg00391.html
>>
> 
> test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail REGR. vs. 
>  115526
> test-armhf-armhf-xl-vhd 15 guest-start/debian.repeat fail REGR. vs.  
> 115526

The log for the xl-vhd contains ([1])

libxl: error: libxl_bootloader.c:283:bootloader_local_detached_cb: Domain 
11:unable to detach locally attached disk
libxl: error: libxl_create.c:1246:domcreate_rebuild_done: Domain 11:cannot 
(re-)build domain: -3
libxl: debug: libxl_domain.c:1138:devices_destroy_cb: Domain 11:Forked pid 5103 
for destroy of domain
libxl: debug: libxl_create.c:1683:do_domain_create: Domain 0:ao 0x5d6e8: 
inprogress: poller=0x56ad8, flags=i
libxl: debug: libxl_event.c:1869:libxl__ao_complete: ao 0x5d6e8: complete, rc=-3
libxl: debug: libxl_event.c:1838:libxl__ao__destroy: ao 0x5d6e8: destroy
libxl: debug: libxl_domain.c:868:libxl_domain_destroy: Domain 11:ao 0x5a170: 
create: how=(nil) callback=(nil) poller=0x56ad8
libxl: error: libxl_domain.c:1000:libxl__destroy_domid: Domain 11:Non-existant 
domain
libxl: error: libxl_domain.c:959:domain_destroy_callback: Domain 11:Unable to 
destroy guest
libxl: error: libxl_domain.c:886:domain_destroy_cb: Domain 11:Destruction of 
domain failed
libxl: debug: libxl_event.c:1869:libxl__ao_complete: ao 0x5a170: complete, 
rc=-21
libxl: debug: libxl_domain.c:877:libxl_domain_destroy

Re: [Xen-devel] [RFC 0/4] TEE mediator framework + OP-TEE mediator

[Julien Grall]

Hi Volodymyr,

On 02/11/17 20:07, Volodymyr Babchuk wrote:

On Thu, Nov 02, 2017 at 05:49:12PM +, Julien Grall wrote:

On 02/11/17 16:53, Volodymyr Babchuk wrote:

On Thu, Nov 02, 2017 at 01:17:26PM +, Julien Grall wrote:

On 24/10/17 20:02, Volodymyr Babchuk wrote:

But parameters are mapped every call and only needed ones.
Example: I have shared buffers A, B, C, D.

1) I call OpenSession(TA_UUID, A, B).
TA sees only buffers A, B (okay, actually it sees whole page, because
buffer is mapped from userspace).

2) I call InvokeCommand(Session, CMD_ID, B, C).
TA sees only buffers B & C.

3) I call InvokeCommand(Session, CMD_ID, A, D).
TA sees only buffers A & D.

Note, that such buffers are not mapped at OP-TEE address space at all.
They will be mapped only to TA address space.


To confirm, what you are saying is as soon as any call is returned by TA,
the region will be unmapped from the TA address space?

Yes.
Also, just to clarify: TA executes only by request from client. It
can't have external events. So, TA address space is somewhat ephemeral
entity. It exists only during time between TA entry and TA exit. At
all other times, TA does have no address space, no thread context,
anything. Just code and data somewhere in memory.


That's quite a good news :). Thank you for the explanation.





[...]

To be clear, this series don't look controversial at least for OP-TEE. What
I am more concerned is about DomU supports.

Your concern is that rogue DomU can compromise whole system, right?


Yes. You seem to assume that DomU using TEE will always be trusted, I think
this is the wrong approach if the use is able to interact directly with
those guests. See above.

No, I am not assuming that DomU that calls TEE should be trusted. Why do you
think so? It should be able to use TEE services, but this does not mean that
XEN should trust it.


In a previous answer you said: "So, if you don't trust your guest - don't
let it". For me, this clearly means you consider that DomU using TEE are
trusted.

So can you clarify by what you mean by trust then?

Well... In real world "trust" isn't binary option. You don't want to
allow all domains to access TEE. Breached TEE user domain doesn't
automatically mean that your whole system is compromised. But this
certainly increases attack surface. So it is safer to give TEE access
only to those domains, which really require it. You can call them
sligtly more trusted, then others.


Do you have an example of guest you would slightly trust more?

I have an example of guest I would trust less: if I'm running server,
and I'm selling virtual machines on that server, I don't want to them
to access TEE.


Make sense.



I will trust slightly more to my own guest.


I kind of agree if there are either no interaction with the user or the user
is not able to gain privilege permissions.

Okay, if user can execute arbitrary code at EL1... Even then nothing bad
will happen. They must be able to hack mediator/hypervisor/OP-TEE to realy
gain priviegs in system.


My worry here is you base the trust on OP-TEE and not only the hypervisor.
At the moment we had to trust the hardware to do the right thing and the
software is owned by Xen.

How about firmware? E.g. ARM TF?


My point here was anything involved in virtualization is at the moment 
the hardware and Xen. The ARM TF/firmware cannot be accessed 
directly/indirectly by any guest. So there are no concern to me.





Now you are telling me, we have this TEE running in EL3 and have to trust
him to do the isolation between guests. Until the last 2 e-mails, it was not
clear for me how OP-TEE could ensure this isolation.

Actually, OP-TEE is running at S-EL1 :-) Only ARM TF (or whatever
firmware is used) has ultimate control over the system. If we are
talking about modern ARMv8 platforms.


I would advise to explain a bit more in your cover letter of your next
version the design of OP-TEE. This would help people to see how this can
work with the hypervisor and also understanding the consequence...

I see. I'll do this, certainly. I just didn't expected that someone will
be interested in OP-TEE internals at such level.


I like to understand what I sign for :).



But, I think, cover leter for next OP-TEE will be done much later. Now,
I'm busy with OP-TEE part, then there will be changes to support
multi-domain boot and only then OP-TEE specific patches...

BTW, if anyone is interested in current state of OP-TEE mediator, you
can find it at [1]. I was able to pass OP-TEE tests from DomU in the
last version. I use it for OP-TEE development, so it is not
production-ready.

Julien, I want to ask about VM monitor feature in XEN. monitor_smc()
function and whole xen/arch/arm/monitor.c... Looks like it was
introduced for some sort of debugging. Do you know any users of this


It was originally introduced to allow an external application trapping 
SMC and executing an action. This is part of the VM introsp

Re: [Xen-devel] [xen-unstable bisection] complete test-amd64-amd64-i386-pvgrub

[Julien Grall]

Hi Wei,

On 06/11/17 14:55, Wei Liu wrote:

On Mon, Nov 06, 2017 at 01:47:56PM +, osstest service owner wrote:

branch xen-unstable
xenbranch xen-unstable
job test-amd64-amd64-i386-pvgrub
testid guest-start

Tree: linux git://xenbits.xen.org/linux-pvops.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git

*** Found and reproduced problem changeset ***

   Bug is in tree:  xen git://xenbits.xen.org/xen.git
   Bug introduced:  f48b5449dabc770acdde6d25cfbd265cfb71034d
   Bug not present: 86cf189a957129ea1ad6468fe9a0887b9e2819f3
   Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/115612/


   commit f48b5449dabc770acdde6d25cfbd265cfb71034d
   Author: Wei Liu <wei.l...@citrix.com>
   Date:   Thu Oct 12 20:19:07 2017 +0100
   
   tools/dombuilder: Switch to using gfn terminology for console and xenstore rings
   
   The sole use of xc_dom_translated() and xc_dom_p2m() outside of the domain

   builder is for libxl_dom() to translate the console and xenstore pfns 
back
   into useful values.  PV guest pfns are only interesting to the domain 
builder,
   and gfns are the address space used by all other hypercalls.
   
   Renaming the fields in xc_dom_image is deliberate, as it will cause

   out-of-tree users of the dombuilder to notice the different semantics.
   
   Correct the terminology throughout xc_dom_gnttab{_hvm,}_seed(), which are all

   using gfns despite the existing variable names.
   
   Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>

   Reviewed-by: Roger Pau Monn?? <roger@citrix.com>
   Acked-by: Wei Liu <wei.l...@citrix.com>
   Tested-by: Julien Grall <julien.gr...@arm.com>
   Release-acked-by: Julien Grall <julien.gr...@linaro.org>
   [ wei: fix stubdom build ]
   Signed-off-by: Wei Liu <wei.l...@citrix.com>


This has broken pvgrub. The problem is more than just the name of the
variables. I have reverted this and its successor patch.


It looks like osstest is still broken after the patches you reverted 
(see [1] and [2]).


AFAICT, the only series between the two flights is the dombuilder, there 
are 2 patches not reverted.


Do you have an idea of what's going on?

Cheers,

[1] http://logs.test-lab.xenproject.org/osstest/logs/115624/
[2] 
https://lists.xenproject.org/archives/html/xen-devel/2017-11/msg00391.html


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v3 for-next 0/4] xen: Convert __page_to_mfn and _mfn_to_page to use typesafe MFN

[Julien Grall]

On 06/11/17 12:11, Jan Beulich wrote:

On 06.11.17 at 12:47, <julien.gr...@linaro.org> wrote:

Hi Jan,

On 06/11/17 11:37, Jan Beulich wrote:

On 01.11.17 at 15:03, <julien.gr...@linaro.org> wrote:

Most of the users of page_to_mfn and mfn_to_page are either overriding
the macros to make them work with mfn_t or use mfn_x/_mfn becaue the rest
of the function use mfn_t.

So I think it is time to make __page_to_mfn and __mfn_to_page using typesafe
MFN.


I have to admit that I still find the overall goal confusing: Afaict
the double-underscore-prefixed versions exist only to allow
easily overriding the non-prefixed ones. Hence the first and
foremost goal ought to be to convert everyone to using the
non-prefixed versions. Files wanting to avoid the typed forms
could then continue to use / be switched to the prefixed ones.

What you're doing here is producing a mess: The prefixed
versions should never have been touched in the first place.
And iirc this was discussed before, with the suggestion to use
overrides (for the non-prefixed versions) to limit overall patch
size.


At the end of the discussion in the previous version, you were happy
with the modification done here (see [1]).


 From the angle looked at it back then I indeed was, but I'm looking
at this from a slightly different angle now with the reply above.


Overall, I think this is an improvement compare to what we have today.
Because we enforce the use of MFN typesafe by default. The developer
would have to override the helpers if he wants to to use the
non-typesafe version.

With your suggestion here, you would just keep the override around even
when they are not necessary. They will have to be dropped at some point,
so why not now?


Why would we keep the overrides in place once no longer needed?
All I'm asking for is that the double-underscore prefixed macros be
left alone, and the non-prefixed versions be converted to be
type-safe by default (with the option to override). That'll allow the
prefixed variants to go way altogether once all code was switched
to typesafe, no longer requiring any overrides.


If you left the double-underscore alone, then you can't convert headers 
using them to typesafe. This is because they can't use the non-prefixed 
version as they may be override.


So what you are suggesting here is will avoid converting those headers 
until someone step up and finish to convert all the source to MFN 
typesafe. Personally, I find quite silly to have to delay that...


Cheers,



Jan



--
Julien Grall

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

Re: [Xen-devel] [PATCH v3 for-next 0/4] xen: Convert __page_to_mfn and _mfn_to_page to use typesafe MFN

[Julien Grall]

Hi Jan,

On 06/11/17 11:37, Jan Beulich wrote:

On 01.11.17 at 15:03, <julien.gr...@linaro.org> wrote:

Most of the users of page_to_mfn and mfn_to_page are either overriding
the macros to make them work with mfn_t or use mfn_x/_mfn becaue the rest
of the function use mfn_t.

So I think it is time to make __page_to_mfn and __mfn_to_page using typesafe
MFN.


I have to admit that I still find the overall goal confusing: Afaict
the double-underscore-prefixed versions exist only to allow
easily overriding the non-prefixed ones. Hence the first and
foremost goal ought to be to convert everyone to using the
non-prefixed versions. Files wanting to avoid the typed forms
could then continue to use / be switched to the prefixed ones.

What you're doing here is producing a mess: The prefixed
versions should never have been touched in the first place.
And iirc this was discussed before, with the suggestion to use
overrides (for the non-prefixed versions) to limit overall patch
size.


At the end of the discussion in the previous version, you were happy 
with the modification done here (see [1]).


Overall, I think this is an improvement compare to what we have today. 
Because we enforce the use of MFN typesafe by default. The developer 
would have to override the helpers if he wants to to use the 
non-typesafe version.


With your suggestion here, you would just keep the override around even 
when they are not necessary. They will have to be dropped at some point, 
so why not now?


Cheers,

[1] 
https://lists.xenproject.org/archives/html/xen-devel/2017-10/msg00986.html




Jan



--
Julien Grall

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

Re: [Xen-devel] [PATCH 1/1] hw/intc/arm_gicv3_its: Fix the VM termination in vm_change_state_handler()

[Julien Grall]

Hi Shanker,

I think you sent this patch to the wrong ML and people. This patch seem 
KVM specific.


Cheers,

On 03/11/17 11:33, Shanker Donthineni wrote:

The commit cddafd8f353d ("hw/intc/arm_gicv3_its: Implement state save
/restore") breaks the backward compatibility with the older kernels
where vITS save/restore support is not available. The vmstate function
vm_change_state_handler() should not be registered if the running kernel
doesn't support ITS save/restore feature. Otherwise VM instance will be
killed whenever vmstate callback function is invoked.

Observed a virtual machine shutdown with QEMU-2.10+linux-4.11 when testing
the reboot command "virsh reboot  --mode acpi" instead of reboot.

KVM Error: 'KVM_SET_DEVICE_ATTR failed: Group 4 attr 0x01'

Signed-off-by: Shanker Donthineni <shank...@codeaurora.org>
---
  hw/intc/arm_gicv3_its_kvm.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
index 39903d5..9b00ce5 100644
--- a/hw/intc/arm_gicv3_its_kvm.c
+++ b/hw/intc/arm_gicv3_its_kvm.c
@@ -111,13 +111,13 @@ static void kvm_arm_its_realize(DeviceState *dev, Error 
**errp)
  error_free(s->migration_blocker);
  return;
  }
+} else {
+qemu_add_vm_change_state_handler(vm_change_state_handler, s);
  }
  
  kvm_msi_use_devid = true;

  kvm_gsi_direct_mapping = false;
  kvm_msi_via_irqfd_allowed = kvm_irqfds_enabled();
-
-qemu_add_vm_change_state_handler(vm_change_state_handler, s);
  }
  
  /**




--
Julien Grall

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

Re: [Xen-devel] [xen-unstable-smoke test] 115515: regressions - FAIL

[Julien Grall]

Hi,

On 03/11/17 10:29, osstest service owner wrote:

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
  test-amd64-amd64-libvirt 15 guest-saverestorefail REGR. vs. 115490
  test-amd64-amd64-xl-qemuu-debianhvm-i386 13 guest-saverestore fail REGR. vs. 
115490

Tests which did not succeed, but are not blocking:
  test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
  test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
  test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass

version targeted for testing:
  xen  29028ed8db226ad3b7bc576c1e8891983acaa3ff
baseline version:
  xen  ff93dc55431517ed29c70dbff6721c6b0803acf9


The only difference between the two is the series from Andrew about the 
tools. Any idea, why it would break?


Cheers,



Last test of basis   115490  2017-11-02 17:01:26 Z0 days
Testing same since   115497  2017-11-02 20:01:37 Z0 days6 attempts


People who touched revisions under test:
   Andrew Cooper <andrew.coop...@citrix.com>
   Julien Grall <julien.gr...@arm.com>
   Wei Liu <wei.l...@citrix.com>

jobs:
  build-amd64  pass
  build-armhf  pass
  build-amd64-libvirt  pass
  test-armhf-armhf-xl  pass
  test-amd64-amd64-xl-qemuu-debianhvm-i386 fail
  test-amd64-amd64-libvirt fail



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


Not pushing.


commit 29028ed8db226ad3b7bc576c1e8891983acaa3ff
Merge: 9ff6dbf ff93dc5
Author: Wei Liu <wei.l...@citrix.com>
Date:   Thu Nov 2 17:07:58 2017 +

 Merge remote-tracking branch 'origin/staging' into staging

commit 9ff6dbfa7576cc1c5d6f9a3c59c69a8503e36f11
Author: Andrew Cooper <andrew.coop...@citrix.com>
Date:   Thu Oct 12 20:19:09 2017 +0100

 tools/dombuilder: Prevent failures of xc_dom_gnttab_init()
 
 Recent changes in grant table configuration have caused calls to

 xc_dom_gnttab_init() to fail if not proceeded with a call to
 xc_domain_set_gnttab_limits().  This is backwards from the point of view of
 3rd party dombuilder users.
 
 Add max_{grant,maptrack}_frames parameters to struct xc_dom_image, and require

 them to be set by callers using xc_dom_gnttab_init().  Libxl, which uses
 xc_dom_gnttab_init() itself is updated appropriately.
 
 Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>

 Acked-by: Wei Liu <wei.l...@citrix.com>
 Tested-by: Julien Grall <julien.gr...@arm.com>
 Reviewed-by: Juergen Gross <jgr...@suse.com>
 Release-acked-by: Julien Grall <julien.gr...@linaro.org>

commit 87b0ae7e8277d2fa13486ce2e11a941e55f8df40
Author: Andrew Cooper <andrew.coop...@citrix.com>
Date:   Thu Oct 12 20:19:08 2017 +0100

 tools/dombuilder: Fix asymmetry when setting up console and xenstore rings
 
 libxl always uses xc_dom_gnttab_init(), which internally calls

 xc_dom_gnttab{_hvm,}_seed() to set up the grants point at the console and
 xenstore rings.  For HVM guests, libxl then asks Xen for the information 
set
 up previously, and calls xc_dom_gnttab_hvm_seed() a second time, which is
 wasteful.  ARM construction expects libxl to have set up
 dom->{console,xenstore}_evtchn earlier, so only actually functions because 
of
 this second call.
 
 Rationalise everything and make it consistent for all guests.
 
  1) Users of the domain builder are expected to provide

 dom->{console,xenstore}_{evtchn,domid} unconditionally.  This is 
checked
 by setting invalid values in xc_dom_allocate(), and checking in
 xc_dom_boot_image().
 
  2) For x86 HVM and ARM guests, the event channels are given to Xen at the

 same time as the ring gfns.  ARM already did this, but x86 is updated 
to
 match.  x86 PV already provides this information in the start_info 
page.

Re: [Xen-devel] [RFC 0/4] TEE mediator framework + OP-TEE mediator

[Julien Grall]

Hi Volodymyr,

On 02/11/17 16:53, Volodymyr Babchuk wrote:

On Thu, Nov 02, 2017 at 01:17:26PM +, Julien Grall wrote:

On 24/10/17 20:02, Volodymyr Babchuk wrote:

If it is not safe, this means you have a whitelist solution and therefore
tie Xen to a specific OP-TEE version. So if you need to use a new function
you would need to upgrade Xen making the code of using new version
potentially high.

Yes, any ABI change between OP-TEE and its clients will require mediator
upgrade. Luckilly, OP-TEE maintains ABI backward-compatible, so if you'll
install old XEN and new OP-TEE, OP-TEE will use only that subset of ABI,
which is known to XEN.


Also, correct me if I am wrong, OP-TEE is a BSD 2-Clause. This means you
impose anyone wanted to modify OP-TEE for their own purpose can make a
closed version of the TEE. But if you need to introspect/whitelist call, you
impose the vendor to expose their API.

Basically yes. Is this bad? OP-TEE driver in Linux is licensed under GPL v2.
If vendor modifies interface between OP-TEE and Linux, they anyways obligued
to expose API.


Pardon me for potential stupid questions, my knowledge of OP-TEE is limited.

My understanding is the OP-TEE will provide a generic way to access
different Trusted Application. While OP-TEE API may be generic, the TA API
is custom. AFAICT the latter is not part of Linux driver.

Yes, you are perfectly right there.


So here my questions:
1) Are you planning allow all the guests to access every Trusted
Applications?

This is a good question. There are two types of TAs supported in
OP-TEE: real TAs (as they are described in GlobalPlatform specs) and
PseudoTAs.  The latter ones are statically linked right into OP-TEE
kernel and execute at S-EL1 level.
Real TAs are provided by client. That means that NW userspace
supplicant loads TA into OP-TEE. OP-TEE checks signature for the TA
and then runs it in S-EL0.
So, I'm planning to allow client to work with any real TA. I can't see
real problem there.


Are the real TAs going to be shared between guests? Or will each guest have
their own one?

No, we don't plan this. At least at this momoent. Every guest will have
own instance of TA.


Will you allow every guests loading real TAs?

Yes, if guest has access to TEE, it can load TA. Either there is no
sense to use TEE. OP-TEE core itself does not provide useful services
to clients.


In a previous e-mail you mentioned OP-TEE has limited memory. How will you
ensure that guest A will not use all the memory of OP-TEE and prevent guest
B to load TAs?

There are no way to do this right now. Even on bare-metal system, one client
call load huge TA or eat up memory in another way to prevent other clients
to use OP-TEE. This is known limitation. It can be mitigated by enforcing
quotas.


Yes, but those clients only serve one OS. Here you would serve multiple 
OSes, clients from OS A could eat up the memory and prevent a client 
from OS B to run.


This could be a serious issue depending on how important the clients for 
OS are.


So likely enforcing quotas will be needed.




[...]


Not really, you could the domain could block when issuing an SMC until the
mediator is up and running.

Do you mean, that if domain tries to execute SMC, and mediator is not
ready, then hypervisor should pause all domain's vCPUs? That can be
destructive for hw domain.


Xen is free to unschedule any vCPU at any time. So why would it be
destructive?

Suppose that mediator domain needs 0.5s to boot up and be ready to
serve calls. For half of a second HW domain will be blocked. I don't
like the idea, that it will not be able to serve IRQs and other
requests. IMHO, it is okay for DomU, but not for Dom0.






And yes, it seems obvious, but I want to say this explicitly: generic
TEE mediator framework should and will use XSM to control which domain
can work with TEE. So, if you don't trust your guest - don't let it
to call TEE at all.


Correct me if I am wrong. TEE could be used by Android guest which likely
run the user apps... right? So are you saying you fully trust that guest and
obviously the user installing rogue app?

I don't think that app downloaded from Play Marget can access OP-TEE directly.
OP-TEE can be used by Android itself as a key storage or to access to a SE,
for example. But 3rd app that issues TEE calls... I don't think so.


You didn't get my point here. That rogue app may be able to break into
kernel via an exploit or have enough privilege to break the guest. Who knows
what it will be able to do after...

Only what hypervisor and TEE will allow it to do. Look, OP-TEE was not designed
to rule the machine. There is ARM TF for that :) OP-TEE's task is to provide
some safer environment for sensitive data and code. This environment has
well-defined interfaces and is desgined to be as safe as possible.

If rogue app breaks into kernel, then it can issue any SMC which it wants.
But OP-TEE does not trust to NW. Hypervisor does not trust to guests.
Mediator should

Re: [Xen-devel] [PATCH v3 for-next 3/4] xen/tmem: Convert the file common/tmem_xen.c to use typesafe MFN

[Julien Grall]

Hi Konrad,

On 01/11/17 18:54, Konrad Rzeszutek Wilk wrote:

On Wed, Nov 01, 2017 at 02:03:15PM +, Julien Grall wrote:

The file common/tmem_xen.c is now converted to use typesafe. This is
requiring to override the macro page_to_mfn to make it work with mfn_t.

Note that all variables converted to mfn_t havem there initial value,
when set, switch from 0 to INVALID_MFN. This is fine because the initial
values was always overriden before used.

Also add a couple of missing newlines suggested by Andrew in the code.

Signed-off-by: Julien Grall <julien.gr...@linaro.org>
Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>

---

Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>



Acked-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>

But could you confirm that you did compile this on x86 and with CONFIG_TMEM=y 
in the .config?


Yes, I have CONFIG_TMEM enabled in .config.

Thank you for the ack,

--
Julien Grall

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

Re: [Xen-devel] [PATCH for-4.10] gdbsx: prefer privcmd character device

[Julien Grall]

Hi Wei,

On 01/11/17 10:20, Wei Liu wrote:

Cc Julien and change tag. I think this is safe to be applied to 4.10.


I agree.

Release-acked-by: Julien Grall <julien.gr...@linaro.org>

Cheers,



On Tue, Oct 31, 2017 at 01:58:07PM -0700, Elena Ufimtseva wrote:

On Tue, Oct 31, 2017 at 03:25:39PM +, Wei Liu wrote:

On Tue, Oct 31, 2017 at 10:20:11AM -0500, Doug Goldstein wrote:

Prefer using the character device over the proc file if the character
device exists.

CC: Elena Ufimtseva <elena.ufimts...@oracle.com>
CC: Ian Jackson <ian.jack...@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabell...@eu.citrix.com>
CC: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Doug Goldstein <car...@cardoe.com>
---
So this was originally submitted with 9c89dc95201 and 7d418eab3b6 and
was rejected since the goal was to convert gdbsx to use libxc but that
hasn't happened. /dev/xen/privcmd should be preferred and this change
makes that happen. It would be nice if we landed this with the plan
to convert gdbsx happening when it happens.


Oh well... I think this is fine.

Elena has the final verdict.


I think this is fine.
I will look into the conversion and relevant discussions if I find them and
see what can be done.

Thanks!

Meanwhile,
Reviewed-by: Elena Ufimtseva <elena.ufimts...@oracle.com>

Elena


--
Julien Grall

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

Re: [Xen-devel] [PATCH for-4.10] common/multicall: Increase debugability for bad hypercalls

[Julien Grall]

Hi Andrew,

On 31/10/17 17:18, Andrew Cooper wrote:

While investigating an issue (in a new codepath I'd introduced, as it turns
out), leaving interrupts disabled manifested as a subsequent op in the
multicall failing a check_lock() test.

The codepath would have hit the ASSERT_NOT_IN_ATOMIC on the return-to-guest
path, had it not hit the check_lock() first.

Call ASSERT_NOT_IN_ATOMIC() after each operation in the multicall, to make
failures more obvious.

Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
---
CC: George Dunlap <george.dun...@eu.citrix.com>
CC: Jan Beulich <jbeul...@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
CC: Stefano Stabellini <sstabell...@kernel.org>
CC: Tim Deegan <t...@xen.org>
CC: Wei Liu <wei.l...@citrix.com>
CC: Julien Grall <julien.gr...@arm.com>

As with the related check_lock() patch, this only affects debug builds, so is
a very low risk change for 4.10


Release-acked-by: Julien Grall <julien.gr...@linaro.org>

With a couple of typos below.


---
  xen/common/multicall.c | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/xen/common/multicall.c b/xen/common/multicall.c
index c7af4e0..d98e59d 100644
--- a/xen/common/multicall.c
+++ b/xen/common/multicall.c
@@ -66,6 +66,13 @@ do_multicall(
  
  disp = arch_do_multicall_call(mcs);
  
+/*

+ * In the unlikley event that a hypercall has left interrupts,


s/unlikley/unlikely/


+ * spinlocks, or other things in a bad way, continuting the multicall


s/continuting/continuing/


+ * will typically lead to far more subtle issues to debug.
+ */
+ASSERT_NOT_IN_ATOMIC();
+
  #ifndef NDEBUG
  {
  /*



Cheers,

--
Julien Grall

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

Re: [Xen-devel] Xen 4.10.0 RC1 test result

[Julien Grall]

Hi,

On 30/10/17 02:21, Hao, Xudong wrote:



-Original Message-
From: Jan Beulich [mailto:jbeul...@suse.com]
Sent: Friday, October 27, 2017 5:19 PM
To: Hao, Xudong <xudong@intel.com>
Cc: Julien Grall <julien.gr...@arm.com>; Lars Kurth <lars.ku...@citrix.com>;
xen-devel@lists.xen.org
Subject: Re: [Xen-devel] Xen 4.10.0 RC1 test result


On 27.10.17 at 10:28, <xudong@intel.com> wrote:

RAS:
[BUG] xen-mceinj tool testing cause dom0 crash
https://www.mail-archive.com/xen-devel@lists.xen.org/msg108671.html


Please can you provide helpful links? This doesn't point to the beginning of the
thread, and the mail archive chosen doesn't appear to have an easy way to go
back to the head of a thread. And when I go through the parts of the thread


Unfortunately I didn't find the original link from mail-archive, but I pick up 
it in my mail client, attach the original mail.


which are easily accessible there, it looks like you've never followed up on the
additional information (log) request.


I've provided the full log which included Xen and Dom0's, even though there was 
no valid error message from Dom0.


This way I don't see how we can make
progress there.


Yes, this is the end mail 
https://www.mail-archive.com/xen-devel@lists.xen.org/msg108894.html.


Plus, looking over the Cc lists there, Linux maintainers also don't
appear to have been involved at any time.



I'm not sure if it's related with Dom0's kernel. My intention is we could 
discuss in Xen list only till we make sure it's Dom0's issue.


At the moment the discussion seem to be stuck on Xen list... Jan 
mentioned that for now he is not convinced it is a Xen bug. How about 
you CC Linux maintainers to get more feedback?


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH for-4.10] common/spinlock: Improve the output from check_lock() if it trips

[Julien Grall]

Hi Andrew,

On 31/10/17 10:49, Andrew Cooper wrote:

If check_lock() triggers, a crash will occur.  Instead of simply identifying
"the irq context was different", indicate the expected and current irq
context.

Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>


Release-acked-by: Julien Grall <julien.gr...@linaro.org>

Cheers,


---
CC: George Dunlap <george.dun...@eu.citrix.com>
CC: Jan Beulich <jbeul...@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
CC: Stefano Stabellini <sstabell...@kernel.org>
CC: Tim Deegan <t...@xen.org>
CC: Wei Liu <wei.l...@citrix.com>
CC: Julien Grall <julien.gr...@arm.com>

check_lock() only exists in debug builds, which makes this a low risk change
for 4.10.
---
  xen/common/spinlock.c | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
index 44b07b7..8f2ba08 100644
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -44,7 +44,13 @@ static void check_lock(struct lock_debug *debug)
  if ( unlikely(debug->irq_safe != irq_safe) )
  {
  int seen = cmpxchg(>irq_safe, -1, irq_safe);
-BUG_ON(seen == !irq_safe);
+
+if ( seen == !irq_safe )
+{
+printk("CHECKLOCK FAILURE: prev irqsafe: %d, curr irqsafe %d\n",
+   seen, irq_safe);
+BUG();
+}
  }
  }
  



--
Julien Grall

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

Re: [Xen-devel] [PATCH for-4.10] tools/hotplug: create XEN_LOG_DIR at runtime

[Julien Grall]

On 30/10/17 14:39, Ian Jackson wrote:

Wei Liu writes ("Re: [PATCH] tools/hotplug: create XEN_LOG_DIR at runtime"):

On Fri, Oct 27, 2017 at 07:52:37PM +0300, Andrii Anisov wrote:

From: Andrii Anisov <andrii_ani...@epam.com>

/var/log could be a tmpfs mount point, so create xen subfolder at runtime.

Signed-off-by: Andrii Anisov <andrii_ani...@epam.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babc...@epam.com>
Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushche...@epam.com>


Acked-by: Wei Liu <wei.l...@citrix.com>

Julien I think we should apply this for 4.10.


I agree.  Subject line tag added.

Acked-by: Ian Jackson <ian.jack...@eu.citrix.com>


Release-acked-by: Julien Grall <julien.gr...@linaro.org>

Cheers,



Ian.

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



--
Julien Grall

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

Re: [Xen-devel] [BUG] win2008 guest cannot get ip through sriov

[Julien Grall]

Hi,

On 27/10/17 21:16, Stefano Stabellini wrote:

On Fri, 27 Oct 2017, Julien Grall wrote:

On 27/10/17 08:27, Hao, Xudong wrote:

This bug exist much long time, there are many discussion last year but not a
solution then. I call out it now because there is a fix in qemu upstream:
commit a8036336609d2e184fc3543a4c439c0ba7d7f3a2
Author: Roger Pau Monne <roger@citrix.com>
Date:   Thu Aug 24 16:07:03 2017 +0100

  xen/pt: allow QEMU to request MSI unmasking at bind time

The fix is not in qemu-xen tree yet, when will qemu-xen sync this fix? Is it
possible to catch Xen 4.10's qemu-xen?


I will let Stefano and Anthony providing feedback before giving a release-ack
here.


Yes, I think we should backport the commit as it fixes a genuine bug.
The backport is not risk-free but it only affects PCI Passthrough. Also
the commit has been in QEMU for 2 months now.


Does anyone actually tested it with QEMU Xen tree?

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH for-4.10 v2 0/5] tools/dombuilder: Fixes and improvements to grant handling

[Julien Grall]

Actually CC them...

On 02/11/17 13:44, Julien Grall wrote:

Hi,

I noticed I forgot to CC Wei and Ian for committing this patch series.

On 27/10/17 14:31, Julien Grall wrote:

Hi Juergen,

On 25/10/17 08:08, Juergen Gross wrote:

On 24/10/17 18:06, Julien Grall wrote:

Hi,

I think this is 4.10 material (particularly patch #5). Juergen, 
would it

be possible get the some feedback on this series?


Patch 5: Reviewed-by: Juergen Gross <jgr...@suse.com>


Thank you!

For the series:

Release-acked-by: Julien Grall <julien.gr...@linaro.org>

Cheers,





--
Julien Grall

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

Re: [Xen-devel] [PATCH for-4.10 v2 0/5] tools/dombuilder: Fixes and improvements to grant handling

[Julien Grall]

Hi,

I noticed I forgot to CC Wei and Ian for committing this patch series.

On 27/10/17 14:31, Julien Grall wrote:

Hi Juergen,

On 25/10/17 08:08, Juergen Gross wrote:

On 24/10/17 18:06, Julien Grall wrote:

Hi,

I think this is 4.10 material (particularly patch #5). Juergen, would it
be possible get the some feedback on this series?


Patch 5: Reviewed-by: Juergen Gross <jgr...@suse.com>


Thank you!

For the series:

Release-acked-by: Julien Grall <julien.gr...@linaro.org>

Cheers,



--
Julien Grall

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

Re: [Xen-devel] linux-arm-xen branch, commit access, etc.

[Julien Grall]

Hi,

On 23/10/17 22:33, Stefano Stabellini wrote:

On Fri, 20 Oct 2017, Julien Grall wrote:

   Julien, do you think we need to keep a special linux tree around for Xen
   on ARM testing in OSSTest or we can start using vanilla kernel releases?
   I would love to get rid of it, if you know of any reasons why we have to
   keep it, this is the time to speak :-)


I think it would be better to keep aroundSome platform may be available before 
the code is merged.


Sure.


Ian,

let's create a /arm/linux.git tree on xenbits where both Julien and I
can push. The idea is that we'll try to use vanilla kernel releases but
we'll keep it around just in case we'll need special patches for
hardware support in the future. If it turns out that we don't actually
need it, we can get rid of it in a year or two.

We'll initialize /arm/linux.git based on the current linux-arm-xen
branch. /arm/linux.git will replace linux-arm-xen in OSSTest.

Sounds good?


I don't mind as long as I have access to the arm linux tree. Ian do you 
have any opinions?


Cheers,

--
Julien Grall

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

Re: [Xen-devel] Commit moratorium to staging [and 1 more messages]

[Julien Grall]

Hi Ian,

On 02/11/17 13:27, Ian Jackson wrote:

Julien Grall writes ("Re: Commit moratorium to staging"):

Thank you for the explanation. I agree with the force push to unblock
master (and other tree I mentioned).


I will force push all the affected trees, but in a reactive way
because I base each force push on a test report - so it won't be right
away for all of them.

osstest service owner writes ("[xen-unstable test] 115471: regressions - FAIL"):

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
  test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stopfail REGR. vs. 114644
  test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop   fail REGR. vs. 114644
  test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop   fail REGR. vs. 114644


The above are justifiable as discussed, leaving no blockers.


version targeted for testing:
  xen  bb2c1a1cc98a22e2d4c14b18421aa7be6c2adf0d


So I have forced pushed that.


Thank you! With that, the tree is re-opened. I will go through my 
backlog of Xen 4.10 and have a look whether they are suitable.


Cheers,


--
Julien Grall

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

Re: [Xen-devel] [RFC 0/4] TEE mediator framework + OP-TEE mediator

[Julien Grall]

to
maintain it.


Well, in a way or another someone will have to maintain the mediator... The
kernel does not need to be specific to TEE, it could be a unikernel.

Right. But for me XEN looks better maintained "kernel" :)
IMHO, XEN is mature, there are less bugs (especially security ones)
than in any other kernel.


And before you say again no-one in the community seem to be interested. I
should remind you that Arm is working on it (see development update).

You are talking about that "unicore" project by NEC guys? Sorry,
can't find mentioned development update. Looks like search on markmail
is down (or I'm doing something terribly wrong).


Sorry, I meant Mini-OS. I don't know any work on "unicore" for Arm64 for
now.

Ah, good to hear. So there will be active maintainer for ARM64
Mini-OS? Sorry, still can't find that "development update".


At the moment, the series is still in development. But yet the plan is 
to get Arm64 fully supported in Mini-OS.








This is a lot of a work. It requires changes in generic parts of XEN.
I fear it will be very hard to upstream such changes, because no one
sees an immediate value in them. How do you think, what are my chances
to upstream this?


It is fairly annoying to see you justifying back most of this thread with
"no one sees an immediate value in them".

I am not the only maintainers in Xen, so effectively can't promise whether
it is going to be upstreamed. But I believe the community has been very
supportive so far, a lot of discussions happened (see [2]) because of the
OP-TEE support. So what more do you expect from us?

I'm sorry, I didn't mean to offend you or someone else. You, guys, can
be harsh sometimes, but I really appreciate help provided by the
community. And I, certainly, don't ask you about any guarantees or
something of that sort.

I'm just bothered by amount of required work and by upstreaming
process. But this is not a strong argument against mediators in
stubdoms, I think :)

Currently I'm developing virtualization support in OP-TEE, so in
meantime we'll have much time to discuss mediators and stubdomain
approach (if you have time). To test this feature in OP-TEE I'm
extending this RFC, making optee.c to look like full-scale mediator.
I need to do this anyways, to test OP-TEE. When I'll finish, I can
show you how mediator can look like. Maybe this will persuade you to
one or another approach.


I think this would be useful. Can you also keep both Stefano (I assume he
wants too) and I  in the loop for the OP-TEE virtualization side?

Okay. I'm planning to produce first RFC for OP-TEE folks in a few
days. I'll subscribe you. In then meantime you can check out [2]


Thank you!


[1] http://markmail.org/message/tdbg5mgxjvsoj2ph
[2] https://github.com/OP-TEE/optee_os/issues/1890


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 2/2] xen: Fix 16550 UART console for HP Moonshot (Aarch64) platform

[Julien Grall]

Hi Bhupinder,

On 02/11/17 10:13, Bhupinder Thakur wrote:

 The console was not working on HP Moonshot (HPE Proliant Aarch64) because
 the UART registers were accessed as 8-bit aligned addresses. However,
 registers are 32-bit aligned for HP Moonshot.

 Since ACPI/SPCR table does not specify the register shift to be applied to 
the
 register offset, this patch implements an erratum to correctly set the 
register
 shift for HP Moonshot.

 Similar erratum was implemented in linux:


Can you remove the tab at the beginning of each of the lines above?



 commit 79a648328d2a604524a30523ca763fbeca0f70e3
 Author: Loc Ho <l...@apm.com>
 Date:   Mon Jul 3 14:33:09 2017 -0700

 ACPI: SPCR: Workaround for APM X-Gene 8250 UART 32-alignment errata

 APM X-Gene verion 1 and 2 have an 8250 UART with its register
 aligned to 32-bit. In addition, the latest released BIOS
 encodes the access field as 8-bit access instead 32-bit access.
 This causes no console with ACPI boot as the console
 will not match X-Gene UART port due to the lack of mmio32
 option.

 Signed-off-by: Loc Ho <l...@apm.com>
 Acked-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
 Signed-off-by: Rafael J. Wysocki <rafael.j.wyso...@intel.com>

Signed-off-by: Bhupinder Thakur <bhupinder.tha...@linaro.org>
---
CC: Andrew Cooper <andrew.coop...@citrix.com>
CC: George Dunlap <george.dun...@eu.citrix.com>
CC: Ian Jackson <ian.jack...@eu.citrix.com>
CC: Jan Beulich <jbeul...@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
CC: Stefano Stabellini <sstabell...@kernel.org>
CC: Tim Deegan <t...@xen.org>
CC: Wei Liu <wei.l...@citrix.com>
CC: Julien Grall <julien.gr...@arm.com>

  xen/drivers/char/ns16550.c | 42 --
  1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index b3f6d85..e716aba 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -1542,6 +1542,33 @@ DT_DEVICE_END
  #ifdef CONFIG_ACPI
  #include 
  
+/*

+ * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
+ * register aligned to 32-bit. In addition, the BIOS also encoded the
+ * access width to be 8 bits. This function detects this errata condition.
+ */
+static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
+{
+bool xgene_8250 = false;
+
+if ( tb->interface_type != ACPI_DBG2_16550_COMPATIBLE )
+return false;
+
+if ( memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
+ memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE) )
+return false;
+
+if ( !memcmp(tb->header.oem_table_id, "XGENESPC",
+ ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0 )

Please align ACPI_OEM_TABLE_ID_SIZE with ( after memcmp.


+xgene_8250 = true;
+
+if ( !memcmp(tb->header.oem_table_id, "ProLiant",
+ ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1 )


Ditto.


+xgene_8250 = true;
+
+return xgene_8250;
+}
+
  static int __init ns16550_acpi_uart_init(const void *data)
  {
  struct ns16550 *uart;
@@ -1568,9 +1595,20 @@ static int __init ns16550_acpi_uart_init(const void 
*data)
  uart->io_base = spcr->serial_port.address;
  uart->irq = spcr->interrupt;
  uart->reg_width = spcr->serial_port.bit_width/8;
-uart->reg_shift = 0;
-uart->io_size = UART_MAX_REG<reg_shift;
  
+if ( xgene_8250_erratum_present(spcr) )

+{
+/*
+ * for xgene v1 and v2 the registers are 32-bit and so a
+ * register shift of 2 has to be applied to get the
+ * correct register offset.
+ */
+uart->reg_shift = 2;
+}
+else
+uart->reg_shift = 0;
+
+uart->io_size = UART_MAX_REG<reg_shift;


I am not why this change here. It might because of diff is confused?


  irq_set_type(spcr->interrupt, spcr->interrupt_type);
  
  uart->vuart.base_addr = uart->io_base;




Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 1/2] xen: Add support for initializing 16550 UART using ACPI

[Julien Grall]

Hi Bhupinder,

Please write a cover letter even if it is small when your send a series 
with multiple patches.


On 02/11/17 10:13, Bhupinder Thakur wrote:

Currently, Xen supports only DT based initialization of 16550 UART.
This patch adds support for initializing 16550 UART using ACPI SPCR table.

Signed-off-by: Bhupinder Thakur <bhupinder.tha...@linaro.org>
---
CC: Andrew Cooper <andrew.coop...@citrix.com>
CC: George Dunlap <george.dun...@eu.citrix.com>
CC: Ian Jackson <ian.jack...@eu.citrix.com>
CC: Jan Beulich <jbeul...@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
CC: Stefano Stabellini <sstabell...@kernel.org>
CC: Tim Deegan <t...@xen.org>
CC: Wei Liu <wei.l...@citrix.com>
CC: Julien Grall <julien.gr...@arm.com>

  xen/drivers/char/ns16550.c  | 57 +
  xen/include/xen/8250-uart.h |  1 +
  2 files changed, 58 insertions(+)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index e0f8199..b3f6d85 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -1538,6 +1538,63 @@ DT_DEVICE_START(ns16550, "NS16550 UART", DEVICE_SERIAL)
  DT_DEVICE_END
  
  #endif /* HAS_DEVICE_TREE */

+
+#ifdef CONFIG_ACPI


The code below is going to break x86 build. You need to do #if 
defined(CONFIG_ACPI) && defined(CONFIG_ARM)



+#include 
+
+static int __init ns16550_acpi_uart_init(const void *data)
+{
+struct ns16550 *uart;
+acpi_status status;
+struct acpi_table_spcr *spcr = NULL;
+
+status = acpi_get_table(ACPI_SIG_SPCR, 0,
+(struct acpi_table_header **));
+
+if ( ACPI_FAILURE(status) )
+{
+printk("ns16550: Failed to get SPCR table\n");
+return -EINVAL;
+}
+
+uart = _com[0];
+
+ns16550_init_common(uart);
+
+uart->baud  = BAUD_AUTO;
+uart->data_bits = 8;
+uart->parity= spcr->parity;
+uart->stop_bits = spcr->stop_bits;
+uart->io_base = spcr->serial_port.address;
+uart->irq = spcr->interrupt;
+uart->reg_width = spcr->serial_port.bit_width/8;


width / 8;


+uart->reg_shift = 0;
+uart->io_size = UART_MAX_REG<reg_shift;


space before and after <<.

Also, io_size seems to be computed differently in pci_uart_config. I am 
not sure why the difference here?



+
+irq_set_type(spcr->interrupt, spcr->interrupt_type);
+
+uart->vuart.base_addr = uart->io_base;
+uart->vuart.size = uart->io_size;
+uart->vuart.data_off = UART_THR <reg_shift;


Ditto for the space.


+uart->vuart.status_off = UART_LSR<reg_shift;


Ditto.


+uart->vuart.status = UART_LSR_THRE|UART_LSR_TEMT;


Ditto.

Also, the code looks very similar to the DT version. Is there any way to 
share it?



+
+/* Register with generic serial driver. */
+serial_register_uart(uart - ns16550_com, _driver, uart);
+
+return 0;
+}
+
+ACPI_DEVICE_START(ns16550c, "16550 COMPAT UART", DEVICE_SERIAL)
+.class_type = ACPI_DBG2_16550_COMPATIBLE,
+.init = ns16550_acpi_uart_init,
+ACPI_DEVICE_END
+ACPI_DEVICE_START(ns16550s, "16550 SUBSET UART", DEVICE_SERIAL)
+.class_type = ACPI_DBG2_16550_SUBSET,
+.init = ns16550_acpi_uart_init,
+ACPI_DEVICE_END
+
+#endif
  /*
   * Local variables:
   * mode: C
diff --git a/xen/include/xen/8250-uart.h b/xen/include/xen/8250-uart.h
index 5c3bac3..1b3e137 100644
--- a/xen/include/xen/8250-uart.h
+++ b/xen/include/xen/8250-uart.h
@@ -35,6 +35,7 @@
  #define UART_USR  0x1f/* Status register (DW) */
  #define UART_DLL  0x00/* divisor latch (ls) (DLAB=1) */
  #define UART_DLM  0x01    /* divisor latch (ms) (DLAB=1) */
+#define UART_MAX_REG  (UART_USR+1)
  
  /* Interrupt Enable Register */

  #define UART_IER_ERDAI0x01/* rx data recv'd   */



Cheers,

--
Julien Grall

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

Re: [Xen-devel] Commit moratorium to staging

[Julien Grall]

Hi Ian,

On 11/01/2017 04:54 PM, Ian Jackson wrote:

Julien Grall writes ("Re: Commit moratorium to staging"):

Hi Ian,

Thank you for the detailed e-mail.

On 11/01/2017 02:07 PM, Ian Jackson wrote:

Furthermore, the test is not intermittent, so a force push will be
effective in the following sense: we would only get a "spurious" pass,
resulting in the relevant osstest branch becoming stuck again, if a
future test was unlucky and got an unaffected host.  That will happen
infrequently enough.

...

I am not entirely sure to understand this paragraph. Are you saying that
osstest will not get stuck if we get a "spurious" pass on some hardware
in the future? Or will we need another force push?


osstest *would* get stuck *if* we got such a spurious push.  However,
because osstest likes to retest failing tests on the same host as they
failed on previously, such spurious passes are fairly unlikely.

I say "likes to".  The allocation system uses a set of heuristics to
calculate a score for each possible host.  The score takes into
account both when the host will be available to this job, and
information like "did the most recent run of this test, on this host,
pass or fail".  So I can't make guarantees but the amount of manual
work to force push stuck branches will be tolerable.


Thank you for the explanation. I agree with the force push to unblock 
master (and other tree I mentioned).


However, it would still be nice to find the root causes of this bug and 
fix it.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] Commit moratorium to staging

[Julien Grall]

Hi Ian,

Thank you for the detailed e-mail.

On 11/01/2017 02:07 PM, Ian Jackson wrote:

So, investigations (mostly by Roger, and also a bit of archaeology in
the osstest db by me) have determined:

* This bug is 100% reproducible on affected hosts.  The repro is
   to boot the Windows guest, save/restore it, then migrate it,
   then shut down.  (This is from an IRL conversation with Roger and
   may not be 100% accurate.  Roger, please correct me.)

* Affected hosts differ from unaffected hosts according to cpuid.
   Roger has repro'd the bug on an unaffected host by masking out
   certain cpuid bits.  There are 6 implicated bits and he is working
   to narrow that down.

* It seems likely that this is therefore a real bug.  Maybe in Xen and
   perhaps indeed one that should indeed be a release blocker.

* But this is not a regresson between master and staging.  It affects
   many osstest branches apparently equally.

* This test is, effectively, new: before the osstest change
   "HostDiskRoot: bump to 20G", these jobs would always fail earlier
   and the affected step would not be run.

* The passes we got on various osstest branches before were just
   because those branches hadn't tested on an affected host yet.  As
   branches test different hosts, they will stick on affected hosts.

ISTM that this situation would therefore justify a force push.  We
have established that this bug is very unlikely to be anything to do
with the commits currently blocked by the failing pushes.

Furthermore, the test is not intermittent, so a force push will be
effective in the following sense: we would only get a "spurious" pass,
resulting in the relevant osstest branch becoming stuck again, if a
future test was unlucky and got an unaffected host.  That will happen
infrequently enough.
I am not entirely sure to understand this paragraph. Are you saying that 
osstest will not get stuck if we get a "spurious" pass on some hardware

in the future? Or will we need another force push?



So unless anyone objects (and for xen.git#master, with Julien's
permission), I intend to force push all affected osstest branches when
the test report shows the only blockage is ws16 and/or win10 tests
failing the "guest-stop" step.


This is not only blocking xen.git#master but also blocking other trees:
- linux-linus
    - linux-4.9

Cheers,

--
Julien Grall

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

[Xen-devel] [PATCH for-4.10] xen/x86: p2m-pod: Prevent infinite loop when shattering 1GB pages

[Julien Grall]

The PoD subsystem only have pool of 4KB and 2MB pages. When it comes
accross a 1GB mapping, it will be splitted in 2MB one using
p2m_set_entry and request the caller to retry (see ept_get_entry for
instance).

p2m_set_entry may fail to shatter if it is not possible to
allocate memory for the new page table. However, the error is not
progated resulting to the callers to retry infinitely the PoD.

Prevent the infinite loop by return false when it is not possible to
shatter the 1GB mapping.

Signed-off-by: Julien Grall <julien.gr...@linaro.org>

---

This is a potential candidate to backport and for Xen 4.10. Without
it, there a potential for infinite loop if the memory is exhausted.
---
 xen/arch/x86/mm/p2m-pod.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index 0a811ccf28..69269a0bd1 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -1103,6 +1103,8 @@ p2m_pod_demand_populate(struct p2m_domain *p2m, gfn_t gfn,
  */
 if ( order == PAGE_ORDER_1G )
 {
+int rc;
+
 pod_unlock(p2m);
 /*
  * Note that we are supposed to call p2m_set_entry() 512 times to
@@ -1113,9 +1115,9 @@ p2m_pod_demand_populate(struct p2m_domain *p2m, gfn_t gfn,
  * NOTE: In a fine-grained p2m locking scenario this operation
  * may need to promote its locking from gfn->1g superpage
  */
-p2m_set_entry(p2m, gfn_aligned, INVALID_MFN, PAGE_ORDER_2M,
-  p2m_populate_on_demand, p2m->default_access);
-return true;
+rc = p2m_set_entry(p2m, gfn_aligned, INVALID_MFN, PAGE_ORDER_2M,
+   p2m_populate_on_demand, p2m->default_access);
+return !rc;
 }
 
 /* Only reclaim if we're in actual need of more cache. */
-- 
2.11.0


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

[Xen-devel] [PATCH v3 for-next 2/4] xen/arm32: mm: Rework is_xen_heap_page to avoid nameclash

[Julien Grall]

The arm32 version of the function is_xen_heap_page currently define a
variable _mfn. This will lead to a compiler when use typesafe MFN in a
follow-up patch:

called object '_mfn' is not a function or function pointer

Fix it by renaming the local variable _mfn to mfn_.

Signed-off-by: Julien Grall <julien.gr...@linaro.org>

---

Cc: Stefano Stabellini <sstabell...@kernel.org>

Changes in v3:
- Fix typo in the commit message
---
 xen/include/asm-arm/mm.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index cd6dfb54b9..737a429409 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -140,9 +140,9 @@ extern vaddr_t xenheap_virt_start;
 #ifdef CONFIG_ARM_32
 #define is_xen_heap_page(page) is_xen_heap_mfn(page_to_mfn(page))
 #define is_xen_heap_mfn(mfn) ({ \
-unsigned long _mfn = (mfn); \
-(_mfn >= mfn_x(xenheap_mfn_start) &&\
- _mfn < mfn_x(xenheap_mfn_end));\
+unsigned long mfn_ = (mfn); \
+(mfn_ >= mfn_x(xenheap_mfn_start) &&\
+ mfn_ < mfn_x(xenheap_mfn_end));\
 })
 #else
 #define is_xen_heap_page(page) ((page)->count_info & PGC_xen_heap)
-- 
2.11.0


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

[Xen-devel] [PATCH v3 for-next 0/4] xen: Convert __page_to_mfn and _mfn_to_page to use typesafe MFN

[Julien Grall]

Hi all,

Most of the users of page_to_mfn and mfn_to_page are either overriding
the macros to make them work with mfn_t or use mfn_x/_mfn becaue the rest
of the function use mfn_t.

So I think it is time to make __page_to_mfn and __mfn_to_page using typesafe
MFN.

The first 3 patches will convert of the code to use typesafe MFN, easing
the tree-wide conversion in patch 4.

Note that this was only build tested it on x86.

Cheers,

Cc: Andrew Cooper <andrew.coop...@citrix.com>
Cc: Boris Ostrovsky <boris.ostrov...@oracle.com>
Cc: Gang Wei <gang@intel.com>
Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Julien Grall <julien.gr...@arm.com>
Cc: Jun Nakajima <jun.nakaj...@intel.com>
Cc: Kevin Tian <kevin.t...@intel.com>
Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Cc: Paul Durrant <paul.durr...@citrix.com>
Cc: Razvan Cojocaru <rcojoc...@bitdefender.com>
Cc: Shane Wang <shane.w...@intel.com>
Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
Cc: Tamas K Lengyel <ta...@tklengyel.com>
Cc: Tim Deegan <t...@xen.org>
Cc: Wei Liu <wei.l...@citrix.com>

Julien Grall (4):
  xen/arm: domain_build: Clean-up insert_11_bank
  xen/arm32: mm: Rework is_xen_heap_page to avoid nameclash
  xen/tmem: Convert the file common/tmem_xen.c to use typesafe MFN
  xen: Convert __page_to_mfn and __mfn_to_page to use typesafe MFN

 xen/arch/arm/domain_build.c | 15 ---
 xen/arch/arm/kernel.c   |  2 +-
 xen/arch/arm/mem_access.c   |  2 +-
 xen/arch/arm/mm.c   |  8 
 xen/arch/arm/p2m.c  | 10 ++
 xen/arch/x86/cpu/vpmu.c |  4 ++--
 xen/arch/x86/domain.c   | 21 +++--
 xen/arch/x86/domain_page.c  |  6 +++---
 xen/arch/x86/domctl.c   |  2 +-
 xen/arch/x86/hvm/dm.c   |  2 +-
 xen/arch/x86/hvm/dom0_build.c   |  6 +++---
 xen/arch/x86/hvm/emulate.c  |  6 +++---
 xen/arch/x86/hvm/hvm.c  | 16 
 xen/arch/x86/hvm/ioreq.c|  6 +++---
 xen/arch/x86/hvm/stdvga.c   |  2 +-
 xen/arch/x86/hvm/svm/svm.c  |  4 ++--
 xen/arch/x86/hvm/viridian.c |  6 +++---
 xen/arch/x86/hvm/vmx/vmcs.c |  2 +-
 xen/arch/x86/hvm/vmx/vmx.c  | 10 +-
 xen/arch/x86/hvm/vmx/vvmx.c |  6 +++---
 xen/arch/x86/mm.c   |  6 --
 xen/arch/x86/mm/guest_walk.c|  6 +++---
 xen/arch/x86/mm/hap/guest_walk.c|  2 +-
 xen/arch/x86/mm/hap/hap.c   |  6 --
 xen/arch/x86/mm/hap/nested_ept.c|  2 +-
 xen/arch/x86/mm/mem_sharing.c   |  5 -
 xen/arch/x86/mm/p2m-ept.c   |  4 
 xen/arch/x86/mm/p2m-pod.c   |  6 --
 xen/arch/x86/mm/p2m.c   |  6 --
 xen/arch/x86/mm/paging.c|  6 --
 xen/arch/x86/mm/shadow/private.h| 16 ++--
 xen/arch/x86/numa.c |  2 +-
 xen/arch/x86/physdev.c  |  2 +-
 xen/arch/x86/pv/callback.c  |  6 --
 xen/arch/x86/pv/descriptor-tables.c | 10 --
 xen/arch/x86/pv/dom0_build.c|  6 ++
 xen/arch/x86/pv/domain.c|  6 --
 xen/arch/x86/pv/emul-gate-op.c  |  6 --
 xen/arch/x86/pv/emul-priv-op.c  | 10 --
 xen/arch/x86/pv/grant_table.c   |  6 --
 xen/arch/x86/pv/ro-page-fault.c |  6 --
 xen/arch/x86/smpboot.c  |  6 --
 xen/arch/x86/tboot.c|  4 ++--
 xen/arch/x86/traps.c|  4 ++--
 xen/arch/x86/x86_64/mm.c|  6 ++
 xen/common/domain.c |  4 ++--
 xen/common/grant_table.c|  6 ++
 xen/common/kimage.c |  6 --
 xen/common/memory.c |  6 ++
 xen/common/page_alloc.c |  6 ++
 xen/common/tmem.c   |  2 +-
 xen/common/tmem_xen.c   | 26 ++
 xen/common/trace.c  |  6 ++
 xen/common/vmap.c   |  9 +
 xen/common/xenoprof.c   |  2 --
 xen/drivers/passthrough/amd/iommu_map.c |  6 ++
 xen/drivers/passthrough/iommu.c |  2 +-
 xen/drivers/passthrough/x86/iommu.c |  2 +-
 xen/include/asm-arm/mm.h| 22 --
 xen/include/asm-arm/p2m.h   |  4 ++--
 xen/include/asm-x86/mm.h| 12 ++--
 xen/include/asm-x86/p2m.h   |  2 +-
 xen/include/asm-x86/page.h  | 32 +

[Xen-devel] [PATCH v3 for-next 4/4] xen: Convert __page_to_mfn and __mfn_to_page to use typesafe MFN

[Julien Grall]

Most of the users of page_to_mfn and mfn_to_page are either overriding
the macros to make them work with mfn_t or use mfn_x/_mfn because the
rest of the function use mfn_t.

So make __page_to_mfn and __mfn_to_page return mfn_t by default.

Only reasonable clean-ups are done in this patch because it is
already quite big. So some of the files now override page_to_mfn and
mfn_to_page to avoid using mfn_t.

Lastly, domain_page_to_mfn is also converted to use mfn_t given that
most of the callers are now switched to _mfn(domain_page_to_mfn(...)).

Signed-off-by: Julien Grall <julien.gr...@linaro.org>

---

Andrew suggested to drop IS_VALID_PAGE in xen/tmem_xen.h. His comment
was:

"/sigh  This is tautological.  The definition of a "valid mfn" in this
case is one for which we have frametable entry, and by having a struct
page_info in our hands, this is by definition true (unless you have a
wild pointer, at which point your bug is elsewhere).

IS_VALID_PAGE() is only ever used in assertions and never usefully, so
instead I would remove it entirely rather than trying to fix it up."

I can remove the function in a separate patch at the begining of the
series if Konrad (TMEM maintainer) is happy with that.

Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Julien Grall <julien.gr...@arm.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Cc: Tim Deegan <t...@xen.org>
Cc: Wei Liu <wei.l...@citrix.com>
Cc: Razvan Cojocaru <rcojoc...@bitdefender.com>
Cc: Tamas K Lengyel <ta...@tklengyel.com>
Cc: Paul Durrant <paul.durr...@citrix.com>
Cc: Boris Ostrovsky <boris.ostrov...@oracle.com>
Cc: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
Cc: Jun Nakajima <jun.nakaj...@intel.com>
Cc: Kevin Tian <kevin.t...@intel.com>
Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Gang Wei <gang@intel.com>
Cc: Shane Wang <shane.w...@intel.com>

Changes in v3:
- Rebase on the latest staging and fix some conflicts. Tags
haven't be retained.
- Switch the printf format to PRI_mfn

Changes in v2:
- Some part have been moved in separate patch
- Remove one spurious comment
- Convert domain_page_to_mfn to use mfn_t
---
 xen/arch/arm/domain_build.c |  2 --
 xen/arch/arm/kernel.c   |  2 +-
 xen/arch/arm/mem_access.c   |  2 +-
 xen/arch/arm/mm.c   |  8 
 xen/arch/arm/p2m.c  | 10 ++
 xen/arch/x86/cpu/vpmu.c |  4 ++--
 xen/arch/x86/domain.c   | 21 +++--
 xen/arch/x86/domain_page.c  |  6 +++---
 xen/arch/x86/domctl.c   |  2 +-
 xen/arch/x86/hvm/dm.c   |  2 +-
 xen/arch/x86/hvm/dom0_build.c   |  6 +++---
 xen/arch/x86/hvm/emulate.c  |  6 +++---
 xen/arch/x86/hvm/hvm.c  | 16 
 xen/arch/x86/hvm/ioreq.c|  6 +++---
 xen/arch/x86/hvm/stdvga.c   |  2 +-
 xen/arch/x86/hvm/svm/svm.c  |  4 ++--
 xen/arch/x86/hvm/viridian.c |  6 +++---
 xen/arch/x86/hvm/vmx/vmcs.c |  2 +-
 xen/arch/x86/hvm/vmx/vmx.c  | 10 +-
 xen/arch/x86/hvm/vmx/vvmx.c |  6 +++---
 xen/arch/x86/mm.c   |  6 --
 xen/arch/x86/mm/guest_walk.c|  6 +++---
 xen/arch/x86/mm/hap/guest_walk.c|  2 +-
 xen/arch/x86/mm/hap/hap.c   |  6 --
 xen/arch/x86/mm/hap/nested_ept.c|  2 +-
 xen/arch/x86/mm/mem_sharing.c   |  5 -
 xen/arch/x86/mm/p2m-ept.c   |  4 
 xen/arch/x86/mm/p2m-pod.c   |  6 --
 xen/arch/x86/mm/p2m.c   |  6 --
 xen/arch/x86/mm/paging.c|  6 --
 xen/arch/x86/mm/shadow/private.h| 16 ++--
 xen/arch/x86/numa.c |  2 +-
 xen/arch/x86/physdev.c  |  2 +-
 xen/arch/x86/pv/callback.c  |  6 --
 xen/arch/x86/pv/descriptor-tables.c | 10 --
 xen/arch/x86/pv/dom0_build.c|  6 ++
 xen/arch/x86/pv/domain.c|  6 --
 xen/arch/x86/pv/emul-gate-op.c  |  6 --
 xen/arch/x86/pv/emul-priv-op.c  | 10 --
 xen/arch/x86/pv/grant_table.c   |  6 --
 xen/arch/x86/pv/ro-page-fault.c |  6 --
 xen/arch/x86/smpboot.c  |  6 --
 xen/arch/x86/tboot.c|  4 ++--
 xen/arch/x86/traps.c|  4 ++--
 xen/arch/x86/x86_64/mm.c|  6 ++
 xen/common/domain.c |  4 ++--
 xen/common/grant_table.c| 

[Xen-devel] [PATCH v3 for-next 3/4] xen/tmem: Convert the file common/tmem_xen.c to use typesafe MFN

[Julien Grall]

The file common/tmem_xen.c is now converted to use typesafe. This is
requiring to override the macro page_to_mfn to make it work with mfn_t.

Note that all variables converted to mfn_t havem there initial value,
when set, switch from 0 to INVALID_MFN. This is fine because the initial
values was always overriden before used.

Also add a couple of missing newlines suggested by Andrew in the code.

Signed-off-by: Julien Grall <julien.gr...@linaro.org>
Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>

---

Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>

Changes in v2:
- Add missing newlines
- Add Andrew's reviewed-by
---
 xen/common/tmem_xen.c | 30 ++
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/xen/common/tmem_xen.c b/xen/common/tmem_xen.c
index 20f74b268f..bd52e44faf 100644
--- a/xen/common/tmem_xen.c
+++ b/xen/common/tmem_xen.c
@@ -14,6 +14,10 @@
 #include 
 #include 
 
+/* Override macros from asm/page.h to make them work with mfn_t */
+#undef page_to_mfn
+#define page_to_mfn(pg) _mfn(__page_to_mfn(pg))
+
 bool __read_mostly opt_tmem;
 boolean_param("tmem", opt_tmem);
 
@@ -31,7 +35,7 @@ static DEFINE_PER_CPU_READ_MOSTLY(unsigned char *, dstmem);
 static DEFINE_PER_CPU_READ_MOSTLY(void *, scratch_page);
 
 #if defined(CONFIG_ARM)
-static inline void *cli_get_page(xen_pfn_t cmfn, unsigned long *pcli_mfn,
+static inline void *cli_get_page(xen_pfn_t cmfn, mfn_t *pcli_mfn,
  struct page_info **pcli_pfp, bool cli_write)
 {
 ASSERT_UNREACHABLE();
@@ -39,14 +43,14 @@ static inline void *cli_get_page(xen_pfn_t cmfn, unsigned 
long *pcli_mfn,
 }
 
 static inline void cli_put_page(void *cli_va, struct page_info *cli_pfp,
-unsigned long cli_mfn, bool mark_dirty)
+mfn_t cli_mfn, bool mark_dirty)
 {
 ASSERT_UNREACHABLE();
 }
 #else
 #include 
 
-static inline void *cli_get_page(xen_pfn_t cmfn, unsigned long *pcli_mfn,
+static inline void *cli_get_page(xen_pfn_t cmfn, mfn_t *pcli_mfn,
  struct page_info **pcli_pfp, bool cli_write)
 {
 p2m_type_t t;
@@ -68,16 +72,17 @@ static inline void *cli_get_page(xen_pfn_t cmfn, unsigned 
long *pcli_mfn,
 
 *pcli_mfn = page_to_mfn(page);
 *pcli_pfp = page;
-return map_domain_page(_mfn(*pcli_mfn));
+
+return map_domain_page(*pcli_mfn);
 }
 
 static inline void cli_put_page(void *cli_va, struct page_info *cli_pfp,
-unsigned long cli_mfn, bool mark_dirty)
+mfn_t cli_mfn, bool mark_dirty)
 {
 if ( mark_dirty )
 {
 put_page_and_type(cli_pfp);
-paging_mark_dirty(current->domain, _mfn(cli_mfn));
+paging_mark_dirty(current->domain, cli_mfn);
 }
 else
 put_page(cli_pfp);
@@ -88,14 +93,14 @@ static inline void cli_put_page(void *cli_va, struct 
page_info *cli_pfp,
 int tmem_copy_from_client(struct page_info *pfp,
 xen_pfn_t cmfn, tmem_cli_va_param_t clibuf)
 {
-unsigned long tmem_mfn, cli_mfn = 0;
+mfn_t tmem_mfn, cli_mfn = INVALID_MFN;
 char *tmem_va, *cli_va = NULL;
 struct page_info *cli_pfp = NULL;
 int rc = 1;
 
 ASSERT(pfp != NULL);
 tmem_mfn = page_to_mfn(pfp);
-tmem_va = map_domain_page(_mfn(tmem_mfn));
+tmem_va = map_domain_page(tmem_mfn);
 if ( guest_handle_is_null(clibuf) )
 {
 cli_va = cli_get_page(cmfn, _mfn, _pfp, 0);
@@ -125,7 +130,7 @@ int tmem_compress_from_client(xen_pfn_t cmfn,
 unsigned char *wmem = this_cpu(workmem);
 char *scratch = this_cpu(scratch_page);
 struct page_info *cli_pfp = NULL;
-unsigned long cli_mfn = 0;
+mfn_t cli_mfn = INVALID_MFN;
 void *cli_va = NULL;
 
 if ( dmem == NULL || wmem == NULL )
@@ -152,7 +157,7 @@ int tmem_compress_from_client(xen_pfn_t cmfn,
 int tmem_copy_to_client(xen_pfn_t cmfn, struct page_info *pfp,
 tmem_cli_va_param_t clibuf)
 {
-unsigned long tmem_mfn, cli_mfn = 0;
+mfn_t tmem_mfn, cli_mfn = INVALID_MFN;
 char *tmem_va, *cli_va = NULL;
 struct page_info *cli_pfp = NULL;
 int rc = 1;
@@ -165,7 +170,8 @@ int tmem_copy_to_client(xen_pfn_t cmfn, struct page_info 
*pfp,
 return -EFAULT;
 }
 tmem_mfn = page_to_mfn(pfp);
-tmem_va = map_domain_page(_mfn(tmem_mfn));
+tmem_va = map_domain_page(tmem_mfn);
+
 if ( cli_va )
 {
 memcpy(cli_va, tmem_va, PAGE_SIZE);
@@ -181,7 +187,7 @@ int tmem_copy_to_client(xen_pfn_t cmfn, struct page_info 
*pfp,
 int tmem_decompress_to_client(xen_pfn_t cmfn, void *tmem_va,
 size_t size, tmem_cli_va_param_t clibuf)
 {
-unsigned long cli_mfn = 0;
+mfn_t cli_mfn = INVALID_MFN;
 struct page_info *cli_pfp = NULL;
 void *cli_va = NULL;
 char *scratch = this_cpu(scratch_page);
-- 
2.11.0


__

[Xen-devel] [PATCH v3 for-next 1/4] xen/arm: domain_build: Clean-up insert_11_bank

[Julien Grall]

- Remove spurious ()
- Add missing spaces
- Turn 1 << to 1UL <<
- Rename spfn to smfn and switch to mfn_t

Signed-off-by: Julien Grall <julien.gr...@linaro.org>

---

Cc: Stefano Stabellini <sstabell...@kernel.org>

Changes in v2:
- Remove double space
- s/spfn/smfn/ and switch to mfn_t
---
 xen/arch/arm/domain_build.c | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index bf29299707..5532068ab1 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -50,6 +50,8 @@ struct map_range_data
 /* Override macros from asm/page.h to make them work with mfn_t */
 #undef virt_to_mfn
 #define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
+#undef page_to_mfn
+#define page_to_mfn(pg) _mfn(__page_to_mfn(pg))
 
 //#define DEBUG_11_ALLOCATION
 #ifdef DEBUG_11_ALLOCATION
@@ -104,16 +106,16 @@ static bool insert_11_bank(struct domain *d,
unsigned int order)
 {
 int res, i;
-paddr_t spfn;
+mfn_t smfn;
 paddr_t start, size;
 
-spfn = page_to_mfn(pg);
-start = pfn_to_paddr(spfn);
-size = pfn_to_paddr((1 << order));
+smfn = page_to_mfn(pg);
+start = mfn_to_maddr(smfn);
+size = pfn_to_paddr(1UL << order);
 
 D11PRINT("Allocated %#"PRIpaddr"-%#"PRIpaddr" (%ldMB/%ldMB, order %d)\n",
  start, start + size,
- 1UL << (order+PAGE_SHIFT-20),
+ 1UL << (order + PAGE_SHIFT - 20),
  /* Don't want format this as PRIpaddr (16 digit hex) */
  (unsigned long)(kinfo->unassigned_mem >> 20),
  order);
@@ -126,7 +128,7 @@ static bool insert_11_bank(struct domain *d,
 goto fail;
 }
 
-res = guest_physmap_add_page(d, _gfn(spfn), _mfn(spfn), order);
+res = guest_physmap_add_page(d, _gfn(mfn_x(smfn)), smfn, order);
 if ( res )
 panic("Failed map pages to DOM0: %d", res);
 
@@ -167,7 +169,8 @@ static bool insert_11_bank(struct domain *d,
  */
 if ( start + size < bank->start && kinfo->mem.nr_banks < NR_MEM_BANKS )
 {
-memmove(bank + 1, bank, sizeof(*bank)*(kinfo->mem.nr_banks - i));
+memmove(bank + 1, bank,
+sizeof(*bank) * (kinfo->mem.nr_banks - i));
 kinfo->mem.nr_banks++;
 bank->start = start;
 bank->size = size;
-- 
2.11.0


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

Re: [Xen-devel] [RFC] [Draft Design] ACPI/IORT Support in Xen.

[Julien Grall]

Hi Manish,

On 10/31/2017 12:05 PM, Manish Jaggi wrote:

On 10/27/2017 7:35 PM, Andre Przywara wrote:

When PCI device passthrough is supported, the PCIRC is itself virtual
(emulated by Xen).
One can have any number of virtual PCIRC  and may be virtual SMMUs.
Hence the topology can vary.

I think I don't disagree, my initial comment was just about the
confusion that this "IORT topology is *different* from" term created.

Ok, I will move it in a different section and remove the term "different".



Now read the below lines.

At a minimum domU IORT should include a single PCIRC and ITS Group.
Similar PCIRC can be added in DSDT.
Additional node can be added if platform device is assigned to domU.
No extra node should be required for PCI device pass-through.

Again I don't fully understand this last sentence.

The last line is continuation of the first line "At a minimum..."

OK, but still I don't get how we would end up with an IORT without
(pass-throughed) PCI devices in the first place?

If hypothetically a platform device uses MSI.


I would move out of the equation platform device passthrough. Most of 
use case I am aware is around embedded and controlled environment. It 
would be difficult to provide a generic way for Server.


To give a bit more details, when using device-tree the user needs to 
provide a partial device-tree describing the device passthrough. For 
ACPI, you would need to do the same but with DSDT.



I will let Sameer comment on it.
Our platform does not have a Named Component node in IORT.


[...]


6. IORT Generation
---
There would be a common code to generate IORT table from
iort_table_struct.

That sounds useful, but we would need to be careful with sharing code
between Xen and the tool stack. Has this actually been done before?

I added the code sharing part here, but I am not hopeful that this would
work as it would require lot of code change on toolstack.
A simple difference is that the acpi header structures have different
member variables. This is same for other structures.
So we might have to create a lot of defines in common code for sharing
and possibility of errors.

See: struct acpi_header in acpi2_0.h (tools/libacpi)
and struct acpi_table_header in actbl.h (xen/include/acpi)
What do you think about this difference in basic structures in toolstack 
and xen code.
When we write a common library should I include a #define for mapping 
xen structure to toolstack.
Would it have more overhead than duplication, that is an implementation 
issue.

That is why I preferred a domctl, so xen coud prepare IORT for DomU.

I don't this it's justified to move a simple table generation task
into Xen, just to allow code sharing. After all this does not require
any Xen internal knowledge. So it should be done definitely in the
toolstack.

Yes. Fully agree.
The point here is duplication or code reuse.
See above.
Can we please focus on what matters, i.e what is necessary from an 
high-level perspective to support IORT in the hypervisor.


We can discuss about code sharing/duplication when we get to the support 
IORT for the guests.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC] [Draft Design] ACPI/IORT Support in Xen.

[Julien Grall]

Hi Sameer,

On 10/30/2017 11:33 PM, Goel, Sameer wrote:

On 10/12/2017 3:03 PM, Manish Jaggi wrote:

5. Parsing of IORT in Xen
--

I think a Linux like approach will solve the following use cases:
1. Identify the SMMU devices and initialize the devices as needed.
2. API function to setup SMMUs in response to a discovery notification from DOM0
- We will still need a path for non pcie devices.
- I agree with Andre that the use cases for the named nodes in IORT should 
be treated the same as PCIe RC devices.
3. The concept of fwnode is still valid as per 4.14 and we can try reuse most 
of the parsing code.

Manish, I looked at your old patch and had a couple of questions before I 
comment more on this design. From an initial
glance, it seems that you should be able to hide SMMUs by calling the already 
defined API functions in the iort.c implementation
(for most part :)).

I am wondering if we really need to keep a list of parsed nodes. Or which use 
case apart from hw dom IORT mandates this?
I want to see the parsing separated from the generation. This means we 
need a list of parsed nodes for the IORT.


As we have them in hand, I was thinking to re-use them than lookup the IORT.

Cheers,

--
Julien Grall

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

[Xen-devel] Commit moratorium to staging

[Julien Grall]

Hi all,

Master lags 15 days behind staging due to tests failing reliably on some 
of the hardware in osstest (see [1]).


At the moment a force push is not feasible because the same tests passes 
on different hardware (see [2]).


Please avoid committing any more patches unless it is fixing a test 
failure in osstest.


Tree will be re-opened once we get a push.

Cheers,

[1] 
https://lists.xenproject.org/archives/html/xen-devel/2017-10/msg03351.html
[2] 
https://lists.xenproject.org/archives/html/xen-devel/2017-10/msg02932.html


--
Julien Grall

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

Re: [Xen-devel] [PATCH v12 05/11] x86/mm: add HYPERVISOR_memory_op to acquire guest resources

[Julien Grall]

Hi Paul,

On 27/10/17 16:19, Paul Durrant wrote:

-Original Message-
From: Julien Grall [mailto:julien.gr...@linaro.org]
Sent: 27 October 2017 12:46
To: Jan Beulich <jbeul...@suse.com>; Paul Durrant
<paul.durr...@citrix.com>
Cc: Julien Grall <julien.gr...@arm.com>; Andrew Cooper
<andrew.coop...@citrix.com>; Wei Liu <wei.l...@citrix.com>; George
Dunlap <george.dun...@citrix.com>; Ian Jackson <ian.jack...@citrix.com>;
Stefano Stabellini <sstabell...@kernel.org>; xen-de...@lists.xenproject.org;
Konrad Rzeszutek Wilk <konrad.w...@oracle.com>; Daniel De Graaf
<dgde...@tycho.nsa.gov>; Tim (Xen.org) <t...@xen.org>
Subject: Re: [Xen-devel] [PATCH v12 05/11] x86/mm: add
HYPERVISOR_memory_op to acquire guest resources

Hi,

On 26/10/17 16:39, Jan Beulich wrote:

On 26.10.17 at 17:32, <julien.gr...@linaro.org> wrote:

On 26/10/17 16:26, Jan Beulich wrote:

On 17.10.17 at 15:24, <paul.durr...@citrix.com> wrote:

+/* IN/OUT - If the tools domain is PV then, upon return, frame_list
+ *  will be populated with the MFNs of the resource.
+ *  If the tools domain is HVM then it is expected that, on
+ *  entry, frame_list will be populated with a list of GFNs
+ *  that will be mapped to the MFNs of the resource.
+ *  If -EIO is returned then the frame_list has only been
+ *  partially mapped and it is up to the caller to unmap all
+ *  the GFNs.
+ *  This parameter may be NULL if nr_frames is 0.
+ */
+XEN_GUEST_HANDLE(xen_ulong_t) frame_list;


This is still xen_ulong_t, which I can live with, but then you shouldn't
copy into / out of arrays of other types in acquire_resource() (the
more that this is common code, and iirc xen_ulong_t and
unsigned long aren't the same thing on ARM32).


xen_ulong_t is always 64-bit on Arm (32-bit and 64-bit). But shouldn't
we use xen_pfn_t here?


I had put this question up earlier, but iirc Paul didn't like it.


I'd like to understand why Paul doesn't like it. We should never assume
that a frame fit in xen_ulong_t. xen_pfn_t was exactly introduced for
that purpose.


My reservation is whether xen_pfn_t is intended to hold either gfns or mfns, 
since this hypercall uses the same array for both. If it suitable then I am 
happy to change it, but Andrew led me to believe otherwise.


Looking at the public hearders, xen_pfn_t is been used for both MFN (see 
xenpf_add_memtype) and GFN (see gnttab_setup_table).


So I think it would be fine to do the same here.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC 0/4] TEE mediator framework + OP-TEE mediator

[Julien Grall]

On 27 Oct 2017 20:59, "Stefano Stabellini" <sstabell...@kernel.org> wrote:

On Fri, 27 Oct 2017, Julien Grall wrote:
> Hi,
>
> Just answering to dom0 been 1:1 domain.
>
> On 24/10/17 22:33, Stefano Stabellini wrote:
> > On Tue, 24 Oct 2017, Volodymyr Babchuk wrote:
> > > > For this series, I think we need a way to specify which domains can
talk
> > > > to TEE, so that we can only allow it for a specific subset of
DomUs. I
> > > > would probably use XSM for that.
> > > I am afraid, this is not possible. As other domains aren't 1:1 mapped,
> > > I need to have special translation code in mediator. Actually, I'm
> > > writing it rigth now to test my changes in OP-TEE. But event this is
> > > not enought for decent OP-TEE support.
> > > What can be done right now: 100% Dom0-only support with vanilla
> > > OP-TEE (i.e. no virtualization support in OP-TEE is needed). This is
> > > even simplier task, so I can throw out some code from this patch
> > > series. On other hand, in the future this will lead to sutiation when
> > > two mediators for the same TEE shall be supported: one, simple, in
> > > XEN, another, fully-functional in stubdom.
> >
> > I think it is fine to support OP-TEE only in Dom0 to begin with.
> >
> > Ideally, it would be in Dom0 for convenience and speed and the OP-TEE
> > capability would be specified as an XSM label. Ideally, it would not be
> > only in Dom0 because it is tied to the 1:1 map, but I understand now
> > that it is a requirement. I still think that the XSM label would be good
> > to have even if today it cannot be changed as only Dom0 is 1:1.
>
> I thought a bit more about Dom0 been a 1:1 domain. It is only true for
Device
> Memory and the initial RAM allocated for Dom0.
>
> Dom0 may balloon out some pages because it has to map region belonging to
> other domain. Those regions will not be 1:1 mapped and translation will be
> needed if used.
>
> The problem is very similar to DMA in dom0. I can't see any reason to not
use
> those regions with OP-TEE. Am I wrong here?

I think you are right. For DMA, Dom0 is expected to use the swiotlb-xen
driver to solve the problem, because it is a genuine use case to have
foreign grants involved in a DMA operation.

For OP-TEE, I don't think we need to support this case? Xen could fail
the request if it involves a page that is not 1:1 mapped?


You would need to introspect the message in order to know that. So
supporting non 1:1 mapped page would not be more difficult.

This assuming that you know when you OP-TEE is done with the page.

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

Re: [Xen-devel] [PATCH for-4.10 1/9] gcov: return ENOSYS for unimplemented gcov domctl

[Julien Grall]

Hi,

On 27/10/17 14:59, Ian Jackson wrote:

Roger Pau Monne writes ("[PATCH for-next 1/9] gcov: return ENOSYS for unimplemented 
gcov domctl"):

Signed-off-by: Roger Pau Monné <roger@citrix.com>

...

  default:
-ret = -EINVAL;
+ret = -ENOSYS;
  break;
  }


Reviewed-by: Ian Jackson <ian.jack...@eu.citrix.com>

I think this is a bugfix which should go into 4.10.  Julien ?
(Subject line changed by me.)


Yes I agree.

Release-acked-by: Julien Grall <julien.gr...@linaro.org>

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC 0/4] TEE mediator framework + OP-TEE mediator

[Julien Grall]

Hi,

Just answering to dom0 been 1:1 domain.

On 24/10/17 22:33, Stefano Stabellini wrote:

On Tue, 24 Oct 2017, Volodymyr Babchuk wrote:

For this series, I think we need a way to specify which domains can talk
to TEE, so that we can only allow it for a specific subset of DomUs. I
would probably use XSM for that.

I am afraid, this is not possible. As other domains aren't 1:1 mapped,
I need to have special translation code in mediator. Actually, I'm
writing it rigth now to test my changes in OP-TEE. But event this is
not enought for decent OP-TEE support.
What can be done right now: 100% Dom0-only support with vanilla
OP-TEE (i.e. no virtualization support in OP-TEE is needed). This is
even simplier task, so I can throw out some code from this patch
series. On other hand, in the future this will lead to sutiation when
two mediators for the same TEE shall be supported: one, simple, in
XEN, another, fully-functional in stubdom.


I think it is fine to support OP-TEE only in Dom0 to begin with.

Ideally, it would be in Dom0 for convenience and speed and the OP-TEE
capability would be specified as an XSM label. Ideally, it would not be
only in Dom0 because it is tied to the 1:1 map, but I understand now
that it is a requirement. I still think that the XSM label would be good
to have even if today it cannot be changed as only Dom0 is 1:1.


I thought a bit more about Dom0 been a 1:1 domain. It is only true for 
Device Memory and the initial RAM allocated for Dom0.


Dom0 may balloon out some pages because it has to map region belonging 
to other domain. Those regions will not be 1:1 mapped and translation 
will be needed if used.


The problem is very similar to DMA in dom0. I can't see any reason to 
not use those regions with OP-TEE. Am I wrong here?


Cheers,

--
Julien Grall

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