[Xen-devel] [PATCH] xen/x86: fix paging_max_paddr_bits()

[Juergen Gross]

paging_max_paddr_bits() has an invalid use of IS_ENABLED(): instead of
IS_ENABLED(CONFIG_BIGMEM) it is using IS_ENABLED(BIGMEM). Fix that.

Signed-off-by: Juergen Gross 
---
 xen/include/asm-x86/paging.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/include/asm-x86/paging.h b/xen/include/asm-x86/paging.h
index dd0d6b5159..fdcc22844b 100644
--- a/xen/include/asm-x86/paging.h
+++ b/xen/include/asm-x86/paging.h
@@ -371,7 +371,8 @@ static inline unsigned int paging_max_paddr_bits(const 
struct domain *d)
 {
 unsigned int bits = paging_mode_hap(d) ? hap_paddr_bits : paddr_bits;
 
-if ( !IS_ENABLED(BIGMEM) && paging_mode_shadow(d) && !is_pv_domain(d) )
+if ( !IS_ENABLED(CONFIG_BIGMEM) && paging_mode_shadow(d) &&
+ !is_pv_domain(d) )
 {
 /* Shadowed superpages store GFNs in 32-bit page_info fields. */
 bits = min(bits, 32U + PAGE_SHIFT);
-- 
2.16.4


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

Re: [Xen-devel] remove the ->mapping_error method from dma_map_ops V2

[Christoph Hellwig]

On Fri, Nov 23, 2018 at 07:55:11AM +0100, Christoph Hellwig wrote:
> On Thu, Nov 22, 2018 at 09:55:25AM -0800, Linus Torvalds wrote:
> > No, the big immediate benefit of allowing "return -EINVAL" etc is
> > simply legibility and error avoidance.
> 
> Well, I can tweak the last patch to return -EINVAL from dma_mapping_error
> instead of the old 1 is as bool true.  The callers should all be fine,
> although I'd have to audit them.  Still wouldn't help with being able to
> return different errors.

Any opinions?  I'd really like to make some forward progress on this
series.

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

Re: [Xen-devel] [PATCH 0/3] xen/x86: support setting dom0_mem depending on host size

[Juergen Gross]

On 27/11/2018 20:33, Stefano Stabellini wrote:
> Hi Juergen,
> 
> I don't mean to scope-creep your series, and I think it is fine if you
> don't want to do it, but it would be fantastic if you took the
> opportunity to make dom0_mem common across architectures.
> 
> On ARM, we parse it in xen/arch/arm/domain_build.c:parse_dom0_mem.
> I think the ARM and x86 implementations should be the same.

This should be done by someone more familiar with the ARM requirements
and internals.

A major difference between ARM and x86 seems to be the default size of
dom0: on ARM it is limited to 512MB, while on x86 it is most of the
host's memory. I guess there is a reason for that difference, but I
don't really know why.


Juergen

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

Re: [Xen-devel] Problems building and running Xen on Hikey960

[Matthew Daley]

On Mon, 26 Nov 2018 at 14:07, Matthew Daley  wrote:
>
> On Fri, 23 Nov 2018 at 09:51, Julien Grall  wrote:
>>
>> Hi Matthew,
>>
>> Sorry for the late answer and thank you for testing the patch.
>>
>> On 11/13/18 10:43 PM, Matthew Daley wrote:
>> > On Tue, 13 Nov 2018 at 02:01, Julien Grall  wrote:
>> >> On 11/11/18 1:15 AM, Matthew Daley wrote:
>> > I gave this a go but unfortunately the same problem occurs (error
>> > -9s). Just to check nothing weird is happening I added a printk to
>> > check the value of __pa(init_secondary) in call_psci_cpu_on, giving
>> > 0xdfe00180.
>>
>> I have posted a patch that disables completely the relocation [1]. Could
>> you have a try and see whether it works now?
>>
>> Cheers,
>>
>> [1] https://lists.xen.org/archives/html/xen-devel/2018-11/msg02638.html
>>
>> --
>> Julien Grall
>
>
> Hi Julien,
>
> Unfortunately with this patch it appears to freeze after the call to 
> switch_ttbr(ttbr).
>
> - Matthew

So looking at setup_pagetables, my understanding is that it sets up a
2MiB mapping for Xen's .text section (before then splitting it into
4KiB pages). However, GRUB is loading Xen in RAM at 0xB8736000 for me.
This isn't 2MiB aligned (and isn't in low memory either!), so I can't
see how this would work; it still requires relocation or a more
granular mapping, no?

- Matthew

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

Re: [Xen-devel] Xen 4.12 Development Update

[Chao Gao]

Hi Juergen,

"Improve late microcode loading" (v4) is expected to be merged in
Xen 4.12. Is it possible to add this series into the task list below?

On Mon, Oct 08, 2018 at 04:44:11PM +0800, Juergen Gross wrote:
>This email only tracks big items for xen.git tree. Please reply for items you
>would like to see in 4.12 so that people have an idea what is going on and
>prioritise accordingly.
>
>You're welcome to provide description and use cases of the feature you're
>working on.
>
>= Timeline =
>
>We now adopt a fixed cut-off date scheme. We will release about every 8 months.
>The upcoming 4.12 timeline are as followed:
>
>* Last posting date: December 14th, 2018
>  [ as this is just before Christmas some maintainers might ask for an
>earlier last posting date if their Ack is needed. ]
>* Hard code freeze: January 11th, 2019
>* RC1: TBD
>* Release: March 7th, 2019
>
>Note that we don't have freeze exception scheme anymore. All patches
>that wish to go into 4.12 must be posted no later than the last posting
>date. All patches posted after that date will be automatically queued
>into next release.
>
>RCs will be arranged immediately after freeze.
>
>We recently introduced a jira instance to track all the tasks (not only big)
>for the project. See: https://xenproject.atlassian.net/projects/XEN/issues.
>
>Most of the tasks tracked by this e-mail also have a corresponding jira task
>referred by XEN-N.
>
>I have started to include the version number of series associated to each
>feature. Can each owner send an update on the version number if the series
>was posted upstream?
>
>= Projects =
>
>== Hypervisor ==
>
>*  Per-cpu tasklet
>  -  XEN-28
>  -  Konrad Rzeszutek Wilk
>
>*  Improvements to domain creation (v2)
>  -  Andrew Cooper
>
>*  Argo (inter-VM communication)
>  -  Christopher Clark
>
>*  Make credit2 scheduler the default
>  -  George Dunlap
>
>=== x86 ===
>
>*  hypervisor x86 instruction emulator additions (v2)
>  -  Jan Beulich
>
>*  PV-IOMMU (v6)
>  -  Paul Durrant
>
>*  HVM guest CPU topology support (RFC)
>  -  Chao Gao
>
>*  Intel Processor Trace virtualization enabling (v1)
>  -  Luwei Kang
>
>*  Linux stub domains (RFC)
>  -  Marek Marczykowski-Górecki
>
>== Grub2 ==
>
>*  Support PVH guest boot (v1)
>  -  Juergen Gross
>
>== Completed ==
>
>*  guest resource mapping
>  -  Paul Durrant
>
>*  PV-only hypervisor
>  -  Wei Liu
>
>
>Juergen Gross
>
>___
>Xen-devel mailing list
>Xen-devel@lists.xenproject.org
>https://lists.xenproject.org/mailman/listinfo/xen-devel

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

Re: [Xen-devel] [PATCH 41/41] scsi: xen-scsifront: mark expected switch fall-through

[Juergen Gross]

On 28/11/2018 05:34, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
> 
> Notice that, in this particular case, I replaced
> "Missed the backend's Closing state -- fallthrough" with
> "fall through - Missed the backend's Closing state", which
> contains the "fall through" annotation at the beginnig of
> the code comment, which is what GCC is expecting to find.
> 
> Signed-off-by: Gustavo A. R. Silva 

Reviewed-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [PATCH 0/3] Remove tmem

[Juergen Gross]

On 27/11/2018 19:44, Wei Liu wrote:
> It is agreed that tmem can be removed from xen.git. See the thread starting
> from .
> 
> Wei Liu (3):
>   xen: remove tmem from hypervisor
>   tools: remove tmem code and commands
>   docs: remove tmem related text

Shouldn't the tools patch be the first in order to make the series
bisectable? After current patch 1 Xen won't build now.


Juergen

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

Re: [Xen-devel] [PATCH 0/3] Remove tmem

[Juergen Gross]

On 27/11/2018 19:44, Wei Liu wrote:
> It is agreed that tmem can be removed from xen.git. See the thread starting
> from .
> 
> Wei Liu (3):
>   xen: remove tmem from hypervisor
>   tools: remove tmem code and commands
>   docs: remove tmem related text
> 
>  docs/man/xl.conf.pod.5   |9 +-
>  docs/man/xl.pod.1.in |   68 -
>  docs/misc/tmem-internals.html|  789 --
>  docs/misc/xen-command-line.markdown  |6 -
>  docs/misc/xsm-flask.txt  |   36 -
>  tools/flask/policy/modules/dom0.te   |4 +-
>  tools/flask/policy/modules/guest_features.te |3 -
>  tools/libxc/Makefile |1 -
>  tools/libxc/include/xenctrl.h|   17 -
>  tools/libxc/xc_tmem.c|  507 ---
>  tools/libxl/libxl_tmem.c |  119 +-
>  tools/misc/Makefile  |1 -
>  tools/misc/xen-tmem-list-parse.c |  339 -
>  tools/python/xen/lowlevel/xc/xc.c|   87 --
>  tools/xenstat/libxenstat/src/xenstat.c   |   52 +-
>  tools/xenstat/libxenstat/src/xenstat.h   |   15 -
>  tools/xenstat/libxenstat/src/xenstat_priv.h  |8 -
>  tools/xenstat/xentop/xentop.c|   36 +-
>  tools/xl/Makefile|2 +-
>  tools/xl/xl.h|6 -
>  tools/xl/xl_cmdtable.c   |   40 -
>  tools/xl/xl_tmem.c   |  251 ---
>  xen/arch/arm/configs/tiny64.conf |1 -
>  xen/arch/x86/configs/pvshim_defconfig|1 -
>  xen/arch/x86/hvm/hypercall.c |3 -
>  xen/arch/x86/pv/hypercall.c  |3 -
>  xen/arch/x86/setup.c |8 -
>  xen/common/Kconfig   |   13 -
>  xen/common/Makefile  |4 -
>  xen/common/compat/tmem_xen.c |   23 -
>  xen/common/domain.c  |3 -
>  xen/common/memory.c  |4 +-
>  xen/common/page_alloc.c  |   40 +-
>  xen/common/sysctl.c  |5 -
>  xen/common/tmem.c| 2095 
> --
>  xen/common/tmem_control.c|  560 ---
>  xen/common/tmem_xen.c|  277 
>  xen/include/Makefile |1 -
>  xen/include/public/sysctl.h  |  108 +-
>  xen/include/public/tmem.h|  124 --
>  xen/include/xen/hypercall.h  |7 -
>  xen/include/xen/sched.h  |3 -
>  xen/include/xen/tmem.h   |   45 -
>  xen/include/xen/tmem_control.h   |   39 -
>  xen/include/xen/tmem_xen.h   |  343 -
>  xen/include/xlat.lst |2 -
>  46 files changed, 27 insertions(+), 6081 deletions(-)
>  delete mode 100644 docs/misc/tmem-internals.html
>  delete mode 100644 tools/libxc/xc_tmem.c
>  delete mode 100644 tools/misc/xen-tmem-list-parse.c
>  delete mode 100644 tools/xl/xl_tmem.c
>  delete mode 100644 xen/common/compat/tmem_xen.c
>  delete mode 100644 xen/common/tmem.c
>  delete mode 100644 xen/common/tmem_control.c
>  delete mode 100644 xen/common/tmem_xen.c
>  delete mode 100644 xen/include/public/tmem.h
>  delete mode 100644 xen/include/xen/tmem.h
>  delete mode 100644 xen/include/xen/tmem_control.h
>  delete mode 100644 xen/include/xen/tmem_xen.h

Great!

As soon as the series is committed I'll post a related Linux kernel
series removing tmem and associated dependent bits from it.


Juergen

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

[Xen-devel] out of memory issue on dom0_hvm start

[andrei . semenov]

Hi all,

I have some troubles to start xen when using dom0 in HVM mode on X86 Intel 
nuc7i5bnh 
board. After some investigation it seems that  "domheap" memory allocator is 
out of memory while 
constructing the p2m mapping  (pvh_setup_p2m).  

This misbehavior, from my understanding, is the result of wrong computation of 
number of pages 
to "give" to dom0 (dom0_compute_nr_pages function). In fact the pages needed to 
paging 
(dom0_paging_pages function)  are not reserved in this function if  IOMMU 
mappings are shareable 
with HAP(EPT) mappings. Moreover this memory (for paging needs) is allocated 
just after that in 
"pvh_setup_p2m" in subroutine "paging_set_allocation". 

Generally speaking from what I understood it's not the IOMMU driver that offers 
its mappings to HAP,
but it's rather the HAP that shares its mappings with IOMMU driver. So the 
obvious patch IMHO would 
be:

--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -294,8 +294,7 @@ unsigned long __init dom0_compute_nr_pages(
 avail -= max_pdx >> s;
 }
 
-    need_paging = is_hvm_domain(d) &&
-    (!iommu_hap_pt_share || !paging_mode_hap(d));
+    need_paging = is_hvm_domain(d);
 for ( ; ; need_paging = false )
 {
 nr_pages = dom0_nrpages; 

Did anyone encounter the same problem or has some thoughts on this issue?

Andrei.
1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 1/6] microcode/intel: extend microcode_update_match()

[Chao Gao]

to a more generic function. The benefit is that this function can be
used to check whether a microcode is newer than another as well. We
rely on this function to decide to perform a replacement or an add when
updating the global microcode cache (introduced by later patches in
this series).

Signed-off-by: Chao Gao 
---
 xen/arch/x86/microcode_intel.c | 57 +++---
 1 file changed, 31 insertions(+), 26 deletions(-)

diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c
index 9657575..8d9a3b2 100644
--- a/xen/arch/x86/microcode_intel.c
+++ b/xen/arch/x86/microcode_intel.c
@@ -127,14 +127,37 @@ static int collect_cpu_info(unsigned int cpu_num, struct 
cpu_signature *csig)
 return 0;
 }
 
-static inline int microcode_update_match(
-unsigned int cpu_num, const struct microcode_header_intel *mc_header,
-int sig, int pf)
+enum {
+OLD_UCODE, /* signature matched, but revision id isn't newer */
+NEW_UCODE, /* signature matched, but revision id is newer */
+MIS_UCODE, /* signature mismatched */
+};
+
+static int microcode_update_match(const void *mc,
+unsigned int sig, unsigned int pf, unsigned int rev)
 {
-struct ucode_cpu_info *uci = _cpu(ucode_cpu_info, cpu_num);
+const struct microcode_header_intel *mc_header = mc;
+const struct extended_sigtable *ext_header;
+unsigned long total_size = get_totalsize(mc_header);
+int ext_sigcount, i;
+struct extended_signature *ext_sig;
 
-return (sigmatch(sig, uci->cpu_sig.sig, pf, uci->cpu_sig.pf) &&
-(mc_header->rev > uci->cpu_sig.rev));
+if ( sigmatch(sig, mc_header->sig, pf, mc_header->pf) )
+return (mc_header->rev > rev) ? NEW_UCODE : OLD_UCODE;
+
+if ( total_size <= (get_datasize(mc_header) + MC_HEADER_SIZE) )
+return MIS_UCODE;
+
+ext_header = mc + get_datasize(mc_header) + MC_HEADER_SIZE;
+ext_sigcount = ext_header->count;
+ext_sig = (void *)ext_header + EXT_HEADER_SIZE;
+for ( i = 0; i < ext_sigcount; i++ )
+{
+if ( sigmatch(sig, ext_sig->sig, pf, ext_sig->pf) )
+return (mc_header->rev > rev) ? NEW_UCODE : OLD_UCODE;
+ext_sig++;
+}
+return MIS_UCODE;
 }
 
 static int microcode_sanity_check(void *mc)
@@ -236,31 +259,13 @@ static int get_matching_microcode(const void *mc, 
unsigned int cpu)
 {
 struct ucode_cpu_info *uci = _cpu(ucode_cpu_info, cpu);
 const struct microcode_header_intel *mc_header = mc;
-const struct extended_sigtable *ext_header;
 unsigned long total_size = get_totalsize(mc_header);
-int ext_sigcount, i;
-struct extended_signature *ext_sig;
 void *new_mc;
 
-if ( microcode_update_match(cpu, mc_header,
-mc_header->sig, mc_header->pf) )
-goto find;
-
-if ( total_size <= (get_datasize(mc_header) + MC_HEADER_SIZE) )
+if ( microcode_update_match(mc, uci->cpu_sig.sig, uci->cpu_sig.pf,
+uci->cpu_sig.rev) != NEW_UCODE )
 return 0;
 
-ext_header = mc + get_datasize(mc_header) + MC_HEADER_SIZE;
-ext_sigcount = ext_header->count;
-ext_sig = (void *)ext_header + EXT_HEADER_SIZE;
-for ( i = 0; i < ext_sigcount; i++ )
-{
-if ( microcode_update_match(cpu, mc_header,
-ext_sig->sig, ext_sig->pf) )
-goto find;
-ext_sig++;
-}
-return 0;
- find:
 pr_debug("microcode: CPU%d found a matching microcode update with"
  " version %#x (current=%#x)\n",
  cpu, mc_header->rev, uci->cpu_sig.rev);
-- 
1.8.3.1


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

[Xen-devel] [PATCH v4 3/6] microcode: delete 'mc' field from struct ucode_cpu_info

[Chao Gao]

apply_microcode() now uses the cached microcode rather than
the microcode stored in "mc" field of ucode_cpu_info. Also remove
'microcode_resume_match' from microcode_ops because the check is
done in find_patch() in apply_microcode() callback.

Signed-off-by: Chao Gao 
---
 xen/arch/x86/microcode.c| 33 +-
 xen/arch/x86/microcode_amd.c| 75 ++---
 xen/arch/x86/microcode_intel.c  | 27 +++
 xen/include/asm-x86/microcode.h |  6 
 4 files changed, 8 insertions(+), 133 deletions(-)

diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index 4f2db88..8350d22 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -199,7 +199,6 @@ static void __microcode_fini_cpu(unsigned int cpu)
 {
 struct ucode_cpu_info *uci = _cpu(ucode_cpu_info, cpu);
 
-xfree(uci->mc.mc_valid);
 memset(uci, 0, sizeof(*uci));
 }
 
@@ -214,8 +213,6 @@ int microcode_resume_cpu(unsigned int cpu)
 {
 int err;
 struct ucode_cpu_info *uci = _cpu(ucode_cpu_info, cpu);
-struct cpu_signature nsig;
-unsigned int cpu2;
 
 if ( !microcode_ops )
 return 0;
@@ -230,35 +227,7 @@ int microcode_resume_cpu(unsigned int cpu)
 return err;
 }
 
-if ( uci->mc.mc_valid )
-{
-err = microcode_ops->microcode_resume_match(cpu, uci->mc.mc_valid);
-if ( err >= 0 )
-{
-if ( err )
-err = microcode_ops->apply_microcode(cpu);
-spin_unlock(_mutex);
-return err;
-}
-}
-
-nsig = uci->cpu_sig;
-__microcode_fini_cpu(cpu);
-uci->cpu_sig = nsig;
-
-err = -EIO;
-for_each_online_cpu ( cpu2 )
-{
-uci = _cpu(ucode_cpu_info, cpu2);
-if ( uci->mc.mc_valid &&
- microcode_ops->microcode_resume_match(cpu, uci->mc.mc_valid) > 0 )
-{
-err = microcode_ops->apply_microcode(cpu);
-break;
-}
-}
-
-__microcode_fini_cpu(cpu);
+err = microcode_ops->apply_microcode(cpu);
 spin_unlock(_mutex);
 
 return err;
diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c
index a686a87..6e6598a 100644
--- a/xen/arch/x86/microcode_amd.c
+++ b/xen/arch/x86/microcode_amd.c
@@ -459,10 +459,10 @@ static bool_t check_final_patch_levels(unsigned int cpu)
 static int cpu_request_microcode(unsigned int cpu, const void *buf,
  size_t bufsize)
 {
-struct microcode_amd *mc_amd, *mc_old;
+struct microcode_amd *mc_amd;
 size_t offset = 0;
 size_t last_offset, applied_offset = 0;
-int error = 0, save_error = 1;
+int error = 0;
 struct ucode_cpu_info *uci = _cpu(ucode_cpu_info, cpu);
 unsigned int current_cpu_id;
 unsigned int equiv_cpu_id;
@@ -545,10 +545,6 @@ static int cpu_request_microcode(unsigned int cpu, const 
void *buf,
 goto out;
 }
 
-mc_old = uci->mc.mc_amd;
-/* implicitely validates uci->mc.mc_valid */
-uci->mc.mc_amd = mc_amd;
-
 /*
  * It's possible the data file has multiple matching ucode,
  * lets keep searching till the latest version
@@ -612,26 +608,6 @@ static int cpu_request_microcode(unsigned int cpu, const 
void *buf,
 break;
 }
 
-/* On success keep the microcode patch for
- * re-apply on resume.
- */
-if ( applied_offset )
-{
-save_error = get_ucode_from_buffer_amd(
-mc_amd, buf, bufsize, _offset);
-
-if ( save_error )
-error = save_error;
-}
-
-if ( save_error )
-{
-xfree(mc_amd);
-uci->mc.mc_amd = mc_old;
-}
-else
-xfree(mc_old);
-
   out:
 #if CONFIG_HVM
 svm_host_osvw_init();
@@ -646,52 +622,6 @@ static int cpu_request_microcode(unsigned int cpu, const 
void *buf,
 return error;
 }
 
-static int microcode_resume_match(unsigned int cpu, const void *mc)
-{
-struct ucode_cpu_info *uci = _cpu(ucode_cpu_info, cpu);
-struct microcode_amd *mc_amd = uci->mc.mc_amd;
-const struct microcode_amd *src = mc;
-
-if ( !microcode_fits(src, cpu) )
-return 0;
-
-if ( src != mc_amd )
-{
-if ( mc_amd )
-{
-xfree(mc_amd->equiv_cpu_table);
-xfree(mc_amd->mpb);
-xfree(mc_amd);
-}
-
-mc_amd = xmalloc(struct microcode_amd);
-uci->mc.mc_amd = mc_amd;
-if ( !mc_amd )
-return -ENOMEM;
-mc_amd->equiv_cpu_table = xmalloc_bytes(src->equiv_cpu_table_size);
-if ( !mc_amd->equiv_cpu_table )
-goto err1;
-mc_amd->mpb = xmalloc_bytes(src->mpb_size);
-if ( !mc_amd->mpb )
-goto err2;
-
-mc_amd->equiv_cpu_table_size = src->equiv_cpu_table_size;
-mc_amd->mpb_size = src->mpb_size;
-memcpy(mc_amd->mpb, src->mpb, src->mpb_size);
-memcpy(mc_amd->equiv_cpu_table, src->equiv_cpu_table,
-   

[Xen-devel] [PATCH v4 5/6] microcode: delete microcode pointer and size from microcode_info

[Chao Gao]

Microcode pointer and size is passed to other cpu to parse microcode
locally. Now, parsing microcode is done on one CPU. Others just
find a suitable microcode stored in microcode_cache.

Signed-off-by: Chao Gao 
---
 xen/arch/x86/microcode.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index cca7b2c..0b435f4 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -190,9 +190,7 @@ DEFINE_PER_CPU(struct ucode_cpu_info, ucode_cpu_info);
 
 struct microcode_info {
 unsigned int cpu;
-uint32_t buffer_size;
 int error;
-char buffer[1];
 };
 
 static void __microcode_fini_cpu(unsigned int cpu)
@@ -270,6 +268,7 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) 
buf, unsigned long len)
 struct microcode_info *info;
 unsigned int cpu = smp_processor_id();
 struct ucode_cpu_info *uci = _cpu(ucode_cpu_info, cpu);
+void * buffer;
 
 if ( len != (uint32_t)len )
 return -E2BIG;
@@ -277,11 +276,12 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) 
buf, unsigned long len)
 if ( microcode_ops == NULL )
 return -EINVAL;
 
-info = xmalloc_bytes(sizeof(*info) + len);
-if ( info == NULL )
+info = xmalloc(struct microcode_info);
+buffer = xmalloc_bytes(len);
+if ( !info || !buffer )
 return -ENOMEM;
 
-ret = copy_from_guest(info->buffer, buf, len);
+ret = copy_from_guest(buffer, buf, len);
 if ( ret != 0 )
 {
 xfree(info);
@@ -302,7 +302,7 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) 
buf, unsigned long len)
 
 ret = microcode_ops->collect_cpu_info(cpu, >cpu_sig);
 if ( likely(!ret) )
-ret = microcode_ops->cpu_request_microcode(cpu, info->buffer, len);
+ret = microcode_ops->cpu_request_microcode(cpu, buffer, len);
 else
 __microcode_fini_cpu(cpu);
 
@@ -314,7 +314,6 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) 
buf, unsigned long len)
 return -EINVAL;
 }
 
-info->buffer_size = len;
 info->error = 0;
 info->cpu = cpumask_first(_online_map);
 
-- 
1.8.3.1


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

[Xen-devel] [PATCH v4 2/6] microcode: save all microcodes which pass sanity check

[Chao Gao]

... and search caches to find a suitable one when loading.

With this cache, the existing 'uci->mc' structure is redundent.
I deliberately avoid touching 'uci->mc' as I am going to remove
it completely in the next patch.

Signed-off-by: Chao Gao 
---
 xen/arch/x86/microcode.c|  2 +
 xen/arch/x86/microcode_amd.c| 93 +++---
 xen/arch/x86/microcode_intel.c  | 99 ++---
 xen/include/asm-x86/microcode.h | 11 +
 4 files changed, 193 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index 4163f50..4f2db88 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -61,6 +61,8 @@ static struct ucode_mod_blob __initdata ucode_blob;
  */
 static bool_t __initdata ucode_scan;
 
