[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/2] exec: Check Xen is enabled before calling the Xen API
From: |
Juan Quintela |
Subject: |
Re: [PATCH v2 1/2] exec: Check Xen is enabled before calling the Xen API |
Date: |
Fri, 08 May 2020 10:39:51 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Philippe Mathieu-Daudé <address@hidden> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
> include/exec/ram_addr.h | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index 5e59a3d8d7..dd8713179e 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -330,7 +330,9 @@ static inline void
> cpu_physical_memory_set_dirty_range(ram_addr_t start,
> }
> }
>
> - xen_hvm_modified_memory(start, length);
> + if (xen_enabled()) {
> + xen_hvm_modified_memory(start, length);
> + }
> }
>
> #if !defined(_WIN32)
> @@ -388,7 +390,9 @@ static inline void
> cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap,
> }
> }
>
> - xen_hvm_modified_memory(start, pages << TARGET_PAGE_BITS);
> + if (xen_enabled()) {
> + xen_hvm_modified_memory(start, pages << TARGET_PAGE_BITS);
> + }
> } else {
> uint8_t clients = tcg_enabled() ? DIRTY_CLIENTS_ALL :
> DIRTY_CLIENTS_NOCODE;
I don't object moving the xen code to accell. But I think that this
change is bad.
On the following patch:
- You export xen_allowed
(ok, it was already exported, but I think it shouldn't)
(master)$ find . -type f | xargs grep xen_allowed
./hw/xen/xen-common.c: ac->allowed = &xen_allowed;
./include/hw/xen/xen.h:extern bool xen_allowed;
./include/hw/xen/xen.h: return xen_allowed;
./softmmu/vl.c:bool xen_allowed;
This are all the users that I can find.
And xen_havm_modified_memory() is an empty function if xen is not
compiled in. And in the case that xen is compiled in, the 1st thing
that it checks is:
if (unlikely(xen_in_migration)) {
That is way more restrictive that xen_enabled().
So, I think that it is better to drop this patch, maintain next one, but
just un-exporting xen_allowed.
What do you think?
Later, Juan.