+LIST_HEAD(microcode_cache);
+
 void __init microcode_set_module(unsigned int idx)
 {
 ucode_mod_idx = idx;
diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c
index fba44cc..a686a87 100644
--- a/xen/arch/x86/microcode_amd.c
+++ b/xen/arch/x86/microcode_amd.c
@@ -190,22 +190,90 @@ static bool_t microcode_fits(const struct microcode_amd 
*mc_amd,
 return 1;
 }
 
+static struct ucode_patch *alloc_ucode_patch(struct microcode_amd *mc_amd)
+{
+struct ucode_patch *ucode_patch = xmalloc(struct ucode_patch);
+struct microcode_amd *cache = xmalloc(struct microcode_amd);
+void *mpb = xmalloc_bytes(mc_amd->mpb_size);
+struct equiv_cpu_entry *equiv_cpu_table =
+xmalloc_bytes(mc_amd->equiv_cpu_table_size);
+
+if ( !ucode_patch || !cache || !mpb || !equiv_cpu_table )
+return ERR_PTR(-ENOMEM);
+
+memcpy(cache->equiv_cpu_table, mc_amd->equiv_cpu_table,
+   mc_amd->equiv_cpu_table_size);
+memcpy(cache->mpb, mc_amd->mpb, mc_amd->mpb_size);
+cache->equiv_cpu_table_size = mc_amd->equiv_cpu_table_size;
+cache->mpb_size = mc_amd->mpb_size;
+ucode_patch->data = cache;
+return ucode_patch;
+}
+
+static void free_ucode_patch(struct ucode_patch *ucode_patch)
+{
+struct microcode_amd *mc_amd = ucode_patch->data;
+
+xfree(mc_amd->equiv_cpu_table);
+xfree(mc_amd->mpb);
+xfree(mc_amd);
+xfree(ucode_patch);
+}
+
+/*
+ * save a micrcode to the cache list
+ * return 1: added successfully
+ *0: replaced an existing entry
+ *   -1: failed as a newer microcode was already cached
+ */
+static int save_patch(struct ucode_patch *new_patch)
+{
+struct ucode_patch *ucode_patch;
+struct microcode_amd *new_mc = new_patch->data;
+struct microcode_header_amd *new_header = new_mc->mpb;
+
+list_for_each_entry(ucode_patch, _cache, list)
+{
+struct microcode_amd *old_mc = ucode_patch->data;
+struct microcode_header_amd *old_header = old_mc->mpb;
+
+if ( new_header->processor_rev_id == old_header->processor_rev_id )
+{
+if ( new_header->patch_id <= old_header->patch_id )
+return -1;
+list_replace(_patch->list, _patch->list);
+free_ucode_patch(ucode_patch);
+return 0;
+}
+}
+list_add_tail(_patch->list, _cache);
+return 1;
+}
+
+static struct microcode_header_amd *find_patch(unsigned int cpu)
+{
+struct ucode_patch *ucode_patch;
+
+list_for_each_entry(ucode_patch, _cache, list)
+{
+if ( microcode_fits(ucode_patch->data, cpu) )
+return ((struct microcode_amd *)ucode_patch->data)->mpb;
+}
+return NULL;
+}
+
 static int apply_microcode(unsigned int cpu)
 {
 unsigned long flags;
 struct ucode_cpu_info *uci = _cpu(ucode_cpu_info, cpu);
 uint32_t rev;
-struct microcode_amd *mc_amd = uci->mc.mc_amd;
 struct microcode_header_amd *hdr;
 int hw_err;
 
 /* We should bind the task to the CPU */
 BUG_ON(raw_smp_processor_id() != cpu);
 
-if ( mc_amd == NULL )
-return -EINVAL;
-
-hdr = mc_amd->mpb;
+hdr = find_patch(cpu);
 if ( hdr == NULL )
 return -EINVAL;
 
@@ -491,6 +559,21 @@ static int cpu_request_microcode(unsigned int cpu, const 
void *buf,
 while ( (error = get_ucode_from_buffer_amd(mc_amd, buf, bufsize,
)) == 0 )
 {
+struct ucode_patch *ucode_patch;
+
+/*
+ * Save this microcode before checking the signature. It is to
+ * optimize microcode update on a mixed family system. Parsing
+ * microcode file is only done once on one of the CPUs, and
+ * during this process microcode cache is created. Other CPUs
+ * needn't parse the same micrcode file again and again.
+ * Instead, they just load the matched and latest microcode in
+ * the caches.
+ */
+ucode_patch = alloc_ucode_patch(mc_amd);
+if ( !IS_ERR_OR_NULL(ucode_patch) && (save_patch(ucode_patch) < 0) )
+free_ucode_patch(ucode_patch);
+
 if ( 

[Xen-devel] [PATCH v4 0/6] improve late microcode loading

[Chao Gao]

The intention of this series is to make the late microcode loading
more reliable by rendezvousing all cpus in stop_machine context and
updating microcode of each cpu core one-by-one. This idea comes from
Ashok. I am porting his linux patch to Xen (see patch 6 for more
detail).

This series makes two changes:
 1. Patch 1-5: introduce a global microcode cache
 2. Patch 6: synchronize late microcode loading

Currently, late microcode loading does a lot of things including
parsing microcode file, checking the signature/revision and performing
update. Putting all of them into stop_machine context is a bad idea
because of complexity (One issue I observed is memory allocation
triggered one assertion in stop_machine context). In order to simplify
the load process, I move parsing microcode out of the load process.
The microcode file is parsed and a global microcode cache is built on
a single CPU before rendezvousing all cpus to update microcode. Other
CPUs just get a suitable microcode from the global cache and load it.
With this global cache, it is safe to put simplified load process to
stop_machine context.

Regarding changes to AMD side, I didn't do any tests for them due to
lack of hardware. Hence it wonldn't be surprising to me if you found
some bugs on a AMD machine. Is there anyone who has a AMD machine
at hand willing to do some basic tests, like
* do a microcode update
* don't bring all pCPUs up at startup by specifying maxcpus in xen
  command line and then do a microcode update and online all
  offlined CPUs via 'xen-hptool'.

For your convenience, you can also find this series at:
https://github.intel.com/chaogao/xen

Chao Gao (6):
  microcode/intel: extend microcode_update_match()
  microcode: save all microcodes which pass sanity check
  microcode: delete 'mc' field from struct ucode_cpu_info
  microcode: don't call apply_microcode() in cpu_request_microcode()
  microcode: delete microcode pointer and size from microcode_info
  x86/microcode: Synchronize late microcode loading

 xen/arch/x86/microcode.c| 215 ++--
 xen/arch/x86/microcode_amd.c| 183 ++
 xen/arch/x86/microcode_intel.c  | 174 ++--
 xen/include/asm-x86/microcode.h |  17 ++--
 4 files changed, 371 insertions(+), 218 deletions(-)

-- 
1.8.3.1


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

[Xen-devel] [PATCH v4 6/6] x86/microcode: Synchronize late microcode loading

[Chao Gao]

This patch ports microcode improvement patches from linux kernel.

Before you read any further: the early loading method is still the
preferred one and you should always do that. The following patch is
improving the late loading mechanism for long running jobs and cloud use
cases.

Gather all cores and serialize the microcode update on them by doing it
one-by-one to make the late update process as reliable as possible and
avoid potential issues caused by the microcode update.

Signed-off-by: Chao Gao 
Tested-by: Chao Gao 
[linux commit: a5321aec6412b20b5ad15db2d6b916c05349dbff]
[linux commit: bb8c13d61a629276a162c1d2b1a20a815cbcfbb7]
Cc: Kevin Tian 
Cc: Jun Nakajima 
Cc: Ashok Raj 
Cc: Borislav Petkov 
Cc: Thomas Gleixner 
Cc: Andrew Cooper 
Cc: Jan Beulich 
---
 xen/arch/x86/microcode.c | 123 +--
 1 file changed, 97 insertions(+), 26 deletions(-)

diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index 0b435f4..d5a2a94 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -22,6 +22,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -30,18 +31,25 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
+#include 
 
+#include 
 #include 
 #include 
 #include 
 #include 
 
+/* By default, wait for 3us */
+#define MICROCODE_DEFAULT_TIMEOUT_US 3
+
 static module_t __initdata ucode_mod;
 static signed int __initdata ucode_mod_idx;
 static bool_t __initdata ucode_mod_forced;
+static unsigned int nr_cores;
 
 /*
  * If we scan the initramfs.cpio for the early microcode code
@@ -189,8 +197,7 @@ static DEFINE_SPINLOCK(microcode_mutex);
 DEFINE_PER_CPU(struct ucode_cpu_info, ucode_cpu_info);
 
 struct microcode_info {
-unsigned int cpu;
-int error;
+atomic_t cpu_in, cpu_out;
 };
 
 static void __microcode_fini_cpu(unsigned int cpu)
@@ -242,31 +249,62 @@ static int microcode_update_cpu(void)
 return err;
 }
 
-static long do_microcode_update(void *_info)
+/* Wait for all CPUs to rendezvous with a timeout (us) */
+static int wait_for_cpus(atomic_t *cnt, unsigned int timeout)
 {
-struct microcode_info *info = _info;
-int error;
+unsigned int cpus = num_online_cpus();
 
-BUG_ON(info->cpu != smp_processor_id());
+atomic_inc(cnt);
 
-error = microcode_update_cpu();
-if ( error )
-info->error = error;
+while ( atomic_read(cnt) != cpus )
+{
+if ( timeout <= 0 )
+{
+printk("Timeout when waiting for CPUs calling in\n");
+return -EBUSY;
+}
+udelay(1);
+timeout--;
+}
 
-info->cpu = cpumask_next(info->cpu, _online_map);
-if ( info->cpu < nr_cpu_ids )
-return continue_hypercall_on_cpu(info->cpu, do_microcode_update, info);
+return 0;
+}
 
-error = info->error;
-xfree(info);
-return error;
+static int do_microcode_update(void *_info)
+{
+struct microcode_info *info = _info;
+unsigned int cpu = smp_processor_id();
+int ret;
+
+ret = wait_for_cpus(>cpu_in, MICROCODE_DEFAULT_TIMEOUT_US);
+if ( ret )
+return ret;
+
+/*
+ * Initiate an update on all processors which don't have an online sibling
+ * thread with a lower thread id. Other sibling threads just await the
+ * completion of microcode update.
+ */
+if ( cpu == cpumask_first(per_cpu(cpu_sibling_mask, cpu)) )
+ret = microcode_update_cpu();
+/*
+ * Increase the wait timeout to a safe value here since we're serializing
+ * the microcode update and that could take a while on a large number of
+ * CPUs. And that is fine as the *actual* timeout will be determined by
+ * the last CPU finished updating and thus cut short
+ */
+if ( wait_for_cpus(>cpu_out, MICROCODE_DEFAULT_TIMEOUT_US *
+   nr_cores) )
+panic("Timeout when finishing updating microcode");
+
+return ret;
 }
 
 int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long len)
 {
 int ret;
-struct microcode_info *info;
 unsigned int cpu = smp_processor_id();
+struct microcode_info *info;
 struct ucode_cpu_info *uci = _cpu(ucode_cpu_info, cpu);
 void * buffer;
 
@@ -283,19 +321,20 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) 
buf, unsigned long len)
 
 ret = copy_from_guest(buffer, buf, len);
 if ( ret != 0 )
+goto free;
+
+/* cpu_online_map must not change during update */
+if ( !get_cpu_maps() )
 {
-xfree(info);
-return ret;
+ret = -EBUSY;
+goto free;
 }
 
 if ( microcode_ops->start_update )
 {
 ret = microcode_ops->start_update();
 if ( ret != 0 )
-{
-xfree(info);
-return ret;
-}
+goto put;
 }
 
 spin_lock(_mutex);
@@ -311,13 +350,45 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) 
buf, unsigned long 

[Xen-devel] [PATCH v4 4/6] microcode: don't call apply_microcode() in cpu_request_microcode()

[Chao Gao]

cpu_request_microcode() will only parse microcode file and save
suitable microcodes to microcode_cache. To update microcode,
apply_microcode() should be invoked explicitly.

On AMD side, svm_host_osvw_init() is supposed to be called after
microcode update. As apply_micrcode() won't be called by
cpu_request_microcode() now, svm_host_osvw_init() is also moved to the
end of apply_microcode().

Signed-off-by: Chao Gao 
---
 xen/arch/x86/microcode.c   | 58 ++
 xen/arch/x86/microcode_amd.c   | 15 +--
 xen/arch/x86/microcode_intel.c |  5 +---
 3 files changed, 50 insertions(+), 28 deletions(-)

diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index 8350d22..cca7b2c 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -233,20 +233,12 @@ int microcode_resume_cpu(unsigned int cpu)
 return err;
 }
 
-static int microcode_update_cpu(const void *buf, size_t size)
+static int microcode_update_cpu(void)
 {
 int err;
-unsigned int cpu = smp_processor_id();
-struct ucode_cpu_info *uci = _cpu(ucode_cpu_info, cpu);
 
 spin_lock(_mutex);
-
-err = microcode_ops->collect_cpu_info(cpu, >cpu_sig);
-if ( likely(!err) )
-err = microcode_ops->cpu_request_microcode(cpu, buf, size);
-else
-__microcode_fini_cpu(cpu);
-
+err = microcode_ops->apply_microcode(smp_processor_id());
 spin_unlock(_mutex);
 
 return err;
@@ -259,7 +251,7 @@ static long do_microcode_update(void *_info)
 
 BUG_ON(info->cpu != smp_processor_id());
 
-error = microcode_update_cpu(info->buffer, info->buffer_size);
+error = microcode_update_cpu();
 if ( error )
 info->error = error;
 
@@ -276,6 +268,8 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) 
buf, unsigned long len)
 {
 int ret;
 struct microcode_info *info;
+unsigned int cpu = smp_processor_id();
+struct ucode_cpu_info *uci = _cpu(ucode_cpu_info, cpu);
 
 if ( len != (uint32_t)len )
 return -E2BIG;
@@ -294,10 +288,6 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) 
buf, unsigned long len)
 return ret;
 }
 
-info->buffer_size = len;
-info->error = 0;
-info->cpu = cpumask_first(_online_map);
-
 if ( microcode_ops->start_update )
 {
 ret = microcode_ops->start_update();
@@ -308,6 +298,26 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) 
buf, unsigned long len)
 }
 }
 
+spin_lock(_mutex);
+
+ret = microcode_ops->collect_cpu_info(cpu, >cpu_sig);
+if ( likely(!ret) )
+ret = microcode_ops->cpu_request_microcode(cpu, info->buffer, len);
+else
+__microcode_fini_cpu(cpu);
+
+spin_unlock(_mutex);
+
+if ( ret <= 0 )
+{
+printk("No valid or newer microcode found. Update abort!\n");
+return -EINVAL;
+}
+
+info->buffer_size = len;
+info->error = 0;
+info->cpu = cpumask_first(_online_map);
+
 return continue_hypercall_on_cpu(info->cpu, do_microcode_update, info);
 }
 
@@ -370,13 +380,29 @@ int __init early_microcode_update_cpu(bool start_update)
 }
 if ( data )
 {
+unsigned int cpu = smp_processor_id();
+struct ucode_cpu_info *uci = _cpu(ucode_cpu_info, cpu);
+
 if ( start_update && microcode_ops->start_update )
 rc = microcode_ops->start_update();
 
 if ( rc )
 return rc;
 
-return microcode_update_cpu(data, len);
+spin_lock(_mutex);
+
+rc = microcode_ops->collect_cpu_info(cpu, >cpu_sig);
+if ( likely(!rc) )
+rc = microcode_ops->cpu_request_microcode(cpu, data, len);
+else
+__microcode_fini_cpu(cpu);
+
+spin_unlock(_mutex);
+
+if ( rc <= 0 )
+return -EINVAL;
+
+return microcode_update_cpu();
 }
 else
 return -ENOMEM;
diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c
index 6e6598a..6d860f3 100644
--- a/xen/arch/x86/microcode_amd.c
+++ b/xen/arch/x86/microcode_amd.c
@@ -299,6 +299,10 @@ static int apply_microcode(unsigned int cpu)
 
 uci->cpu_sig.rev = rev;
 
+#if CONFIG_HVM
+svm_host_osvw_init();
+#endif
+
 return 0;
 }
 
@@ -466,6 +470,7 @@ static int cpu_request_microcode(unsigned int cpu, const 
void *buf,
 struct ucode_cpu_info *uci = _cpu(ucode_cpu_info, cpu);
 unsigned int current_cpu_id;
 unsigned int equiv_cpu_id;
+unsigned int matched_cnt = 0;
 
 /* We should bind the task to the CPU */
 BUG_ON(cpu != raw_smp_processor_id());
@@ -572,9 +577,7 @@ static int cpu_request_microcode(unsigned int cpu, const 
void *buf,
 
 if ( microcode_fits(mc_amd, cpu) )
 {
-error = apply_microcode(cpu);
-if ( error )
-break;
+matched_cnt++;
 applied_offset = last_offset;
 }
 
@@ -609,17 +612,13 @@ static int cpu_request_microcode(unsigned int cpu, const 

[Xen-devel] [PATCH 41/41] scsi: xen-scsifront: mark expected switch fall-through

[Gustavo A. R. Silva]

In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Notice that, in this particular case, I replaced
"Missed the backend's Closing state -- fallthrough" with
"fall through - Missed the backend's Closing state", which
contains the "fall through" annotation at the beginnig of
the code comment, which is what GCC is expecting to find.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/scsi/xen-scsifront.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c
index 61389bdc7926..bb76d0d2022b 100644
--- a/drivers/scsi/xen-scsifront.c
+++ b/drivers/scsi/xen-scsifront.c
@@ -1112,7 +1112,7 @@ static void scsifront_backend_changed(struct 
xenbus_device *dev,
case XenbusStateClosed:
if (dev->state == XenbusStateClosed)
break;
-   /* Missed the backend's Closing state -- fallthrough */
+   /* fall through - Missed the backend's Closing state */
case XenbusStateClosing:
scsifront_disconnect(info);
break;
-- 
2.17.1


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

[Xen-devel] [ovmf test] 130806: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i386-xsm6 xen-buildfail REGR. vs. 129475
 build-i3866 xen-buildfail REGR. vs. 129475
 build-amd64-xsm   6 xen-buildfail REGR. vs. 129475
 build-amd64   6 xen-buildfail REGR. vs. 129475

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf e8f40b770f5eeb1031a56fcab9afc9c12a4ecafa
baseline version:
 ovmf 5ae3184d8c59f7bbb84bad482df6b8020ba58188

Last test of basis   129475  2018-11-05 21:13:11 Z   22 days
Failing since129526  2018-11-06 20:49:26 Z   21 days  141 attempts
Testing same since   130806  2018-11-26 08:13:19 Z1 days1 attempts


People who touched revisions under test:
  Ard Biesheuvel 
  BobCF 
  Chasel Chiu 
  Chasel, Chiu 
  Dandan Bi 
  David Wei 
  Eric Dong 
  Feng, Bob C 
  Fu Siyuan 
  Hao Wu 
  Jeff Brasen 
  Jian J Wang 
  Jiaxin Wu 
  Jiewen Yao 
  Laszlo Ersek 
  Liming Gao 
  Liu Yu 
  Marc Zyngier 
  Marcin Wojtas 
  Ming Huang 
  Pedroa Liu 
  Ruiyu Ni 
  shenglei 
  Shenglei Zhang 
  Star Zeng 
  Sun, Zailiang 
  Tomasz Michalec 
  Vijayenthiran Subramaniam 
  Wu Jiaxin 
  yuchenlin 
  Zailiang Sun 
  Zhang, Chao B 
  zwei4 

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



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.

(No revision log; it would be 1720 lines long.)

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

[Xen-devel] [PATCH 00/41] scsi: Mark expected switch fall-throughs

[Gustavo A. R. Silva]

In preparation to enabling -Wimplicit-fallthrough, this patchset aims
to mark switch cases where we are expecting to fall through.

I reviewed case by case and concluded that each of them is an
intentional fall-through. However, it doesn't hurt that the
maintainers and supporters of each driver take a look. :)

Each commit log contains the particular details for the changes in the
corresponding file.

This series fix a total of 110 of the following type of warnings in
drivers/scsi:

drivers/scsi/aic7xxx/aic7xxx_core.c:4921:3: warning: this statement may fall 
through [-Wimplicit-fallthrough=]
   ahc_dma_tag_destroy(ahc, scb_data->sg_dmat);
   ^~~
drivers/scsi/aic7xxx/aic7xxx_core.c:4923:2: note: here
  case 6:
  ^~~~

Thanks!

Gustavo A. R. Silva (41):
  scsi: BusLogic: mark expected switch fall-through
  scsi: NCR5380: Mark expected switch fall-through
  scsi: aacraid: aachba: Mark expected switch fall-throughs
  scsi: aacraid: linit: Mark expected switch fall-through
  scsi: aic7xxx: aic79xx: mark expected switch fall-through
  scsi: aic7xxx: mark expected switch fall-throughs
  scsi: be2iscsi: be_iscsi: Mark expected switch fall-through
  scsi: be2iscsi: be_main: Mark expected switch fall-through
  scsi: bfa: bfa_fcpim: Mark expected switch fall-throughs
  scsi: bfa: bfa_fcs_lport: Mark expected switch fall-throughs
  scsi: bfa: bfa_fcs_rport: Mark expected switch fall-throughs
  scsi: bfa: bfa_ioc: Mark expected switch fall-throughs
  scsi: csiostor: csio_wr: mark expected switch fall-through
  scsi: esas2r: esas2r_init: mark expected switch fall-throughs
  scsi: hpsa: mark expected switch fall-throughs
  scsi: imm: mark expected switch fall-throughs
  scsi: isci: phy: Mark expected switch fall-through
  scsi: isci: remote_device: Mark expected switch fall-throughs
  scsi: isci: remote_node_context: mark expected switch fall-throughs
  scsi: isci: request: mark expected switch fall-through
  scsi: libfc: fc_rport: Mark expected switch fall-through
  scsi: lpfc: lpfc_ct: Mark expected switch fall-throughs
  scsi: lpfc: lpfc_els: Mark expected switch fall-throughs
  scsi: lpfc: lpfc_hbadisc: Mark expected switch fall-throughs
  scsi: lpfc: lpfc_nportdisc: Mark expected switch fall-through
  scsi: lpfc: lpfc_nvme: Mark expected switch fall-through
  scsi: lpfc: lpfc_scsi: Mark expected switch fall-throughs
  scsi: lpfc: lpfc_sli: Mark expected switch fall-throughs
  scsi: megaraid: megaraid_sas_base: Mark expected switch fall-through
  scsi: megaraid_sas_fusion: Mark expected switch fall-through
  scsi: mpt3sas: mpt3sas_scsih: Mark expected switch fall-through
  scsi: myrb: Mark expected switch fall-throughs
  scsi: osd: osd_initiator: mark expected switch fall-throughs
  scsi: osst: mark expected switch fall-throughs
  scsi: ppa: mark expected switch fall-through
  scsi: qla4xxx: ql4_os: mark expected switch fall-through
  scsi: st: mark expected switch fall-throughs
  scsi: sym53c8xx_2: sym_hipd: mark expected switch fall-throughs
  scsi: sym53c8xx_2: sym_nvram: Mark expected switch fall-through
  scsi: ufs: ufshcd: mark expected switch fall-throughs
  scsi: xen-scsifront: mark expected switch fall-through

 drivers/scsi/BusLogic.c |  1 +
 drivers/scsi/NCR5380.c  |  3 +-
 drivers/scsi/aacraid/aachba.c   |  5 +++-
 drivers/scsi/aacraid/linit.c|  1 +
 drivers/scsi/aic7xxx/aic79xx_core.c | 14 +
 drivers/scsi/aic7xxx/aic7xxx_core.c | 12 ++--
 drivers/scsi/be2iscsi/be_iscsi.c|  1 +
 drivers/scsi/be2iscsi/be_main.c |  1 +
 drivers/scsi/bfa/bfa_fcpim.c|  6 ++--
 drivers/scsi/bfa/bfa_fcs_lport.c|  8 ++---
 drivers/scsi/bfa/bfa_fcs_rport.c| 19 +---
 drivers/scsi/bfa/bfa_ioc.c  |  9 ++
 drivers/scsi/csiostor/csio_wr.c |  1 +
 drivers/scsi/esas2r/esas2r_init.c   |  3 +-
 drivers/scsi/hpsa.c |  5 
 drivers/scsi/imm.c  | 33 +++--
 drivers/scsi/isci/phy.c |  1 +
 drivers/scsi/isci/remote_device.c   |  4 +--
 drivers/scsi/isci/remote_node_context.c |  4 +--
 drivers/scsi/isci/request.c |  2 +-
 drivers/scsi/libfc/fc_rport.c   |  1 +
 drivers/scsi/lpfc/lpfc_ct.c |  2 ++
 drivers/scsi/lpfc/lpfc_els.c|  1 +
 drivers/scsi/lpfc/lpfc_hbadisc.c|  4 ++-
 drivers/scsi/lpfc/lpfc_nportdisc.c  |  1 +
 drivers/scsi/lpfc/lpfc_nvme.c   |  1 +
 drivers/scsi/lpfc/lpfc_scsi.c   |  8 ++---
 drivers/scsi/lpfc/lpfc_sli.c| 20 +++--
 drivers/scsi/megaraid/megaraid_sas_base.c   |  1 +
 drivers/scsi/megaraid/megaraid_sas_fusion.c |  2 +-
 drivers/scsi/mpt3sas/mpt3sas_scsih.c|  1 +
 drivers/scsi/myrb.c |  3 ++
 drivers/scsi/osd/osd_initiator.c  

[Xen-devel] [xen-unstable-smoke test] 130841: tolerable all pass - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

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  3e0c9519d8dd68970116ba6c79b82b5b7317329d
baseline version:
 xen  43fa95ae6a64132b8ebe3025bd187ab9df68677b

Last test of basis   130839  2018-11-27 18:00:32 Z0 days
Testing same since   130841  2018-11-28 01:00:39 Z0 days1 attempts


People who touched revisions under test:
  Andrii Anisov 
  Julien Grall 
  Stefano Stabellini 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-amd64-amd64-xl-qemuu-debianhvm-i386 pass
 test-amd64-amd64-libvirt pass



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   43fa95ae6a..3e0c9519d8  3e0c9519d8dd68970116ba6c79b82b5b7317329d -> smoke

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

[Xen-devel] [qemu-mainline test] 130798: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

flight 130798 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/130798/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64  broken
 build-arm64-pvopsbroken
 build-arm64-xsm  broken
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail REGR. vs. 129996
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail REGR. vs. 129996

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-shadow7 xen-boot   fail pass in 130739
 test-armhf-armhf-xl-cubietruck 16 guest-start/debian.repeat fail pass in 130739

Regressions which are regarded as allowable (not blocking):
 build-arm64-pvops 2 hosts-allocate broken REGR. vs. 129996
 build-arm64   2 hosts-allocate broken REGR. vs. 129996
 build-arm64-xsm   2 hosts-allocate broken REGR. vs. 129996

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit1   1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 build-arm64   3 capture-logs  broken blocked in 129996
 build-arm64-xsm   3 capture-logs  broken blocked in 129996
 build-arm64-pvops 3 capture-logs  broken blocked in 129996
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 129996
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 129996
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 129996
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 129996
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 129996
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass

version targeted for testing:
 qemuu5298f4d67a911dd9cefa4c4185eed242074d64c2
baseline version:
 qemuucb968d275c145467c8b385a3618a207ec111eab1

Last test of basis   129996  2018-11-13 22:49:16 Z   14 days
Failing since130168  2018-11-16 04:27:30 Z   11 days8 attempts
Testing same since   130739  

[Xen-devel] [linux-3.18 test] 130796: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64-pvopsbroken
 build-arm64  broken
 build-arm64-xsm  broken
 build-arm64-pvops 3 capture-logs   broken REGR. vs. 128858
 test-amd64-i386-examine   8 reboot   fail REGR. vs. 128858
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 
128858
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 
128858
 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 128858
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. 
vs. 128858
 test-amd64-amd64-xl-multivcpu  7 xen-bootfail REGR. vs. 128858
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. 
vs. 128858
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-boot fail REGR. vs. 128858
 test-amd64-i386-freebsd10-i386  7 xen-boot   fail REGR. vs. 128858
 test-amd64-i386-xl-qemuu-debianhvm-amd64  7 xen-boot fail REGR. vs. 128858
 test-amd64-i386-xl-qemuu-ovmf-amd64  7 xen-boot  fail REGR. vs. 128858
 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 128858
 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 128858
 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 128858
 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 128858
 test-amd64-amd64-rumprun-amd64  7 xen-boot   fail REGR. vs. 128858
 test-amd64-amd64-libvirt-pair 10 xen-boot/src_host   fail REGR. vs. 128858
 test-amd64-amd64-libvirt-pair 11 xen-boot/dst_host   fail REGR. vs. 128858
 test-amd64-amd64-xl-xsm   7 xen-boot fail REGR. vs. 128858
 test-amd64-amd64-pair10 xen-boot/src_hostfail REGR. vs. 128858
 test-amd64-amd64-pair11 xen-boot/dst_hostfail REGR. vs. 128858

Tests which are failing intermittently (not blocking):
 test-amd64-i386-xl7 xen-boot fail in 130716 pass in 130796
 test-armhf-armhf-xl-rtds  5 host-ping-check-native fail pass in 130716
 test-armhf-armhf-xl-cubietruck 16 guest-start/debian.repeat fail pass in 130716

Regressions which are regarded as allowable (not blocking):
 build-arm64-pvops 2 hosts-allocate broken REGR. vs. 128858
 build-arm64-xsm   2 hosts-allocate broken REGR. vs. 128858
 build-arm64   2 hosts-allocate broken REGR. vs. 128858

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit1   1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-examine  1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 build-arm64   3 capture-logs  broken blocked in 128858
 build-arm64-xsm   3 capture-logs  broken blocked in 128858
 test-amd64-i386-xl-qemut-debianhvm-amd64 16 guest-localmigrate/x10 fail in 
130716 like 128807
 test-armhf-armhf-libvirt-raw 10 debian-di-install   fail in 130716 like 128841
 test-armhf-armhf-xl-rtds13 migrate-support-check fail in 130716 never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-check fail in 130716 never pass
 test-amd64-amd64-examine  4 memdisk-try-append   fail  like 128807
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 128858
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 128858
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 128858
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 128858
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 128858
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 128858
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail never pass
 test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvhv2-amd 12 guest-start  fail  never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-libvirt 13 

Re: [Xen-devel] [PATCH 1/4] xen/arm: gic: Ensure we have an ISB between ack and do_IRQ()

[Stefano Stabellini]

On Thu, 22 Nov 2018, Julien Grall wrote:
> > > If you are worried about performance, then I would recommend to try the
> > > new vGIC and see whether it improves.
> > You know, we are based on XEN 4.10. Initially, when a customer said about
> > their dissatisfaction about performance drop in benchmark due to XEN
> > existence, I tried 4.12-unstable, both an old and a new VGIC. So performance
> > with 4.12-unstable with the old VGIC was worse than 4.10, and the new VGIC
> > made things even much worse. I can't remember the exact numbers or
> > proportions, but that was the reason we do not offer upgrading XEN yet.
> 
> I can't comment without any numbers here. Bear in mind that we fixed bugs in
> Xen 4.12 (including spectre/meltdown and missing barriers) that wasn't
> backported to Xen 4.10. It is entirely possible that it introduced slowness
> but it also ensure the code is behaving correctly.
> 
> Anyway, if there are performance regression we should investigate them and
> discuss how we can address/limit them. Similarly for the new vGIC, if you
> think it is too slow, then we need to know why before we get rid of the old
> vGIC.

Well said! We care about interrupt performance very much and we
definitely need to address any regressions with either the old or the
new driver. But to do that, we need reliable numbers and to figure out
exactly what the problem is so that we can fix it.

I sent the way I used to measure performance in a separate email.

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

Re: [Xen-devel] [ARM] gvirt_to_maddr fails when DomU is created

[Andrew Cooper]

On 28/11/2018 00:05, Andrew Cooper wrote:
> On 27/11/2018 19:40, Julien Grall wrote:
>> (+ Stefano)
>>
>> On 11/27/18 5:12 PM, Volodymyr Babchuk wrote:
>>> Hello community,
>> Hi Volodymyr,
>>
>>> After creating domU, I'm seeing lots of this messages from hypervisor:
>>>
>>> (XEN) p2m.c:1442: d1v0: gvirt_to_maddr failed va=0x8efc7f0f
>>> flags=0x1 par=0x809
>>> (XEN) p2m.c:1442: d1v0: gvirt_to_maddr failed va=0x8efc7f00
>>> flags=0x1 par=0x809
>>> (XEN) p2m.c:1442: d1v0: gvirt_to_maddr failed va=0x8efc7f0f
>>> flags=0x1 par=0x809
>>>
>>> Interestingly, I'm getting them from both Dom0 and DomU:
>>>
>>> (XEN) p2m.c:1442: d0v0: gvirt_to_maddr failed va=0x80003efd7f0f
>>> flags=0x1 par=0x809
>>> (XEN) p2m.c:1442: d1v0: gvirt_to_maddr failed va=0x8efc7f0f
>>> flags=0x1 par=0x809
>>>
>>> But only after DomU is created.
>>>
>>> I attached GDB and found that this is caused by update_runstate_area:
>>>
>>> (gdb) bt
>>> #0  get_page_from_gva (v=0x80005dbe2000, v@entry=0x22f2c8
>>> ,
>>>  va=va@entry=18446603337277996815, flags=flags@entry=1) at
>>> p2m.c:1440
>>> #1  0x0024e320 in translate_get_page (write=true, linear=true,
>>> addr=18446603337277996815,
>>>  info=...) at guestcopy.c:37
>>> #2  copy_guest (buf=buf@entry=0x80005dbe20d7,
>>> addr=addr@entry=18446603337277996815, len=len@entry=1,
>>>  info=..., flags=flags@entry=6) at guestcopy.c:69
>>> #3  0x0024e45c in raw_copy_to_guest
>>> (to=to@entry=0x80003efd7f0f,
>>>  from=from@entry=0x80005dbe20d7, len=len@entry=1) at guestcopy.c:110
>>> #4  0x002497b4 in update_runstate_area
>>> (v=v@entry=0x80005dbe2000) at domain.c:287
>>> #5  0x00249eb8 in context_switch
>>> (prev=prev@entry=0x80005dbe2000,
>>>  next=next@entry=0x80005bf3c000) at domain.c:344
>>> #6  0x0022f2c8 in schedule () at schedule.c:1583
>>> #7  0x00232c10 in __do_softirq
>>> (ignore_mask=ignore_mask@entry=0) at softirq.c:50
>>> #8  0x00232ca4 in do_softirq () at softirq.c:64
>>> #9  0x00258254 in leave_hypervisor_tail () at traps.c:2302
>>>
>>> This issue is encountered on QEMU-ARMv8. Dom0 kernel is Linux 4.19.0
>>> My XEN master is at d8ffac1f7 "xen/arm: gic: Remove duplicated comment
>>> in do_sgi"
>>>
>>> The same setup worked perfectly with Xen 4.10.2
>> The message is only printed in debug build. Do you have CONFIG_DEBUG
>> enabled?
>>
>>> Julien, I saw on mailing list, that you paid attention to issues with
>>> gvirt_to_maddr,
>>> so maybe you can be interested in this.
>> Which thread are you speaking about? The problem is not because of
>> gvirt_to_maddr but of how update_runstate_area is working at the moment.
>>
>> update_runstate_area is using a guest virtual address to update the
>> vCPU runstate. It blindly assumes the vCPU runstate will always be
>> mapped in stage-1 page-tables. However, if KPTI (Kernel Page Table
>> Isolation) is enabled the kernel address space (and therefore the vCPU
>> runstate) will not be mapped when running at EL0.
>>
>> So if you are restoring a vCPU that was executing code at EL0 then
>> update_runstate_area will fail as the address is not mapped. There are
>> a few solution suggested on the ML (see [1]). However I haven't had
>> time to look at properly how to implement them.
>>
>> KPTI is getting used more widely (e.g meltdown and KASLR). So it would
>> be good if we try to solve this problem sooner. I would be happy to
>> review patches and/or provide advice if you want to tackle the problem.
>>
>> Cheers,
>>
>> [1] https://lists.xen.org/archives/html/xen-devel/2018-03/msg00223.html
>>
> update_runstate_area() using a virtual address is a complete misfeature,
> and the sooner we can replace it, the better.  It's history is with x86
> PV guests, where the early ABIs were designed in terms of Linux's
> copy_{to,from}_user().
>
> It is similarly broken in x86 with meltdown mitigations, as well as SMAP
> considerations (PAN in ARM, iirc).
>
> We've got two options.  Invent a new API which takes a gfn/gaddr, or
> retrofit the API to be "you pass a virtual address, we translate to
> gfn/gaddr, then update that".  Perhaps both.
>
> When this was last discussed, I think the "onetime translate to
> gfn/gaddr" was a good enough compatibility to cope with existing guests,
> but that we should have a more clean way for modern guests.

Or alternatively, see if we can actually get away without it.  A lot of
the early Xen paravirtual functionality can probably be done without, or
designed in a better way entirely.

~Andrew

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

Re: [Xen-devel] [PATCH 2/2] arm/irq: Fix block parathenses and whitespaces

[Stefano Stabellini]

On Fri, 16 Nov 2018, Andrii Anisov wrote:
> From: Andrii Anisov 
> 
> Signed-off-by: Andrii Anisov 

Acked-by: Stefano Stabellini 

> ---
>  xen/arch/arm/irq.c | 12 
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index d5ad277..d6a0273 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -61,7 +61,9 @@ static DEFINE_PER_CPU(irq_desc_t[NR_LOCAL_IRQS], 
> local_irq_desc);
>  
>  irq_desc_t *__irq_to_desc(int irq)
>  {
> -if (irq < NR_LOCAL_IRQS) return _cpu(local_irq_desc)[irq];
> +if ( irq < NR_LOCAL_IRQS )
> +return _cpu(local_irq_desc)[irq];
> +
>  return _desc[irq-NR_LOCAL_IRQS];
>  }
>  
> @@ -76,7 +78,8 @@ static int __init init_irq_data(void)
>  {
>  int irq;
>  
> -for (irq = NR_LOCAL_IRQS; irq < NR_IRQS; irq++) {
> +for ( irq = NR_LOCAL_IRQS; irq < NR_IRQS; irq++ )
> +{
>  struct irq_desc *desc = irq_to_desc(irq);
>  init_one_irq_desc(desc);
>  desc->irq = irq;
> @@ -92,7 +95,8 @@ static int init_local_irq_data(void)
>  
>  spin_lock(_irqs_type_lock);
>  
> -for (irq = 0; irq < NR_LOCAL_IRQS; irq++) {
> +for ( irq = 0; irq < NR_LOCAL_IRQS; irq++ )
> +{
>  struct irq_desc *desc = irq_to_desc(irq);
>  init_one_irq_desc(desc);
>  desc->irq = irq;
> @@ -193,7 +197,7 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, 
> int is_fiq)
>  
>  ASSERT(irq >= 16); /* SGIs do not come down this path */
>  
> -if (irq < 32)
> +if ( irq < 32 )
>  perfc_incr(ppis);
>  else
>  perfc_incr(spis);
> -- 
> 2.7.4
> 

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

Re: [Xen-devel] [ARM] gvirt_to_maddr fails when DomU is created

[Andrew Cooper]

On 27/11/2018 19:40, Julien Grall wrote:
> (+ Stefano)
>
> On 11/27/18 5:12 PM, Volodymyr Babchuk wrote:
>> Hello community,
>
> Hi Volodymyr,
>
>>
>> After creating domU, I'm seeing lots of this messages from hypervisor:
>>
>> (XEN) p2m.c:1442: d1v0: gvirt_to_maddr failed va=0x8efc7f0f
>> flags=0x1 par=0x809
>> (XEN) p2m.c:1442: d1v0: gvirt_to_maddr failed va=0x8efc7f00
>> flags=0x1 par=0x809
>> (XEN) p2m.c:1442: d1v0: gvirt_to_maddr failed va=0x8efc7f0f
>> flags=0x1 par=0x809
>>
>> Interestingly, I'm getting them from both Dom0 and DomU:
>>
>> (XEN) p2m.c:1442: d0v0: gvirt_to_maddr failed va=0x80003efd7f0f
>> flags=0x1 par=0x809
>> (XEN) p2m.c:1442: d1v0: gvirt_to_maddr failed va=0x8efc7f0f
>> flags=0x1 par=0x809
>>
>> But only after DomU is created.
>>
>> I attached GDB and found that this is caused by update_runstate_area:
>>
>> (gdb) bt
>> #0  get_page_from_gva (v=0x80005dbe2000, v@entry=0x22f2c8
>> ,
>>  va=va@entry=18446603337277996815, flags=flags@entry=1) at
>> p2m.c:1440
>> #1  0x0024e320 in translate_get_page (write=true, linear=true,
>> addr=18446603337277996815,
>>  info=...) at guestcopy.c:37
>> #2  copy_guest (buf=buf@entry=0x80005dbe20d7,
>> addr=addr@entry=18446603337277996815, len=len@entry=1,
>>  info=..., flags=flags@entry=6) at guestcopy.c:69
>> #3  0x0024e45c in raw_copy_to_guest
>> (to=to@entry=0x80003efd7f0f,
>>  from=from@entry=0x80005dbe20d7, len=len@entry=1) at guestcopy.c:110
>> #4  0x002497b4 in update_runstate_area
>> (v=v@entry=0x80005dbe2000) at domain.c:287
>> #5  0x00249eb8 in context_switch
>> (prev=prev@entry=0x80005dbe2000,
>>  next=next@entry=0x80005bf3c000) at domain.c:344
>> #6  0x0022f2c8 in schedule () at schedule.c:1583
>> #7  0x00232c10 in __do_softirq
>> (ignore_mask=ignore_mask@entry=0) at softirq.c:50
>> #8  0x00232ca4 in do_softirq () at softirq.c:64
>> #9  0x00258254 in leave_hypervisor_tail () at traps.c:2302
>>
>> This issue is encountered on QEMU-ARMv8. Dom0 kernel is Linux 4.19.0
>> My XEN master is at d8ffac1f7 "xen/arm: gic: Remove duplicated comment
>> in do_sgi"
>>
>> The same setup worked perfectly with Xen 4.10.2
>
> The message is only printed in debug build. Do you have CONFIG_DEBUG
> enabled?
>
>>
>> Julien, I saw on mailing list, that you paid attention to issues with
>> gvirt_to_maddr,
>> so maybe you can be interested in this.
>
> Which thread are you speaking about? The problem is not because of
> gvirt_to_maddr but of how update_runstate_area is working at the moment.
>
> update_runstate_area is using a guest virtual address to update the
> vCPU runstate. It blindly assumes the vCPU runstate will always be
> mapped in stage-1 page-tables. However, if KPTI (Kernel Page Table
> Isolation) is enabled the kernel address space (and therefore the vCPU
> runstate) will not be mapped when running at EL0.
>
> So if you are restoring a vCPU that was executing code at EL0 then
> update_runstate_area will fail as the address is not mapped. There are
> a few solution suggested on the ML (see [1]). However I haven't had
> time to look at properly how to implement them.
>
> KPTI is getting used more widely (e.g meltdown and KASLR). So it would
> be good if we try to solve this problem sooner. I would be happy to
> review patches and/or provide advice if you want to tackle the problem.
>
> Cheers,
>
> [1] https://lists.xen.org/archives/html/xen-devel/2018-03/msg00223.html
>

update_runstate_area() using a virtual address is a complete misfeature,
and the sooner we can replace it, the better.  It's history is with x86
PV guests, where the early ABIs were designed in terms of Linux's
copy_{to,from}_user().

It is similarly broken in x86 with meltdown mitigations, as well as SMAP
considerations (PAN in ARM, iirc).

We've got two options.  Invent a new API which takes a gfn/gaddr, or
retrofit the API to be "you pass a virtual address, we translate to
gfn/gaddr, then update that".  Perhaps both.

When this was last discussed, I think the "onetime translate to
gfn/gaddr" was a good enough compatibility to cope with existing guests,
but that we should have a more clean way for modern guests.

~Andrew

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

Re: [Xen-devel] [PATCH] pvcalls-front: fixes incorrect error handling

[Stefano Stabellini]

On Tue, 27 Nov 2018, Boris Ostrovsky wrote:
> On 11/27/18 4:08 PM, Stefano Stabellini wrote:
> > On Tue, 27 Nov 2018, Boris Ostrovsky wrote:
> >> On 11/27/18 3:37 PM, Stefano Stabellini wrote:
> >>> On Tue, 27 Nov 2018, PanBian wrote:
>  On Mon, Nov 26, 2018 at 03:31:39PM -0500, Boris Ostrovsky wrote:
> > On 11/21/18 9:07 PM, Pan Bian wrote:
> >> kfree() is incorrectly used to release the pages allocated by
> >> __get_free_page() and __get_free_pages(). Use the matching deallocators
> >> i.e., free_page() and free_pages(), respectively.
> >>
> >> Signed-off-by: Pan Bian 
> >> ---
> >>  drivers/xen/pvcalls-front.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c
> >> index 2f11ca7..77224d8 100644
> >> --- a/drivers/xen/pvcalls-front.c
> >> +++ b/drivers/xen/pvcalls-front.c
> >> @@ -385,8 +385,8 @@ static int create_active(struct sock_mapping *map, 
> >> int *evtchn)
> >>  out_error:
> >>if (*evtchn >= 0)
> >>xenbus_free_evtchn(pvcalls_front_dev, *evtchn);
> >> -  kfree(map->active.data.in);
> >> -  kfree(map->active.ring);
> >> +  free_pages((unsigned long)map->active.data.in, 
> >> PVCALLS_RING_ORDER);
> > Is map->active.data.in guaranteed to be NULL when entering this routine?
>  I am not sure yet. Sorry for that. I observed the mismatches between
>  __get_free_page and kfree, and submitted the patch.
> 
>  But I think your consideration is reasonable. A better solution is to
>  directly free bytes, a local variable that holds __get_free_pages return
>  value. If you agree, I will rewrite the patch.
> >>> Like Boris said, map->active.ring and map->active.data.in are not
> >>> guaranteed to be NULL or != NULL here. For instance,map->active.ring can
> >>> be != NULL and map->active.data.in can be NULL. However, free_pages and
> >>> free_page should be able to cope with it, the same way that kfree is
> >>> able to cope with it?
> >> If map->active.data.in can be non-NULL on entry to this routine then I
> >> think this has been a problem all along. Pan's suggestion to use bytes
> >> for freeing is going to solve this (assuming bytes will be initialized
> >> to NULL).
> > Why is it a problem? map->active.data.in and map->active.ring are only
> > != NULL if they need to be freed. Otherwise, they are NULL. 
> 
> That was my question --- I wasn't sure about it, and I read your
> previous message as if it was possible to be calling create_active()
> with map->active.data.in pointing somewhere non-NULL.
> 
> If it is NULL *upon entry* to calling_create() then Pan's original patch
> is fine.

Right, I think it is fine too.

Reviewed-by: Stefano Stabellini 

 
> > All structs
> > are always initialized to zero. I don't think there are any issues.
> 

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

Re: [Xen-devel] [PATCH] pvcalls-front: fixes incorrect error handling

[Boris Ostrovsky]

On 11/27/18 4:08 PM, Stefano Stabellini wrote:
> On Tue, 27 Nov 2018, Boris Ostrovsky wrote:
>> On 11/27/18 3:37 PM, Stefano Stabellini wrote:
>>> On Tue, 27 Nov 2018, PanBian wrote:
 On Mon, Nov 26, 2018 at 03:31:39PM -0500, Boris Ostrovsky wrote:
> On 11/21/18 9:07 PM, Pan Bian wrote:
>> kfree() is incorrectly used to release the pages allocated by
>> __get_free_page() and __get_free_pages(). Use the matching deallocators
>> i.e., free_page() and free_pages(), respectively.
>>
>> Signed-off-by: Pan Bian 
>> ---
>>  drivers/xen/pvcalls-front.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c
>> index 2f11ca7..77224d8 100644
>> --- a/drivers/xen/pvcalls-front.c
>> +++ b/drivers/xen/pvcalls-front.c
>> @@ -385,8 +385,8 @@ static int create_active(struct sock_mapping *map, 
>> int *evtchn)
>>  out_error:
>>  if (*evtchn >= 0)
>>  xenbus_free_evtchn(pvcalls_front_dev, *evtchn);
>> -kfree(map->active.data.in);
>> -kfree(map->active.ring);
>> +free_pages((unsigned long)map->active.data.in, 
>> PVCALLS_RING_ORDER);
> Is map->active.data.in guaranteed to be NULL when entering this routine?
 I am not sure yet. Sorry for that. I observed the mismatches between
 __get_free_page and kfree, and submitted the patch.

 But I think your consideration is reasonable. A better solution is to
 directly free bytes, a local variable that holds __get_free_pages return
 value. If you agree, I will rewrite the patch.
>>> Like Boris said, map->active.ring and map->active.data.in are not
>>> guaranteed to be NULL or != NULL here. For instance,map->active.ring can
>>> be != NULL and map->active.data.in can be NULL. However, free_pages and
>>> free_page should be able to cope with it, the same way that kfree is
>>> able to cope with it?
>> If map->active.data.in can be non-NULL on entry to this routine then I
>> think this has been a problem all along. Pan's suggestion to use bytes
>> for freeing is going to solve this (assuming bytes will be initialized
>> to NULL).
> Why is it a problem? map->active.data.in and map->active.ring are only
> != NULL if they need to be freed. Otherwise, they are NULL. 

That was my question --- I wasn't sure about it, and I read your
previous message as if it was possible to be calling create_active()
with map->active.data.in pointing somewhere non-NULL.

If it is NULL *upon entry* to calling_create() then Pan's original patch
is fine.


-boris


> All structs
> are always initialized to zero. I don't think there are any issues.


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

Re: [Xen-devel] [ARM] gvirt_to_maddr fails when DomU is created

[Stefano Stabellini]

On Tue, 27 Nov 2018, Julien Grall wrote:
> (+ Stefano)
> 
> On 11/27/18 5:12 PM, Volodymyr Babchuk wrote:
> > Hello community,
> 
> Hi Volodymyr,
> 
> > 
> > After creating domU, I'm seeing lots of this messages from hypervisor:
> > 
> > (XEN) p2m.c:1442: d1v0: gvirt_to_maddr failed va=0x8efc7f0f
> > flags=0x1 par=0x809
> > (XEN) p2m.c:1442: d1v0: gvirt_to_maddr failed va=0x8efc7f00
> > flags=0x1 par=0x809
> > (XEN) p2m.c:1442: d1v0: gvirt_to_maddr failed va=0x8efc7f0f
> > flags=0x1 par=0x809
> > 
> > Interestingly, I'm getting them from both Dom0 and DomU:
> > 
> > (XEN) p2m.c:1442: d0v0: gvirt_to_maddr failed va=0x80003efd7f0f
> > flags=0x1 par=0x809
> > (XEN) p2m.c:1442: d1v0: gvirt_to_maddr failed va=0x8efc7f0f
> > flags=0x1 par=0x809
> > 
> > But only after DomU is created.
> > 
> > I attached GDB and found that this is caused by update_runstate_area:
> > 
> > (gdb) bt
> > #0  get_page_from_gva (v=0x80005dbe2000, v@entry=0x22f2c8 ,
> >  va=va@entry=18446603337277996815, flags=flags@entry=1) at p2m.c:1440
> > #1  0x0024e320 in translate_get_page (write=true, linear=true,
> > addr=18446603337277996815,
> >  info=...) at guestcopy.c:37
> > #2  copy_guest (buf=buf@entry=0x80005dbe20d7,
> > addr=addr@entry=18446603337277996815, len=len@entry=1,
> >  info=..., flags=flags@entry=6) at guestcopy.c:69
> > #3  0x0024e45c in raw_copy_to_guest (to=to@entry=0x80003efd7f0f,
> >  from=from@entry=0x80005dbe20d7, len=len@entry=1) at guestcopy.c:110
> > #4  0x002497b4 in update_runstate_area
> > (v=v@entry=0x80005dbe2000) at domain.c:287
> > #5  0x00249eb8 in context_switch (prev=prev@entry=0x80005dbe2000,
> >  next=next@entry=0x80005bf3c000) at domain.c:344
> > #6  0x0022f2c8 in schedule () at schedule.c:1583
> > #7  0x00232c10 in __do_softirq
> > (ignore_mask=ignore_mask@entry=0) at softirq.c:50
> > #8  0x00232ca4 in do_softirq () at softirq.c:64
> > #9  0x00258254 in leave_hypervisor_tail () at traps.c:2302
> > 
> > This issue is encountered on QEMU-ARMv8. Dom0 kernel is Linux 4.19.0
> > My XEN master is at d8ffac1f7 "xen/arm: gic: Remove duplicated comment
> > in do_sgi"
> > 
> > The same setup worked perfectly with Xen 4.10.2
> 
> The message is only printed in debug build. Do you have CONFIG_DEBUG enabled?
> 
> > 
> > Julien, I saw on mailing list, that you paid attention to issues with
> > gvirt_to_maddr,
> > so maybe you can be interested in this.
> 
> Which thread are you speaking about? The problem is not because of
> gvirt_to_maddr but of how update_runstate_area is working at the moment.
> 
> update_runstate_area is using a guest virtual address to update the vCPU
> runstate. It blindly assumes the vCPU runstate will always be mapped in
> stage-1 page-tables. However, if KPTI (Kernel Page Table Isolation) is enabled
> the kernel address space (and therefore the vCPU runstate) will not be mapped
> when running at EL0.
> 
> So if you are restoring a vCPU that was executing code at EL0 then
> update_runstate_area will fail as the address is not mapped. There are a few
> solution suggested on the ML (see [1]). However I haven't had time to look at
> properly how to implement them.
> 
> KPTI is getting used more widely (e.g meltdown and KASLR). So it would be good
> if we try to solve this problem sooner. I would be happy to review patches
> and/or provide advice if you want to tackle the problem.
> 
> Cheers,
> 
> [1] https://lists.xen.org/archives/html/xen-devel/2018-03/msg00223.html

I have seen this issue too, it is quite painful. I haven't had the time
to fix it, but like Julien, I would be happy to help somebody else fix
it.

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

Re: [Xen-devel] [PATCH] Revert "xen/balloon: Mark unallocated host memory as UNUSABLE"

[Boris Ostrovsky]

On 11/27/18 3:58 PM, Igor Druzhinin wrote:
> This reverts commit b3cf8528bb21febb650a7ecbf080d0647be40b9f.
>
> That commit unintentionally broke Xen balloon memory hotplug with
> "hotplug_unpopulated" set to 1. As long as "System RAM" resource
> got assigned under a new "Unusable memory" resource in IO/Mem tree
> any attempt to online this memory would fail due to general kernel
> restrictions on having "System RAM" resources as 1st level only.
>
> The original issue that commit has tried to workaround fa564ad96366
> ("x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 00-1f, 30-3f,
> 60-7f)") also got amended by the following 03a551734 ("x86/PCI: Move
> and shrink AMD 64-bit window to avoid conflict") which made the
> original fix to Xen ballooning unnecessary.
>
> Signed-off-by: Igor Druzhinin 
> ---
> In mail thread [1] it was agreed to revert the change due to technical
> comlications of fixing it and the fact that there is no any strong reason
> to keep it.
>
> [1] https://lists.xenproject.org/archives/html/xen-devel/2018-11/msg02834.html
> ---
>  arch/x86/xen/enlighten.c | 78 
> 
>  arch/x86/xen/setup.c |  6 ++--
>  drivers/xen/balloon.c| 65 ++--
>  include/xen/balloon.h|  5 
>  4 files changed, 13 insertions(+), 141 deletions(-)


Reviewed-by: Boris Ostrovsky 

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

Re: [Xen-devel] Xen optimization

[Stefano Stabellini]

On Tue, 20 Nov 2018, Andrii Anisov wrote:
> Hello Stefano,
> 
> On 01.11.18 22:20, Stefano Stabellini wrote:
> > No, I haven't had any time. Aside from the Xen version, another
> > difference is the interrupt source. I used the physical timer for
> > testing.
> 
> Could you share your approach for interrupts latency measurement? Are you
> using any HW specifics or it is SoC independent?
> 
> I would like to get more evidences for optimizations of gic/vgic/gic-v2 code I
> did for our customer (its about old vgic, we are still on xen 4.10).

Hi Andrii,

See the following:

https://marc.info/?l=xen-devel=148668817704668

The numbers have improved now thanks to vwfi=native and other
optimizations but the mechanism to setup the experiment are the same.

Cheers,

Stefano

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

Re: [Xen-devel] [PATCH v5 15/20] xen_pvh: add build runes for grub-core

[Daniel Kiper]

On Wed, Nov 21, 2018 at 03:28:50PM +0100, Juergen Gross wrote:
> Add the modifications to the build system needed to build a xen_pvh
> grub.
>
> Signed-off-by: Juergen Gross 
> Reviewed-by: Daniel Kiper 
> ---
> V3: sorted some filenames (Daniel Kiper)
> V4: add bus/pci.c to xen_pvh

V5 drops bus/pci.c from xen_pvh. Is it intentional or mistake?

Daniel

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

Re: [Xen-devel] [PATCH v5 13/20] xen: setup Xen specific data for PVH

[Daniel Kiper]

On Wed, Nov 21, 2018 at 03:28:48PM +0100, Juergen Gross wrote:
> Initialize the needed Xen specific data. This is:
>
> - the Xen start of day page containing the console and Xenstore ring
>   page PFN and event channel
> - the grant table
> - the shared info page
>
> Write back the possibly modified memory map to the hypervisor in case
> the guest is reading it from there again.
>
> Set the RSDP address for the guest from the start_info page passed
> as boot parameter.
>
> Signed-off-by: Juergen Gross 
> Reviewed-by: Daniel Kiper 

One nitpick below...

> ---
> V4: write back memory map to Xen (Roger Pau Monné)
> V5: add comment (Daniel Kiper)
> ---
>  grub-core/kern/i386/xen/pvh.c | 120 
> ++
>  1 file changed, 120 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index bb90874b3..6de84eb8e 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -27,6 +27,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>
>  #define XEN_MEMORY_MAP_SIZE   128
> @@ -37,6 +38,7 @@ static char hypercall_page[GRUB_XEN_PAGE_SIZE]
>__attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
>
>  static grub_uint32_t xen_cpuid_base;
> +static struct start_info grub_xen_start_page;
>  static struct grub_e820_mmap_entry map[XEN_MEMORY_MAP_SIZE];
>  static unsigned int nr_map_entries;
>
> @@ -110,6 +112,36 @@ grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t 
> a0,
>return __res;
>  }
>
> +static grub_uint32_t
> +grub_xen_get_param (int idx)
> +{
> +  struct xen_hvm_param xhv;
> +  int r;
> +
> +  xhv.domid = DOMID_SELF;
> +  xhv.index = idx;
> +  r = grub_xen_hypercall (__HYPERVISOR_hvm_op, HVMOP_get_param,
> +   (grub_uint32_t) (), 0, 0, 0, 0);
> +  if (r < 0)
> +grub_xen_panic ("Could not get parameter from Xen!\n");
> +  return xhv.value;
> +}
> +
> +static void *
> +grub_xen_add_physmap (unsigned int space, void *addr)
> +{
> +  struct xen_add_to_physmap xatp;
> +
> +  xatp.domid = DOMID_SELF;
> +  xatp.idx = 0;
> +  xatp.space = space;
> +  xatp.gpfn = (grub_addr_t) addr >> GRUB_XEN_LOG_PAGE_SIZE;
> +  if (grub_xen_hypercall (__HYPERVISOR_memory_op, XENMEM_add_to_physmap,
> +   (grub_uint32_t) (), 0, 0, 0, 0))
> +grub_xen_panic ("Memory_op hypercall failed!\n");
> +  return addr;
> +}
> +
>  static void
>  grub_xen_sort_mmap (void)
>  {
> @@ -196,12 +228,100 @@ grub_xen_get_mmap (void)
>grub_xen_sort_mmap ();
>  }
>
> +static void
> +grub_xen_set_mmap (void)
> +{
> +  struct xen_foreign_memory_map memmap;
> +
> +  memmap.domid = DOMID_SELF;
> +  memmap.map.nr_entries = nr_map_entries;
> +  set_xen_guest_handle (memmap.map.buffer, map);
> +  grub_xen_hypercall (__HYPERVISOR_memory_op, XENMEM_set_memory_map,
> +   (grub_uint32_t) (), 0, 0, 0, 0);
> +}
> +
> +static grub_uint64_t
> +grub_xen_find_page (grub_uint64_t start)
> +{
> +  unsigned int i, j;
> +  grub_uint64_t last = start;
> +
> +  /* Try to find a e820 map hole below 4G. */
> +  /* Relies on page-aligned entries (addr and len) and input (start). */

I would like to see above two comments as one as below.

/*
 * Try to find a e820 map hole below 4G.
 * Relies on page-aligned entries (addr and len) and input (start).
 */

You can retain my RB if you change that.

Daniel

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

Re: [Xen-devel] [PATCH] pvcalls-front: fixes incorrect error handling

[Stefano Stabellini]

On Tue, 27 Nov 2018, Boris Ostrovsky wrote:
> On 11/27/18 3:37 PM, Stefano Stabellini wrote:
> > On Tue, 27 Nov 2018, PanBian wrote:
> >> On Mon, Nov 26, 2018 at 03:31:39PM -0500, Boris Ostrovsky wrote:
> >>> On 11/21/18 9:07 PM, Pan Bian wrote:
>  kfree() is incorrectly used to release the pages allocated by
>  __get_free_page() and __get_free_pages(). Use the matching deallocators
>  i.e., free_page() and free_pages(), respectively.
> 
>  Signed-off-by: Pan Bian 
>  ---
>   drivers/xen/pvcalls-front.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
>  diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c
>  index 2f11ca7..77224d8 100644
>  --- a/drivers/xen/pvcalls-front.c
>  +++ b/drivers/xen/pvcalls-front.c
>  @@ -385,8 +385,8 @@ static int create_active(struct sock_mapping *map, 
>  int *evtchn)
>   out_error:
>   if (*evtchn >= 0)
>   xenbus_free_evtchn(pvcalls_front_dev, *evtchn);
>  -kfree(map->active.data.in);
>  -kfree(map->active.ring);
>  +free_pages((unsigned long)map->active.data.in, 
>  PVCALLS_RING_ORDER);
> >>> Is map->active.data.in guaranteed to be NULL when entering this routine?
> >> I am not sure yet. Sorry for that. I observed the mismatches between
> >> __get_free_page and kfree, and submitted the patch.
> >>
> >> But I think your consideration is reasonable. A better solution is to
> >> directly free bytes, a local variable that holds __get_free_pages return
> >> value. If you agree, I will rewrite the patch.
> > Like Boris said, map->active.ring and map->active.data.in are not
> > guaranteed to be NULL or != NULL here. For instance,map->active.ring can
> > be != NULL and map->active.data.in can be NULL. However, free_pages and
> > free_page should be able to cope with it, the same way that kfree is
> > able to cope with it?
> 
> If map->active.data.in can be non-NULL on entry to this routine then I
> think this has been a problem all along. Pan's suggestion to use bytes
> for freeing is going to solve this (assuming bytes will be initialized
> to NULL).

Why is it a problem? map->active.data.in and map->active.ring are only
!= NULL if they need to be freed. Otherwise, they are NULL. All structs
are always initialized to zero. I don't think there are any issues.

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

[Xen-devel] [xen-unstable-smoke test] 130839: tolerable all pass - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

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  43fa95ae6a64132b8ebe3025bd187ab9df68677b
baseline version:
 xen  dc80c424844578048b457730e293a65267dea01c

Last test of basis   130819  2018-11-26 18:00:35 Z1 days
Testing same since   130839  2018-11-27 18:00:32 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Jan Beulich 
  Julien Grall 
  Paul Durrant 
  Roger Pau Monne 
  Roger Pau Monné 
  Wei Liu 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-amd64-amd64-xl-qemuu-debianhvm-i386 pass
 test-amd64-amd64-libvirt pass



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   dc80c42484..43fa95ae6a  43fa95ae6a64132b8ebe3025bd187ab9df68677b -> smoke

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

[Xen-devel] [linux-4.9 test] 130793: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64-xsm  broken
 build-arm64  broken
 build-arm64-pvopsbroken
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 16 
guest-localmigrate/x10 fail REGR. vs. 130142

Regressions which are regarded as allowable (not blocking):
 build-arm64-xsm   2 hosts-allocate broken REGR. vs. 130142
 build-arm64   2 hosts-allocate broken REGR. vs. 130142
 build-arm64-pvops 2 hosts-allocate broken REGR. vs. 130142

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit1   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-arm64-arm64-examine  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 build-arm64   3 capture-logs  broken blocked in 130142
 build-arm64-xsm   3 capture-logs  broken blocked in 130142
 build-arm64-pvops 3 capture-logs  broken blocked in 130142
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 130142
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 130142
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 130142
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 130142
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 130142
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail never pass
 test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail never pass
 test-amd64-amd64-xl-pvhv2-amd 12 guest-start  fail  never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 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
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never 

[Xen-devel] [PATCH] Revert "xen/balloon: Mark unallocated host memory as UNUSABLE"

[Igor Druzhinin]

This reverts commit b3cf8528bb21febb650a7ecbf080d0647be40b9f.

That commit unintentionally broke Xen balloon memory hotplug with
"hotplug_unpopulated" set to 1. As long as "System RAM" resource
got assigned under a new "Unusable memory" resource in IO/Mem tree
any attempt to online this memory would fail due to general kernel
restrictions on having "System RAM" resources as 1st level only.

The original issue that commit has tried to workaround fa564ad96366
("x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 00-1f, 30-3f,
60-7f)") also got amended by the following 03a551734 ("x86/PCI: Move
and shrink AMD 64-bit window to avoid conflict") which made the
original fix to Xen ballooning unnecessary.

Signed-off-by: Igor Druzhinin 
---
In mail thread [1] it was agreed to revert the change due to technical
comlications of fixing it and the fact that there is no any strong reason
to keep it.

[1] https://lists.xenproject.org/archives/html/xen-devel/2018-11/msg02834.html
---
 arch/x86/xen/enlighten.c | 78 
 arch/x86/xen/setup.c |  6 ++--
 drivers/xen/balloon.c| 65 ++--
 include/xen/balloon.h|  5 
 4 files changed, 13 insertions(+), 141 deletions(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index e996e8e..750f46a 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -10,7 +10,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include 
 #include 
@@ -346,80 +345,3 @@ void xen_arch_unregister_cpu(int num)
 }
 EXPORT_SYMBOL(xen_arch_unregister_cpu);
 #endif
-
-#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
-void __init arch_xen_balloon_init(struct resource *hostmem_resource)
-{
-   struct xen_memory_map memmap;
-   int rc;
-   unsigned int i, last_guest_ram;
-   phys_addr_t max_addr = PFN_PHYS(max_pfn);
-   struct e820_table *xen_e820_table;
-   const struct e820_entry *entry;
-   struct resource *res;
-
-   if (!xen_initial_domain())
-   return;
-
-   xen_e820_table = kmalloc(sizeof(*xen_e820_table), GFP_KERNEL);
-   if (!xen_e820_table)
-   return;
-
-   memmap.nr_entries = ARRAY_SIZE(xen_e820_table->entries);
-   set_xen_guest_handle(memmap.buffer, xen_e820_table->entries);
-   rc = HYPERVISOR_memory_op(XENMEM_machine_memory_map, );
-   if (rc) {
-   pr_warn("%s: Can't read host e820 (%d)\n", __func__, rc);
-   goto out;
-   }
-
-   last_guest_ram = 0;
-   for (i = 0; i < memmap.nr_entries; i++) {
-   if (xen_e820_table->entries[i].addr >= max_addr)
-   break;
-   if (xen_e820_table->entries[i].type == E820_TYPE_RAM)
-   last_guest_ram = i;
-   }
-
-   entry = _e820_table->entries[last_guest_ram];
-   if (max_addr >= entry->addr + entry->size)
-   goto out; /* No unallocated host RAM. */
-
-   hostmem_resource->start = max_addr;
-   hostmem_resource->end = entry->addr + entry->size;
-
-   /*
-* Mark non-RAM regions between the end of dom0 RAM and end of host RAM
-* as unavailable. The rest of that region can be used for hotplug-based
-* ballooning.
-*/
-   for (; i < memmap.nr_entries; i++) {
-   entry = _e820_table->entries[i];
-
-   if (entry->type == E820_TYPE_RAM)
-   continue;
-
-   if (entry->addr >= hostmem_resource->end)
-   break;
-
-   res = kzalloc(sizeof(*res), GFP_KERNEL);
-   if (!res)
-   goto out;
-
-   res->name = "Unavailable host RAM";
-   res->start = entry->addr;
-   res->end = (entry->addr + entry->size < hostmem_resource->end) ?
-   entry->addr + entry->size : hostmem_resource->end;
-   rc = insert_resource(hostmem_resource, res);
-   if (rc) {
-   pr_warn("%s: Can't insert [%llx - %llx) (%d)\n",
-   __func__, res->start, res->end, rc);
-   kfree(res);
-   goto  out;
-   }
-   }
-
- out:
-   kfree(xen_e820_table);
-}
-#endif /* CONFIG_XEN_BALLOON_MEMORY_HOTPLUG */
diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index 1163e33..075ed47 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -808,6 +808,7 @@ char * __init xen_memory_setup(void)
addr = xen_e820_table.entries[0].addr;
size = xen_e820_table.entries[0].size;
while (i < xen_e820_table.nr_entries) {
+   bool discard = false;
 
chunk_size = size;
type = xen_e820_table.entries[i].type;
@@ -823,10 +824,11 @@ char * __init xen_memory_setup(void)
xen_add_extra_mem(pfn_s, n_pfns);
xen_max_p2m_pfn = 

Re: [Xen-devel] [PATCH] pvcalls-front: fixes incorrect error handling

[Boris Ostrovsky]

On 11/27/18 3:37 PM, Stefano Stabellini wrote:
> On Tue, 27 Nov 2018, PanBian wrote:
>> On Mon, Nov 26, 2018 at 03:31:39PM -0500, Boris Ostrovsky wrote:
>>> On 11/21/18 9:07 PM, Pan Bian wrote:
 kfree() is incorrectly used to release the pages allocated by
 __get_free_page() and __get_free_pages(). Use the matching deallocators
 i.e., free_page() and free_pages(), respectively.

 Signed-off-by: Pan Bian 
 ---
  drivers/xen/pvcalls-front.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c
 index 2f11ca7..77224d8 100644
 --- a/drivers/xen/pvcalls-front.c
 +++ b/drivers/xen/pvcalls-front.c
 @@ -385,8 +385,8 @@ static int create_active(struct sock_mapping *map, int 
 *evtchn)
  out_error:
if (*evtchn >= 0)
xenbus_free_evtchn(pvcalls_front_dev, *evtchn);
 -  kfree(map->active.data.in);
 -  kfree(map->active.ring);
 +  free_pages((unsigned long)map->active.data.in, PVCALLS_RING_ORDER);
>>> Is map->active.data.in guaranteed to be NULL when entering this routine?
>> I am not sure yet. Sorry for that. I observed the mismatches between
>> __get_free_page and kfree, and submitted the patch.
>>
>> But I think your consideration is reasonable. A better solution is to
>> directly free bytes, a local variable that holds __get_free_pages return
>> value. If you agree, I will rewrite the patch.
> Like Boris said, map->active.ring and map->active.data.in are not
> guaranteed to be NULL or != NULL here. For instance,map->active.ring can
> be != NULL and map->active.data.in can be NULL. However, free_pages and
> free_page should be able to cope with it, the same way that kfree is
> able to cope with it?

If map->active.data.in can be non-NULL on entry to this routine then I
think this has been a problem all along. Pan's suggestion to use bytes
for freeing is going to solve this (assuming bytes will be initialized
to NULL).


-boris


>
>>> -boris
>>>
 +  free_page((unsigned long)map->active.ring);
return ret;
  }
  


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

Re: [Xen-devel] [PATCH] pvcalls-front: fixes incorrect error handling

[Stefano Stabellini]

On Tue, 27 Nov 2018, PanBian wrote:
> On Mon, Nov 26, 2018 at 03:31:39PM -0500, Boris Ostrovsky wrote:
> > On 11/21/18 9:07 PM, Pan Bian wrote:
> > > kfree() is incorrectly used to release the pages allocated by
> > > __get_free_page() and __get_free_pages(). Use the matching deallocators
> > > i.e., free_page() and free_pages(), respectively.
> > >
> > > Signed-off-by: Pan Bian 
> > > ---
> > >  drivers/xen/pvcalls-front.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c
> > > index 2f11ca7..77224d8 100644
> > > --- a/drivers/xen/pvcalls-front.c
> > > +++ b/drivers/xen/pvcalls-front.c
> > > @@ -385,8 +385,8 @@ static int create_active(struct sock_mapping *map, 
> > > int *evtchn)
> > >  out_error:
> > >   if (*evtchn >= 0)
> > >   xenbus_free_evtchn(pvcalls_front_dev, *evtchn);
> > > - kfree(map->active.data.in);
> > > - kfree(map->active.ring);
> > > + free_pages((unsigned long)map->active.data.in, PVCALLS_RING_ORDER);
> > 
> > Is map->active.data.in guaranteed to be NULL when entering this routine?
> 
> I am not sure yet. Sorry for that. I observed the mismatches between
> __get_free_page and kfree, and submitted the patch.
> 
> But I think your consideration is reasonable. A better solution is to
> directly free bytes, a local variable that holds __get_free_pages return
> value. If you agree, I will rewrite the patch.

Like Boris said, map->active.ring and map->active.data.in are not
guaranteed to be NULL or != NULL here. For instance,map->active.ring can
be != NULL and map->active.data.in can be NULL. However, free_pages and
free_page should be able to cope with it, the same way that kfree is
able to cope with it?


> > -boris
> > 
> > > + free_page((unsigned long)map->active.ring);
> > >   return ret;
> > >  }
> > >  
> > 
> 

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

Re: [Xen-devel] [PATCH v5 11/20] xen: setup hypercall page for PVH

[Daniel Kiper]

On Wed, Nov 21, 2018 at 03:28:46PM +0100, Juergen Gross wrote:
> Add the needed code to setup the hypercall page for calling into the
> Xen hypervisor.
>
> Import the XEN_HVM_DEBUGCONS_IOPORT define from Xen unstable into
> include/xen/arch-x86/xen.h
>
> Signed-off-by: Juergen Gross 
> ---
> V3: grub_xen_early_halt->grub_xen_panic (Roger Pau Monné)
> issue panic message (Roger Pau Monné)
> rewrite grub_xen_hypercall to avoid register variables (Daniel Kiper)
> V5: Use XEN_HVM_DEBUGCONS_IOPORT from Xen unstable (Roger Pau Monné)
> Issue "System halted!" in panic (Daniel Kiper)
> Clear interrupts and loop for halting (Roger Pau Monné, Daniel Kiper)
> Use only one dummy variable for hypercall asm statement
> ---
>  grub-core/kern/i386/xen/pvh.c | 79 
> +++
>  include/xen/arch-x86/xen.h|  7 
>  2 files changed, 86 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index 4f629b15e..478cef0d1 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -20,15 +20,94 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
>
>  grub_uint64_t grub_rsdp_addr;
>
> +static char hypercall_page[GRUB_XEN_PAGE_SIZE]
> +  __attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
> +
> +static grub_uint32_t xen_cpuid_base;
> +
> +static void
> +grub_xen_cons_msg (const char *msg)
> +{
> +  const char *c;
> +
> +  for (c = msg; *c; c++)
> +grub_outb (*c, XEN_HVM_DEBUGCONS_IOPORT);
> +}
> +
> +static void
> +grub_xen_panic (const char *msg)
> +{
> +  grub_xen_cons_msg (msg);
> +  grub_xen_cons_msg ("System halted!\n");
> +
> +  asm volatile ("cli");
> +
> +  while (1)
> +{
> +  asm volatile ("hlt");
> +}
> +}
> +
> +static void
> +grub_xen_cpuid_base (void)
> +{
> +  grub_uint32_t base, eax, signature[3];
> +
> +  for (base = 0x4000; base < 0x4001; base += 0x100)
> +{
> +  grub_cpuid (base, eax, signature[0], signature[1], signature[2]);
> +  if (!grub_memcmp ("XenVMMXenVMM", signature, 12) && (eax - base) >= 2)
> + {
> +   xen_cpuid_base = base;
> +   return;
> + }
> +}
> +
> +  grub_xen_panic ("Found no Xen signature!\n");
> +}
> +
> +static void
> +grub_xen_setup_hypercall_page (void)
> +{
> +  grub_uint32_t msr, pfn, eax, ebx, ecx, edx;
> +
> +  grub_cpuid (xen_cpuid_base + 2, eax, ebx, ecx, edx);

Could not you use a constant instead of plain 2 here? Is there anything
like that in Xen headers? If not please add a comment what grub_cpuid ()
and wrmsr do. One line is sufficient.

> +  msr = ebx;
> +  pfn = (grub_uint32_t) (_page);

Is it PFN? Really? I do not think so.

> +
> +  asm volatile ("wrmsr" : : "c" (msr), "a" (pfn), "d" (0) : "memory");
> +}
> +
> +int
> +grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t a0,
> + grub_uint32_t a1, grub_uint32_t a2,
> + grub_uint32_t a3, grub_uint32_t a4,
> + grub_uint32_t a5 __attribute__ ((unused)))
> +{
> +  grub_uint32_t __res, dummy;
> +
> +  asm volatile ("call *%[callno]"
> + : "=a" (__res), "=b" (dummy), "=c" (dummy), "=d" (dummy),
> +   "=S" (dummy), "=D" (dummy)
> + : "1" (a0), "2" (a1), "3" (a2), "4" (a3), "5" (a4),
> +   [callno] "a" (_page[callno * 32])
> + : "memory");

Have you tried "+b", "+c", ... instead of "=b", "=c", ...?

Daniel

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

Re: [Xen-devel] [PATCH v5 09/20] xen: add basic hooks for PVH in current code

[Daniel Kiper]

On Wed, Nov 21, 2018 at 03:28:44PM +0100, Juergen Gross wrote:
> Add the hooks to current code needed for Xen PVH. They will be filled
> with code later when the related functionality is being added.
>
> loader/i386/linux.c needs to include machine/kernel.h now as it needs
> to get GRUB_KERNEL_USE_RSDP_ADDR from there.
>
> Signed-off-by: Juergen Gross 

Reviewed-by: Daniel Kiper 

Daniel

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

Re: [Xen-devel] [PATCH v5 07/20] xen: modify grub_xen_ptr2mfn() for xen-pvh

[Daniel Kiper]

On Wed, Nov 21, 2018 at 03:28:42PM +0100, Juergen Gross wrote:
> grub_xen_ptr2mfn() returns the machine frame number for a given pointer
> value. For Xen-PVH guests this is just the PFN. Add the PVH specific
> variant.
>
> Signed-off-by: Juergen Gross 

Reviewed-by: Daniel Kiper 

Daniel

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

Re: [Xen-devel] [Qemu-devel] [PATCH for-3.2 v4 15/28] hw: apply accel compat properties without touching globals

[Marc-André Lureau]

Hi

On Tue, Nov 27, 2018 at 11:40 PM Eduardo Habkost  wrote:
>
> On Tue, Nov 27, 2018 at 01:27:48PM +0400, Marc-André Lureau wrote:
> > Introduce object_apply_global_props() function, to apply compatibility
> > properties from a GPtrArray.
> >
> > For accel compatibility properties, apply them during
> > device_post_init(), after accel_register_compat_props() has set them.
> >
> > To populate the compatibility properties, introduce SET_COMPAT(), a
> > more generic version of SET_MACHINE_COMPAT() that can set compat
> > properties on other objects than Machine, and using GPtrArray.
> >
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  include/hw/qdev-core.h | 13 +
> >  include/qom/object.h   |  3 +++
> >  include/sysemu/accel.h |  4 +---
> >  accel/accel.c  | 12 
> >  hw/core/qdev.c | 11 +++
> >  hw/xen/xen-common.c| 38 +++---
> >  qom/object.c   | 25 +
> >  vl.c   |  2 +-
> >  8 files changed, 73 insertions(+), 35 deletions(-)
> >
> > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> > index a24d0dd566..82afd3c50d 100644
> > --- a/include/hw/qdev-core.h
> > +++ b/include/hw/qdev-core.h
> > @@ -267,6 +267,19 @@ typedef struct GlobalProperty {
> >  Error **errp;
> >  } GlobalProperty;
> >
> > +#define SET_COMPAT(S, COMPAT)   \
> > +do {\
> > +int i;  \
> > +static GlobalProperty props[] = {   \
> > +COMPAT  \
> > +};  \
> > +for (i = 0; i < G_N_ELEMENTS(props); i++) { \
> > +g_ptr_array_add((S)->compat_props, (void *)[i]);  \
> > +}   \
> > +} while (0)
>
> I think this macro would be an acceptable alternative to the
> existing SET_MACHINE_COMPAT macro trickery, but:
>
> > +
> > +void accel_register_compat_props(const GPtrArray *props);
> [...]
> > @@ -185,7 +183,9 @@ static void xen_accel_class_init(ObjectClass *oc, void 
> > *data)
> >  ac->init_machine = xen_init;
> >  ac->setup_post = xen_setup_post;
> >  ac->allowed = _allowed;
> > -ac->global_props = xen_compat_props;
> > +ac->compat_props = g_ptr_array_new();
> > +
> > +SET_COMPAT(ac, XEN_COMPAT);
>
> I think this is a step backwards.  I like us to be able to
> register compat properties without macro magic.  The existence of
> SET_MACHINE_COMPAT is a bug and not a feature.
>
> If you really want to use GPtrArray instead of a simple
> GlobalProperty* field (I'm not sure I understand the reasoning
> behind the choice to use GPtrArray), what about:

Except in the Xen case, It needs to register multiple GlobalProperty*,
not necessarily from contiguous in memory. That's why we have an array
of ptr.

>
> static GPtrArray *build_compat_props_array(GlobalProperty *props)
> {
> GlobalProperty *p = props;
> GPtrArray *array = g_ptr_array_new();
> while (p->driver) {
> g_ptr_array_add(array, (void *)p);
> }
> return array;
> }
>
>
> static void xen_accel_class_init(ObjectClass *oc, void *data)
> {
> ...
> ac->compat_props = build_compat_props_array(xen_compat_props);

If we would register from one place, that would be fine.

We could replace the macro by a function, then we would have to
declare the GlobalProperty arrays manually basically.

> }
>
>
>
> >  }
> >
> >  #define TYPE_XEN_ACCEL ACCEL_CLASS_NAME("xen")
> > diff --git a/qom/object.c b/qom/object.c
> > index 17921c0a71..dbdab0aead 100644
> > --- a/qom/object.c
> > +++ b/qom/object.c
> > @@ -370,6 +370,31 @@ static void object_post_init_with_type(Object *obj, 
> > TypeImpl *ti)
> >  }
> >  }
> >
> > +void object_apply_global_props(Object *obj, const GPtrArray *props, Error 
> > **errp)
> > +{
> > +Error *err = NULL;
> > +int i;
> > +
> > +if (!props) {
> > +return;
> > +}
> > +
> > +for (i = 0; i < props->len; i++) {
> > +GlobalProperty *p = g_ptr_array_index(props, i);
> > +
> > +if (object_dynamic_cast(obj, p->driver) == NULL) {
> > +continue;
> > +}
> > +p->used = true;
> > +object_property_parse(obj, p->value, p->property, );
> > +if (err != NULL) {
> > +error_prepend(, "can't apply global %s.%s=%s: ",
> > +  p->driver, p->property, p->value);
> > +error_propagate(errp, err);
> > +}
> > +}
> > +}
> > +
> >  static void object_initialize_with_type(void *data, size_t size, TypeImpl 
> > *type)
> >  {
> >  Object *obj = data;
> > diff --git a/vl.c b/vl.c
> > index fa25d1ae2d..c06e94271c 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ 

Re: [Xen-devel] [PATCH v5 05/20] xen: add some dummy headers for PVH mode

[Daniel Kiper]

On Wed, Nov 21, 2018 at 03:28:40PM +0100, Juergen Gross wrote:
> With Xen PVH mode adding a new machine type the machine related headers
> need to be present for the build to succeed. Most of the headers just
> need to include the related common i386 headers. Add those to the tree.
>
> Note that xen_pvh/int.h needs to include pc/int_types.h instead of
> pc/int.h in order to avoid the definition of grub_bios_interrupt().
>
> xen_pvh/memory.h needs to include coreboot/memory.h (like some other
> /memory.h do as well) as this contains just the needed stubs.
>
> Signed-off-by: Juergen Gross 

Reviewed-by: Daniel Kiper 

Daniel

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

Re: [Xen-devel] [PATCH v5 02/20] loader/linux: support passing rsdp address via boot params

[Daniel Kiper]

On Wed, Nov 21, 2018 at 03:28:37PM +0100, Juergen Gross wrote:
> Xen PVH guests will have the RSDP at an arbitrary address. Support that
> by passing the RSDP address via the boot parameters to Linux.
>
> Signed-off-by: Juergen Gross 

Reviewed-by: Daniel Kiper 

Daniel

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

Re: [Xen-devel] [Qemu-devel] [PATCH for-3.2 v4 15/28] hw: apply accel compat properties without touching globals

[Eduardo Habkost]

On Tue, Nov 27, 2018 at 01:27:48PM +0400, Marc-André Lureau wrote:
> Introduce object_apply_global_props() function, to apply compatibility
> properties from a GPtrArray.
> 
> For accel compatibility properties, apply them during
> device_post_init(), after accel_register_compat_props() has set them.
> 
> To populate the compatibility properties, introduce SET_COMPAT(), a
> more generic version of SET_MACHINE_COMPAT() that can set compat
> properties on other objects than Machine, and using GPtrArray.
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  include/hw/qdev-core.h | 13 +
>  include/qom/object.h   |  3 +++
>  include/sysemu/accel.h |  4 +---
>  accel/accel.c  | 12 
>  hw/core/qdev.c | 11 +++
>  hw/xen/xen-common.c| 38 +++---
>  qom/object.c   | 25 +
>  vl.c   |  2 +-
>  8 files changed, 73 insertions(+), 35 deletions(-)
> 
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index a24d0dd566..82afd3c50d 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -267,6 +267,19 @@ typedef struct GlobalProperty {
>  Error **errp;
>  } GlobalProperty;
>  
> +#define SET_COMPAT(S, COMPAT)   \
> +do {\
> +int i;  \
> +static GlobalProperty props[] = {   \
> +COMPAT  \
> +};  \
> +for (i = 0; i < G_N_ELEMENTS(props); i++) { \
> +g_ptr_array_add((S)->compat_props, (void *)[i]);  \
> +}   \
> +} while (0)

I think this macro would be an acceptable alternative to the
existing SET_MACHINE_COMPAT macro trickery, but:

> +
> +void accel_register_compat_props(const GPtrArray *props);
[...]
> @@ -185,7 +183,9 @@ static void xen_accel_class_init(ObjectClass *oc, void 
> *data)
>  ac->init_machine = xen_init;
>  ac->setup_post = xen_setup_post;
>  ac->allowed = _allowed;
> -ac->global_props = xen_compat_props;
> +ac->compat_props = g_ptr_array_new();
> +
> +SET_COMPAT(ac, XEN_COMPAT);

I think this is a step backwards.  I like us to be able to
register compat properties without macro magic.  The existence of
SET_MACHINE_COMPAT is a bug and not a feature.

If you really want to use GPtrArray instead of a simple
GlobalProperty* field (I'm not sure I understand the reasoning
behind the choice to use GPtrArray), what about:

static GPtrArray *build_compat_props_array(GlobalProperty *props)
{
GlobalProperty *p = props;
GPtrArray *array = g_ptr_array_new();
while (p->driver) {
g_ptr_array_add(array, (void *)p);
}
return array;
}


static void xen_accel_class_init(ObjectClass *oc, void *data)
{
...
ac->compat_props = build_compat_props_array(xen_compat_props);
}



>  }
>  
>  #define TYPE_XEN_ACCEL ACCEL_CLASS_NAME("xen")
> diff --git a/qom/object.c b/qom/object.c
> index 17921c0a71..dbdab0aead 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -370,6 +370,31 @@ static void object_post_init_with_type(Object *obj, 
> TypeImpl *ti)
>  }
>  }
>  
> +void object_apply_global_props(Object *obj, const GPtrArray *props, Error 
> **errp)
> +{
> +Error *err = NULL;
> +int i;
> +
> +if (!props) {
> +return;
> +}
> +
> +for (i = 0; i < props->len; i++) {
> +GlobalProperty *p = g_ptr_array_index(props, i);
> +
> +if (object_dynamic_cast(obj, p->driver) == NULL) {
> +continue;
> +}
> +p->used = true;
> +object_property_parse(obj, p->value, p->property, );
> +if (err != NULL) {
> +error_prepend(, "can't apply global %s.%s=%s: ",
> +  p->driver, p->property, p->value);
> +error_propagate(errp, err);
> +}
> +}
> +}
> +
>  static void object_initialize_with_type(void *data, size_t size, TypeImpl 
> *type)
>  {
>  Object *obj = data;
> diff --git a/vl.c b/vl.c
> index fa25d1ae2d..c06e94271c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2963,7 +2963,7 @@ static void user_register_global_props(void)
>   */
>  static void register_global_properties(MachineState *ms)
>  {
> -accel_register_compat_props(ms->accelerator);
> +
> accel_register_compat_props(ACCEL_GET_CLASS(ms->accelerator)->compat_props);
>  machine_register_compat_props(ms);
>  user_register_global_props();
>  }
> -- 
> 2.20.0.rc1
> 
> 

-- 
Eduardo

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

Re: [Xen-devel] [ARM] gvirt_to_maddr fails when DomU is created

[Julien Grall]

(+ Stefano)

On 11/27/18 5:12 PM, Volodymyr Babchuk wrote:

Hello community,


Hi Volodymyr,



After creating domU, I'm seeing lots of this messages from hypervisor:

(XEN) p2m.c:1442: d1v0: gvirt_to_maddr failed va=0x8efc7f0f
flags=0x1 par=0x809
(XEN) p2m.c:1442: d1v0: gvirt_to_maddr failed va=0x8efc7f00
flags=0x1 par=0x809
(XEN) p2m.c:1442: d1v0: gvirt_to_maddr failed va=0x8efc7f0f
flags=0x1 par=0x809

Interestingly, I'm getting them from both Dom0 and DomU:

(XEN) p2m.c:1442: d0v0: gvirt_to_maddr failed va=0x80003efd7f0f
flags=0x1 par=0x809
(XEN) p2m.c:1442: d1v0: gvirt_to_maddr failed va=0x8efc7f0f
flags=0x1 par=0x809

But only after DomU is created.

I attached GDB and found that this is caused by update_runstate_area:

(gdb) bt
#0  get_page_from_gva (v=0x80005dbe2000, v@entry=0x22f2c8 ,
 va=va@entry=18446603337277996815, flags=flags@entry=1) at p2m.c:1440
#1  0x0024e320 in translate_get_page (write=true, linear=true,
addr=18446603337277996815,
 info=...) at guestcopy.c:37
#2  copy_guest (buf=buf@entry=0x80005dbe20d7,
addr=addr@entry=18446603337277996815, len=len@entry=1,
 info=..., flags=flags@entry=6) at guestcopy.c:69
#3  0x0024e45c in raw_copy_to_guest (to=to@entry=0x80003efd7f0f,
 from=from@entry=0x80005dbe20d7, len=len@entry=1) at guestcopy.c:110
#4  0x002497b4 in update_runstate_area
(v=v@entry=0x80005dbe2000) at domain.c:287
#5  0x00249eb8 in context_switch (prev=prev@entry=0x80005dbe2000,
 next=next@entry=0x80005bf3c000) at domain.c:344
#6  0x0022f2c8 in schedule () at schedule.c:1583
#7  0x00232c10 in __do_softirq
(ignore_mask=ignore_mask@entry=0) at softirq.c:50
#8  0x00232ca4 in do_softirq () at softirq.c:64
#9  0x00258254 in leave_hypervisor_tail () at traps.c:2302

This issue is encountered on QEMU-ARMv8. Dom0 kernel is Linux 4.19.0
My XEN master is at d8ffac1f7 "xen/arm: gic: Remove duplicated comment
in do_sgi"

The same setup worked perfectly with Xen 4.10.2


The message is only printed in debug build. Do you have CONFIG_DEBUG 
enabled?




Julien, I saw on mailing list, that you paid attention to issues with
gvirt_to_maddr,
so maybe you can be interested in this.


Which thread are you speaking about? The problem is not because of 
gvirt_to_maddr but of how update_runstate_area is working at the moment.


update_runstate_area is using a guest virtual address to update the vCPU 
runstate. It blindly assumes the vCPU runstate will always be mapped in 
stage-1 page-tables. However, if KPTI (Kernel Page Table Isolation) is 
enabled the kernel address space (and therefore the vCPU runstate) will 
not be mapped when running at EL0.


So if you are restoring a vCPU that was executing code at EL0 then 
update_runstate_area will fail as the address is not mapped. There are a 
few solution suggested on the ML (see [1]). However I haven't had time 
to look at properly how to implement them.


KPTI is getting used more widely (e.g meltdown and KASLR). So it would 
be good if we try to solve this problem sooner. I would be happy to 
review patches and/or provide advice if you want to tackle the problem.


Cheers,

[1] https://lists.xen.org/archives/html/xen-devel/2018-03/msg00223.html

--
Julien Grall

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

Re: [Xen-devel] [PATCH 0/3] xen/x86: support setting dom0_mem depending on host size

[Stefano Stabellini]

Hi Juergen,

I don't mean to scope-creep your series, and I think it is fine if you
don't want to do it, but it would be fantastic if you took the
opportunity to make dom0_mem common across architectures.

On ARM, we parse it in xen/arch/arm/domain_build.c:parse_dom0_mem.
I think the ARM and x86 implementations should be the same.

Cheers,

Stefano

On Thu, 22 Nov 2018, Juergen Gross wrote:
> Setting the memory size of dom0 on a server for the non autoballooning
> case requires always specification of a boot parameter today. The value
> to set will depend mostly on the host memory size.
> 
> In order to support that scenario add the possibility to set dom0_mem
> depending on the amount of physical memory by allowing to specify a
> percentage of host memory (e.g. 10%) with an offset (like 1G+10%).
> 
> To make it easy for a distributor to use such a setting as the default
> make the standard setting for dom0_mem configurable via Kconfig.
> 
> Juergen Gross (3):
>   xen/x86: delay parsing of dom0_mem parameter until needed
>   xen/x86: add dom0 memory sizing variants
>   xen/x86: add CONFIG item for default dom0 memory size
> 
>  docs/misc/xen-command-line.markdown | 21 
>  xen/arch/x86/Kconfig|  9 +
>  xen/arch/x86/dom0_build.c   | 67 
> +++--
>  3 files changed, 73 insertions(+), 24 deletions(-)
> 
> -- 
> 2.16.4


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

Re: [Xen-devel] [PATCH 17/18] MAINTAINERS: add myself as a Xen maintainer

[Stefano Stabellini]

On Wed, 21 Nov 2018, Paul Durrant wrote:
> I have made many significant contributions to the Xen code in QEMU,
> particularly the recent patches introducing a new PV device framework.
> I intend to make further significant contributions, porting other PV back-
> ends to the new framework with the intent of eventually removing the
> legacy code. It therefore seems reasonable that I become a maintiner of
> the Xen code.
> 
> Signed-off-by: Paul Durrant 

Acked-by: Stefano Stabellini 

> ---
> Cc: Stefano Stabellini 
> Cc: Anthony Perard 
> Cc: Paolo Bonzini 
> ---
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5871f035c3..0b668dd205 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -382,6 +382,7 @@ Guest CPU Cores (Xen):
>  X86
>  M: Stefano Stabellini 
>  M: Anthony Perard 
> +M: Paul Durrant 
>  L: xen-devel@lists.xenproject.org
>  S: Supported
>  F: */xen*
> -- 
> 2.11.0
> 

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

Re: [Xen-devel] [PATCH 18/18] xen/arm: Suspend/resume console on Xen suspend/resume

[Julien Grall]

Hi Mirela,

On 11/12/18 11:30 AM, Mirela Simonovic wrote:

This is done using generic console_suspend/resume functions that cause
uart driver specific suspend/resume handlers to be called for each
initialized port (if the port has suspend/resume driver handlers
implemented).

Signed-off-by: Mirela Simonovic 
Signed-off-by: Saeed Nowshadi 
---
  xen/arch/arm/suspend.c | 14 ++
  1 file changed, 14 insertions(+)

diff --git a/xen/arch/arm/suspend.c b/xen/arch/arm/suspend.c
index a05aea9c25..6d7d69539b 100644
--- a/xen/arch/arm/suspend.c
+++ b/xen/arch/arm/suspend.c
@@ -1,5 +1,6 @@
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -149,6 +150,15 @@ static long system_suspend(void *data)
  goto resume_irqs;
  }
  
+dprintk(XENLOG_DEBUG, "Suspend\n");


This message may not appear on the console unless it is synchronized 
(see console_start_sync). Also, it might be a useful message in 
non-debug build.


Ideally the suspend/resume logic should not be much different than x86 
(see arch/x86/acpi/power.c).


Cheers,

--
Julien Grall

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

[Xen-devel] [PATCH 0/3] Remove tmem

[Wei Liu]

It is agreed that tmem can be removed from xen.git. See the thread starting
from .

Wei Liu (3):
  xen: remove tmem from hypervisor
  tools: remove tmem code and commands
  docs: remove tmem related text

 docs/man/xl.conf.pod.5   |9 +-
 docs/man/xl.pod.1.in |   68 -
 docs/misc/tmem-internals.html|  789 --
 docs/misc/xen-command-line.markdown  |6 -
 docs/misc/xsm-flask.txt  |   36 -
 tools/flask/policy/modules/dom0.te   |4 +-
 tools/flask/policy/modules/guest_features.te |3 -
 tools/libxc/Makefile |1 -
 tools/libxc/include/xenctrl.h|   17 -
 tools/libxc/xc_tmem.c|  507 ---
 tools/libxl/libxl_tmem.c |  119 +-
 tools/misc/Makefile  |1 -
 tools/misc/xen-tmem-list-parse.c |  339 -
 tools/python/xen/lowlevel/xc/xc.c|   87 --
 tools/xenstat/libxenstat/src/xenstat.c   |   52 +-
 tools/xenstat/libxenstat/src/xenstat.h   |   15 -
 tools/xenstat/libxenstat/src/xenstat_priv.h  |8 -
 tools/xenstat/xentop/xentop.c|   36 +-
 tools/xl/Makefile|2 +-
 tools/xl/xl.h|6 -
 tools/xl/xl_cmdtable.c   |   40 -
 tools/xl/xl_tmem.c   |  251 ---
 xen/arch/arm/configs/tiny64.conf |1 -
 xen/arch/x86/configs/pvshim_defconfig|1 -
 xen/arch/x86/hvm/hypercall.c |3 -
 xen/arch/x86/pv/hypercall.c  |3 -
 xen/arch/x86/setup.c |8 -
 xen/common/Kconfig   |   13 -
 xen/common/Makefile  |4 -
 xen/common/compat/tmem_xen.c |   23 -
 xen/common/domain.c  |3 -
 xen/common/memory.c  |4 +-
 xen/common/page_alloc.c  |   40 +-
 xen/common/sysctl.c  |5 -
 xen/common/tmem.c| 2095 --
 xen/common/tmem_control.c|  560 ---
 xen/common/tmem_xen.c|  277 
 xen/include/Makefile |1 -
 xen/include/public/sysctl.h  |  108 +-
 xen/include/public/tmem.h|  124 --
 xen/include/xen/hypercall.h  |7 -
 xen/include/xen/sched.h  |3 -
 xen/include/xen/tmem.h   |   45 -
 xen/include/xen/tmem_control.h   |   39 -
 xen/include/xen/tmem_xen.h   |  343 -
 xen/include/xlat.lst |2 -
 46 files changed, 27 insertions(+), 6081 deletions(-)
 delete mode 100644 docs/misc/tmem-internals.html
 delete mode 100644 tools/libxc/xc_tmem.c
 delete mode 100644 tools/misc/xen-tmem-list-parse.c
 delete mode 100644 tools/xl/xl_tmem.c
 delete mode 100644 xen/common/compat/tmem_xen.c
 delete mode 100644 xen/common/tmem.c
 delete mode 100644 xen/common/tmem_control.c
 delete mode 100644 xen/common/tmem_xen.c
 delete mode 100644 xen/include/public/tmem.h
 delete mode 100644 xen/include/xen/tmem.h
 delete mode 100644 xen/include/xen/tmem_control.h
 delete mode 100644 xen/include/xen/tmem_xen.h

-- 
2.11.0


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

[Xen-devel] [PATCH 2/3] tools: remove tmem code and commands

[Wei Liu]

Remove all tmem related code in libxc.

Leave some stubs in libxl in case anyone has linked to those functions
before the removal.

Remove all tmem related commands in xl, all tmem related code in other
utilities we ship.

Signed-off-by: Wei Liu 
---
 tools/libxc/Makefile|   1 -
 tools/libxc/include/xenctrl.h   |  17 -
 tools/libxc/xc_tmem.c   | 507 
 tools/libxl/libxl_tmem.c| 119 +--
 tools/misc/Makefile |   1 -
 tools/misc/xen-tmem-list-parse.c| 339 ---
 tools/python/xen/lowlevel/xc/xc.c   |  87 -
 tools/xenstat/libxenstat/src/xenstat.c  |  52 +--
 tools/xenstat/libxenstat/src/xenstat.h  |  15 -
 tools/xenstat/libxenstat/src/xenstat_priv.h |   8 -
 tools/xenstat/xentop/xentop.c   |  36 +-
 tools/xl/Makefile   |   2 +-
 tools/xl/xl.h   |   6 -
 tools/xl/xl_cmdtable.c  |  40 ---
 tools/xl/xl_tmem.c  | 251 --
 15 files changed, 18 insertions(+), 1463 deletions(-)
 delete mode 100644 tools/libxc/xc_tmem.c
 delete mode 100644 tools/misc/xen-tmem-list-parse.c
 delete mode 100644 tools/xl/xl_tmem.c

diff --git a/tools/libxc/Makefile b/tools/libxc/Makefile
index 44d9d09d4e..1546afd168 100644
--- a/tools/libxc/Makefile
+++ b/tools/libxc/Makefile
@@ -30,7 +30,6 @@ CTRL_SRCS-y   += xc_tbuf.c
 CTRL_SRCS-y   += xc_pm.c
 CTRL_SRCS-y   += xc_cpu_hotplug.c
 CTRL_SRCS-y   += xc_resume.c
-CTRL_SRCS-y   += xc_tmem.c
 CTRL_SRCS-y   += xc_vm_event.c
 CTRL_SRCS-y   += xc_monitor.c
 CTRL_SRCS-y   += xc_mem_paging.c
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 97ae965be7..8334dc5750 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -44,7 +44,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
@@ -1907,22 +1906,6 @@ int xc_set_cpuidle_max_cstate(xc_interface *xch, 
uint32_t value);
 
 int xc_enable_turbo(xc_interface *xch, int cpuid);
 int xc_disable_turbo(xc_interface *xch, int cpuid);
-/**
- * tmem operations
- */
-
-int xc_tmem_control_oid(xc_interface *xch, int32_t pool_id, uint32_t subop,
-uint32_t cli_id, uint32_t len, uint32_t arg,
-struct xen_tmem_oid oid, void *buf);
-int xc_tmem_control(xc_interface *xch,
-int32_t pool_id, uint32_t subop, uint32_t cli_id,
-uint32_t len, uint32_t arg, void *buf);
-int xc_tmem_auth(xc_interface *xch, int cli_id, char *uuid_str, int enable);
-int xc_tmem_save(xc_interface *xch, uint32_t domid, int live, int fd, int 
field_marker);
-int xc_tmem_save_extra(xc_interface *xch, uint32_t domid, int fd, int 
field_marker);
-void xc_tmem_save_done(xc_interface *xch, uint32_t domid);
-int xc_tmem_restore(xc_interface *xch, uint32_t domid, int fd);
-int xc_tmem_restore_extra(xc_interface *xch, uint32_t domid, int fd);
 
 /**
  * altp2m operations
diff --git a/tools/libxc/xc_tmem.c b/tools/libxc/xc_tmem.c
deleted file mode 100644
index a365c74388..00
--- a/tools/libxc/xc_tmem.c
+++ /dev/null
@@ -1,507 +0,0 @@
-/**
- * xc_tmem.c
- *
- * Copyright (C) 2008 Oracle Corp.
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation;
- * version 2.1 of the License.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; If not, see .
- */
-
-#include "xc_private.h"
-#include 
-#include 
-#include 
-
-int xc_tmem_control(xc_interface *xch,
-int32_t pool_id,
-uint32_t cmd,
-uint32_t cli_id,
-uint32_t len,
-uint32_t arg,
-void *buf)
-{
-DECLARE_SYSCTL;
-DECLARE_HYPERCALL_BOUNCE(buf, len, XC_HYPERCALL_BUFFER_BOUNCE_OUT);
-int rc;
-
-sysctl.cmd = XEN_SYSCTL_tmem_op;
-sysctl.u.tmem_op.pool_id = pool_id;
-sysctl.u.tmem_op.cmd = cmd;
-sysctl.u.tmem_op.cli_id = cli_id;
-sysctl.u.tmem_op.len = len;
-sysctl.u.tmem_op.arg = arg;
-sysctl.u.tmem_op.pad = 0;
-sysctl.u.tmem_op.oid.oid[0] = 0;
-sysctl.u.tmem_op.oid.oid[1] = 0;
-sysctl.u.tmem_op.oid.oid[2] = 0;
-
-if ( cmd == XEN_SYSCTL_TMEM_OP_SET_CLIENT_INFO ||
- cmd == XEN_SYSCTL_TMEM_OP_SET_AUTH )
-HYPERCALL_BOUNCE_SET_DIR(buf, 

[Xen-devel] [PATCH 3/3] docs: remove tmem related text

[Wei Liu]

Signed-off-by: Wei Liu 
---
 docs/man/xl.conf.pod.5  |  9 ++---
 docs/man/xl.pod.1.in| 68 -
 docs/misc/xen-command-line.markdown |  6 
 docs/misc/xsm-flask.txt | 36 
 4 files changed, 2 insertions(+), 117 deletions(-)

diff --git a/docs/man/xl.conf.pod.5 b/docs/man/xl.conf.pod.5
index 37262a7ef8..b1bde7d657 100644
--- a/docs/man/xl.conf.pod.5
+++ b/docs/man/xl.conf.pod.5
@@ -148,10 +148,8 @@ The default choice is "xvda".
 =item B
 
 If this option is enabled then when a guest is created there will be an
-guarantee that there is memory available for the guest. This is an
-particularly acute problem on hosts with memory over-provisioned guests
-that use tmem and have self-balloon enabled (which is the default
-option). The self-balloon mechanism can deflate/inflate the balloon
+guarantee that there is memory available for the guest.
+The self-balloon mechanism can deflate/inflate the balloon
 quickly and the amount of free memory (which C can show) is
 stale the moment it is printed. When claim is enabled a reservation for
 the amount of memory (see 'memory' in xl.conf(5)) is set, which is then
@@ -163,9 +161,6 @@ If the reservation cannot be meet the guest creation fails 
immediately
 instead of taking seconds/minutes (depending on the size of the guest)
 while the guest is populated.
 
-Note that to enable tmem type guests, one needs to provide C on the
-Xen hypervisor argument and as well on the Linux kernel command line.
-
 Default: C<1>
 
 =over 4
diff --git a/docs/man/xl.pod.1.in b/docs/man/xl.pod.1.in
index 18006880d6..7c765dbc3c 100644
--- a/docs/man/xl.pod.1.in
+++ b/docs/man/xl.pod.1.in
@@ -1677,74 +1677,6 @@ Obtain information of USB devices connected as such via 
the device model
 
 =back
 
-=head1 TRANSCENDENT MEMORY (TMEM)
-
-=over 4
-
-=item B I<[OPTIONS]> I
-
-List tmem pools.
-
-B
-
-=over 4
-
-=item B<-l>
-
-If this parameter is specified, also list tmem stats.
-
-=back
-
-=item B I
-
-Freeze tmem pools.
-
-=item B I
-
-Thaw tmem pools.
-
-=item B I [I]
-
-Change tmem settings.
-
-B
-
-=over 4
-
-=item B<-w> I
-
-Weight (int)
-
-=item B<-p> I
-
-Compress (int)
-
-=back
-
-=item B I [I]
-
-De/authenticate shared tmem pool.
-
-B
-
-=over 4
-
-=item B<-u> I
-
-Specify uuid (abcdef01-2345-6789-1234-567890abcdef)
-
-=item B<-a> I
-
-0=auth,1=deauth
-
-=back
-
-=item B
-
-Get information about how much freeable memory (MB) is in-use by tmem.
-
-=back
-
 =head1 FLASK
 
 B is a security framework that defines a mandatory access control policy
diff --git a/docs/misc/xen-command-line.markdown 
b/docs/misc/xen-command-line.markdown
index 9028bcde2e..fe891ef074 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1993,12 +1993,6 @@ pages) must also be specified via the tbuf\_size 
parameter.
 ### timer\_slop
 > `= `
 
-### tmem
-> `= `
-
-### tmem\_compress
-> `= `
-
 ### tsc (x86)
 > `= unstable | skewed | stable:socket`
 
diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
index 62f15dde84..40e5fc845e 100644
--- a/docs/misc/xsm-flask.txt
+++ b/docs/misc/xsm-flask.txt
@@ -81,42 +81,6 @@ __HYPERVISOR_memory_op (xen/include/public/memory.h)
  * XENMEM_get_pod_target
  * XENMEM_claim_pages
 
-__HYPERVISOR_tmem_op (xen/include/public/tmem.h)
-
- The following tmem control ops, that is the sub-subops of
- TMEM_CONTROL, are covered by this statement. 
-
- Note that TMEM is also subject to a similar policy arising from
- XSA-15 http://lists.xen.org/archives/html/xen-announce/2012-09/msg6.html.
- Due to this existing policy all TMEM Ops are already subject to
- reduced security support.
-
- * TMEMC_THAW
- * TMEMC_FREEZE
- * TMEMC_FLUSH
- * TMEMC_DESTROY
- * TMEMC_LIST
- * TMEMC_SET_WEIGHT
- * TMEMC_SET_CAP
- * TMEMC_SET_COMPRESS
- * TMEMC_QUERY_FREEABLE_MB
- * TMEMC_SAVE_BEGIN
- * TMEMC_SAVE_GET_VERSION
- * TMEMC_SAVE_GET_MAXPOOLS
- * TMEMC_SAVE_GET_CLIENT_WEIGHT
- * TMEMC_SAVE_GET_CLIENT_CAP
- * TMEMC_SAVE_GET_CLIENT_FLAGS
- * TMEMC_SAVE_GET_POOL_FLAGS
- * TMEMC_SAVE_GET_POOL_NPAGES
- * TMEMC_SAVE_GET_POOL_UUID
- * TMEMC_SAVE_GET_NEXT_PAGE
- * TMEMC_SAVE_GET_NEXT_INV
- * TMEMC_SAVE_END
- * TMEMC_RESTORE_BEGIN
- * TMEMC_RESTORE_PUT_PAGE
- * TMEMC_RESTORE_FLUSH_PAGE
-
-
 
 Setting up FLASK
 
-- 
2.11.0


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

Re: [Xen-devel] [PATCH 17/18] xen/arm: Resume Dom0 after Xen resumes

[Julien Grall]

On 11/17/18 4:01 PM, Mirela Simonovic wrote:

Hi,


Hi Mirela,



On Sat, Nov 17, 2018 at 12:06 AM Stefano Stabellini
 wrote:


On Sat, 17 Nov 2018, Dario Faggioli wrote:

On Fri, 2018-11-16 at 21:58 +, Julien Grall wrote:

On 16/11/2018 21:41, Mirela Simonovic wrote:

On Fri, Nov 16, 2018 at 8:09 PM Stefano Stabellini
 wrote:

It should be possible to figure out which domain needs to
awaken from
there.


Actually, evtchn_send eventually will trigger a proper interrupt
injection into the domain
(xen/arch/arm/vgic.c:arch_evtchn_inject),
which will necessarely wake it up. So it is possible that it will
already work without any need for additional changes?



Absolutely, that sounds great :) Then we could just drop this
patch.


I don't think you can drop this patch... As you tie the host suspend
to
the hardware domain suspend, it may makes sense to resume at the same
time.


FWIW, I think that too.

In fact, let's assume a *fully* disaggregated setup, where dom0 only
has the toolstack, while it has no hardware, no PV backend, etc... If
we don't resume it explicitly together with Xen, who is going to resume
it? :-O


Yes, that's right. However, it should work for driver domains: there is
no need to wake up driver domains explicitly because they will be
woken up by the frontends?



I think we all agree, except some of us weren't so clear about it :)
For now, dom0 issues suspend and should resume as well when Xen
suspends. This is done in the series, resume is covered by this patch,
and commit message should be clarified.

If a domU has a backend, we should verify that it can be woken-up by
an event triggered by a frontend driver in another domain.

One day, this patch could be dropped/reverted if one come up with a
different logic for triggering Xen suspend. This should be of the
table for now, but a good option to remember for future.


Such change cannot be easily dropped because some hardware domain OS may 
rely on that behavior.


I am also interested to see how this is going to fit with the Dom0less 
use case. The end goal is to have no Dom0/Hardware domain. So how do you 
expect suspend/resume to work in that case? Note that I am not asking to 
implement it :).


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v1] Increase framebuffer size to todays standards

[Julien Grall]

Hi Jan,

On 11/27/18 9:44 AM, Jan Beulich wrote:

On 27.11.18 at 10:26,  wrote:

Hi Jan,

On 11/26/18 4:15 PM, Jan Beulich wrote:

On 26.11.18 at 17:03,  wrote:

Am Mon, 26 Nov 2018 03:31:27 -0700
schrieb "Jan Beulich" :


And I think a change like this should (a) address the more general
case rather than just your laptop (or laptops in general) and (b)
actually add some headroom. Hence at the very least I'd see us
go to 4096x3072. WHUXGA would even call for 7680x4800.


So should I resend this patch with higher values, or should I remove
the bounds check entirely? Not sure what it is trying to achieve, the
framebuffer may fail either way if the BIOS provides bogus values.


I have to forward this question: Stefano introduced all five MAX_*
values here when splitting out the LFB code in commit e7cb35e8b1
("xen: introduce a generic framebuffer driver"). I apparently didn't
even notice back then that three of them are entirely unused, and
the two dimension ones had no upper bound before.

Stefano: Why were all of these introduced (there's no explanation
in the description) and what were the values derived from? Will
anything break if we remove them?


FWIW, looking at the logs, this was introduced to cater arm framebuffer
driver. However, we dropped the only driver a few months ago as it was
not maintained. So x86 is the only user of that code today.


Interesting. I assume you mean arm_hdlcd.c. I've looked at its
4.11.0 version, and I'm afraid I still can't see a connection to
the questionable MAX_* values. Whatever we go with is going
to be a backport candidate (as obviously slightly older versions
of Xen would also better work properly with larger monitors),
and hence I'd still need to understand the correlation, perhaps
unless backporting the removal of that driver is also desired.


I would be surprised if someone ever used the HDLCD driver after it was 
merged. I am not even sure if it even works.


Furthermore, this driver only targets development platform (i.g Juno) or 
the models were pretty much everyone tends to use serial console for Xen.


So I would not worry if you break the support when backporting. I am 
also happy to see it completely removed in old Xen version.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2] xl: free bitmaps on exit

[Wei Liu]

On Tue, Nov 27, 2018 at 06:09:33PM +0100, Olaf Hering wrote:
> Am Tue, 27 Nov 2018 16:55:38 +
> schrieb Wei Liu :
> 
> > Looking more closely into this, these lines should be moved even
> > earlier before the call to libxl_ctx_alloc -- there is an exit there.
> 
> Is it required to install the atexit handler before alloc()?
> To me it looks like atexit(xl_ctx_free) should be done right after, or 
> inside, xl_ctx_alloc().

Note that multiple calls to atexit creates a chain of functions to
call, not overriding what is previously registered.

The xl_ctx_alloc function is also called in postfork. I think if you
move atexit call into xl_ctx_alloc you will end up registering
xl_ctx_free more than once, which is wrong.

The other option -- to call atexit right after -- looks plausible to me.

Wei.

> 
> Olaf



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

Re: [Xen-devel] [PATCH] CODING_STYLE: explicitly call out label indentation

[Jan Beulich]

>>> On 27.11.18 at 18:09,  wrote:
>> On Nov 27, 2018, at 4:48 PM, Andrew Cooper  wrote:
>> On 27/11/2018 16:22, Ian Jackson wrote:
>>> Since there was some confusion about what we are talking about, see
>>> below.  Obviously the diff output in my `1' test cases is
>>> prefereable.   Note that `git diff' does the same thing as diff -p
>>> (and it doesn't even need a -p option to do it).
>> 
>> After some investigation, git does the correct thing when you ask to
>> treat c files as c files.
>> 
>> andrewcoop@andrewcoop:~$ cat .config/git/attributes
>> *.[hc] diff=cpp
>> 
>> This has the additional side effect of making `git diff --color-words`
>> and friends far more legible and nice to use.  Its a shame this isn't
>> the default.
>> 
>>> I also observe that by default, emacs wants to indent the label by 1
>>> character - even though usually it likes to align labels to the LHS of
>>> the enclosing block.  Presumably for this reason.
>> 
>> And after some investigation, emacs does the wrong thing in the Xen tree
>> because we explicitly ask for BSD style in the local block.
>> 
>> We should make a choice, then fix our automatic tooling to not force
>> code to be non-compliant.
> 
> FWIW, having labels in column 0 always looked wrong to me.  I’m happy to 
> change the style to require at least 1 space, but I’m *not* happy to have a 
> style enforced that contradicts what we’ve written in the emacs style blocks 
> at the bottom of all the files.
> 
> Would it be OK if we 
> * Checked in this patch, but
> * Weren’t picky about enforcing it yet

FWIW, I've been picky about this for the last so many years.

Jan

> * Looked into an efficient way to get a suitable style update?
> 
>  -George




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

[Xen-devel] [ARM] gvirt_to_maddr fails when DomU is created

[Volodymyr Babchuk]

Hello community,

After creating domU, I'm seeing lots of this messages from hypervisor:

(XEN) p2m.c:1442: d1v0: gvirt_to_maddr failed va=0x8efc7f0f
flags=0x1 par=0x809
(XEN) p2m.c:1442: d1v0: gvirt_to_maddr failed va=0x8efc7f00
flags=0x1 par=0x809
(XEN) p2m.c:1442: d1v0: gvirt_to_maddr failed va=0x8efc7f0f
flags=0x1 par=0x809

Interestingly, I'm getting them from both Dom0 and DomU:

(XEN) p2m.c:1442: d0v0: gvirt_to_maddr failed va=0x80003efd7f0f
flags=0x1 par=0x809
(XEN) p2m.c:1442: d1v0: gvirt_to_maddr failed va=0x8efc7f0f
flags=0x1 par=0x809

But only after DomU is created.

I attached GDB and found that this is caused by update_runstate_area:

(gdb) bt
#0  get_page_from_gva (v=0x80005dbe2000, v@entry=0x22f2c8 ,
va=va@entry=18446603337277996815, flags=flags@entry=1) at p2m.c:1440
#1  0x0024e320 in translate_get_page (write=true, linear=true,
addr=18446603337277996815,
info=...) at guestcopy.c:37
#2  copy_guest (buf=buf@entry=0x80005dbe20d7,
addr=addr@entry=18446603337277996815, len=len@entry=1,
info=..., flags=flags@entry=6) at guestcopy.c:69
#3  0x0024e45c in raw_copy_to_guest (to=to@entry=0x80003efd7f0f,
from=from@entry=0x80005dbe20d7, len=len@entry=1) at guestcopy.c:110
#4  0x002497b4 in update_runstate_area
(v=v@entry=0x80005dbe2000) at domain.c:287
#5  0x00249eb8 in context_switch (prev=prev@entry=0x80005dbe2000,
next=next@entry=0x80005bf3c000) at domain.c:344
#6  0x0022f2c8 in schedule () at schedule.c:1583
#7  0x00232c10 in __do_softirq
(ignore_mask=ignore_mask@entry=0) at softirq.c:50
#8  0x00232ca4 in do_softirq () at softirq.c:64
#9  0x00258254 in leave_hypervisor_tail () at traps.c:2302

This issue is encountered on QEMU-ARMv8. Dom0 kernel is Linux 4.19.0
My XEN master is at d8ffac1f7 "xen/arm: gic: Remove duplicated comment
in do_sgi"

The same setup worked perfectly with Xen 4.10.2

Julien, I saw on mailing list, that you paid attention to issues with
gvirt_to_maddr,
so maybe you can be interested in this.

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

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

Re: [Xen-devel] [PATCH v2] xl: free bitmaps on exit

[Olaf Hering]

Am Tue, 27 Nov 2018 16:55:38 +
schrieb Wei Liu :

> Looking more closely into this, these lines should be moved even
> earlier before the call to libxl_ctx_alloc -- there is an exit there.

Is it required to install the atexit handler before alloc()?
To me it looks like atexit(xl_ctx_free) should be done right after, or inside, 
xl_ctx_alloc().

Olaf


pgpztdhceiwC0.pgp
Description: Digitale Signatur von OpenPGP
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] CODING_STYLE: explicitly call out label indentation

[George Dunlap]

> On Nov 27, 2018, at 4:48 PM, Andrew Cooper  wrote:
> 
> On 27/11/2018 16:22, Ian Jackson wrote:
>> Since there was some confusion about what we are talking about, see
>> below.  Obviously the diff output in my `1' test cases is
>> prefereable.   Note that `git diff' does the same thing as diff -p
>> (and it doesn't even need a -p option to do it).
> 
> After some investigation, git does the correct thing when you ask to
> treat c files as c files.
> 
> andrewcoop@andrewcoop:~$ cat .config/git/attributes
> *.[hc] diff=cpp
> 
> This has the additional side effect of making `git diff --color-words`
> and friends far more legible and nice to use.  Its a shame this isn't
> the default.
> 
>> I also observe that by default, emacs wants to indent the label by 1
>> character - even though usually it likes to align labels to the LHS of
>> the enclosing block.  Presumably for this reason.
> 
> And after some investigation, emacs does the wrong thing in the Xen tree
> because we explicitly ask for BSD style in the local block.
> 
> We should make a choice, then fix our automatic tooling to not force
> code to be non-compliant.

FWIW, having labels in column 0 always looked wrong to me.  I’m happy to change 
the style to require at least 1 space, but I’m *not* happy to have a style 
enforced that contradicts what we’ve written in the emacs style blocks at the 
bottom of all the files.

Would it be OK if we 
* Checked in this patch, but
* Weren’t picky about enforcing it yet
* Looked into an efficient way to get a suitable style update?

 -George
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2] mm: make opt_bootscrub non-init

[Wei Liu]

On Tue, Nov 27, 2018 at 03:25:29PM +0100, Roger Pau Monné wrote:
> On Tue, Nov 27, 2018 at 03:13:27AM -0700, Jan Beulich wrote:
> > >>> On 26.11.18 at 18:55,  wrote:
> > > --- a/xen/common/page_alloc.c
> > > +++ b/xen/common/page_alloc.c
> > > @@ -166,7 +166,14 @@ enum bootscrub_mode {
> > >  BOOTSCRUB_ON,
> > >  BOOTSCRUB_IDLE,
> > >  };
> > > -static enum bootscrub_mode __initdata opt_bootscrub = BOOTSCRUB_IDLE;
> > > +/*
> > > + * opt_bootscrub should live in the init section, since it's not accessed
> > > + * afterwards. However at least LLVM assumes there are no side effects of
> > > + * accessing the variable, and optimizes the condition so opt_bootscrub 
> > > is
> > 
> > ... the condition in init_heap_pages() ...
> > 
> > (which I guess can be folded in while committing).
> 
> Yes please.
> 

I have fixed this and taken the liberty to add a blank line before this
comment block.

Wei.

> Thanks, Roger.

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

Re: [Xen-devel] [PATCH v2] xl: free bitmaps on exit

[Wei Liu]

On Tue, Nov 27, 2018 at 11:06:08AM +0100, Olaf Hering wrote:
> Every invocation of xl via valgrind will show three leaks.
> Since libxl_bitmap_alloc uses NOGC, the caller has to free the memory
> after use. And since xl_ctx_free might be called before
> parse_global_config, also move the libxl_bitmap_init calls into
> xl_ctx_alloc.
> 
> Signed-off-by: Olaf Hering 
> ---
>  tools/xl/xl.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/xl/xl.c b/tools/xl/xl.c
> index 7d2142f16f..f5a17cf0d1 100644
> --- a/tools/xl/xl.c
> +++ b/tools/xl/xl.c
> @@ -209,11 +209,8 @@ static void parse_global_config(const char *configfile,
>  if (!xlu_cfg_get_long (config, "max_maptrack_frames", , 0))
>  max_maptrack_frames = l;
>  
> -libxl_bitmap_init(_vm_affinity_mask);
>  libxl_cpu_bitmap_alloc(ctx, _vm_affinity_mask, 0);
> -libxl_bitmap_init(_hvm_affinity_mask);
>  libxl_cpu_bitmap_alloc(ctx, _hvm_affinity_mask, 0);
> -libxl_bitmap_init(_pv_affinity_mask);
>  libxl_cpu_bitmap_alloc(ctx, _pv_affinity_mask, 0);
>  
>  if (!xlu_cfg_get_string (config, "vm.cpumask", , 0))
> @@ -323,11 +320,17 @@ void xl_ctx_alloc(void) {
>  exit(1);
>  }
>  
> +libxl_bitmap_init(_vm_affinity_mask);
> +libxl_bitmap_init(_hvm_affinity_mask);
> +libxl_bitmap_init(_pv_affinity_mask);

Looking more closely into this, these lines should be moved even
earlier before the call to libxl_ctx_alloc -- there is an exit there.

Sorry for not having noticed this earlier before giving my ack. Please
submit v3.

Wei.

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

Re: [Xen-devel] [PATCH] CODING_STYLE: explicitly call out label indentation

[Andrew Cooper]

On 27/11/2018 16:22, Ian Jackson wrote:
> Since there was some confusion about what we are talking about, see
> below.  Obviously the diff output in my `1' test cases is
> prefereable.   Note that `git diff' does the same thing as diff -p
> (and it doesn't even need a -p option to do it).

After some investigation, git does the correct thing when you ask to
treat c files as c files.

andrewcoop@andrewcoop:~$ cat .config/git/attributes
*.[hc] diff=cpp

This has the additional side effect of making `git diff --color-words`
and friends far more legible and nice to use.  Its a shame this isn't
the default.

> I also observe that by default, emacs wants to indent the label by 1
> character - even though usually it likes to align labels to the LHS of
> the enclosing block.  Presumably for this reason.

And after some investigation, emacs does the wrong thing in the Xen tree
because we explicitly ask for BSD style in the local block.

We should make a choice, then fix our automatic tooling to not force
code to be non-compliant.

~Andrew

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

Re: [Xen-devel] [PATCH RFC] mm/memory_hotplug: Introduce memory block types

[David Hildenbrand]

On 27.11.18 17:32, Michal Suchánek wrote:
> On Mon, 26 Nov 2018 16:59:14 +0100
> David Hildenbrand  wrote:
> 
>> On 26.11.18 15:20, Michal Suchánek wrote:
>>> On Mon, 26 Nov 2018 14:33:29 +0100
>>> David Hildenbrand  wrote:
>>>   
 On 26.11.18 13:30, David Hildenbrand wrote:  
> On 23.11.18 19:06, Michal Suchánek wrote:
>>>   
>>
>> If we are going to fake the driver information we may as well add the
>> type attribute and be done with it.
>>
>> I think the problem with the patch was more with the semantic than the
>> attribute itself.
>>
>> What is normal, paravirtualized, and standby memory?
>>
>> I can understand DIMM device, baloon device, or whatever mechanism for
>> adding memory you might have.
>>
>> I can understand "memory designated as standby by the cluster
>> administrator".
>>
>> However, DIMM vs baloon is orthogonal to standby and should not be
>> conflated into one property.
>>
>> paravirtualized means nothing at all in relationship to memory type and
>> the desired online policy to me.
>
> Right, so with whatever we come up, it should allow to make a decision
> in user space about
> - if memory is to be onlined automatically

 And I will think about if we really should model standby memory. Maybe
 it is really better to have in user space something like (as Dan noted)  
>>>
>>> If it is possible to designate the memory as standby or online in the
>>> s390 admin interface and the kernel does have access to this
>>> information it makes sense to forward it to userspace (as separate
>>> s390-specific property). If not then you need to make some kind of
>>> assumption like below and the user can tune the script according to
>>> their usecase.  
>>
>> Also true, standby memory really represents a distinct type of memory
>> block (memory seems to be there but really isn't). Right now I am
>> thinking about something like this (tried to formulate it on a very
>> generic level because we can't predict which mechanism might want to
>> make use of these types in the future).
>>
>>
>> /*
>>  * Memory block types allow user space to formulate rules if and how to
>>  * online memory blocks. The types are exposed to user space as text
>>  * strings in sysfs. While the typical online strategies are described
>>  * along with the types, there are use cases where that can differ (e.g.
>>  * use MOVABLE zone for more reliable huge page usage, use NORMAL zone
>>  * due to zone imbalance or because memory unplug is not intended).
>>  *
>>  * MEMORY_BLOCK_NONE:
>>  *  No memory block is to be created (e.g. device memory). Used internally
>>  *  only.
>>  *
>>  * MEMORY_BLOCK_REMOVABLE:
>>  *  This memory block type should be treated as if it can be
>>  *  removed/unplugged from the system again. E.g. there is a hardware
>>  *  interface to unplug such memory. This memory block type is usually
>>  *  onlined to the MOVABLE zone, to e.g. make offlining of it more
>>  *  reliable. Examples include ACPI and PPC DIMMs.
>>  *
>>  * MEMORY_BLOCK_UNREMOVABLE:
>>  *  This memory block type should be treated as if it can not be
>>  *  removed/unplugged again. E.g. there is no hardware interface to
>>  *  unplug such memory. This memory block type is usually onlined to
>>  *  the NORMAL zone, as offlining is not beneficial. Examples include boot
>>  *  memory on most architectures and memory added via balloon devices.
> 
> AFAIK baloon device can be inflated as well so this does not really
> describe how this memory type works in any meaningful way. Also it
> should not be possible to see this kind of memory from userspace. The
> baloon driver just takes existing memory that is properly backed,
> allocates it for itself, and allows the hypervisor to use it. Thus it
> creates the equivalent to s390 standby memory which is not backed in
> the VM. When memory is reclaimed from hypervisor the baloon driver
> frees it making it available to the VM kernel again. However, the whole
> time the memory appears present in the machine and no hotplug events
> should be visible unless the docs I am looking at are really outdated.

It's all not optimal yet.

Don't confuse what I describe here with inflated/deflated memory. XEN
and Hyper-V add *new* memory to the system using add_memory(). New
memory blocks. This memory will never be removed using the typical
"offline + remove_memory()" approach. It will be removed using
ballooning (if at all) and only in pieces. So it will usually be onlined
to the NORMAL zone. (but userspace can later on implement whatever rule
it wants)

I am not talking about any kind of inflation/deflation. I am talking
about memory blocks added to the system via add_memory().

Inflation/deflation does not belong into the memory block interface.

> 
>>  *
>>  * MEMORY_BLOCK_STANDBY:
>>  *  The memory block type should be treated as if it can be
>>  *  removed/unplugged again, however the 

Re: [Xen-devel] [PATCH] tools/libs: xenforeignmemory_unmap_resource() should be idempotent...

[Wei Liu]

On Tue, Nov 27, 2018 at 04:39:17PM +, Paul Durrant wrote:
> ...and is not because linux osdep_xenforeignmemory_unmap_resource() is not.
> 
> Reported-by: Andrew Cooper 
> Signed-off-by: Paul Durrant 

Acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v2] amd-iommu: remove page merging code

[Paul Durrant]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 27 November 2018 16:38
> To: Paul Durrant 
> Cc: Brian Woods ; Suravee Suthikulpanit
> ; xen-devel  de...@lists.xenproject.org>
> Subject: RE: [Xen-devel] [PATCH v2] amd-iommu: remove page merging code
> 
> >>> On 27.11.18 at 17:05,  wrote:
> > Well, I could just leave PV-IOMMU unimplemented for AMD h/w if you think
> the
> > page merging code is more important since, without the spare ignored
> bits, I
> > have no way to track pages added by the hypercall vs. those added at
> start of
> > day. I personally think that the simpler code is worthwhile in itself
> but I
> > guess you disagree.
> 
> Not exactly: With a proper reason (here: you need to free up at least
> one of the bits; "for other potential use" simply is too vague for my
> taste, and to be honest I had already forgotten about this need of
> yours), I can live with this. Plus I'm not the maintainer of this code
> anyway.
> 

Ok, I'll send v3 adjusting the comment again.

  Paul

> Jan
> 


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

[Xen-devel] [PATCH] tools/libs: xenforeignmemory_unmap_resource() should be idempotent...

[Paul Durrant]

...and is not because linux osdep_xenforeignmemory_unmap_resource() is not.

Reported-by: Andrew Cooper 
Signed-off-by: Paul Durrant 
---
Cc: Andrew Cooper 
Cc: Ian Jackson 
Cc: Wei Liu 

This is an alternative to the similarly named patch posted by Andrew. This
one fixes the underlying issue in the osdep implementation.
Andrew requested this be backported to 4.11.
---
 tools/libs/foreignmemory/linux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libs/foreignmemory/linux.c b/tools/libs/foreignmemory/linux.c
index 132875df8a..8daa5828e3 100644
--- a/tools/libs/foreignmemory/linux.c
+++ b/tools/libs/foreignmemory/linux.c
@@ -298,7 +298,7 @@ int osdep_xenforeignmemory_restrict(xenforeignmemory_handle 
*fmem,
 int osdep_xenforeignmemory_unmap_resource(
 xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
 {
-return munmap(fres->addr, fres->nr_frames << PAGE_SHIFT);
+return fres ? munmap(fres->addr, fres->nr_frames << PAGE_SHIFT) : 0;
 }
 
 int osdep_xenforeignmemory_map_resource(
-- 
2.11.0


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

Re: [Xen-devel] [PATCH v2] amd-iommu: remove page merging code

[Jan Beulich]

>>> On 27.11.18 at 17:05,  wrote:
> Well, I could just leave PV-IOMMU unimplemented for AMD h/w if you think the 
> page merging code is more important since, without the spare ignored bits, I 
> have no way to track pages added by the hypercall vs. those added at start of 
> day. I personally think that the simpler code is worthwhile in itself but I 
> guess you disagree.

Not exactly: With a proper reason (here: you need to free up at least
one of the bits; "for other potential use" simply is too vague for my
taste, and to be honest I had already forgotten about this need of
yours), I can live with this. Plus I'm not the maintainer of this code
anyway.

Jan



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

[Xen-devel] [linux-4.14 test] 130790: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64  broken
 build-arm64-xsm  broken
 build-arm64-pvopsbroken
 test-amd64-amd64-xl-multivcpu  7 xen-bootfail REGR. vs. 130155
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail REGR. vs. 130155
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail REGR. vs. 130155
 test-armhf-armhf-xl-vhd 15 guest-start/debian.repeat fail REGR. vs. 130155

Regressions which are regarded as allowable (not blocking):
 build-arm64-xsm   2 hosts-allocate broken REGR. vs. 130155
 build-arm64   2 hosts-allocate broken REGR. vs. 130155
 build-arm64-pvops 2 hosts-allocate broken REGR. vs. 130155

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl-credit1   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-arm64-arm64-examine  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 build-arm64   3 capture-logs  broken blocked in 130155
 build-arm64-xsm   3 capture-logs  broken blocked in 130155
 build-arm64-pvops 3 capture-logs  broken blocked in 130155
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 

Re: [Xen-devel] [PATCH v2] makedumpfile: exclude pages that are logically offline

[Kazuhito Hagio]

> Linux marks pages that are logically offline via a page flag (map count).
> Such pages e.g. include pages infated as part of a balloon driver or
> pages that were not actually onlined when onlining the whole section.
> 
> While the hypervisor usually allows to read such inflated memory, we
> basically read and dump data that is completely irrelevant. Also, this
> might result in quite some overhead in the hypervisor. In addition,
> we saw some problems under Hyper-V, whereby we can crash the kernel by
> dumping, when reading memory of a partially onlined memory segment
> (for memory added by the Hyper-V balloon driver).
> 
> Therefore, don't read and dump pages that are marked as being logically
> offline.
> 
> Signed-off-by: David Hildenbrand 

Thanks for the v2 update.
I'm going to merge this patch after the kernel patches are merged
and it tests fine with the kernel.

Kazu

> ---
> 
> v1 -> v2:
> - Fix PAGE_BUDDY_MAPCOUNT_VALUE vs. PAGE_OFFLINE_MAPCOUNT_VALUE
> 
>  makedumpfile.c | 34 ++
>  makedumpfile.h |  1 +
>  2 files changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index 8923538..a5f2ea9 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -88,6 +88,7 @@ mdf_pfn_t pfn_cache_private;
>  mdf_pfn_t pfn_user;
>  mdf_pfn_t pfn_free;
>  mdf_pfn_t pfn_hwpoison;
> +mdf_pfn_t pfn_offline;
> 
>  mdf_pfn_t num_dumped;
> 
> @@ -249,6 +250,21 @@ isHugetlb(unsigned long dtor)
>  && (SYMBOL(free_huge_page) == dtor));
>  }
> 
> +static int
> +isOffline(unsigned long flags, unsigned int _mapcount)
> +{
> + if (NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE) == NOT_FOUND_NUMBER)
> + return FALSE;
> +
> + if (flags & (1UL << NUMBER(PG_slab)))
> + return FALSE;
> +
> + if (_mapcount == (int)NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE))
> + return TRUE;
> +
> + return FALSE;
> +}
> +
>  static int
>  is_cache_page(unsigned long flags)
>  {
> @@ -2287,6 +2303,8 @@ write_vmcoreinfo_data(void)
>   WRITE_NUMBER("PG_hwpoison", PG_hwpoison);
> 
>   WRITE_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
> + WRITE_NUMBER("PAGE_OFFLINE_MAPCOUNT_VALUE",
> +  PAGE_OFFLINE_MAPCOUNT_VALUE);
>   WRITE_NUMBER("phys_base", phys_base);
> 
>   WRITE_NUMBER("HUGETLB_PAGE_DTOR", HUGETLB_PAGE_DTOR);
> @@ -2687,6 +2705,7 @@ read_vmcoreinfo(void)
>   READ_SRCFILE("pud_t", pud_t);
> 
>   READ_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
> + READ_NUMBER("PAGE_OFFLINE_MAPCOUNT_VALUE", PAGE_OFFLINE_MAPCOUNT_VALUE);
>   READ_NUMBER("phys_base", phys_base);
>  #ifdef __aarch64__
>   READ_NUMBER("VA_BITS", VA_BITS);
> @@ -6041,6 +6060,12 @@ __exclude_unnecessary_pages(unsigned long mem_map,
>   else if (isHWPOISON(flags)) {
>   pfn_counter = _hwpoison;
>   }
> + /*
> +  * Exclude pages that are logically offline.
> +  */
> + else if (isOffline(flags, _mapcount)) {
> + pfn_counter = _offline;
> + }
>   /*
>* Unexcludable page
>*/
> @@ -7522,7 +7547,7 @@ write_elf_pages_cyclic(struct cache_data *cd_header, 
> struct cache_data *cd_page)
>*/
>   if (info->flag_cyclic) {
>   pfn_zero = pfn_cache = pfn_cache_private = 0;
> - pfn_user = pfn_free = pfn_hwpoison = 0;
> + pfn_user = pfn_free = pfn_hwpoison = pfn_offline = 0;
>   pfn_memhole = info->max_mapnr;
>   }
> 
> @@ -8804,7 +8829,7 @@ write_kdump_pages_and_bitmap_cyclic(struct cache_data 
> *cd_header, struct cache_d
>* Reset counter for debug message.
>*/
>   pfn_zero = pfn_cache = pfn_cache_private = 0;
> - pfn_user = pfn_free = pfn_hwpoison = 0;
> + pfn_user = pfn_free = pfn_hwpoison = pfn_offline = 0;
>   pfn_memhole = info->max_mapnr;
> 
>   /*
> @@ -9749,7 +9774,7 @@ print_report(void)
>   pfn_original = info->max_mapnr - pfn_memhole;
> 
>   pfn_excluded = pfn_zero + pfn_cache + pfn_cache_private
> - + pfn_user + pfn_free + pfn_hwpoison;
> + + pfn_user + pfn_free + pfn_hwpoison + pfn_offline;
>   shrinking = (pfn_original - pfn_excluded) * 100;
>   shrinking = shrinking / pfn_original;
> 
> @@ -9763,6 +9788,7 @@ print_report(void)
>   REPORT_MSG("User process data pages : 0x%016llx\n", pfn_user);
>   REPORT_MSG("Free pages  : 0x%016llx\n", pfn_free);
>   REPORT_MSG("Hwpoison pages  : 0x%016llx\n", pfn_hwpoison);
> + REPORT_MSG("Offline pages   : 0x%016llx\n", pfn_offline);
>   REPORT_MSG("  Remaining pages  : 0x%016llx\n",
>   pfn_original - pfn_excluded);
>   REPORT_MSG("  (The number of pages is reduced to %lld%%.)\n",
> @@ -9790,7 +9816,7 @@ 

Re: [Xen-devel] [PATCH RFC] mm/memory_hotplug: Introduce memory block types

[Michal Suchánek]

On Mon, 26 Nov 2018 16:59:14 +0100
David Hildenbrand  wrote:

> On 26.11.18 15:20, Michal Suchánek wrote:
> > On Mon, 26 Nov 2018 14:33:29 +0100
> > David Hildenbrand  wrote:
> >   
> >> On 26.11.18 13:30, David Hildenbrand wrote:  
> >>> On 23.11.18 19:06, Michal Suchánek wrote:
> >   
> 
>  If we are going to fake the driver information we may as well add the
>  type attribute and be done with it.
> 
>  I think the problem with the patch was more with the semantic than the
>  attribute itself.
> 
>  What is normal, paravirtualized, and standby memory?
> 
>  I can understand DIMM device, baloon device, or whatever mechanism for
>  adding memory you might have.
> 
>  I can understand "memory designated as standby by the cluster
>  administrator".
> 
>  However, DIMM vs baloon is orthogonal to standby and should not be
>  conflated into one property.
> 
>  paravirtualized means nothing at all in relationship to memory type and
>  the desired online policy to me.
> >>>
> >>> Right, so with whatever we come up, it should allow to make a decision
> >>> in user space about
> >>> - if memory is to be onlined automatically
> >>
> >> And I will think about if we really should model standby memory. Maybe
> >> it is really better to have in user space something like (as Dan noted)  
> > 
> > If it is possible to designate the memory as standby or online in the
> > s390 admin interface and the kernel does have access to this
> > information it makes sense to forward it to userspace (as separate
> > s390-specific property). If not then you need to make some kind of
> > assumption like below and the user can tune the script according to
> > their usecase.  
> 
> Also true, standby memory really represents a distinct type of memory
> block (memory seems to be there but really isn't). Right now I am
> thinking about something like this (tried to formulate it on a very
> generic level because we can't predict which mechanism might want to
> make use of these types in the future).
> 
> 
> /*
>  * Memory block types allow user space to formulate rules if and how to
>  * online memory blocks. The types are exposed to user space as text
>  * strings in sysfs. While the typical online strategies are described
>  * along with the types, there are use cases where that can differ (e.g.
>  * use MOVABLE zone for more reliable huge page usage, use NORMAL zone
>  * due to zone imbalance or because memory unplug is not intended).
>  *
>  * MEMORY_BLOCK_NONE:
>  *  No memory block is to be created (e.g. device memory). Used internally
>  *  only.
>  *
>  * MEMORY_BLOCK_REMOVABLE:
>  *  This memory block type should be treated as if it can be
>  *  removed/unplugged from the system again. E.g. there is a hardware
>  *  interface to unplug such memory. This memory block type is usually
>  *  onlined to the MOVABLE zone, to e.g. make offlining of it more
>  *  reliable. Examples include ACPI and PPC DIMMs.
>  *
>  * MEMORY_BLOCK_UNREMOVABLE:
>  *  This memory block type should be treated as if it can not be
>  *  removed/unplugged again. E.g. there is no hardware interface to
>  *  unplug such memory. This memory block type is usually onlined to
>  *  the NORMAL zone, as offlining is not beneficial. Examples include boot
>  *  memory on most architectures and memory added via balloon devices.

AFAIK baloon device can be inflated as well so this does not really
describe how this memory type works in any meaningful way. Also it
should not be possible to see this kind of memory from userspace. The
baloon driver just takes existing memory that is properly backed,
allocates it for itself, and allows the hypervisor to use it. Thus it
creates the equivalent to s390 standby memory which is not backed in
the VM. When memory is reclaimed from hypervisor the baloon driver
frees it making it available to the VM kernel again. However, the whole
time the memory appears present in the machine and no hotplug events
should be visible unless the docs I am looking at are really outdated.

>  *
>  * MEMORY_BLOCK_STANDBY:
>  *  The memory block type should be treated as if it can be
>  *  removed/unplugged again, however the actual memory hot(un)plug is
>  *  performed by onlining/offlining. In virtual environments, such memory
>  *  is usually added during boot and never removed. Onlining memory will
>  *  result in memory getting allocated to a VM. This memory type is usually
>  *  not onlined automatically but explicitly by the administrator. One
>  *  example is standby memory on s390x.

Again, this does not meaningfully describe the memory type. There is
no memory on standby. There is in fact no backing at all unless you
online it. So this probably is some kind of shared memory. However, the
(de)allocation is controlled differently compared to the baloon device.
The concept is very similar, though.

Thanks

Michal

___

Re: [Xen-devel] [PATCH] tools/libs: Make xenforeignmemory_unmap_resource() idempotent

[Paul Durrant]

> -Original Message-
> From: Andrew Cooper
> Sent: 27 November 2018 16:10
> To: Paul Durrant ; Xen-devel  de...@lists.xen.org>
> Cc: Ian Jackson ; Wei Liu 
> Subject: Re: [PATCH] tools/libs: Make xenforeignmemory_unmap_resource()
> idempotent
> 
> On 24/11/2018 15:05, Paul Durrant wrote:
> >> -Original Message-
> >> From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
> >> Sent: 23 November 2018 15:12
> >> To: Xen-devel 
> >> Cc: Andrew Cooper ; Ian Jackson
> >> ; Wei Liu ; Paul Durrant
> >> 
> >> Subject: [PATCH] tools/libs: Make xenforeignmemory_unmap_resource()
> >> idempotent
> >>
> >> Most other close/unmap functions are.
> >>
> >> Signed-off-by: Andrew Cooper 
> >> ---
> >> CC: Ian Jackson 
> >> CC: Wei Liu 
> >> CC: Paul Durrant 
> >>
> >> This ideally wants backporting to 4.11 to hit 4.11.1
> >>
> >> I got an unexpected shock while trying to diagnose why GVT-g is still
> >> broken (differently!) on staging.
> >> ---
> >>  tools/libs/foreignmemory/core.c | 7 ++-
> >>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tools/libs/foreignmemory/core.c
> >> b/tools/libs/foreignmemory/core.c
> >> index 63f12e2..2516fd4 100644
> >> --- a/tools/libs/foreignmemory/core.c
> >> +++ b/tools/libs/foreignmemory/core.c
> >> @@ -182,7 +182,12 @@ xenforeignmemory_resource_handle
> >> *xenforeignmemory_map_resource(
> >>  int xenforeignmemory_unmap_resource(
> >>  xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle
> >> *fres)
> >>  {
> >> -int rc = osdep_xenforeignmemory_unmap_resource(fmem, fres);
> >> +int rc;
> >> +
> >> +if ( !fres )
> >> +return 0;
> >> +
> >> +rc = osdep_xenforeignmemory_unmap_resource(fmem, fres);
> > Freeing NULL should not be problem as this defined to do nothing so
> there is nothing about this function which is not idempotent. I assume,
> without looking yet, that it is the osdep function which needs fixing.
> 
> Why?  You can fix it once for the entire library here, or once in every
> osdep flavour.
> 
> One of these is rather less code...

Actually I can argue my patch is both complete and smaller since there is a 
single non-idempotent osdep implementation at the moment :-) I'd also prefer 
that the osdep implementations are made (and kept) idempotent anyway.

  Paul
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] CODING_STYLE: explicitly call out label indentation

[Ian Jackson]

Since there was some confusion about what we are talking about, see
below.  Obviously the diff output in my `1' test cases is
prefereable.   Note that `git diff' does the same thing as diff -p
(and it doesn't even need a -p option to do it).

I also observe that by default, emacs wants to indent the label by 1
character - even though usually it likes to align labels to the LHS of
the enclosing block.  Presumably for this reason.

Ian.

$ cat t.c
function(){
code;
out:
context1;
context2;
context3;
context4;
will be edited;
context5;
context6;
context7;
}
$ perl -pe 's/will be/was/' u.c
$ perl -pe 's/out:/ $&/' t1.c
$ perl -pe 's/will be/was/' u1.c
$ diff -up t.c u.c
--- t.c 2018-11-27 16:14:33.614941668 +
+++ u.c 2018-11-27 16:15:39.020244882 +
@@ -5,7 +5,7 @@ out:
 context2;
 context3;
 context4;
-will be edited;
+was edited;
 context5;
 context6;
 context7;
$ diff -up t1.c u1.c
--- t1.c2018-11-27 16:15:52.424512251 +
+++ u1.c2018-11-27 16:15:57.064604805 +
@@ -5,7 +5,7 @@ function(){
 context2;
 context3;
 context4;
-will be edited;
+was edited;
 context5;
 context6;
 context7;
$ 

diff --git a/t.c b/t.c
index e0a0315..d09f948 100644
--- a/t.c
+++ b/t.c
@@ -5,7 +5,7 @@ out:
 context2;
 context3;
 context4;
-will be edited;
+was edited;
 context5;
 context6;
 context7;
diff --git a/t1.c b/t1.c
index d7ef2c4..bc8bba1 100644
--- a/t1.c
+++ b/t1.c
@@ -5,7 +5,7 @@ function(){
 context2;
 context3;
 context4;
-will be edited;
+was edited;
 context5;
 context6;
 context7;

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

Re: [Xen-devel] [PATCH] tools/libs: Make xenforeignmemory_unmap_resource() idempotent

[Andrew Cooper]

On 24/11/2018 15:05, Paul Durrant wrote:
>> -Original Message-
>> From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
>> Sent: 23 November 2018 15:12
>> To: Xen-devel 
>> Cc: Andrew Cooper ; Ian Jackson
>> ; Wei Liu ; Paul Durrant
>> 
>> Subject: [PATCH] tools/libs: Make xenforeignmemory_unmap_resource()
>> idempotent
>>
>> Most other close/unmap functions are.
>>
>> Signed-off-by: Andrew Cooper 
>> ---
>> CC: Ian Jackson 
>> CC: Wei Liu 
>> CC: Paul Durrant 
>>
>> This ideally wants backporting to 4.11 to hit 4.11.1
>>
>> I got an unexpected shock while trying to diagnose why GVT-g is still
>> broken (differently!) on staging.
>> ---
>>  tools/libs/foreignmemory/core.c | 7 ++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/libs/foreignmemory/core.c
>> b/tools/libs/foreignmemory/core.c
>> index 63f12e2..2516fd4 100644
>> --- a/tools/libs/foreignmemory/core.c
>> +++ b/tools/libs/foreignmemory/core.c
>> @@ -182,7 +182,12 @@ xenforeignmemory_resource_handle
>> *xenforeignmemory_map_resource(
>>  int xenforeignmemory_unmap_resource(
>>  xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle
>> *fres)
>>  {
>> -int rc = osdep_xenforeignmemory_unmap_resource(fmem, fres);
>> +int rc;
>> +
>> +if ( !fres )
>> +return 0;
>> +
>> +rc = osdep_xenforeignmemory_unmap_resource(fmem, fres);
> Freeing NULL should not be problem as this defined to do nothing so there is 
> nothing about this function which is not idempotent. I assume, without 
> looking yet, that it is the osdep function which needs fixing.

Why?  You can fix it once for the entire library here, or once in every
osdep flavour.

One of these is rather less code...

~Andrew

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

Re: [Xen-devel] [PATCH v2] amd-iommu: remove page merging code

[Paul Durrant]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 27 November 2018 15:58
> To: Paul Durrant 
> Cc: Brian Woods ; Suravee Suthikulpanit
> ; xen-devel  de...@lists.xenproject.org>
> Subject: Re: [Xen-devel] [PATCH v2] amd-iommu: remove page merging code
> 
> >>> On 27.11.18 at 15:58,  wrote:
> > The page merging logic makes use of bits 1-8 and bit 63 of a PTE, which
> used
> > to be specified as ignored. However, bits 5 and 6 are now specified as
> > 'accessed' and 'dirty' bits and their use only remains safe as long as
> > the DTE 'Host Access Dirty' bits remain unused by Xen.
> 
> ... or as long the two page table bits don't get made use of
> (by Xen or hardware) before the domain starts running (i.e.
> the "hardware" part is always true afaict).
> 

Yes, that would also be true. I still don't like the re-use of bits that are no 
longer marked explicitly as ignored though.

> > The code was also the subject of XSA-275 and, since then, has been
> disabled
> > after domain creation.
> >
> > This patch removes the code, freeing up the remaining PTE 'ignored' bits
> > for other potential use and shortening the source by 170 lines. There
> may
> > be some marginal performance cost since higher order mappings will now
> be
> > ruled out until a mapping order parameter is passed to iommu_ops.
> 
> "Marginal" is a guess, or supported by actual measurements? With
> heavy S/G of small blocks of data I could easily see this become
> more than a marginal increase of overhead. How bad it is certainly
> also depends on IOTLB capacity.

Yes, it is down to IOTLB capacity and therefore I do not know what the reality 
of the performance cost is. I saw no problem in manual testing of a GPU but I 
don't have comparative benchmark numbers.

> 
> What I would find more convincing would be if there was a reason
> why a fair part of the large page mappings get shattered anyway
> today.
> 

Well, I could just leave PV-IOMMU unimplemented for AMD h/w if you think the 
page merging code is more important since, without the spare ignored bits, I 
have no way to track pages added by the hypercall vs. those added at start of 
day. I personally think that the simpler code is worthwhile in itself but I 
guess you disagree.


  Paul

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

Re: [Xen-devel] [PATCH] amd-iommu: remove page merging code

[Paul Durrant]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 27 November 2018 15:50
> To: Paul Durrant 
> Cc: Brian Woods ; Suravee Suthikulpanit
> ; xen-devel  de...@lists.xenproject.org>
> Subject: RE: [Xen-devel] [PATCH] amd-iommu: remove page merging code
> 
> >>> On 27.11.18 at 15:20,  wrote:
> >>  -Original Message-
> >> From: Jan Beulich [mailto:jbeul...@suse.com]
> >> Sent: 27 November 2018 13:07
> >> To: Paul Durrant 
> >> Cc: Brian Woods ; Suravee Suthikulpanit
> >> ; xen-devel  >> de...@lists.xenproject.org>
> >> Subject: Re: [Xen-devel] [PATCH] amd-iommu: remove page merging code
> >>
> >> >>> On 26.11.18 at 18:30,  wrote:
> >> > The page merging logic makes use of bits 1-8 and bit 63 of a PTE,
> which
> >> used
> >> > to be specified as ignored. However, bits 5 and 6 are now specified
> as
> >> > 'accessed' and 'dirty' bits and their use only remains safe as long
> as
> >> > the DTE 'Host Access Dirty' bits remain clear.
> >>
> >> Upon second thought - is this actually true with the XSA-275
> >> changes in place? As long as the domain is not running yet,
> >> how would A and/or D bits get set?
> >
> > Ok, I can amend the comment. The risk is, as I say, predicated on the
> bits
> > in the DTE anyway but the tables are wired into the DTE *before* being
> > populated so I don't think there is anything to stop h/w DMAing whilst
> they
> > are being constructed.
> 
> This way of thinking recurs: When the tables get constructed, the
> domain doesn't run yet, or has no device assigned yet. In both
> cases I can't see who/what would initiate DMA.
>

Ah yes. I was thinking that the page table construction was asynchronous with 
the assignment for some reason.

  Paul

> Jan
> 


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

Re: [Xen-devel] [PATCH v2] amd-iommu: remove page merging code

[Jan Beulich]

>>> On 27.11.18 at 15:58,  wrote:
> The page merging logic makes use of bits 1-8 and bit 63 of a PTE, which used
> to be specified as ignored. However, bits 5 and 6 are now specified as
> 'accessed' and 'dirty' bits and their use only remains safe as long as
> the DTE 'Host Access Dirty' bits remain unused by Xen.

... or as long the two page table bits don't get made use of
(by Xen or hardware) before the domain starts running (i.e.
the "hardware" part is always true afaict).

> The code was also the subject of XSA-275 and, since then, has been disabled
> after domain creation.
> 
> This patch removes the code, freeing up the remaining PTE 'ignored' bits
> for other potential use and shortening the source by 170 lines. There may
> be some marginal performance cost since higher order mappings will now be
> ruled out until a mapping order parameter is passed to iommu_ops.

"Marginal" is a guess, or supported by actual measurements? With
heavy S/G of small blocks of data I could easily see this become
more than a marginal increase of overhead. How bad it is certainly
also depends on IOTLB capacity.

What I would find more convincing would be if there was a reason
why a fair part of the large page mappings get shattered anyway
today.

Jan



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

Re: [Xen-devel] [PATCH] tools/libs: Make xenforeignmemory_unmap_resource() idempotent

[Paul Durrant]

> -Original Message-
> From: Wei Liu [mailto:wei.l...@citrix.com]
> Sent: 27 November 2018 15:48
> To: Paul Durrant 
> Cc: Andrew Cooper ; Xen-devel  de...@lists.xen.org>; Wei Liu ; Ian Jackson
> 
> Subject: Re: [PATCH] tools/libs: Make xenforeignmemory_unmap_resource()
> idempotent
> 
> On Sat, Nov 24, 2018 at 03:09:33PM +, Paul Durrant wrote:
> > > -Original Message-
> > > From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On
> Behalf
> > > Of Paul Durrant
> > > Sent: 24 November 2018 15:06
> > > To: Andrew Cooper ; Xen-devel  > > de...@lists.xen.org>
> > > Cc: Andrew Cooper ; Wei Liu
> > > ; Ian Jackson 
> > > Subject: Re: [Xen-devel] [PATCH] tools/libs: Make
> > > xenforeignmemory_unmap_resource() idempotent
> > >
> > > > -Original Message-
> > > > From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
> > > > Sent: 23 November 2018 15:12
> > > > To: Xen-devel 
> > > > Cc: Andrew Cooper ; Ian Jackson
> > > > ; Wei Liu ; Paul
> Durrant
> > > > 
> > > > Subject: [PATCH] tools/libs: Make xenforeignmemory_unmap_resource()
> > > > idempotent
> > > >
> > > > Most other close/unmap functions are.
> > > >
> > > > Signed-off-by: Andrew Cooper 
> > > > ---
> > > > CC: Ian Jackson 
> > > > CC: Wei Liu 
> > > > CC: Paul Durrant 
> > > >
> > > > This ideally wants backporting to 4.11 to hit 4.11.1
> > > >
> > > > I got an unexpected shock while trying to diagnose why GVT-g is
> still
> > > > broken (differently!) on staging.
> > > > ---
> > > >  tools/libs/foreignmemory/core.c | 7 ++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/tools/libs/foreignmemory/core.c
> > > > b/tools/libs/foreignmemory/core.c
> > > > index 63f12e2..2516fd4 100644
> > > > --- a/tools/libs/foreignmemory/core.c
> > > > +++ b/tools/libs/foreignmemory/core.c
> > > > @@ -182,7 +182,12 @@ xenforeignmemory_resource_handle
> > > > *xenforeignmemory_map_resource(
> > > >  int xenforeignmemory_unmap_resource(
> > > >  xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle
> > > > *fres)
> > > >  {
> > > > -int rc = osdep_xenforeignmemory_unmap_resource(fmem, fres);
> > > > +int rc;
> > > > +
> > > > +if ( !fres )
> > > > +return 0;
> > > > +
> > > > +rc = osdep_xenforeignmemory_unmap_resource(fmem, fres);
> > >
> > > Freeing NULL should not be problem as this defined to do nothing so
> there
> > > is nothing about this function which is not idempotent. I assume,
> without
> > > looking yet, that it is the osdep function which needs fixing.
> > >
> >
> > ...and indeed it does. I think this is patch you need...
> >
> > diff --git a/tools/libs/foreignmemory/linux.c
> b/tools/libs/foreignmemory/linux.c
> > index 132875d..8daa582 100644
> > --- a/tools/libs/foreignmemory/linux.c
> > +++ b/tools/libs/foreignmemory/linux.c
> > @@ -298,7 +298,7 @@ int
> osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
> >  int osdep_xenforeignmemory_unmap_resource(
> >  xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle
> *fres)
> >  {
> > -return munmap(fres->addr, fres->nr_frames << PAGE_SHIFT);
> > +return fres ? munmap(fres->addr, fres->nr_frames << PAGE_SHIFT) :
> 0;
> 
> Can you submit a patch for this? And please do this for other osdep
> functions as well.
> 

Sure.

  Paul

> Wei.
> 
> >  }
> >
> >   Paul

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

Re: [Xen-devel] [PATCH] amd-iommu: remove page merging code

[Jan Beulich]

>>> On 27.11.18 at 15:20,  wrote:
>>  -Original Message-
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>> Sent: 27 November 2018 13:07
>> To: Paul Durrant 
>> Cc: Brian Woods ; Suravee Suthikulpanit
>> ; xen-devel > de...@lists.xenproject.org>
>> Subject: Re: [Xen-devel] [PATCH] amd-iommu: remove page merging code
>> 
>> >>> On 26.11.18 at 18:30,  wrote:
>> > The page merging logic makes use of bits 1-8 and bit 63 of a PTE, which
>> used
>> > to be specified as ignored. However, bits 5 and 6 are now specified as
>> > 'accessed' and 'dirty' bits and their use only remains safe as long as
>> > the DTE 'Host Access Dirty' bits remain clear.
>> 
>> Upon second thought - is this actually true with the XSA-275
>> changes in place? As long as the domain is not running yet,
>> how would A and/or D bits get set?
> 
> Ok, I can amend the comment. The risk is, as I say, predicated on the bits 
> in the DTE anyway but the tables are wired into the DTE *before* being 
> populated so I don't think there is anything to stop h/w DMAing whilst they 
> are being constructed.

This way of thinking recurs: When the tables get constructed, the
domain doesn't run yet, or has no device assigned yet. In both
cases I can't see who/what would initiate DMA.

Jan



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

Re: [Xen-devel] [PATCH] amd-iommu: remove page merging code

[Jan Beulich]

>>> On 27.11.18 at 15:12,  wrote:
>>  -Original Message-
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>> Sent: 27 November 2018 13:02
>> To: Paul Durrant 
>> Cc: Brian Woods ; Suravee Suthikulpanit
>> ; xen-devel > de...@lists.xenproject.org>
>> Subject: Re: [Xen-devel] [PATCH] amd-iommu: remove page merging code
>> 
>> >>> On 26.11.18 at 18:30,  wrote:
>> > The page merging logic makes use of bits 1-8 and bit 63 of a PTE, which
>> used
>> > to be specified as ignored. However, bits 5 and 6 are now specified as
>> > 'accessed' and 'dirty' bits and their use only remains safe as long as
>> > the DTE 'Host Access Dirty' bits remain clear. The code is also of
>> dubious
>> > benefit and was the subject XSA-275.
>> >
>> > This patch removes the code, freeing up the remaining PTE 'ignored' bits
>> > for other potential use and shortening the source by 170 lines.
>> 
>> No word at all about the performance implications? Do you have
>> any plans to re-introduce properly working page recombining
>> code? I realize VT-d doesn't have any either (the maintainers at
>> some point in the distant past had promised to implement it, but
>> I guess that's long been forgotten), but anyway...
>> 
> 
> I hope to wire through the order parameter to the implementations 
> eventually, which is the right way to do things I think. It also means I'll 
> probably need to tweak the PV-IOMMU interface to handle an order parameter 
> before I send v.next too.

That's going to help only partially, but at least as far as domain
creation goes it should get us back to current state, as guest
memory population should happen in large enough chunks. Very
large guests may then still have more levels than they actually
need on AMD hardware, buts that's a corner case I consider
acceptable.

Re-combination of large pages when the domain is running,
otoh, is not going to start working again with what you describe.
Arguably there may not be too many cases where there
actually is an opportunity to do so.

Jan



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

Re: [Xen-devel] [PATCH] tools/libs: Make xenforeignmemory_unmap_resource() idempotent

[Wei Liu]

On Sat, Nov 24, 2018 at 03:09:33PM +, Paul Durrant wrote:
> > -Original Message-
> > From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf
> > Of Paul Durrant
> > Sent: 24 November 2018 15:06
> > To: Andrew Cooper ; Xen-devel  > de...@lists.xen.org>
> > Cc: Andrew Cooper ; Wei Liu
> > ; Ian Jackson 
> > Subject: Re: [Xen-devel] [PATCH] tools/libs: Make
> > xenforeignmemory_unmap_resource() idempotent
> > 
> > > -Original Message-
> > > From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
> > > Sent: 23 November 2018 15:12
> > > To: Xen-devel 
> > > Cc: Andrew Cooper ; Ian Jackson
> > > ; Wei Liu ; Paul Durrant
> > > 
> > > Subject: [PATCH] tools/libs: Make xenforeignmemory_unmap_resource()
> > > idempotent
> > >
> > > Most other close/unmap functions are.
> > >
> > > Signed-off-by: Andrew Cooper 
> > > ---
> > > CC: Ian Jackson 
> > > CC: Wei Liu 
> > > CC: Paul Durrant 
> > >
> > > This ideally wants backporting to 4.11 to hit 4.11.1
> > >
> > > I got an unexpected shock while trying to diagnose why GVT-g is still
> > > broken (differently!) on staging.
> > > ---
> > >  tools/libs/foreignmemory/core.c | 7 ++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/libs/foreignmemory/core.c
> > > b/tools/libs/foreignmemory/core.c
> > > index 63f12e2..2516fd4 100644
> > > --- a/tools/libs/foreignmemory/core.c
> > > +++ b/tools/libs/foreignmemory/core.c
> > > @@ -182,7 +182,12 @@ xenforeignmemory_resource_handle
> > > *xenforeignmemory_map_resource(
> > >  int xenforeignmemory_unmap_resource(
> > >  xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle
> > > *fres)
> > >  {
> > > -int rc = osdep_xenforeignmemory_unmap_resource(fmem, fres);
> > > +int rc;
> > > +
> > > +if ( !fres )
> > > +return 0;
> > > +
> > > +rc = osdep_xenforeignmemory_unmap_resource(fmem, fres);
> > 
> > Freeing NULL should not be problem as this defined to do nothing so there
> > is nothing about this function which is not idempotent. I assume, without
> > looking yet, that it is the osdep function which needs fixing.
> >
> 
> ...and indeed it does. I think this is patch you need...
> 
> diff --git a/tools/libs/foreignmemory/linux.c 
> b/tools/libs/foreignmemory/linux.c
> index 132875d..8daa582 100644
> --- a/tools/libs/foreignmemory/linux.c
> +++ b/tools/libs/foreignmemory/linux.c
> @@ -298,7 +298,7 @@ int 
> osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
>  int osdep_xenforeignmemory_unmap_resource(
>  xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
>  {
> -return munmap(fres->addr, fres->nr_frames << PAGE_SHIFT);
> +return fres ? munmap(fres->addr, fres->nr_frames << PAGE_SHIFT) : 0;

Can you submit a patch for this? And please do this for other osdep
functions as well.

Wei.

>  }
> 
>   Paul

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

Re: [Xen-devel] [PATCH] amd-iommu: remove page merging code

[Jan Beulich]

>>> On 27.11.18 at 15:19,  wrote:
> On 27/11/2018 13:06, Jan Beulich wrote:
> On 26.11.18 at 18:30,  wrote:
>>> The page merging logic makes use of bits 1-8 and bit 63 of a PTE, which used
>>> to be specified as ignored. However, bits 5 and 6 are now specified as
>>> 'accessed' and 'dirty' bits and their use only remains safe as long as
>>> the DTE 'Host Access Dirty' bits remain clear.
>> Upon second thought - is this actually true with the XSA-275
>> changes in place? As long as the domain is not running yet,
>> how would A and/or D bits get set?
> 
> DTE.HAD is an IOMMU control which Xen doesn't currently enable, so this
> isn't an issue in current usage.

Well, of course, and Paul did say so. My point is that even when we
start using the feature, as long as we don't set A and/or D bits
ourselves, nothing is getting in the way of the merging code, as it
ceases to do anything once the domain starts running.

Jan



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

Re: [Xen-devel] [PATCH] CODING_STYLE: explicitly call out label indentation

[Jan Beulich]

>>> On 27.11.18 at 16:23,  wrote:
> On Mon, Nov 26, 2018 at 02:04:05AM -0700, Jan Beulich wrote:
>> Since the behavior of "diff -p" to use an unindented label as context
>> identifier often makes it harder to review patches, make explicit the
>> requirement for labels to be indented.
>> 
>> Signed-off-by: Jan Beulich 
>> 
>> --- a/CODING_STYLE
>> +++ b/CODING_STYLE
>> @@ -31,6 +31,10 @@ void fun(void)
>>  }
>>  }
>>  
>> +Due to the behavior of GNU diffutils "diff -p", labels should be
>> +indented by at least one blank.  Non-case labels inside switch() bodies
>> +are preferred to be indented the same as the block's case labels.
>> +
> 
> Sorry, I don't follow this rationale.
> 
> I actually tried `diff -p` with and without indenting the label. Here is
> the result.
> 
> With:
> 
> *** kernel.c.orig   2018-11-27 15:15:20.841296089 +
> --- kernel.c2018-11-27 15:20:23.192022064 +
> *** static int assign_integer_param(const st
> *** 48,54 
>   default:
>   BUG();
>   }
> !
>   return 0;
>   }
> 
> --- 48,54 
>   default:
>   BUG();
>   }
> !  label:
>   return 0;
>   }
> 
> 
> Without:
> [...]
> They look the same to me.

Well, that's because you used a change as example where you're
_adding_ a label, whereas the issue is with other additions which
_follow_ an earlier label.

> And frankly having an extra space in front make Xen
> rather too unique. That's an issue for new comers and writing automated tool
> to check patch.

If other projects don't care about this and are happy to review
patches to files with, say, many unindented "retry" labels (in
different functions), then that's their issue. _I_ dislike reviewing
patches where I can't easily identify which function is getting
changed. And based on that I also dislike submitting patches
where this isn't easily possible.

I think I've also come to a conclusion as to why they may prefer
to leave diff behavior as it is: For assembler files the behavior is
actually useful.

Jan



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

[Xen-devel] [PATCH v6 0/2] amd/iommu: fixes for PVH Dom0

[Roger Pau Monne]

Hello,

This is the remaining of the PVH Dom0 fixes, which now only contains the
AMD IOMMU patches. The series can be found at:

git://xenbits.xen.org/people/royger/xen.git fixes-pvh-v6

Roger Pau Monne (2):
  amd/iommu: assign iommu devices to Xen
  amd/iommu: skip bridge devices when updating IOMMU page tables

 xen/drivers/passthrough/amd/iommu_init.c | 2 ++
 xen/drivers/passthrough/amd/iommu_map.c  | 3 +++
 2 files changed, 5 insertions(+)

-- 
2.19.1


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

[Xen-devel] [PATCH v6 1/2] amd/iommu: assign iommu devices to Xen

[Roger Pau Monne]

AMD IOMMU devices are exposed on the PCI bus, and thus are assigned by
default to the hardware domain. This can cause issues because the
IOMMU devices themselves are not behind an IOMMU, so update_paging_mode will
return an error if Xen tries to expand the page tables of a domain
that has assigned devices not behind an IOMMU. update_paging_mode
failing will cause the domain to be destroyed.

Fix this by hiding PCI IOMMU devices, so they are not assigned to the
hardware domain.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Jan Beulich 
---
Cc: Suravee Suthikulpanit 
Cc: Brian Woods 
---
Changes since v4:
 - Use pci_hide_device.
 - Expand commit message.
---
 xen/drivers/passthrough/amd/iommu_init.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/drivers/passthrough/amd/iommu_init.c 
b/xen/drivers/passthrough/amd/iommu_init.c
index 15c10b0929..17f39552a9 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -993,6 +993,8 @@ static void * __init allocate_ppr_log(struct amd_iommu 
*iommu)
 
 static int __init amd_iommu_init_one(struct amd_iommu *iommu)
 {
+pci_hide_device(iommu->seg, PCI_BUS(iommu->bdf), PCI_DEVFN2(iommu->bdf));
+
 if ( map_iommu_mmio_region(iommu) != 0 )
 goto error_out;
 
-- 
2.19.1


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

[Xen-devel] [PATCH v6 2/2] amd/iommu: skip bridge devices when updating IOMMU page tables

[Roger Pau Monne]

Bridges are not behind an IOMMU, and are already special cased and
skipped in amd_iommu_add_device. Apply the same special casing when
updating page tables.

This is required or else update_paging_mode will fail and return an
error to the caller (amd_iommu_{un}map_page) which will destroy the
domain.

Signed-off-by: Roger Pau Monné 
---
Cc: Suravee Suthikulpanit 
Cc: Brian Woods 
---
Changes since v5:
 - Remove the hardware domain check.

Changes since v4:
 - Invert condition order so they match the order in
   amd_iommu_add_device.
 - Expand commit message to spell out why this is required.
---
 xen/drivers/passthrough/amd/iommu_map.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/drivers/passthrough/amd/iommu_map.c 
b/xen/drivers/passthrough/amd/iommu_map.c
index c1daba8422..e4904bb3c5 100644
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -612,6 +612,9 @@ static int update_paging_mode(struct domain *d, unsigned 
long dfn)
 /* Update device table entries using new root table and paging mode */
 for_each_pdev( d, pdev )
 {
+if ( pdev->type == DEV_TYPE_PCI_HOST_BRIDGE )
+continue;
+
 bdf = PCI_BDF2(pdev->bus, pdev->devfn);
 iommu = find_iommu_for_device(pdev->seg, bdf);
 if ( !iommu )
-- 
2.19.1


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

Re: [Xen-devel] [PATCH] CODING_STYLE: explicitly call out label indentation

[Wei Liu]

On Mon, Nov 26, 2018 at 02:04:05AM -0700, Jan Beulich wrote:
> Since the behavior of "diff -p" to use an unindented label as context
> identifier often makes it harder to review patches, make explicit the
> requirement for labels to be indented.
> 
> Signed-off-by: Jan Beulich 
> 
> --- a/CODING_STYLE
> +++ b/CODING_STYLE
> @@ -31,6 +31,10 @@ void fun(void)
>  }
>  }
>  
> +Due to the behavior of GNU diffutils "diff -p", labels should be
> +indented by at least one blank.  Non-case labels inside switch() bodies
> +are preferred to be indented the same as the block's case labels.
> +

Sorry, I don't follow this rationale.

I actually tried `diff -p` with and without indenting the label. Here is
the result.

With:

*** kernel.c.orig   2018-11-27 15:15:20.841296089 +
--- kernel.c2018-11-27 15:20:23.192022064 +
*** static int assign_integer_param(const st
*** 48,54 
  default:
  BUG();
  }
!
  return 0;
  }

--- 48,54 
  default:
  BUG();
  }
!  label:
  return 0;
  }


Without:

*** kernel.c.orig   2018-11-27 15:15:20.841296089 +
--- kernel.c2018-11-27 15:21:01.456360128 +
*** static int assign_integer_param(const st
*** 48,54 
  default:
  BUG();
  }
!
  return 0;
  }

--- 48,54 
  default:
  BUG();
  }
! label:
  return 0;
  }


They look the same to me. And frankly having an extra space in front make Xen
rather too unique. That's an issue for new comers and writing automated tool
to check patch.

Wei.

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

Re: [Xen-devel] [PATCH 1/4] xen/arm: gic: Ensure we have an ISB between ack and do_IRQ()

[Julien Grall]

On 11/27/18 1:30 PM, Andrii Anisov wrote:

Hello Julien,


Hi Andrii,



On 23.11.18 15:27, Julien Grall wrote:

But we can't use it, because our system is overcommitted.


Can you describe how overcommitted your system is?
I did mean that we have a requirement to not limit VCPU number with PCPU 
number. Also we should not use cpu pinning. I guess I used a wrong word, 
it must be "oversubscribed".


Oversubscribing is usually a pretty bad idea if you want to have good 
latency. There are no promise when the vCPU will get scheduled.


Furthermore, if you want to have more vCPU than pCPU, then you still 
need to make sure that the total amount of vCPU usage is always lower 
(or equal) than the total capacity of your pCPUs.


So can you describe how oversubscribed your platform is when doing the 
benchmark?





I don't understand what you mean. Can you details it?
I hope you do not mind I draw a picture for this. I failed to find a 
simple wording for it :(
At some rate of IRQs from different sources, slight reducing of IRQ 
processing time in the hypervisor might result in an overly slower IRQs 
processing.


So, on the picture attaches, in case2 IRQ processing path in XEN made 
shorter. But when we have IRQ1 and IRQ2 asserted at some rate, we result 
in two context switches and going through the IRQ processing path twice. 
What is longer than catching the IRQ2 right away with IRQ1 in XEN itself.
We come to this idea after observing a strange effect: dropping a bit of 
code from IRQ processing path (i.e. several if's) led to the benchmark 
shown us a bit smaller numbers.


I think I now understand your problem. The problem is not because of 
re-enabling the interrupt. Instead, it is because the GIC CPU priority 
is been dropped using EOI early (via desc->handler->end()). As soon as 
you drop the priority another interrupt with the same (or lower) 
priority can fire.


Looking at do_IRQ, we don't handle the same way guest IRQ and Xen IRQ.

The steps for Xen IRQ is roughly:
-> read_irq
-> local_irq_enable
-> do_IRQ
   -> local_irq_enable (via spin_unlock_irq)
   -> call handlers
   -> local_irq_disable (via spin_lock_irq)
   -> EOI
   -> DIR
-> local_irq_disable

The steps of for Guest IRQ is roughly:
-> read_irq
-> local_irq_enable
-> do_IRQ
-> EOI
-> vgic_inject_irq
-> local_irq_disable  (via spin_lock_irqsave)
-> local_irq_enable   (via spin_lock_irqrestore)
-> local_irq_disable

All vgic_inject_irq is pretty much running with interrupts disabled. The 
Xen IRQ path seem to continue pointless enable/disable.


So I think the following steps should suit you.

Xen IRQ:
-> read_irq
-> do_IRQ
-> local_irq_enable (via spin_unlock_irq)
-> call handlers
-> local_irq_disable (via spin_lock_irq)
-> EOI
-> DIR
Guest IRQ:
-> read_irq
-> local_irq_enable
-> do_IRQ
-> EOI
-> vgic_inject_irq

SGIs seems to be handled with IRQ disabled, so no change there. For 
LPIs, we might want to do the same (needs some investigation).


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v3] xen/balloon: Mark unallocated host memory as UNUSABLE

[Igor Druzhinin]

On 27/11/2018 03:28, Boris Ostrovsky wrote:
> On 11/26/18 2:57 PM, Igor Druzhinin wrote:
>> On 26/11/2018 19:42, Boris Ostrovsky wrote:
>>> On 11/26/18 12:10 PM, Igor Druzhinin wrote:
 On 26/11/2018 16:25, Boris Ostrovsky wrote:
> On 11/25/18 8:00 PM, Igor Druzhinin wrote:
>> On 20/12/2017 14:05, Boris Ostrovsky wrote:
>>> Commit f5775e0b6116 ("x86/xen: discard RAM regions above the maximum
>>> reservation") left host memory not assigned to dom0 as available for
>>> memory hotplug.
>>>
>>> Unfortunately this also meant that those regions could be used by
>>> others. Specifically, commit fa564ad96366 ("x86/PCI: Enable a 64bit BAR
>>> on AMD Family 15h (Models 00-1f, 30-3f, 60-7f)") may try to map those
>>> addresses as MMIO.
>>>
>>> To prevent this mark unallocated host memory as E820_TYPE_UNUSABLE (thus
>>> effectively reverting f5775e0b6116) and keep track of that region as
>>> a hostmem resource that can be used for the hotplug.
>>>
>>> Signed-off-by: Boris Ostrovsky 
>> This commit breaks Xen balloon memory hotplug for us in Dom0 with
>> "hoplug_unpopulated" set to 1. The issue is that the common kernel
>> memory onlining procedures require "System RAM" resource to be 1-st
>> level. That means by inserting it under "Unusable memory" as the commit
>> above does (intentionally or not) we make it 2-nd level and break memory
>> onlining.
> What do you mean by 1st and 2nd level?
>
 I mean the level of a resource in IOMEM tree (the one that's printed
 from /proc/iomem). 1-st level means its parent is root and so on.
>>> Ah, OK. Doesn't
>>> additional_memory_resource()->insert_resource(iomem_resource) place the
>>> RAM at 1st level? And if not, can we make it so?
>>>
>> That'd mean splitting "Unusable memory" resource. Since it's allocated
>> from bootmem it has proven to be quite difficult but there are seem to
>> be special functions available particularly for memory resource
>> management operations that I've not yet experimented with. So the answer
>> is probably - maybe yes but not straightforward.
>>
>> There are multiple ways to fix it depending on what was the intention of
>> original commit and what exactly it tried to workaround. It seems it
>> does several things at once:
>> 1) Marks non-Dom0 host memory "Unusable memory" in resource tree.
>> 2) Keeps track of all the areas safe for hotplug in Dom0
>> 3) Changes allocation algorithms itself in balloon driver to use those 
>> areas
> Pretty much. (3) is true in the sense that memory is first allocated
> from hostmem_resource (which is non-dom0 RAM).
>
>> Are all the things above necessary to cover the issue in fa564ad96366
>> ("x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 00-1f, 30-3f,
>> 60-7f)")?
> Not anymore, as far as that particular commit is concerned, but that's
> because of 03a551734 ("x86/PCI: Move and shrink AMD 64-bit window to
> avoid conflict") which was introduced after balloon patch. IIRC there
> were some issues with fa564ad96366 unrelated to balloon.
>
 If it's not a problem anymore IIUC, can we revert the change as it still
 breaks "hotplug_unpopulated=1" for the reasons I described above?
>>> Since this seems to have broken existing feature this would be an
>>> option. But before going that route I'd like to see if we can fix the patch.
>>>
>>> I have been unable to reproduce your problem. Can you describe what you did?
>>>
>> It doesn't happen on all configurations as sometimes the memory is
>> successfully hotplugged to a hole depending on the size of Dom0 memory.
>> But we reproduced it quite reliably with small Dom0 sizes like 752MB.
>>
>> XenServer is using this feature to hotplug additional memory for grant
>> table operations so we started a VM and observed a stable hang.
>>
>> Can we remove "Unusable memory" resources as soon as we finished
>> booting? Is removing on-demand is preferable over "shoot them all" in
>> that case?
> The concern is that in principle nothing prevents someone else to do
> exact same thing fa564ad96366 did, which is grab something from right
> above end of RAM as the kernel sees it. And that can be done at any point.
>
 Nothing prevents - true, but that's plainly wrong from OS point of view
 to grab physical ranges for something without knowing what's actually
 behind on that platform. 
>>> I am not sure I agree that this is plainly wrong. If not for BIOS issues
>>> that 03a551734cf mentions I think what the original implementation of
>>> fa564ad963 did was perfectly reasonable. Which is why I would prefer to
>>> keep keep the hostmem resource *if possible*.
>>>
>> Exactly, those *are* BIOS issues and are not supposed to be workarounded
>> by the OS. And as the next commit showed even the workaround didn't
>> quite helped with it.
>>
>> I agree that having hotmem 

[Xen-devel] [PATCH v2] amd-iommu: remove page merging code

[Paul Durrant]

The page merging logic makes use of bits 1-8 and bit 63 of a PTE, which used
to be specified as ignored. However, bits 5 and 6 are now specified as
'accessed' and 'dirty' bits and their use only remains safe as long as
the DTE 'Host Access Dirty' bits remain unused by Xen.
The code was also the subject of XSA-275 and, since then, has been disabled
after domain creation.

This patch removes the code, freeing up the remaining PTE 'ignored' bits
for other potential use and shortening the source by 170 lines. There may
be some marginal performance cost since higher order mappings will now be
ruled out until a mapping order parameter is passed to iommu_ops. That
will be dealt with by a subsequent patch though.

Signed-off-by: Paul Durrant 
---
Cc: Suravee Suthikulpanit 
Cc: Brian Woods 

v2:
 - Remove 'no_merge' boolean
 - Expand commit comment
---
 xen/drivers/passthrough/amd/iommu_map.c | 175 +---
 xen/include/asm-x86/iommu.h |   1 -
 2 files changed, 1 insertion(+), 175 deletions(-)

diff --git a/xen/drivers/passthrough/amd/iommu_map.c 
b/xen/drivers/passthrough/amd/iommu_map.c
index 0ac3f473b3..04cb7b3182 100644
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -323,134 +323,6 @@ uint64_t amd_iommu_get_address_from_pte(void *pte)
 return ptr;
 }
 
-/* For each pde, We use ignored bits (bit 1 - bit 8 and bit 63)
- * to save pde count, pde count = 511 is a candidate of page coalescing.
- */
-static unsigned int get_pde_count(uint64_t pde)
-{
-unsigned int count;
-uint64_t upper_mask = 1ULL << 63 ;
-uint64_t lower_mask = 0xFF << 1;
-
-count = ((pde & upper_mask) >> 55) | ((pde & lower_mask) >> 1);
-return count;
-}
-
-/* Convert pde count into iommu pte ignored bits */
-static void set_pde_count(uint64_t *pde, unsigned int count)
-{
-uint64_t upper_mask = 1ULL << 8 ;
-uint64_t lower_mask = 0xFF;
-uint64_t pte_mask = (~(1ULL << 63)) & (~(0xFF << 1));
-
-*pde &= pte_mask;
-*pde |= ((count & upper_mask ) << 55) | ((count & lower_mask ) << 1);
-}
-
-/* Return 1, if pages are suitable for merging at merge_level.
- * otherwise increase pde count if mfn is contigous with mfn - 1
- */
-static bool iommu_update_pde_count(struct domain *d, unsigned long pt_mfn,
-   unsigned long dfn, unsigned long mfn,
-   unsigned int merge_level)
-{
-unsigned int pde_count, next_level;
-unsigned long first_mfn;
-uint64_t *table, *pde, *ntable;
-uint64_t ntable_maddr, mask;
-struct domain_iommu *hd = dom_iommu(d);
-bool ok = false;
-
-ASSERT( spin_is_locked(>arch.mapping_lock) && pt_mfn );
-
-next_level = merge_level - 1;
-
-/* get pde at merge level */
-table = map_domain_page(_mfn(pt_mfn));
-pde = table + pfn_to_pde_idx(dfn, merge_level);
-
-/* get page table of next level */
-ntable_maddr = amd_iommu_get_address_from_pte(pde);
-ntable = map_domain_page(_mfn(paddr_to_pfn(ntable_maddr)));
-
-/* get the first mfn of next level */
-first_mfn = amd_iommu_get_address_from_pte(ntable) >> PAGE_SHIFT;
-
-if ( first_mfn == 0 )
-goto out;
-
-mask = (1ULL<< (PTE_PER_TABLE_SHIFT * next_level)) - 1;
-
-if ( ((first_mfn & mask) == 0) &&
- (((dfn & mask) | first_mfn) == mfn) )
-{
-pde_count = get_pde_count(*pde);
-
-if ( pde_count == (PTE_PER_TABLE_SIZE - 1) )
-ok = true;
-else if ( pde_count < (PTE_PER_TABLE_SIZE - 1))
-{
-pde_count++;
-set_pde_count(pde, pde_count);
-}
-}
-
-else
-/* non-contiguous mapping */
-set_pde_count(pde, 0);
-
-out:
-unmap_domain_page(ntable);
-unmap_domain_page(table);
-
-return ok;
-}
-
-static int iommu_merge_pages(struct domain *d, unsigned long pt_mfn,
- unsigned long dfn, unsigned int flags,
- unsigned int merge_level)
-{
-uint64_t *table, *pde, *ntable;
-uint64_t ntable_mfn;
-unsigned long first_mfn;
-struct domain_iommu *hd = dom_iommu(d);
-
-ASSERT( spin_is_locked(>arch.mapping_lock) && pt_mfn );
-
-table = map_domain_page(_mfn(pt_mfn));
-pde = table + pfn_to_pde_idx(dfn, merge_level);
-
-/* get first mfn */
-ntable_mfn = amd_iommu_get_address_from_pte(pde) >> PAGE_SHIFT;
-
-if ( ntable_mfn == 0 )
-{
-unmap_domain_page(table);
-return 1;
-}
-
-ntable = map_domain_page(_mfn(ntable_mfn));
-first_mfn = amd_iommu_get_address_from_pte(ntable) >> PAGE_SHIFT;
-
-if ( first_mfn == 0 )
-{
-unmap_domain_page(ntable);
-unmap_domain_page(table);
-return 1;
-}
-
-/* setup super page mapping, next level = 0 */
-set_iommu_pde_present((uint32_t *)pde, first_mfn, 0,
-  !!(flags & IOMMUF_writable),
-  !!(flags & 

[Xen-devel] [linux-4.4 test] 130791: trouble: blocked/broken/fail/pass

[osstest service owner]

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64-pvopsbroken
 build-arm64-xsm  broken
 build-arm64  broken

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-qemut-win7-amd64  7 xen-boot fail in 130705 pass in 130791
 test-armhf-armhf-xl-arndale   7 xen-boot fail in 130705 pass in 130791
 test-armhf-armhf-libvirt  7 xen-boot   fail pass in 130705

Regressions which are regarded as allowable (not blocking):
 build-arm64   2 hosts-allocate broken REGR. vs. 129898
 build-arm64-pvops 2 hosts-allocate broken REGR. vs. 129898
 build-arm64-xsm   2 hosts-allocate broken REGR. vs. 129898

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit1   1 build-check(1)   blocked  n/a
 test-arm64-arm64-examine  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 build-arm64-xsm   3 capture-logs broken never pass
 build-arm64   3 capture-logs broken never pass
 build-arm64-pvops 3 capture-logs broken never pass
 test-armhf-armhf-libvirt13 migrate-support-check fail in 130705 never pass
 test-armhf-armhf-libvirt 14 saverestore-support-check fail in 130705 never pass
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail never pass
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvhv2-amd 12 guest-start  fail  never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 

[Xen-devel] [PATCH] xen: xlate_mmu: add missing header to fix 'W=1' warning

[Srikanth Boddepalli]

Add a missing header otherwise compiler warns about missed prototype:

drivers/xen/xlate_mmu.c:183:5: warning: no previous prototype for 
'xen_xlate_unmap_gfn_range?' [-Wmissing-prototypes]
  int xen_xlate_unmap_gfn_range(struct vm_area_struct *vma,
  ^

Signed-off-by: Srikanth Boddepalli 
---
 drivers/xen/xlate_mmu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/xen/xlate_mmu.c b/drivers/xen/xlate_mmu.c
index 23f1387..e7df65d 100644
--- a/drivers/xen/xlate_mmu.c
+++ b/drivers/xen/xlate_mmu.c
@@ -36,6 +36,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 #include 
-- 
2.7.4


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

Re: [Xen-devel] [PATCH v2] mm: make opt_bootscrub non-init

[Roger Pau Monné]

On Tue, Nov 27, 2018 at 03:13:27AM -0700, Jan Beulich wrote:
> >>> On 26.11.18 at 18:55,  wrote:
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -166,7 +166,14 @@ enum bootscrub_mode {
> >  BOOTSCRUB_ON,
> >  BOOTSCRUB_IDLE,
> >  };
> > -static enum bootscrub_mode __initdata opt_bootscrub = BOOTSCRUB_IDLE;
> > +/*
> > + * opt_bootscrub should live in the init section, since it's not accessed
> > + * afterwards. However at least LLVM assumes there are no side effects of
> > + * accessing the variable, and optimizes the condition so opt_bootscrub is
> 
> ... the condition in init_heap_pages() ...
> 
> (which I guess can be folded in while committing).

Yes please.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH] amd-iommu: remove page merging code

[Paul Durrant]

> -Original Message-
> From: Andrew Cooper
> Sent: 27 November 2018 14:19
> To: Jan Beulich ; Paul Durrant
> 
> Cc: xen-devel ; Brian Woods
> ; Suravee Suthikulpanit
> 
> Subject: Re: [Xen-devel] [PATCH] amd-iommu: remove page merging code
> 
> On 27/11/2018 13:06, Jan Beulich wrote:
>  On 26.11.18 at 18:30,  wrote:
> >> The page merging logic makes use of bits 1-8 and bit 63 of a PTE, which
> used
> >> to be specified as ignored. However, bits 5 and 6 are now specified as
> >> 'accessed' and 'dirty' bits and their use only remains safe as long as
> >> the DTE 'Host Access Dirty' bits remain clear.
> > Upon second thought - is this actually true with the XSA-275
> > changes in place? As long as the domain is not running yet,
> > how would A and/or D bits get set?
> 
> DTE.HAD is an IOMMU control which Xen doesn't currently enable, so this
> isn't an issue in current usage.

Yes, that was the point of my comment but I'll clarify that 'remain clear' 
means that Xen does not set them.

  Paul

> 
> ~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] amd-iommu: remove page merging code

[Paul Durrant]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 27 November 2018 13:07
> To: Paul Durrant 
> Cc: Brian Woods ; Suravee Suthikulpanit
> ; xen-devel  de...@lists.xenproject.org>
> Subject: Re: [Xen-devel] [PATCH] amd-iommu: remove page merging code
> 
> >>> On 26.11.18 at 18:30,  wrote:
> > The page merging logic makes use of bits 1-8 and bit 63 of a PTE, which
> used
> > to be specified as ignored. However, bits 5 and 6 are now specified as
> > 'accessed' and 'dirty' bits and their use only remains safe as long as
> > the DTE 'Host Access Dirty' bits remain clear.
> 
> Upon second thought - is this actually true with the XSA-275
> changes in place? As long as the domain is not running yet,
> how would A and/or D bits get set?

Ok, I can amend the comment. The risk is, as I say, predicated on the bits in 
the DTE anyway but the tables are wired into the DTE *before* being populated 
so I don't think there is anything to stop h/w DMAing whilst they are being 
constructed.

> 
> > @@ -698,55 +569,14 @@ int amd_iommu_map_page(struct domain *d, dfn_t
> dfn, mfn_t mfn,
> >  return -EFAULT;
> >  }
> >
> > -/* Install 4k mapping first */
> > +/* Install 4k mapping */
> >  need_flush = set_iommu_pte_present(pt_mfn[1], dfn_x(dfn),
> mfn_x(mfn), 1,
> > !!(flags & IOMMUF_writable),
> > !!(flags & IOMMUF_readable));
> >
> >  if ( need_flush )
> > -{
> >  amd_iommu_flush_pages(d, dfn_x(dfn), 0);
> > -/* No further merging, as the logic doesn't cope. */
> > -hd->arch.no_merge = true;
> 
> Besides removing the uses of this field, which was introduced for
> XSA-275, you should then also remove the field itself I think.

Yes, I missed that.

  Paul

> 
> Jan
> 


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

Re: [Xen-devel] [PATCH] amd-iommu: remove page merging code

[Andrew Cooper]

On 27/11/2018 13:06, Jan Beulich wrote:
 On 26.11.18 at 18:30,  wrote:
>> The page merging logic makes use of bits 1-8 and bit 63 of a PTE, which used
>> to be specified as ignored. However, bits 5 and 6 are now specified as
>> 'accessed' and 'dirty' bits and their use only remains safe as long as
>> the DTE 'Host Access Dirty' bits remain clear.
> Upon second thought - is this actually true with the XSA-275
> changes in place? As long as the domain is not running yet,
> how would A and/or D bits get set?

DTE.HAD is an IOMMU control which Xen doesn't currently enable, so this
isn't an issue in current usage.

~Andrew

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

Re: [Xen-devel] [PATCH] amd-iommu: remove page merging code

[Paul Durrant]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 27 November 2018 13:02
> To: Paul Durrant 
> Cc: Brian Woods ; Suravee Suthikulpanit
> ; xen-devel  de...@lists.xenproject.org>
> Subject: Re: [Xen-devel] [PATCH] amd-iommu: remove page merging code
> 
> >>> On 26.11.18 at 18:30,  wrote:
> > The page merging logic makes use of bits 1-8 and bit 63 of a PTE, which
> used
> > to be specified as ignored. However, bits 5 and 6 are now specified as
> > 'accessed' and 'dirty' bits and their use only remains safe as long as
> > the DTE 'Host Access Dirty' bits remain clear. The code is also of
> dubious
> > benefit and was the subject XSA-275.
> >
> > This patch removes the code, freeing up the remaining PTE 'ignored' bits
> > for other potential use and shortening the source by 170 lines.
> 
> No word at all about the performance implications? Do you have
> any plans to re-introduce properly working page recombining
> code? I realize VT-d doesn't have any either (the maintainers at
> some point in the distant past had promised to implement it, but
> I guess that's long been forgotten), but anyway...
> 

I hope to wire through the order parameter to the implementations eventually, 
which is the right way to do things I think. It also means I'll probably need 
to tweak the PV-IOMMU interface to handle an order parameter before I send 
v.next too.

  Paul

> Jan
> 


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

Re: [Xen-devel] [Qemu-devel] [PATCH for-3.2 v3 10/14] qdev-props: call object_apply_global_props()

[Igor Mammedov]

On Tue, 27 Nov 2018 00:02:35 +0400
Marc-André Lureau  wrote:

> Hi
> On Mon, Nov 26, 2018 at 5:27 PM Igor Mammedov  wrote:
> >
> > On Wed,  7 Nov 2018 16:36:48 +0400
> > Marc-André Lureau  wrote:
> >  
> > > It's now possible to use the common function.
> > >
> > > Teach object_apply_global_props() to warn if Error argument is NULL.
> > >
> > > Signed-off-by: Marc-André Lureau 
> > > ---
> > >  hw/core/qdev-properties.c | 24 ++--
> > >  qom/object.c  |  6 +-
> > >  2 files changed, 7 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> > > index 8728cbab9f..239535a4cb 100644
> > > --- a/hw/core/qdev-properties.c
> > > +++ b/hw/core/qdev-properties.c
> > > @@ -1223,28 +1223,8 @@ int qdev_prop_check_globals(void)
> > >
> > >  void qdev_prop_set_globals(DeviceState *dev)
> > >  {
> > > -int i;
> > > -
> > > -for (i = 0; i < global_props()->len; i++) {
> > > -GlobalProperty *prop;
> > > -Error *err = NULL;
> > > -
> > > -prop = g_array_index(global_props(), GlobalProperty *, i);
> > > -if (object_dynamic_cast(OBJECT(dev), prop->driver) == NULL) {
> > > -continue;
> > > -}
> > > -prop->used = true;
> > > -object_property_parse(OBJECT(dev), prop->value, prop->property, 
> > > );
> > > -if (err != NULL) {
> > > -error_prepend(, "can't apply global %s.%s=%s: ",
> > > -  prop->driver, prop->property, prop->value);
> > > -if (!dev->hotplugged) {
> > > -error_propagate(_fatal, err);
> > > -} else {
> > > -warn_report_err(err);
> > > -}
> > > -}
> > > -}
> > > +object_apply_global_props(OBJECT(dev), global_props(),
> > > +  dev->hotplugged ? NULL : _fatal);  
> > arguably, it's up to caller to decide it warn or not.
> > I'd leave it warning code out of object_apply_global_props() and let caller 
> > do the job  
> 
> The problem is that there may be multiple errors, and the remaining
> globals should be applied.
>
> I'll add a comment.
ok

 
> > >  }
> > >
> > >  /* --- 64bit unsigned int 'size' type --- */
> > > diff --git a/qom/object.c b/qom/object.c
> > > index 9acdf9e16d..b1a7f70550 100644
> > > --- a/qom/object.c
> > > +++ b/qom/object.c
> > > @@ -392,7 +392,11 @@ void object_apply_global_props(Object *obj, GArray 
> > > *props, Error **errp)
> > >  if (err != NULL) {
> > >  error_prepend(, "can't apply global %s.%s=%s: ",
> > >p->driver, p->property, p->value);
> > > -error_propagate(errp, err);
> > > +if (errp) {
> > > +error_propagate(errp, err);
> > > +} else {
> > > +warn_report_err(err);
> > > +}
> > >  }
> > >  }
> > >  }  
> >
> >  
> 
> 


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

Re: [Xen-devel] [Qemu-devel] [PATCH v5 20/24] hw: acpi: Define ACPI tables builder interface

[Igor Mammedov]

On Thu, 22 Nov 2018 00:57:21 +0100
Samuel Ortiz  wrote:

> On Fri, Nov 16, 2018 at 05:02:26PM +0100, Igor Mammedov wrote:
> > On Mon,  5 Nov 2018 02:40:43 +0100
> > Samuel Ortiz  wrote:
> >   
> > > In order to decouple ACPI APIs from specific machine types, we are
> > > creating an ACPI builder interface that each ACPI platform can choose to
> > > implement.
> > > This way, a new machine type can re-use the high level ACPI APIs and
> > > define some custom table build methods, without having to duplicate most
> > > of the existing implementation only to add small variations to it.  
> > I'm not sure about motivation behind so high APIs,
> > what obvious here is an extra level of indirection for not clear gain.
> > 
> > Yep using table callbacks, one can attempt to generalize
> > acpi_setup() and help boards to decide which tables do not build
> > (MCFG comes to the mind). But I'm not convinced that acpi_setup()
> > could be cleanly generalized as a whole (probably some parts but
> > not everything)  
> It's more about generalizing acpi_build(), and then having one
> acpi_setup for non hardware-reduced ACPI and a acpi_reduced_setup() for
> hardware-reduced.
> 
> Right now there's no generalization at all but with this patch we could
> already use the same acpi_reduced_setup() implementation for both arm
> and i386/virt.
> 
> > so it's minor benefit for extra headache of
> > figuring out what callback will be actually called when reading code.  
> This is the same complexity that already exists for essentially all
> current interfaces.
in case of callback vs plain function call, I'd choose the later
if it does the job and resort to the former when I have to.
 
> > However if board needs a slightly different table, it will have to
> > duplicate an exiting one and then modify to suit its needs.
> > 
> > to me it pretty much looks the same as calling build_foo()
> > we use now but with an extra indirection level and then
> > duplicating the later for usage in another board in slightly
> > different manner.  
> I believe what you're trying to say is that this abstraction may be
> useful but you're arguing the granularity is not properly defined? Am I
> getting this right?
yep, something along those lines. So far it's not useful much if at all.
So I'll not introduce it for now and try to get by with plain functions
calls. Later we might add fine-grained callbacks on case by case basis
(like 'adevc->madt_cpu') where it's possible to generalize.
 
> Cheers,
> Samuel.


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

Re: [Xen-devel] [PATCH] xen/x86: add diagnostic printout to xen_mc_flush() in case of error

[Boris Ostrovsky]

On 11/27/18 2:46 AM, Juergen Gross wrote:
> On 26/11/2018 21:11, Boris Ostrovsky wrote:
>> On 11/23/18 11:24 AM, Juergen Gross wrote:
>>> Failure of an element of a Xen multicall is signalled via a WARN()
>>> only unless the kernel is compiled with MC_DEBUG. It is impossible to
>> s/unless/if
>>
>>
>>> know which element failed and why it did so.
>>>
>>> Change that by printing the related information even without MC_DEBUG,
>>> even if maybe in some limited form (e.g. without information which
>>> caller produced the failing element).
>>>
>>> Move the printing out of the switch statement in order to have the
>>> same information for a single call.
>>>
>>> Signed-off-by: Juergen Gross 
>>> ---
>>>  arch/x86/xen/multicalls.c | 35 ---
>>>  1 file changed, 20 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/arch/x86/xen/multicalls.c b/arch/x86/xen/multicalls.c
>>> index 2bce7958ce8b..0766a08bdf45 100644
>>> --- a/arch/x86/xen/multicalls.c
>>> +++ b/arch/x86/xen/multicalls.c
>>> @@ -69,6 +69,11 @@ void xen_mc_flush(void)
>>>  
>>> trace_xen_mc_flush(b->mcidx, b->argidx, b->cbidx);
>>>  
>>> +#if MC_DEBUG
>>> +   memcpy(b->debug, b->entries,
>>> +  b->mcidx * sizeof(struct multicall_entry));
>>> +#endif
>>> +
>>> switch (b->mcidx) {
>>> case 0:
>>> /* no-op */
>>> @@ -87,32 +92,34 @@ void xen_mc_flush(void)
>>> break;
>>>  
>>> default:
>>> -#if MC_DEBUG
>>> -   memcpy(b->debug, b->entries,
>>> -  b->mcidx * sizeof(struct multicall_entry));
>>> -#endif
>>> -
>>> if (HYPERVISOR_multicall(b->entries, b->mcidx) != 0)
>>> BUG();
>>> for (i = 0; i < b->mcidx; i++)
>>> if (b->entries[i].result < 0)
>>> ret++;
>>> +   }
>>>  
>>> +   if (WARN_ON(ret)) {
>>> +   pr_err("%d of %d multicall(s) failed: cpu %d\n",
>>> +  ret, b->mcidx, smp_processor_id());
>>> +   for (i = 0; i < b->mcidx; i++) {
>>> +   if (b->entries[i].result < 0) {
>>>  #if MC_DEBUG
>>> -   if (ret) {
>>> -   printk(KERN_ERR "%d multicall(s) failed: cpu %d\n",
>>> -  ret, smp_processor_id());
>>> -   dump_stack();
>>> -   for (i = 0; i < b->mcidx; i++) {
>>> -   printk(KERN_DEBUG "  call %2d/%d: op=%lu 
>>> arg=[%lx] result=%ld\t%pF\n",
>>> -  i+1, b->mcidx,
>>> +   pr_err("  call %2d: op=%lu arg=[%lx] 
>>> result=%ld\t%pF\n",
>>> +  i + 1,
>>>b->debug[i].op,
>>>b->debug[i].args[0],
>>>b->entries[i].result,
>>>b->caller[i]);
>>> +#else
>>> +   pr_err("  call %2d: op=%lu arg=[%lx] 
>>> result=%ld\n",
>>> +  i + 1,
>>> +  b->entries[i].op,
>>> +  b->entries[i].args[0],
>>> +  b->entries[i].result);
>>> +#endif
>> Doesn't (non-debug) hypervisor corrupt op and args?
> No. Only debug hypervisor does so.
>
> See my patch (and rather lengthy discussion) on xen-devel:
>
> https://lists.xen.org/archives/html/xen-devel/2018-11/msg02714.html

Yes, I saw that later, after I responded to the patch.


>
>> (Also, we don't really need to print anything when b->entries[i].result
>> == 0)
> Right. Did you miss the:

I did.

Reviewed-by: Boris Ostrovsky 

(with commit message fixed)



>
> + if (b->entries[i].result < 0) {
>
> above?
>
>
> Juergen


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