* [PATCH-for-9.0 v2 01/19] tests/avocado: Add 'guest:xen' tag to tests running Xen guest
2023-11-14 14:37 [PATCH-for-9.0 v2 00/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
@ 2023-11-14 14:37 ` Philippe Mathieu-Daudé
2023-11-14 14:50 ` David Woodhouse
2023-11-14 14:37 ` [PATCH-for-9.0 v2 02/19] sysemu/xen: Forbid using Xen headers in user emulation Philippe Mathieu-Daudé
` (17 subsequent siblings)
18 siblings, 1 reply; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 14:37 UTC (permalink / raw)
To: David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
David Woodhouse, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, Anthony Perard, kvm, Thomas Huth,
Philippe Mathieu-Daudé,
Cleber Rosa, Wainer dos Santos Moschetta, Beraldo Leal
Add a tag to run all Xen-specific tests using:
$ make check-avocado AVOCADO_TAGS='guest:xen'
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
tests/avocado/boot_xen.py | 3 +++
tests/avocado/kvm_xen_guest.py | 1 +
2 files changed, 4 insertions(+)
diff --git a/tests/avocado/boot_xen.py b/tests/avocado/boot_xen.py
index fc2faeedb5..f7f35d4740 100644
--- a/tests/avocado/boot_xen.py
+++ b/tests/avocado/boot_xen.py
@@ -61,6 +61,9 @@ def launch_xen(self, xen_path):
class BootXen(BootXenBase):
+ """
+ :avocado: tags=guest:xen
+ """
def test_arm64_xen_411_and_dom0(self):
"""
diff --git a/tests/avocado/kvm_xen_guest.py b/tests/avocado/kvm_xen_guest.py
index 5391283113..63607707d6 100644
--- a/tests/avocado/kvm_xen_guest.py
+++ b/tests/avocado/kvm_xen_guest.py
@@ -22,6 +22,7 @@ class KVMXenGuest(QemuSystemTest, LinuxSSHMixIn):
:avocado: tags=arch:x86_64
:avocado: tags=machine:q35
:avocado: tags=accel:kvm
+ :avocado: tags=guest:xen
:avocado: tags=kvm_xen_guest
"""
--
2.41.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 01/19] tests/avocado: Add 'guest:xen' tag to tests running Xen guest
2023-11-14 14:37 ` [PATCH-for-9.0 v2 01/19] tests/avocado: Add 'guest:xen' tag to tests running Xen guest Philippe Mathieu-Daudé
@ 2023-11-14 14:50 ` David Woodhouse
2023-11-14 15:00 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 50+ messages in thread
From: David Woodhouse @ 2023-11-14 14:50 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
Stefano Stabellini, Richard Henderson, xen-devel, qemu-block,
Anthony Perard, kvm, Thomas Huth, Cleber Rosa,
Wainer dos Santos Moschetta, Beraldo Leal
On 14 November 2023 09:37:57 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>Add a tag to run all Xen-specific tests using:
>
> $ make check-avocado AVOCADO_TAGS='guest:xen'
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> tests/avocado/boot_xen.py | 3 +++
> tests/avocado/kvm_xen_guest.py | 1 +
> 2 files changed, 4 insertions(+)
Those two are very different. One runs on Xen, the other on KVM. Do we want to use the same tag for both?
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 01/19] tests/avocado: Add 'guest:xen' tag to tests running Xen guest
2023-11-14 14:50 ` David Woodhouse
@ 2023-11-14 15:00 ` Philippe Mathieu-Daudé
2023-11-14 15:08 ` David Woodhouse
0 siblings, 1 reply; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 15:00 UTC (permalink / raw)
To: David Woodhouse, David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
Stefano Stabellini, Richard Henderson, xen-devel, qemu-block,
Anthony Perard, kvm, Thomas Huth, Cleber Rosa,
Wainer dos Santos Moschetta, Beraldo Leal
On 14/11/23 15:50, David Woodhouse wrote:
> On 14 November 2023 09:37:57 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>> Add a tag to run all Xen-specific tests using:
>>
>> $ make check-avocado AVOCADO_TAGS='guest:xen'
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> tests/avocado/boot_xen.py | 3 +++
>> tests/avocado/kvm_xen_guest.py | 1 +
>> 2 files changed, 4 insertions(+)
>
> Those two are very different. One runs on Xen, the other on KVM. Do we want to use the same tag for both?
My understanding is,
- boot_xen.py runs Xen on TCG
- kvm_xen_guest.py runs Xen on KVM
so both runs Xen guests.
Alex, is that incorrect?
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 01/19] tests/avocado: Add 'guest:xen' tag to tests running Xen guest
2023-11-14 15:00 ` Philippe Mathieu-Daudé
@ 2023-11-14 15:08 ` David Woodhouse
2023-11-14 15:13 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 50+ messages in thread
From: David Woodhouse @ 2023-11-14 15:08 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
Stefano Stabellini, Richard Henderson, xen-devel, qemu-block,
Anthony Perard, kvm, Thomas Huth, Cleber Rosa,
Wainer dos Santos Moschetta, Beraldo Leal
On 14 November 2023 10:00:09 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>On 14/11/23 15:50, David Woodhouse wrote:
>> On 14 November 2023 09:37:57 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>>> Add a tag to run all Xen-specific tests using:
>>>
>>> $ make check-avocado AVOCADO_TAGS='guest:xen'
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>> tests/avocado/boot_xen.py | 3 +++
>>> tests/avocado/kvm_xen_guest.py | 1 +
>>> 2 files changed, 4 insertions(+)
>>
>> Those two are very different. One runs on Xen, the other on KVM. Do we want to use the same tag for both?
>
>My understanding is,
>- boot_xen.py runs Xen on TCG
>- kvm_xen_guest.py runs Xen on KVM
>so both runs Xen guests.
Does boot_xen.py actually boot *Xen*? And presumably at least one Xen guest *within* Xen?
kvm_xen_guest.py boots a "Xen guest" under KVM directly without any real Xen being present. It's *emulating* Xen.
They do both run Xen guests (or at least guests which use Xen hypercalls and *think* they're running under Xen). But is that the important classification for lumping them together?
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 01/19] tests/avocado: Add 'guest:xen' tag to tests running Xen guest
2023-11-14 15:08 ` David Woodhouse
@ 2023-11-14 15:13 ` Philippe Mathieu-Daudé
2023-11-14 15:19 ` David Woodhouse
0 siblings, 1 reply; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 15:13 UTC (permalink / raw)
To: David Woodhouse, David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
Stefano Stabellini, Richard Henderson, xen-devel, qemu-block,
Anthony Perard, kvm, Thomas Huth, Cleber Rosa,
Wainer dos Santos Moschetta, Beraldo Leal
On 14/11/23 16:08, David Woodhouse wrote:
> On 14 November 2023 10:00:09 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>> On 14/11/23 15:50, David Woodhouse wrote:
>>> On 14 November 2023 09:37:57 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>>>> Add a tag to run all Xen-specific tests using:
>>>>
>>>> $ make check-avocado AVOCADO_TAGS='guest:xen'
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>> tests/avocado/boot_xen.py | 3 +++
>>>> tests/avocado/kvm_xen_guest.py | 1 +
>>>> 2 files changed, 4 insertions(+)
>>>
>>> Those two are very different. One runs on Xen, the other on KVM. Do we want to use the same tag for both?
>>
>> My understanding is,
>> - boot_xen.py runs Xen on TCG
>> - kvm_xen_guest.py runs Xen on KVM
>> so both runs Xen guests.
>
> Does boot_xen.py actually boot *Xen*? And presumably at least one Xen guest *within* Xen?
I'll let Alex confirm, but yes, I expect Xen guest within Xen guest
within TCG. So the tags "accel:tcg" (already present) and "guest:xen".
> kvm_xen_guest.py boots a "Xen guest" under KVM directly without any real Xen being present. It's *emulating* Xen.
Yes, so the tag "guest:xen" is correct.
> They do both run Xen guests (or at least guests which use Xen hypercalls and *think* they're running under Xen). But is that the important classification for lumping them together?
The idea of AVOCADO_TAGS is to restrict testing to what you want to
cover. So here this allow running 'anything that can run Xen guest'
in a single command, for example it is handy on my macOS aarch64 host.
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 01/19] tests/avocado: Add 'guest:xen' tag to tests running Xen guest
2023-11-14 15:13 ` Philippe Mathieu-Daudé
@ 2023-11-14 15:19 ` David Woodhouse
2023-11-14 15:42 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 50+ messages in thread
From: David Woodhouse @ 2023-11-14 15:19 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
Stefano Stabellini, Richard Henderson, xen-devel, qemu-block,
Anthony Perard, kvm, Thomas Huth, Cleber Rosa,
Wainer dos Santos Moschetta, Beraldo Leal
On 14 November 2023 10:13:14 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>On 14/11/23 16:08, David Woodhouse wrote:
>> On 14 November 2023 10:00:09 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>>> On 14/11/23 15:50, David Woodhouse wrote:
>>>> On 14 November 2023 09:37:57 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>>>>> Add a tag to run all Xen-specific tests using:
>>>>>
>>>>> $ make check-avocado AVOCADO_TAGS='guest:xen'
>>>>>
>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>> ---
>>>>> tests/avocado/boot_xen.py | 3 +++
>>>>> tests/avocado/kvm_xen_guest.py | 1 +
>>>>> 2 files changed, 4 insertions(+)
>>>>
>>>> Those two are very different. One runs on Xen, the other on KVM. Do we want to use the same tag for both?
>>>
>>> My understanding is,
>>> - boot_xen.py runs Xen on TCG
>>> - kvm_xen_guest.py runs Xen on KVM
>>> so both runs Xen guests.
>>
>> Does boot_xen.py actually boot *Xen*? And presumably at least one Xen guest *within* Xen?
>
>I'll let Alex confirm, but yes, I expect Xen guest within Xen guest within TCG. So the tags "accel:tcg" (already present) and "guest:xen".
>
>> kvm_xen_guest.py boots a "Xen guest" under KVM directly without any real Xen being present. It's *emulating* Xen.
>
>Yes, so the tag "guest:xen" is correct.
>
>> They do both run Xen guests (or at least guests which use Xen hypercalls and *think* they're running under Xen). But is that the important classification for lumping them together?
>
>The idea of AVOCADO_TAGS is to restrict testing to what you want to cover. So here this allow running 'anything that can run Xen guest'
>in a single command, for example it is handy on my macOS aarch64 host.
Ok, that makes sense then. Thanks for your patience.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 01/19] tests/avocado: Add 'guest:xen' tag to tests running Xen guest
2023-11-14 15:19 ` David Woodhouse
@ 2023-11-14 15:42 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 15:42 UTC (permalink / raw)
To: David Woodhouse, David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
Stefano Stabellini, Richard Henderson, xen-devel, qemu-block,
Anthony Perard, kvm, Thomas Huth, Cleber Rosa,
Wainer dos Santos Moschetta, Beraldo Leal
On 14/11/23 16:19, David Woodhouse wrote:
> On 14 November 2023 10:13:14 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>> On 14/11/23 16:08, David Woodhouse wrote:
>>> On 14 November 2023 10:00:09 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>>>> On 14/11/23 15:50, David Woodhouse wrote:
>>>>> On 14 November 2023 09:37:57 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>>>>>> Add a tag to run all Xen-specific tests using:
>>>>>>
>>>>>> $ make check-avocado AVOCADO_TAGS='guest:xen'
>>>>>>
>>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>>> ---
>>>>>> tests/avocado/boot_xen.py | 3 +++
>>>>>> tests/avocado/kvm_xen_guest.py | 1 +
>>>>>> 2 files changed, 4 insertions(+)
>>>>>
>>>>> Those two are very different. One runs on Xen, the other on KVM. Do we want to use the same tag for both?
>>>>
>>>> My understanding is,
>>>> - boot_xen.py runs Xen on TCG
>>>> - kvm_xen_guest.py runs Xen on KVM
>>>> so both runs Xen guests.
>>>
>>> Does boot_xen.py actually boot *Xen*? And presumably at least one Xen guest *within* Xen?
>>
>> I'll let Alex confirm, but yes, I expect Xen guest within Xen guest within TCG. So the tags "accel:tcg" (already present) and "guest:xen".
>>
>>> kvm_xen_guest.py boots a "Xen guest" under KVM directly without any real Xen being present. It's *emulating* Xen.
>>
>> Yes, so the tag "guest:xen" is correct.
>>
>>> They do both run Xen guests (or at least guests which use Xen hypercalls and *think* they're running under Xen). But is that the important classification for lumping them together?
>>
>> The idea of AVOCADO_TAGS is to restrict testing to what you want to cover. So here this allow running 'anything that can run Xen guest'
>> in a single command, for example it is handy on my macOS aarch64 host.
>
> Ok, that makes sense then. Thanks for your patience.
No problem, I'll add a better description in v3.
> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Thanks!
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH-for-9.0 v2 02/19] sysemu/xen: Forbid using Xen headers in user emulation
2023-11-14 14:37 [PATCH-for-9.0 v2 00/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
2023-11-14 14:37 ` [PATCH-for-9.0 v2 01/19] tests/avocado: Add 'guest:xen' tag to tests running Xen guest Philippe Mathieu-Daudé
@ 2023-11-14 14:37 ` Philippe Mathieu-Daudé
2023-11-14 14:37 ` [PATCH-for-9.0 v2 03/19] sysemu/xen-mapcache: Check Xen availability with CONFIG_XEN_IS_POSSIBLE Philippe Mathieu-Daudé
` (16 subsequent siblings)
18 siblings, 0 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 14:37 UTC (permalink / raw)
To: David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
David Woodhouse, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, Anthony Perard, kvm, Thomas Huth,
Philippe Mathieu-Daudé
Xen is a system specific accelerator, it makes no sense
to include its headers in user emulation.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
---
include/sysemu/xen.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/sysemu/xen.h b/include/sysemu/xen.h
index bc13ad5692..a9f591f26d 100644
--- a/include/sysemu/xen.h
+++ b/include/sysemu/xen.h
@@ -10,6 +10,10 @@
#ifndef SYSEMU_XEN_H
#define SYSEMU_XEN_H
+#ifdef CONFIG_USER_ONLY
+#error Cannot include sysemu/xen.h from user emulation
+#endif
+
#include "exec/cpu-common.h"
#ifdef NEED_CPU_H
@@ -26,16 +30,13 @@ extern bool xen_allowed;
#define xen_enabled() (xen_allowed)
-#ifndef CONFIG_USER_ONLY
void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length);
void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
struct MemoryRegion *mr, Error **errp);
-#endif
#else /* !CONFIG_XEN_IS_POSSIBLE */
#define xen_enabled() 0
-#ifndef CONFIG_USER_ONLY
static inline void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
{
/* nothing */
@@ -45,7 +46,6 @@ static inline void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
{
g_assert_not_reached();
}
-#endif
#endif /* CONFIG_XEN_IS_POSSIBLE */
--
2.41.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH-for-9.0 v2 03/19] sysemu/xen-mapcache: Check Xen availability with CONFIG_XEN_IS_POSSIBLE
2023-11-14 14:37 [PATCH-for-9.0 v2 00/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
2023-11-14 14:37 ` [PATCH-for-9.0 v2 01/19] tests/avocado: Add 'guest:xen' tag to tests running Xen guest Philippe Mathieu-Daudé
2023-11-14 14:37 ` [PATCH-for-9.0 v2 02/19] sysemu/xen: Forbid using Xen headers in user emulation Philippe Mathieu-Daudé
@ 2023-11-14 14:37 ` Philippe Mathieu-Daudé
2023-11-14 14:38 ` [PATCH-for-9.0 v2 04/19] system/physmem: Do not include 'hw/xen/xen.h' but 'sysemu/xen.h' Philippe Mathieu-Daudé
` (15 subsequent siblings)
18 siblings, 0 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 14:37 UTC (permalink / raw)
To: David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
David Woodhouse, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, Anthony Perard, kvm, Thomas Huth,
Philippe Mathieu-Daudé
"sysemu/xen.h" defines CONFIG_XEN_IS_POSSIBLE as a target-agnostic
version of CONFIG_XEN accelerator.
Use it in order to use "sysemu/xen-mapcache.h" in target-agnostic files.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
---
include/sysemu/xen-mapcache.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/sysemu/xen-mapcache.h b/include/sysemu/xen-mapcache.h
index c8e7c2f6cf..10c2e3082a 100644
--- a/include/sysemu/xen-mapcache.h
+++ b/include/sysemu/xen-mapcache.h
@@ -10,10 +10,11 @@
#define XEN_MAPCACHE_H
#include "exec/cpu-common.h"
+#include "sysemu/xen.h"
typedef hwaddr (*phys_offset_to_gaddr_t)(hwaddr phys_offset,
ram_addr_t size);
-#ifdef CONFIG_XEN
+#ifdef CONFIG_XEN_IS_POSSIBLE
void xen_map_cache_init(phys_offset_to_gaddr_t f,
void *opaque);
--
2.41.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH-for-9.0 v2 04/19] system/physmem: Do not include 'hw/xen/xen.h' but 'sysemu/xen.h'
2023-11-14 14:37 [PATCH-for-9.0 v2 00/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2023-11-14 14:37 ` [PATCH-for-9.0 v2 03/19] sysemu/xen-mapcache: Check Xen availability with CONFIG_XEN_IS_POSSIBLE Philippe Mathieu-Daudé
@ 2023-11-14 14:38 ` Philippe Mathieu-Daudé
2023-11-14 14:59 ` David Woodhouse
2023-11-14 15:14 ` David Hildenbrand
2023-11-14 14:38 ` [PATCH-for-9.0 v2 05/19] hw/display: Restrict xen_register_framebuffer() call to Xen Philippe Mathieu-Daudé
` (14 subsequent siblings)
18 siblings, 2 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 14:38 UTC (permalink / raw)
To: David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
David Woodhouse, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, Anthony Perard, kvm, Thomas Huth,
Philippe Mathieu-Daudé,
Peter Xu, David Hildenbrand
physmem.c doesn't use any declaration from "hw/xen/xen.h",
it only requires "sysemu/xen.h" and "system/xen-mapcache.h".
Suggested-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
system/physmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/system/physmem.c b/system/physmem.c
index fc2b0fee01..04630711d2 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -35,7 +35,7 @@
#include "hw/qdev-core.h"
#include "hw/qdev-properties.h"
#include "hw/boards.h"
-#include "hw/xen/xen.h"
+#include "sysemu/xen.h"
#include "sysemu/kvm.h"
#include "sysemu/tcg.h"
#include "sysemu/qtest.h"
--
2.41.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 04/19] system/physmem: Do not include 'hw/xen/xen.h' but 'sysemu/xen.h'
2023-11-14 14:38 ` [PATCH-for-9.0 v2 04/19] system/physmem: Do not include 'hw/xen/xen.h' but 'sysemu/xen.h' Philippe Mathieu-Daudé
@ 2023-11-14 14:59 ` David Woodhouse
2023-11-14 15:14 ` David Hildenbrand
1 sibling, 0 replies; 50+ messages in thread
From: David Woodhouse @ 2023-11-14 14:59 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
Stefano Stabellini, Richard Henderson, xen-devel, qemu-block,
Anthony Perard, kvm, Thomas Huth, Peter Xu, David Hildenbrand
On 14 November 2023 09:38:00 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>physmem.c doesn't use any declaration from "hw/xen/xen.h",
>it only requires "sysemu/xen.h" and "system/xen-mapcache.h".
>
>Suggested-by: David Woodhouse <dwmw@amazon.co.uk>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 04/19] system/physmem: Do not include 'hw/xen/xen.h' but 'sysemu/xen.h'
2023-11-14 14:38 ` [PATCH-for-9.0 v2 04/19] system/physmem: Do not include 'hw/xen/xen.h' but 'sysemu/xen.h' Philippe Mathieu-Daudé
2023-11-14 14:59 ` David Woodhouse
@ 2023-11-14 15:14 ` David Hildenbrand
1 sibling, 0 replies; 50+ messages in thread
From: David Hildenbrand @ 2023-11-14 15:14 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
David Woodhouse, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, Anthony Perard, kvm, Thomas Huth,
Peter Xu
On 14.11.23 15:38, Philippe Mathieu-Daudé wrote:
> physmem.c doesn't use any declaration from "hw/xen/xen.h",
> it only requires "sysemu/xen.h" and "system/xen-mapcache.h".
>
> Suggested-by: David Woodhouse <dwmw@amazon.co.uk>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH-for-9.0 v2 05/19] hw/display: Restrict xen_register_framebuffer() call to Xen
2023-11-14 14:37 [PATCH-for-9.0 v2 00/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2023-11-14 14:38 ` [PATCH-for-9.0 v2 04/19] system/physmem: Do not include 'hw/xen/xen.h' but 'sysemu/xen.h' Philippe Mathieu-Daudé
@ 2023-11-14 14:38 ` Philippe Mathieu-Daudé
2024-03-27 11:50 ` Anthony PERARD
2023-11-14 14:38 ` [PATCH-for-9.0 v2 06/19] hw/pci/msi: Restrict xen_is_pirq_msi() " Philippe Mathieu-Daudé
` (13 subsequent siblings)
18 siblings, 1 reply; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 14:38 UTC (permalink / raw)
To: David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
David Woodhouse, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, Anthony Perard, kvm, Thomas Huth,
Philippe Mathieu-Daudé,
Gerd Hoffmann
Only call xen_register_framebuffer() when Xen is enabled.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/display/vga.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 37557c3442..f9cf3d6f77 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -25,6 +25,7 @@
#include "qemu/osdep.h"
#include "qemu/units.h"
#include "sysemu/reset.h"
+#include "sysemu/xen.h"
#include "qapi/error.h"
#include "hw/core/cpu.h"
#include "hw/display/vga.h"
@@ -2223,7 +2224,9 @@ bool vga_common_init(VGACommonState *s, Object *obj, Error **errp)
return false;
}
vmstate_register_ram(&s->vram, s->global_vmstate ? NULL : DEVICE(obj));
- xen_register_framebuffer(&s->vram);
+ if (xen_enabled()) {
+ xen_register_framebuffer(&s->vram);
+ }
s->vram_ptr = memory_region_get_ram_ptr(&s->vram);
s->get_bpp = vga_get_bpp;
s->get_offsets = vga_get_offsets;
--
2.41.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 05/19] hw/display: Restrict xen_register_framebuffer() call to Xen
2023-11-14 14:38 ` [PATCH-for-9.0 v2 05/19] hw/display: Restrict xen_register_framebuffer() call to Xen Philippe Mathieu-Daudé
@ 2024-03-27 11:50 ` Anthony PERARD
0 siblings, 0 replies; 50+ messages in thread
From: Anthony PERARD @ 2024-03-27 11:50 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: David Woodhouse, qemu-devel, Alex Bennée, Paul Durrant,
qemu-arm, Paolo Bonzini, David Woodhouse, Stefano Stabellini,
Richard Henderson, xen-devel, qemu-block, kvm, Thomas Huth,
Gerd Hoffmann
On Tue, Nov 14, 2023 at 03:38:01PM +0100, Philippe Mathieu-Daudé wrote:
> Only call xen_register_framebuffer() when Xen is enabled.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
I don't think this patch is very useful but it's fine, so:
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH-for-9.0 v2 06/19] hw/pci/msi: Restrict xen_is_pirq_msi() call to Xen
2023-11-14 14:37 [PATCH-for-9.0 v2 00/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2023-11-14 14:38 ` [PATCH-for-9.0 v2 05/19] hw/display: Restrict xen_register_framebuffer() call to Xen Philippe Mathieu-Daudé
@ 2023-11-14 14:38 ` Philippe Mathieu-Daudé
2023-11-14 15:13 ` David Woodhouse
2023-11-14 14:38 ` [PATCH-for-9.0 v2 07/19] hw/xen: Remove unnecessary xen_hvm_inject_msi() stub Philippe Mathieu-Daudé
` (12 subsequent siblings)
18 siblings, 1 reply; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 14:38 UTC (permalink / raw)
To: David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
David Woodhouse, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, Anthony Perard, kvm, Thomas Huth,
Philippe Mathieu-Daudé,
Michael S. Tsirkin, Marcel Apfelbaum
Similarly to the restriction in hw/pci/msix.c (see commit
e1e4bf2252 "msix: fix msix_vector_masked"), restrict the
xen_is_pirq_msi() call in msi_is_masked() to Xen.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/pci/msi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/pci/msi.c b/hw/pci/msi.c
index 041b0bdbec..8104ac1d91 100644
--- a/hw/pci/msi.c
+++ b/hw/pci/msi.c
@@ -23,6 +23,7 @@
#include "hw/xen/xen.h"
#include "qemu/range.h"
#include "qapi/error.h"
+#include "sysemu/xen.h"
#include "hw/i386/kvm/xen_evtchn.h"
@@ -308,7 +309,7 @@ bool msi_is_masked(const PCIDevice *dev, unsigned int vector)
}
data = pci_get_word(dev->config + msi_data_off(dev, msi64bit));
- if (xen_is_pirq_msi(data)) {
+ if (xen_enabled() && xen_is_pirq_msi(data)) {
return false;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 06/19] hw/pci/msi: Restrict xen_is_pirq_msi() call to Xen
2023-11-14 14:38 ` [PATCH-for-9.0 v2 06/19] hw/pci/msi: Restrict xen_is_pirq_msi() " Philippe Mathieu-Daudé
@ 2023-11-14 15:13 ` David Woodhouse
2023-11-14 15:22 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 50+ messages in thread
From: David Woodhouse @ 2023-11-14 15:13 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
Stefano Stabellini, Richard Henderson, xen-devel, qemu-block,
Anthony Perard, kvm, Thomas Huth, Michael S. Tsirkin,
Marcel Apfelbaum
On 14 November 2023 09:38:02 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>Similarly to the restriction in hw/pci/msix.c (see commit
>e1e4bf2252 "msix: fix msix_vector_masked"), restrict the
>xen_is_pirq_msi() call in msi_is_masked() to Xen.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Hm, we do also support the Xen abomination of snooping on MSI table writes to see if they're targeted at a Xen PIRQ, then actually unmasking the MSI from QEMU when the guest binds the corresponding event channel to that PIRQ.
I think this is going to break in CI as kvm_xen_guest.py does deliberately exercise that use case, doesn't it?
I deliberately *didn't* switch to testing the Xen PV net device, with a comment that testing MSI and irqchip permutations was far more entertaining. So I hope it should catch this?
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 06/19] hw/pci/msi: Restrict xen_is_pirq_msi() call to Xen
2023-11-14 15:13 ` David Woodhouse
@ 2023-11-14 15:22 ` Philippe Mathieu-Daudé
2023-11-14 15:44 ` David Woodhouse
2023-11-14 16:17 ` David Woodhouse
0 siblings, 2 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 15:22 UTC (permalink / raw)
To: David Woodhouse, David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
Stefano Stabellini, Richard Henderson, xen-devel, qemu-block,
Anthony Perard, kvm, Thomas Huth, Michael S. Tsirkin,
Marcel Apfelbaum
On 14/11/23 16:13, David Woodhouse wrote:
> On 14 November 2023 09:38:02 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>> Similarly to the restriction in hw/pci/msix.c (see commit
>> e1e4bf2252 "msix: fix msix_vector_masked"), restrict the
>> xen_is_pirq_msi() call in msi_is_masked() to Xen.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> Hm, we do also support the Xen abomination of snooping on MSI table writes to see if they're targeted at a Xen PIRQ, then actually unmasking the MSI from QEMU when the guest binds the corresponding event channel to that PIRQ.
>
> I think this is going to break in CI as kvm_xen_guest.py does deliberately exercise that use case, doesn't it?
Hmmm I see what you mean.
So you mentioned these checks:
- host Xen accel
- Xen accel emulated to guest via KVM host accel
Maybe we need here:
- guest expected to run Xen
Being (
Xen accel emulated to guest via KVM host accel
OR
host Xen accel
)
If so, possibly few places incorrectly check 'xen_enabled()'
instead of this 'xen_guest()'.
"Xen on KVM" is a tricky case...
> I deliberately *didn't* switch to testing the Xen PV net device, with a comment that testing MSI and irqchip permutations was far more entertaining. So I hope it should catch this?
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 06/19] hw/pci/msi: Restrict xen_is_pirq_msi() call to Xen
2023-11-14 15:22 ` Philippe Mathieu-Daudé
@ 2023-11-14 15:44 ` David Woodhouse
2023-11-14 21:18 ` David Woodhouse
2023-11-14 16:17 ` David Woodhouse
1 sibling, 1 reply; 50+ messages in thread
From: David Woodhouse @ 2023-11-14 15:44 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
Stefano Stabellini, Richard Henderson, xen-devel, qemu-block,
Anthony Perard, kvm, Thomas Huth, Michael S. Tsirkin,
Marcel Apfelbaum
On 14 November 2023 10:22:23 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>On 14/11/23 16:13, David Woodhouse wrote:
>> On 14 November 2023 09:38:02 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>>> Similarly to the restriction in hw/pci/msix.c (see commit
>>> e1e4bf2252 "msix: fix msix_vector_masked"), restrict the
>>> xen_is_pirq_msi() call in msi_is_masked() to Xen.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>> Hm, we do also support the Xen abomination of snooping on MSI table writes to see if they're targeted at a Xen PIRQ, then actually unmasking the MSI from QEMU when the guest binds the corresponding event channel to that PIRQ.
>>
>> I think this is going to break in CI as kvm_xen_guest.py does deliberately exercise that use case, doesn't it?
>
>Hmmm I see what you mean.
>
>So you mentioned these checks:
>
>- host Xen accel
>- Xen accel emulated to guest via KVM host accel
>
>Maybe we need here:
>
>- guest expected to run Xen
>
> Being (
> Xen accel emulated to guest via KVM host accel
> OR
> host Xen accel
> )
>
>If so, possibly few places incorrectly check 'xen_enabled()'
>instead of this 'xen_guest()'.
I think xen_is_pirq_msi() had that test built in, didn't it? Adding a 'xen_enabled() &&' prefix was technically redundant?
What's the actual problem we're trying to solve here? That we had two separate implementations of xen_is_pirq_msi() (three if you count an empty stub?) which are resolved at link time and prevent you from running Xen-accel and KVM-accel VMs within the same QEMU process?
>"Xen on KVM" is a tricky case...
>
>> I deliberately *didn't* switch to testing the Xen PV net device, with a comment that testing MSI and irqchip permutations was far more entertaining. So I hope it should catch this?
>
>¯\_(ツ)_/¯
I believe that if you push your branch to a gitlab tree with the right CI variables defined, it'll run all the CI? And I *hope* it fails with this patch. It's precisely the kind of thing I was *intending* to catch with the testing!
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 06/19] hw/pci/msi: Restrict xen_is_pirq_msi() call to Xen
2023-11-14 15:44 ` David Woodhouse
@ 2023-11-14 21:18 ` David Woodhouse
0 siblings, 0 replies; 50+ messages in thread
From: David Woodhouse @ 2023-11-14 21:18 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
Stefano Stabellini, Richard Henderson, xen-devel, qemu-block,
Anthony Perard, kvm, Thomas Huth, Michael S. Tsirkin,
Marcel Apfelbaum
[-- Attachment #1: Type: text/plain, Size: 1353 bytes --]
On Tue, 2023-11-14 at 10:44 -0500, David Woodhouse wrote:
>
> I believe that if you push your branch to a gitlab tree with the
> right CI variables defined, it'll run all the CI? And I *hope* it
> fails with this patch. It's precisely the kind of thing I was
> *intending* to catch with the testing!
So it doesn't fail. Because virtio-net-pci doesn't use MSI, we need to
test with E1000E or something like that. And actually I'm not 100%
convinced the avocado tests are even running in the gitlab CI.
And also, if I test it manually... it doesn't fail because you didn't
break it :)
You didn't break the xen_evtchn_snoop_msi() call; that still works.
What you were changing is the part which makes msi_is_masked() lie and
unconditionally return false when it's mapped to a Xen PIRQ. And
actually, even in the Xen-emulation case, xen_is_pirq_msi() is always
returning zero. So your patch isn't changing anything.
It *does*, however, make hw/pci/msi.c do this exactly the same way as
hw/pci/msix.c does. That one does check xen_enabled() first.
I need to double-check whether the msi{x,}_is_masked() checks ought to
be doing the same for Xen-emu as they do on real Xen. That whole thing
is a complete abomination. But that isn't your problem. For your patch,
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 06/19] hw/pci/msi: Restrict xen_is_pirq_msi() call to Xen
2023-11-14 15:22 ` Philippe Mathieu-Daudé
2023-11-14 15:44 ` David Woodhouse
@ 2023-11-14 16:17 ` David Woodhouse
1 sibling, 0 replies; 50+ messages in thread
From: David Woodhouse @ 2023-11-14 16:17 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
Stefano Stabellini, Richard Henderson, xen-devel, qemu-block,
Anthony Perard, kvm, Thomas Huth, Michael S. Tsirkin,
Marcel Apfelbaum
[-- Attachment #1: Type: text/plain, Size: 699 bytes --]
On Tue, 2023-11-14 at 16:22 +0100, Philippe Mathieu-Daudé wrote:
>
> If so, possibly few places incorrectly check 'xen_enabled()'
> instead of this 'xen_guest()'.
Sorry, I meant to respond to that one directly. I don't *think* there
are any cases of that. As I added the CONFIG_XEN_EMU support, I moved a
bunch of stuff to live under CONFIG_XEN_BUS instead of CONFIG_XEN,
fixing them up (and implementing the emulated versions of grant table
operations, event channel operations etc. which no longer come from the
actual Xen libraries).
The existing cases of xen_enabled() really *are* being used to mean
'xen_accel_enabled()', AFAIK. Apart from that one you just tried to add
:)
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH-for-9.0 v2 07/19] hw/xen: Remove unnecessary xen_hvm_inject_msi() stub
2023-11-14 14:37 [PATCH-for-9.0 v2 00/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
` (5 preceding siblings ...)
2023-11-14 14:38 ` [PATCH-for-9.0 v2 06/19] hw/pci/msi: Restrict xen_is_pirq_msi() " Philippe Mathieu-Daudé
@ 2023-11-14 14:38 ` Philippe Mathieu-Daudé
2023-11-14 15:27 ` David Woodhouse
2023-11-14 14:38 ` [PATCH-for-9.0 v2 08/19] hw/xen: Remove unused Xen stubs Philippe Mathieu-Daudé
` (11 subsequent siblings)
18 siblings, 1 reply; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 14:38 UTC (permalink / raw)
To: David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
David Woodhouse, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, Anthony Perard, kvm, Thomas Huth,
Philippe Mathieu-Daudé
Since commit 04b0de0ee8 ("xen: factor out common functions")
xen_hvm_inject_msi() stub is not required.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
stubs/xen-hw-stub.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/stubs/xen-hw-stub.c b/stubs/xen-hw-stub.c
index 7d7ffe83a9..6cf0e9a4c1 100644
--- a/stubs/xen-hw-stub.c
+++ b/stubs/xen-hw-stub.c
@@ -24,10 +24,6 @@ int xen_set_pci_link_route(uint8_t link, uint8_t irq)
return -1;
}
-void xen_hvm_inject_msi(uint64_t addr, uint32_t data)
-{
-}
-
int xen_is_pirq_msi(uint32_t msi_data)
{
return 0;
--
2.41.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 07/19] hw/xen: Remove unnecessary xen_hvm_inject_msi() stub
2023-11-14 14:38 ` [PATCH-for-9.0 v2 07/19] hw/xen: Remove unnecessary xen_hvm_inject_msi() stub Philippe Mathieu-Daudé
@ 2023-11-14 15:27 ` David Woodhouse
0 siblings, 0 replies; 50+ messages in thread
From: David Woodhouse @ 2023-11-14 15:27 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
Stefano Stabellini, Richard Henderson, xen-devel, qemu-block,
Anthony Perard, kvm, Thomas Huth
On 14 November 2023 09:38:03 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>Since commit 04b0de0ee8 ("xen: factor out common functions")
>xen_hvm_inject_msi() stub is not required.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH-for-9.0 v2 08/19] hw/xen: Remove unused Xen stubs
2023-11-14 14:37 [PATCH-for-9.0 v2 00/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
` (6 preceding siblings ...)
2023-11-14 14:38 ` [PATCH-for-9.0 v2 07/19] hw/xen: Remove unnecessary xen_hvm_inject_msi() stub Philippe Mathieu-Daudé
@ 2023-11-14 14:38 ` Philippe Mathieu-Daudé
2024-03-27 11:54 ` Anthony PERARD
2023-11-14 14:38 ` [RFC PATCH-for-9.0 v2 09/19] hw/block/xen_blkif: Align structs with QEMU_ALIGNED() instead of #pragma Philippe Mathieu-Daudé
` (10 subsequent siblings)
18 siblings, 1 reply; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 14:38 UTC (permalink / raw)
To: David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
David Woodhouse, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, Anthony Perard, kvm, Thomas Huth,
Philippe Mathieu-Daudé
All these stubs are protected by a 'if (xen_enabled())' check.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
stubs/xen-hw-stub.c | 24 ------------------------
1 file changed, 24 deletions(-)
diff --git a/stubs/xen-hw-stub.c b/stubs/xen-hw-stub.c
index 6cf0e9a4c1..53c6a6f2a0 100644
--- a/stubs/xen-hw-stub.c
+++ b/stubs/xen-hw-stub.c
@@ -8,36 +8,12 @@
#include "qemu/osdep.h"
#include "hw/xen/xen.h"
-#include "hw/xen/xen-x86.h"
-
-int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
-{
- return -1;
-}
-
-void xen_intx_set_irq(void *opaque, int irq_num, int level)
-{
-}
-
-int xen_set_pci_link_route(uint8_t link, uint8_t irq)
-{
- return -1;
-}
int xen_is_pirq_msi(uint32_t msi_data)
{
return 0;
}
-qemu_irq *xen_interrupt_controller_init(void)
-{
- return NULL;
-}
-
void xen_register_framebuffer(MemoryRegion *mr)
{
}
-
-void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion **ram_memory)
-{
-}
--
2.41.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 08/19] hw/xen: Remove unused Xen stubs
2023-11-14 14:38 ` [PATCH-for-9.0 v2 08/19] hw/xen: Remove unused Xen stubs Philippe Mathieu-Daudé
@ 2024-03-27 11:54 ` Anthony PERARD
0 siblings, 0 replies; 50+ messages in thread
From: Anthony PERARD @ 2024-03-27 11:54 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: David Woodhouse, qemu-devel, Alex Bennée, Paul Durrant,
qemu-arm, Paolo Bonzini, David Woodhouse, Stefano Stabellini,
Richard Henderson, xen-devel, qemu-block, kvm, Thomas Huth
On Tue, Nov 14, 2023 at 03:38:04PM +0100, Philippe Mathieu-Daudé wrote:
> All these stubs are protected by a 'if (xen_enabled())' check.
Are you sure? There's still nothing that prevent a compiler from wanting
those, I don't think.
Sure, often compilers will remove dead code in `if(0){...}`, but there's
no guaranty, is there?
Cheers,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 50+ messages in thread
* [RFC PATCH-for-9.0 v2 09/19] hw/block/xen_blkif: Align structs with QEMU_ALIGNED() instead of #pragma
2023-11-14 14:37 [PATCH-for-9.0 v2 00/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
` (7 preceding siblings ...)
2023-11-14 14:38 ` [PATCH-for-9.0 v2 08/19] hw/xen: Remove unused Xen stubs Philippe Mathieu-Daudé
@ 2023-11-14 14:38 ` Philippe Mathieu-Daudé
2023-11-14 15:30 ` David Woodhouse
2024-03-27 13:31 ` Anthony PERARD
2023-11-14 14:38 ` [PATCH-for-9.0 v2 10/19] hw/xen: Rename 'ram_memory' global variable as 'xen_memory' Philippe Mathieu-Daudé
` (9 subsequent siblings)
18 siblings, 2 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 14:38 UTC (permalink / raw)
To: David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
David Woodhouse, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, Anthony Perard, kvm, Thomas Huth,
Philippe Mathieu-Daudé,
Kevin Wolf, Hanna Reitz
Except imported source files, QEMU code base uses
the QEMU_ALIGNED() macro to align its structures.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/block/xen_blkif.h | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/hw/block/xen_blkif.h b/hw/block/xen_blkif.h
index 99733529c1..c1d154d502 100644
--- a/hw/block/xen_blkif.h
+++ b/hw/block/xen_blkif.h
@@ -18,7 +18,6 @@ struct blkif_common_response {
};
/* i386 protocol version */
-#pragma pack(push, 4)
struct blkif_x86_32_request {
uint8_t operation; /* BLKIF_OP_??? */
uint8_t nr_segments; /* number of segments */
@@ -26,7 +25,7 @@ struct blkif_x86_32_request {
uint64_t id; /* private guest value, echoed in resp */
blkif_sector_t sector_number; /* start sector idx on disk (r/w only) */
struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
-};
+} QEMU_ALIGNED(4);
struct blkif_x86_32_request_discard {
uint8_t operation; /* BLKIF_OP_DISCARD */
uint8_t flag; /* nr_segments in request struct */
@@ -34,15 +33,14 @@ struct blkif_x86_32_request_discard {
uint64_t id; /* private guest value, echoed in resp */
blkif_sector_t sector_number; /* start sector idx on disk (r/w only) */
uint64_t nr_sectors; /* # of contiguous sectors to discard */
-};
+} QEMU_ALIGNED(4);
struct blkif_x86_32_response {
uint64_t id; /* copied from request */
uint8_t operation; /* copied from request */
int16_t status; /* BLKIF_RSP_??? */
-};
+} QEMU_ALIGNED(4);
typedef struct blkif_x86_32_request blkif_x86_32_request_t;
typedef struct blkif_x86_32_response blkif_x86_32_response_t;
-#pragma pack(pop)
/* x86_64 protocol version */
struct blkif_x86_64_request {
--
2.41.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [RFC PATCH-for-9.0 v2 09/19] hw/block/xen_blkif: Align structs with QEMU_ALIGNED() instead of #pragma
2023-11-14 14:38 ` [RFC PATCH-for-9.0 v2 09/19] hw/block/xen_blkif: Align structs with QEMU_ALIGNED() instead of #pragma Philippe Mathieu-Daudé
@ 2023-11-14 15:30 ` David Woodhouse
2024-03-27 13:31 ` Anthony PERARD
1 sibling, 0 replies; 50+ messages in thread
From: David Woodhouse @ 2023-11-14 15:30 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
Stefano Stabellini, Richard Henderson, xen-devel, qemu-block,
Anthony Perard, kvm, Thomas Huth, Kevin Wolf, Hanna Reitz
On 14 November 2023 09:38:05 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>Except imported source files, QEMU code base uses
>the QEMU_ALIGNED() macro to align its structures.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Can we have a BUILD_BUG_ON(sizeof==) for these please?
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH-for-9.0 v2 09/19] hw/block/xen_blkif: Align structs with QEMU_ALIGNED() instead of #pragma
2023-11-14 14:38 ` [RFC PATCH-for-9.0 v2 09/19] hw/block/xen_blkif: Align structs with QEMU_ALIGNED() instead of #pragma Philippe Mathieu-Daudé
2023-11-14 15:30 ` David Woodhouse
@ 2024-03-27 13:31 ` Anthony PERARD
2024-03-27 13:34 ` David Woodhouse
1 sibling, 1 reply; 50+ messages in thread
From: Anthony PERARD @ 2024-03-27 13:31 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: David Woodhouse, qemu-devel, Alex Bennée, Paul Durrant,
qemu-arm, Paolo Bonzini, David Woodhouse, Stefano Stabellini,
Richard Henderson, xen-devel, qemu-block, kvm, Thomas Huth,
Kevin Wolf, Hanna Reitz
On Tue, Nov 14, 2023 at 03:38:05PM +0100, Philippe Mathieu-Daudé wrote:
> Except imported source files, QEMU code base uses
> the QEMU_ALIGNED() macro to align its structures.
This patch only convert the alignment, but discard pack. We need both or
the struct is changed.
> ---
> hw/block/xen_blkif.h | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/hw/block/xen_blkif.h b/hw/block/xen_blkif.h
> index 99733529c1..c1d154d502 100644
> --- a/hw/block/xen_blkif.h
> +++ b/hw/block/xen_blkif.h
> @@ -18,7 +18,6 @@ struct blkif_common_response {
> };
>
> /* i386 protocol version */
> -#pragma pack(push, 4)
> struct blkif_x86_32_request {
> uint8_t operation; /* BLKIF_OP_??? */
> uint8_t nr_segments; /* number of segments */
> @@ -26,7 +25,7 @@ struct blkif_x86_32_request {
> uint64_t id; /* private guest value, echoed in resp */
> blkif_sector_t sector_number; /* start sector idx on disk (r/w only) */
> struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
> -};
> +} QEMU_ALIGNED(4);
E.g. for this one, I've compare the output of
`pahole --class_name=blkif_x86_32_request build/qemu-system-i386`:
--- before
+++ after
@@ -1,11 +1,15 @@
struct blkif_x86_32_request {
uint8_t operation; /* 0 1 */
uint8_t nr_segments; /* 1 1 */
uint16_t handle; /* 2 2 */
- uint64_t id; /* 4 8 */
- uint64_t sector_number; /* 12 8 */
- struct blkif_request_segment seg[11]; /* 20 88 */
- /* size: 108, cachelines: 2, members: 6 */
- /* last cacheline: 44 bytes */
-} __attribute__((__packed__));
+ /* XXX 4 bytes hole, try to pack */
+
+ uint64_t id; /* 8 8 */
+ uint64_t sector_number; /* 16 8 */
+ struct blkif_request_segment seg[11]; /* 24 88 */
+
+ /* size: 112, cachelines: 2, members: 6 */
+ /* sum members: 108, holes: 1, sum holes: 4 */
+ /* last cacheline: 48 bytes */
+} __attribute__((__aligned__(8)));
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH-for-9.0 v2 09/19] hw/block/xen_blkif: Align structs with QEMU_ALIGNED() instead of #pragma
2024-03-27 13:31 ` Anthony PERARD
@ 2024-03-27 13:34 ` David Woodhouse
0 siblings, 0 replies; 50+ messages in thread
From: David Woodhouse @ 2024-03-27 13:34 UTC (permalink / raw)
To: Anthony PERARD, Philippe Mathieu-Daudé
Cc: David Woodhouse, qemu-devel, Alex Bennée, Paul Durrant,
qemu-arm, Paolo Bonzini, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, kvm, Thomas Huth, Kevin Wolf, Hanna Reitz
On 27 March 2024 13:31:52 GMT, Anthony PERARD <anthony.perard@cloud.com> wrote:
>On Tue, Nov 14, 2023 at 03:38:05PM +0100, Philippe Mathieu-Daudé wrote:
>> Except imported source files, QEMU code base uses
>> the QEMU_ALIGNED() macro to align its structures.
>
>This patch only convert the alignment, but discard pack. We need both or
>the struct is changed.
Which means we need some build-time asserts on struct size and field offsets. That should never have passed a build test.
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH-for-9.0 v2 10/19] hw/xen: Rename 'ram_memory' global variable as 'xen_memory'
2023-11-14 14:37 [PATCH-for-9.0 v2 00/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
` (8 preceding siblings ...)
2023-11-14 14:38 ` [RFC PATCH-for-9.0 v2 09/19] hw/block/xen_blkif: Align structs with QEMU_ALIGNED() instead of #pragma Philippe Mathieu-Daudé
@ 2023-11-14 14:38 ` Philippe Mathieu-Daudé
2023-11-14 15:49 ` David Woodhouse
2023-11-14 14:38 ` [PATCH-for-9.0 v2 11/19] hw/xen/xen_arch_hvm: Rename prototypes using 'xen_arch_' prefix Philippe Mathieu-Daudé
` (8 subsequent siblings)
18 siblings, 1 reply; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 14:38 UTC (permalink / raw)
To: David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
David Woodhouse, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, Anthony Perard, kvm, Thomas Huth,
Philippe Mathieu-Daudé,
Peter Maydell, Eduardo Habkost, Michael S. Tsirkin,
Marcel Apfelbaum
To avoid a potential global variable shadow in
hw/i386/pc_piix.c::pc_init1(), rename Xen's
"ram_memory" as "xen_memory".
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
include/hw/xen/xen-hvm-common.h | 2 +-
hw/arm/xen_arm.c | 6 +++---
hw/i386/xen/xen-hvm.c | 10 +++++-----
hw/xen/xen-hvm-common.c | 6 +++---
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/include/hw/xen/xen-hvm-common.h b/include/hw/xen/xen-hvm-common.h
index 4e9904f1a6..d3fa5ed29b 100644
--- a/include/hw/xen/xen-hvm-common.h
+++ b/include/hw/xen/xen-hvm-common.h
@@ -16,7 +16,7 @@
#include "qemu/error-report.h"
#include <xen/hvm/ioreq.h>
-extern MemoryRegion ram_memory;
+extern MemoryRegion xen_memory;
extern MemoryListener xen_io_listener;
extern DeviceListener xen_device_listener;
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index a5631529d0..8a185da193 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -111,17 +111,17 @@ static void xen_init_ram(MachineState *machine)
block_len = GUEST_RAM1_BASE + ram_size[1];
}
- memory_region_init_ram(&ram_memory, NULL, "xen.ram", block_len,
+ memory_region_init_ram(&xen_memory, NULL, "xen.ram", block_len,
&error_fatal);
- memory_region_init_alias(&ram_lo, NULL, "xen.ram.lo", &ram_memory,
+ memory_region_init_alias(&ram_lo, NULL, "xen.ram.lo", &xen_memory,
GUEST_RAM0_BASE, ram_size[0]);
memory_region_add_subregion(sysmem, GUEST_RAM0_BASE, &ram_lo);
DPRINTF("Initialized region xen.ram.lo: base 0x%llx size 0x%lx\n",
GUEST_RAM0_BASE, ram_size[0]);
if (ram_size[1] > 0) {
- memory_region_init_alias(&ram_hi, NULL, "xen.ram.hi", &ram_memory,
+ memory_region_init_alias(&ram_hi, NULL, "xen.ram.hi", &xen_memory,
GUEST_RAM1_BASE, ram_size[1]);
memory_region_add_subregion(sysmem, GUEST_RAM1_BASE, &ram_hi);
DPRINTF("Initialized region xen.ram.hi: base 0x%llx size 0x%lx\n",
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index f42621e674..1ae943370b 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -149,12 +149,12 @@ static void xen_ram_init(PCMachineState *pcms,
*/
block_len = (4 * GiB) + x86ms->above_4g_mem_size;
}
- memory_region_init_ram(&ram_memory, NULL, "xen.ram", block_len,
+ memory_region_init_ram(&xen_memory, NULL, "xen.ram", block_len,
&error_fatal);
- *ram_memory_p = &ram_memory;
+ *ram_memory_p = &xen_memory;
memory_region_init_alias(&ram_640k, NULL, "xen.ram.640k",
- &ram_memory, 0, 0xa0000);
+ &xen_memory, 0, 0xa0000);
memory_region_add_subregion(sysmem, 0, &ram_640k);
/* Skip of the VGA IO memory space, it will be registered later by the VGA
* emulated device.
@@ -163,12 +163,12 @@ static void xen_ram_init(PCMachineState *pcms,
* the Options ROM, so it is registered here as RAM.
*/
memory_region_init_alias(&ram_lo, NULL, "xen.ram.lo",
- &ram_memory, 0xc0000,
+ &xen_memory, 0xc0000,
x86ms->below_4g_mem_size - 0xc0000);
memory_region_add_subregion(sysmem, 0xc0000, &ram_lo);
if (x86ms->above_4g_mem_size > 0) {
memory_region_init_alias(&ram_hi, NULL, "xen.ram.hi",
- &ram_memory, 0x100000000ULL,
+ &xen_memory, 0x100000000ULL,
x86ms->above_4g_mem_size);
memory_region_add_subregion(sysmem, 0x100000000ULL, &ram_hi);
}
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index 565dc39c8f..cf4053c9f2 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -9,7 +9,7 @@
#include "hw/boards.h"
#include "hw/xen/arch_hvm.h"
-MemoryRegion ram_memory;
+MemoryRegion xen_memory;
void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr,
Error **errp)
@@ -26,7 +26,7 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr,
return;
}
- if (mr == &ram_memory) {
+ if (mr == &xen_memory) {
return;
}
@@ -53,7 +53,7 @@ static void xen_set_memory(struct MemoryListener *listener,
{
XenIOState *state = container_of(listener, XenIOState, memory_listener);
- if (section->mr == &ram_memory) {
+ if (section->mr == &xen_memory) {
return;
} else {
if (add) {
--
2.41.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 10/19] hw/xen: Rename 'ram_memory' global variable as 'xen_memory'
2023-11-14 14:38 ` [PATCH-for-9.0 v2 10/19] hw/xen: Rename 'ram_memory' global variable as 'xen_memory' Philippe Mathieu-Daudé
@ 2023-11-14 15:49 ` David Woodhouse
2024-03-06 17:14 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 50+ messages in thread
From: David Woodhouse @ 2023-11-14 15:49 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
Stefano Stabellini, Richard Henderson, xen-devel, qemu-block,
Anthony Perard, kvm, Thomas Huth, Peter Maydell, Eduardo Habkost,
Michael S. Tsirkin, Marcel Apfelbaum
On 14 November 2023 09:38:06 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>To avoid a potential global variable shadow in
>hw/i386/pc_piix.c::pc_init1(), rename Xen's
>"ram_memory" as "xen_memory".
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Well OK, but aren't you going to be coming back later to eliminate global variables which are actually per-VM?
Or is that the point, because *then* the conflicting name will actually matter?
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 10/19] hw/xen: Rename 'ram_memory' global variable as 'xen_memory'
2023-11-14 15:49 ` David Woodhouse
@ 2024-03-06 17:14 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-03-06 17:14 UTC (permalink / raw)
To: David Woodhouse, David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
Stefano Stabellini, Richard Henderson, xen-devel, qemu-block,
Anthony Perard, kvm, Thomas Huth, Peter Maydell, Eduardo Habkost,
Michael S. Tsirkin, Marcel Apfelbaum
On 14/11/23 16:49, David Woodhouse wrote:
> On 14 November 2023 09:38:06 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>> To avoid a potential global variable shadow in
>> hw/i386/pc_piix.c::pc_init1(), rename Xen's
>> "ram_memory" as "xen_memory".
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> Well OK, but aren't you going to be coming back later to eliminate global variables which are actually per-VM?
>
> Or is that the point, because *then* the conflicting name will actually matter?
Eh I wasn't thinking about running 2 Xen VMs in the same QEMU process,
but this is a good test case. I was thinking of running a Xen guest VM
and another TCG VM in the same process.
So IIUC xen_memory should be part of PCMachineState.
>
> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Thanks!
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH-for-9.0 v2 11/19] hw/xen/xen_arch_hvm: Rename prototypes using 'xen_arch_' prefix
2023-11-14 14:37 [PATCH-for-9.0 v2 00/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
` (9 preceding siblings ...)
2023-11-14 14:38 ` [PATCH-for-9.0 v2 10/19] hw/xen: Rename 'ram_memory' global variable as 'xen_memory' Philippe Mathieu-Daudé
@ 2023-11-14 14:38 ` Philippe Mathieu-Daudé
2024-03-27 13:37 ` Anthony PERARD
2023-11-14 14:38 ` [PATCH-for-9.0 v2 12/19] hw/xen: Merge 'hw/xen/arch_hvm.h' in 'hw/xen/xen-hvm-common.h' Philippe Mathieu-Daudé
` (7 subsequent siblings)
18 siblings, 1 reply; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 14:38 UTC (permalink / raw)
To: David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
David Woodhouse, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, Anthony Perard, kvm, Thomas Huth,
Philippe Mathieu-Daudé,
Peter Maydell, Michael S. Tsirkin, Marcel Apfelbaum,
Eduardo Habkost
Use a common 'xen_arch_' prefix for architecture-specific functions.
Rename xen_arch_set_memory() and xen_arch_handle_ioreq().
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
include/hw/arm/xen_arch_hvm.h | 4 ++--
include/hw/i386/xen_arch_hvm.h | 4 ++--
hw/arm/xen_arm.c | 4 ++--
hw/i386/xen/xen-hvm.c | 6 +++---
hw/xen/xen-hvm-common.c | 4 ++--
5 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/include/hw/arm/xen_arch_hvm.h b/include/hw/arm/xen_arch_hvm.h
index 8fd645e723..6a974f2020 100644
--- a/include/hw/arm/xen_arch_hvm.h
+++ b/include/hw/arm/xen_arch_hvm.h
@@ -2,8 +2,8 @@
#define HW_XEN_ARCH_ARM_HVM_H
#include <xen/hvm/ioreq.h>
-void arch_handle_ioreq(XenIOState *state, ioreq_t *req);
-void arch_xen_set_memory(XenIOState *state,
+void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req);
+void xen_arch_set_memory(XenIOState *state,
MemoryRegionSection *section,
bool add);
#endif
diff --git a/include/hw/i386/xen_arch_hvm.h b/include/hw/i386/xen_arch_hvm.h
index 1000f8f543..2822304955 100644
--- a/include/hw/i386/xen_arch_hvm.h
+++ b/include/hw/i386/xen_arch_hvm.h
@@ -4,8 +4,8 @@
#include <xen/hvm/ioreq.h>
#include "hw/xen/xen-hvm-common.h"
-void arch_handle_ioreq(XenIOState *state, ioreq_t *req);
-void arch_xen_set_memory(XenIOState *state,
+void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req);
+void xen_arch_set_memory(XenIOState *state,
MemoryRegionSection *section,
bool add);
#endif
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index 8a185da193..bf19407879 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -129,14 +129,14 @@ static void xen_init_ram(MachineState *machine)
}
}
-void arch_handle_ioreq(XenIOState *state, ioreq_t *req)
+void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req)
{
hw_error("Invalid ioreq type 0x%x\n", req->type);
return;
}
-void arch_xen_set_memory(XenIOState *state, MemoryRegionSection *section,
+void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
bool add)
{
}
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 1ae943370b..5150984e46 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -659,8 +659,8 @@ void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
}
}
-void arch_xen_set_memory(XenIOState *state, MemoryRegionSection *section,
- bool add)
+void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
+ bool add)
{
hwaddr start_addr = section->offset_within_address_space;
ram_addr_t size = int128_get64(section->size);
@@ -700,7 +700,7 @@ void arch_xen_set_memory(XenIOState *state, MemoryRegionSection *section,
}
}
-void arch_handle_ioreq(XenIOState *state, ioreq_t *req)
+void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req)
{
switch (req->type) {
case IOREQ_TYPE_VMWARE_PORT:
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index cf4053c9f2..cf6ed11f70 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -65,7 +65,7 @@ static void xen_set_memory(struct MemoryListener *listener,
}
}
- arch_xen_set_memory(state, section, add);
+ xen_arch_set_memory(state, section, add);
}
void xen_region_add(MemoryListener *listener,
@@ -452,7 +452,7 @@ static void handle_ioreq(XenIOState *state, ioreq_t *req)
cpu_ioreq_config(state, req);
break;
default:
- arch_handle_ioreq(state, req);
+ xen_arch_handle_ioreq(state, req);
}
if (req->dir == IOREQ_READ) {
trace_handle_ioreq_read(req, req->type, req->df, req->data_is_ptr,
--
2.41.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 11/19] hw/xen/xen_arch_hvm: Rename prototypes using 'xen_arch_' prefix
2023-11-14 14:38 ` [PATCH-for-9.0 v2 11/19] hw/xen/xen_arch_hvm: Rename prototypes using 'xen_arch_' prefix Philippe Mathieu-Daudé
@ 2024-03-27 13:37 ` Anthony PERARD
0 siblings, 0 replies; 50+ messages in thread
From: Anthony PERARD @ 2024-03-27 13:37 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: David Woodhouse, qemu-devel, Alex Bennée, Paul Durrant,
qemu-arm, Paolo Bonzini, David Woodhouse, Stefano Stabellini,
Richard Henderson, xen-devel, qemu-block, kvm, Thomas Huth,
Peter Maydell, Michael S. Tsirkin, Marcel Apfelbaum,
Eduardo Habkost
On Tue, Nov 14, 2023 at 03:38:07PM +0100, Philippe Mathieu-Daudé wrote:
> Use a common 'xen_arch_' prefix for architecture-specific functions.
> Rename xen_arch_set_memory() and xen_arch_handle_ioreq().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH-for-9.0 v2 12/19] hw/xen: Merge 'hw/xen/arch_hvm.h' in 'hw/xen/xen-hvm-common.h'
2023-11-14 14:37 [PATCH-for-9.0 v2 00/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
` (10 preceding siblings ...)
2023-11-14 14:38 ` [PATCH-for-9.0 v2 11/19] hw/xen/xen_arch_hvm: Rename prototypes using 'xen_arch_' prefix Philippe Mathieu-Daudé
@ 2023-11-14 14:38 ` Philippe Mathieu-Daudé
2024-03-27 14:27 ` Anthony PERARD
2023-11-14 14:38 ` [RFC PATCH-for-9.0 v2 13/19] hw/xen: Remove use of 'target_ulong' in handle_ioreq() Philippe Mathieu-Daudé
` (6 subsequent siblings)
18 siblings, 1 reply; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 14:38 UTC (permalink / raw)
To: David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
David Woodhouse, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, Anthony Perard, kvm, Thomas Huth,
Philippe Mathieu-Daudé,
Peter Maydell, Eduardo Habkost, Michael S. Tsirkin,
Marcel Apfelbaum
We don't need a target-specific header for common target-specific
prototypes. Declare xen_arch_handle_ioreq() and xen_arch_set_memory()
in "hw/xen/xen-hvm-common.h".
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
include/hw/arm/xen_arch_hvm.h | 9 ---------
include/hw/i386/xen_arch_hvm.h | 11 -----------
include/hw/xen/arch_hvm.h | 5 -----
include/hw/xen/xen-hvm-common.h | 6 ++++++
hw/arm/xen_arm.c | 1 -
hw/i386/xen/xen-hvm.c | 1 -
hw/xen/xen-hvm-common.c | 1 -
7 files changed, 6 insertions(+), 28 deletions(-)
delete mode 100644 include/hw/arm/xen_arch_hvm.h
delete mode 100644 include/hw/i386/xen_arch_hvm.h
delete mode 100644 include/hw/xen/arch_hvm.h
diff --git a/include/hw/arm/xen_arch_hvm.h b/include/hw/arm/xen_arch_hvm.h
deleted file mode 100644
index 6a974f2020..0000000000
--- a/include/hw/arm/xen_arch_hvm.h
+++ /dev/null
@@ -1,9 +0,0 @@
-#ifndef HW_XEN_ARCH_ARM_HVM_H
-#define HW_XEN_ARCH_ARM_HVM_H
-
-#include <xen/hvm/ioreq.h>
-void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req);
-void xen_arch_set_memory(XenIOState *state,
- MemoryRegionSection *section,
- bool add);
-#endif
diff --git a/include/hw/i386/xen_arch_hvm.h b/include/hw/i386/xen_arch_hvm.h
deleted file mode 100644
index 2822304955..0000000000
--- a/include/hw/i386/xen_arch_hvm.h
+++ /dev/null
@@ -1,11 +0,0 @@
-#ifndef HW_XEN_ARCH_I386_HVM_H
-#define HW_XEN_ARCH_I386_HVM_H
-
-#include <xen/hvm/ioreq.h>
-#include "hw/xen/xen-hvm-common.h"
-
-void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req);
-void xen_arch_set_memory(XenIOState *state,
- MemoryRegionSection *section,
- bool add);
-#endif
diff --git a/include/hw/xen/arch_hvm.h b/include/hw/xen/arch_hvm.h
deleted file mode 100644
index c7c515220d..0000000000
--- a/include/hw/xen/arch_hvm.h
+++ /dev/null
@@ -1,5 +0,0 @@
-#if defined(TARGET_I386) || defined(TARGET_X86_64)
-#include "hw/i386/xen_arch_hvm.h"
-#elif defined(TARGET_ARM) || defined(TARGET_ARM_64)
-#include "hw/arm/xen_arch_hvm.h"
-#endif
diff --git a/include/hw/xen/xen-hvm-common.h b/include/hw/xen/xen-hvm-common.h
index d3fa5ed29b..8934033eaa 100644
--- a/include/hw/xen/xen-hvm-common.h
+++ b/include/hw/xen/xen-hvm-common.h
@@ -96,4 +96,10 @@ void xen_register_ioreq(XenIOState *state, unsigned int max_cpus,
const MemoryListener *xen_memory_listener);
void cpu_ioreq_pio(ioreq_t *req);
+
+void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req);
+void xen_arch_set_memory(XenIOState *state,
+ MemoryRegionSection *section,
+ bool add);
+
#endif /* HW_XEN_HVM_COMMON_H */
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index bf19407879..6b0e396502 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -33,7 +33,6 @@
#include "sysemu/sysemu.h"
#include "hw/xen/xen-hvm-common.h"
#include "sysemu/tpm.h"
-#include "hw/xen/arch_hvm.h"
#define TYPE_XEN_ARM MACHINE_TYPE_NAME("xenpvh")
OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 5150984e46..0fbe720c8f 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -21,7 +21,6 @@
#include "qemu/range.h"
#include "hw/xen/xen-hvm-common.h"
-#include "hw/xen/arch_hvm.h"
#include <xen/hvm/e820.h>
static MemoryRegion ram_640k, ram_lo, ram_hi;
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index cf6ed11f70..bb3cfb200c 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -7,7 +7,6 @@
#include "hw/xen/xen-hvm-common.h"
#include "hw/xen/xen-bus.h"
#include "hw/boards.h"
-#include "hw/xen/arch_hvm.h"
MemoryRegion xen_memory;
--
2.41.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 12/19] hw/xen: Merge 'hw/xen/arch_hvm.h' in 'hw/xen/xen-hvm-common.h'
2023-11-14 14:38 ` [PATCH-for-9.0 v2 12/19] hw/xen: Merge 'hw/xen/arch_hvm.h' in 'hw/xen/xen-hvm-common.h' Philippe Mathieu-Daudé
@ 2024-03-27 14:27 ` Anthony PERARD
0 siblings, 0 replies; 50+ messages in thread
From: Anthony PERARD @ 2024-03-27 14:27 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: David Woodhouse, qemu-devel, Alex Bennée, Paul Durrant,
qemu-arm, Paolo Bonzini, David Woodhouse, Stefano Stabellini,
Richard Henderson, xen-devel, qemu-block, kvm, Thomas Huth,
Peter Maydell, Eduardo Habkost, Michael S. Tsirkin,
Marcel Apfelbaum
On Tue, Nov 14, 2023 at 03:38:08PM +0100, Philippe Mathieu-Daudé wrote:
> We don't need a target-specific header for common target-specific
> prototypes. Declare xen_arch_handle_ioreq() and xen_arch_set_memory()
> in "hw/xen/xen-hvm-common.h".
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 50+ messages in thread
* [RFC PATCH-for-9.0 v2 13/19] hw/xen: Remove use of 'target_ulong' in handle_ioreq()
2023-11-14 14:37 [PATCH-for-9.0 v2 00/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
` (11 preceding siblings ...)
2023-11-14 14:38 ` [PATCH-for-9.0 v2 12/19] hw/xen: Merge 'hw/xen/arch_hvm.h' in 'hw/xen/xen-hvm-common.h' Philippe Mathieu-Daudé
@ 2023-11-14 14:38 ` Philippe Mathieu-Daudé
2024-03-06 17:16 ` Philippe Mathieu-Daudé
2024-03-27 16:45 ` Anthony PERARD
2023-11-14 14:38 ` [PATCH-for-9.0 v2 14/19] hw/xen: Use target-agnostic qemu_target_page_bits() Philippe Mathieu-Daudé
` (5 subsequent siblings)
18 siblings, 2 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 14:38 UTC (permalink / raw)
To: David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
David Woodhouse, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, Anthony Perard, kvm, Thomas Huth,
Philippe Mathieu-Daudé
Per commit f17068c1c7 ("xen-hvm: reorganize xen-hvm and move common
function to xen-hvm-common"), handle_ioreq() is expected to be
target-agnostic. However it uses 'target_ulong', which is a target
specific definition.
Per xen/include/public/hvm/ioreq.h header:
struct ioreq {
uint64_t addr; /* physical address */
uint64_t data; /* data (or paddr of data) */
uint32_t count; /* for rep prefixes */
uint32_t size; /* size in bytes */
uint32_t vp_eport; /* evtchn for notifications to/from device model */
uint16_t _pad0;
uint8_t state:4;
uint8_t data_is_ptr:1; /* if 1, data above is the guest paddr
* of the real data to use. */
uint8_t dir:1; /* 1=read, 0=write */
uint8_t df:1;
uint8_t _pad1:1;
uint8_t type; /* I/O type */
};
typedef struct ioreq ioreq_t;
If 'data' is not a pointer, it is a u64.
- In PIO / VMWARE_PORT modes, only 32-bit are used.
- In MMIO COPY mode, memory is accessed by chunks of 64-bit
- In PCI_CONFIG mode, access is u8 or u16 or u32.
- None of TIMEOFFSET / INVALIDATE use 'req'.
- Fallback is only used in x86 for VMWARE_PORT.
Masking the upper bits of 'data' to keep 'req->size' low bits
is irrelevant of the target word size. Remove the word size
check and always extract the relevant bits.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/xen/xen-hvm-common.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index bb3cfb200c..fb81bd8fbc 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -1,5 +1,6 @@
#include "qemu/osdep.h"
#include "qemu/units.h"
+#include "qemu/bitops.h"
#include "qapi/error.h"
#include "trace.h"
@@ -426,9 +427,8 @@ static void handle_ioreq(XenIOState *state, ioreq_t *req)
trace_handle_ioreq(req, req->type, req->dir, req->df, req->data_is_ptr,
req->addr, req->data, req->count, req->size);
- if (!req->data_is_ptr && (req->dir == IOREQ_WRITE) &&
- (req->size < sizeof (target_ulong))) {
- req->data &= ((target_ulong) 1 << (8 * req->size)) - 1;
+ if (!req->data_is_ptr && (req->dir == IOREQ_WRITE)) {
+ req->data = extract64(req->data, 0, BITS_PER_BYTE * req->size);
}
if (req->dir == IOREQ_WRITE)
--
2.41.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [RFC PATCH-for-9.0 v2 13/19] hw/xen: Remove use of 'target_ulong' in handle_ioreq()
2023-11-14 14:38 ` [RFC PATCH-for-9.0 v2 13/19] hw/xen: Remove use of 'target_ulong' in handle_ioreq() Philippe Mathieu-Daudé
@ 2024-03-06 17:16 ` Philippe Mathieu-Daudé
2024-03-27 16:45 ` Anthony PERARD
1 sibling, 0 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-03-06 17:16 UTC (permalink / raw)
To: David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
David Woodhouse, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, Anthony Perard, kvm, Thomas Huth,
Anton Johansson
Cc'ing Anton.
On 14/11/23 15:38, Philippe Mathieu-Daudé wrote:
> Per commit f17068c1c7 ("xen-hvm: reorganize xen-hvm and move common
> function to xen-hvm-common"), handle_ioreq() is expected to be
> target-agnostic. However it uses 'target_ulong', which is a target
> specific definition.
>
> Per xen/include/public/hvm/ioreq.h header:
>
> struct ioreq {
> uint64_t addr; /* physical address */
> uint64_t data; /* data (or paddr of data) */
> uint32_t count; /* for rep prefixes */
> uint32_t size; /* size in bytes */
> uint32_t vp_eport; /* evtchn for notifications to/from device model */
> uint16_t _pad0;
> uint8_t state:4;
> uint8_t data_is_ptr:1; /* if 1, data above is the guest paddr
> * of the real data to use. */
> uint8_t dir:1; /* 1=read, 0=write */
> uint8_t df:1;
> uint8_t _pad1:1;
> uint8_t type; /* I/O type */
> };
> typedef struct ioreq ioreq_t;
>
> If 'data' is not a pointer, it is a u64.
>
> - In PIO / VMWARE_PORT modes, only 32-bit are used.
>
> - In MMIO COPY mode, memory is accessed by chunks of 64-bit
>
> - In PCI_CONFIG mode, access is u8 or u16 or u32.
>
> - None of TIMEOFFSET / INVALIDATE use 'req'.
>
> - Fallback is only used in x86 for VMWARE_PORT.
>
> Masking the upper bits of 'data' to keep 'req->size' low bits
> is irrelevant of the target word size. Remove the word size
> check and always extract the relevant bits.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/xen/xen-hvm-common.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
> index bb3cfb200c..fb81bd8fbc 100644
> --- a/hw/xen/xen-hvm-common.c
> +++ b/hw/xen/xen-hvm-common.c
> @@ -1,5 +1,6 @@
> #include "qemu/osdep.h"
> #include "qemu/units.h"
> +#include "qemu/bitops.h"
> #include "qapi/error.h"
> #include "trace.h"
>
> @@ -426,9 +427,8 @@ static void handle_ioreq(XenIOState *state, ioreq_t *req)
> trace_handle_ioreq(req, req->type, req->dir, req->df, req->data_is_ptr,
> req->addr, req->data, req->count, req->size);
>
> - if (!req->data_is_ptr && (req->dir == IOREQ_WRITE) &&
> - (req->size < sizeof (target_ulong))) {
> - req->data &= ((target_ulong) 1 << (8 * req->size)) - 1;
> + if (!req->data_is_ptr && (req->dir == IOREQ_WRITE)) {
> + req->data = extract64(req->data, 0, BITS_PER_BYTE * req->size);
> }
>
> if (req->dir == IOREQ_WRITE)
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH-for-9.0 v2 13/19] hw/xen: Remove use of 'target_ulong' in handle_ioreq()
2023-11-14 14:38 ` [RFC PATCH-for-9.0 v2 13/19] hw/xen: Remove use of 'target_ulong' in handle_ioreq() Philippe Mathieu-Daudé
2024-03-06 17:16 ` Philippe Mathieu-Daudé
@ 2024-03-27 16:45 ` Anthony PERARD
1 sibling, 0 replies; 50+ messages in thread
From: Anthony PERARD @ 2024-03-27 16:45 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: David Woodhouse, qemu-devel, Alex Bennée, Paul Durrant,
qemu-arm, Paolo Bonzini, David Woodhouse, Stefano Stabellini,
Richard Henderson, xen-devel, qemu-block, kvm, Thomas Huth
On Tue, Nov 14, 2023 at 03:38:09PM +0100, Philippe Mathieu-Daudé wrote:
> Per commit f17068c1c7 ("xen-hvm: reorganize xen-hvm and move common
> function to xen-hvm-common"), handle_ioreq() is expected to be
> target-agnostic. However it uses 'target_ulong', which is a target
> specific definition.
>
> Per xen/include/public/hvm/ioreq.h header:
>
> struct ioreq {
> uint64_t addr; /* physical address */
> uint64_t data; /* data (or paddr of data) */
> uint32_t count; /* for rep prefixes */
> uint32_t size; /* size in bytes */
> uint32_t vp_eport; /* evtchn for notifications to/from device model */
> uint16_t _pad0;
> uint8_t state:4;
> uint8_t data_is_ptr:1; /* if 1, data above is the guest paddr
> * of the real data to use. */
> uint8_t dir:1; /* 1=read, 0=write */
> uint8_t df:1;
> uint8_t _pad1:1;
> uint8_t type; /* I/O type */
> };
> typedef struct ioreq ioreq_t;
>
> If 'data' is not a pointer, it is a u64.
>
> - In PIO / VMWARE_PORT modes, only 32-bit are used.
>
> - In MMIO COPY mode, memory is accessed by chunks of 64-bit
Looks like it could also be 8, 16, or 32 as well, depending on
req->size.
> - In PCI_CONFIG mode, access is u8 or u16 or u32.
>
> - None of TIMEOFFSET / INVALIDATE use 'req'.
>
> - Fallback is only used in x86 for VMWARE_PORT.
>
> Masking the upper bits of 'data' to keep 'req->size' low bits
> is irrelevant of the target word size. Remove the word size
> check and always extract the relevant bits.
When building QEMU for Xen, we tend to build the target "i386-softmmu",
which looks like to have target_ulong == uint32_t. So the `data`
clamping would only apply to size 8 and 16. The clamping with
target_ulong was introduce long time ago, here:
https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=b4a663b87df3954557434a2d31bff7f6b2706ec1
and they were more IOREQ types.
So my guess is it isn't relevant anymore, but extending the clamping to
32-bits request should be fine, when using qemu-system-i386 that is, as
it is already be done if one use qemu-system-x86_64.
So I think the patch is fine, and the tests I've ran so far worked fine.
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH-for-9.0 v2 14/19] hw/xen: Use target-agnostic qemu_target_page_bits()
2023-11-14 14:37 [PATCH-for-9.0 v2 00/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
` (12 preceding siblings ...)
2023-11-14 14:38 ` [RFC PATCH-for-9.0 v2 13/19] hw/xen: Remove use of 'target_ulong' in handle_ioreq() Philippe Mathieu-Daudé
@ 2023-11-14 14:38 ` Philippe Mathieu-Daudé
2023-11-14 14:38 ` [PATCH-for-9.0 v2 15/19] hw/xen: Reduce inclusion of 'cpu.h' to target-specific sources Philippe Mathieu-Daudé
` (4 subsequent siblings)
18 siblings, 0 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 14:38 UTC (permalink / raw)
To: David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
David Woodhouse, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, Anthony Perard, kvm, Thomas Huth,
Philippe Mathieu-Daudé
Instead of the target-specific TARGET_PAGE_BITS definition,
use qemu_target_page_bits() which is target agnostic.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
---
hw/xen/xen-hvm-common.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index fb81bd8fbc..73fa2c414d 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -2,6 +2,7 @@
#include "qemu/units.h"
#include "qemu/bitops.h"
#include "qapi/error.h"
+#include "exec/target_page.h"
#include "trace.h"
#include "hw/pci/pci_host.h"
@@ -14,6 +15,7 @@ MemoryRegion xen_memory;
void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr,
Error **errp)
{
+ unsigned target_page_bits = qemu_target_page_bits();
unsigned long nr_pfn;
xen_pfn_t *pfn_list;
int i;
@@ -32,11 +34,11 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr,
trace_xen_ram_alloc(ram_addr, size);
- nr_pfn = size >> TARGET_PAGE_BITS;
+ nr_pfn = size >> target_page_bits;
pfn_list = g_new(xen_pfn_t, nr_pfn);
for (i = 0; i < nr_pfn; i++) {
- pfn_list[i] = (ram_addr >> TARGET_PAGE_BITS) + i;
+ pfn_list[i] = (ram_addr >> target_page_bits) + i;
}
if (xc_domain_populate_physmap_exact(xen_xc, xen_domid, nr_pfn, 0, 0, pfn_list)) {
--
2.41.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH-for-9.0 v2 15/19] hw/xen: Reduce inclusion of 'cpu.h' to target-specific sources
2023-11-14 14:37 [PATCH-for-9.0 v2 00/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
` (13 preceding siblings ...)
2023-11-14 14:38 ` [PATCH-for-9.0 v2 14/19] hw/xen: Use target-agnostic qemu_target_page_bits() Philippe Mathieu-Daudé
@ 2023-11-14 14:38 ` Philippe Mathieu-Daudé
2024-03-27 17:13 ` Anthony PERARD
2023-11-14 14:38 ` [PATCH-for-9.0 v2 16/19] hw/xen/xen_pt: Add missing license Philippe Mathieu-Daudé
` (3 subsequent siblings)
18 siblings, 1 reply; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 14:38 UTC (permalink / raw)
To: David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
David Woodhouse, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, Anthony Perard, kvm, Thomas Huth,
Philippe Mathieu-Daudé,
Peter Maydell, Eduardo Habkost, Michael S. Tsirkin,
Marcel Apfelbaum
We rarely need to include "cpu.h" in headers. Including it
'taint' headers to be target-specific. Here only the i386/arm
implementations requires "cpu.h", so include it there and
remove from the "hw/xen/xen-hvm-common.h" *common* header.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
---
include/hw/xen/xen-hvm-common.h | 1 -
hw/arm/xen_arm.c | 1 +
hw/i386/xen/xen-hvm.c | 1 +
3 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/hw/xen/xen-hvm-common.h b/include/hw/xen/xen-hvm-common.h
index 8934033eaa..83ed16f425 100644
--- a/include/hw/xen/xen-hvm-common.h
+++ b/include/hw/xen/xen-hvm-common.h
@@ -4,7 +4,6 @@
#include "qemu/osdep.h"
#include "qemu/units.h"
-#include "cpu.h"
#include "hw/pci/pci.h"
#include "hw/hw.h"
#include "hw/xen/xen_native.h"
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index 6b0e396502..b478d74ea0 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -33,6 +33,7 @@
#include "sysemu/sysemu.h"
#include "hw/xen/xen-hvm-common.h"
#include "sysemu/tpm.h"
+#include "cpu.h"
#define TYPE_XEN_ARM MACHINE_TYPE_NAME("xenpvh")
OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 0fbe720c8f..f1c30d1384 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -22,6 +22,7 @@
#include "hw/xen/xen-hvm-common.h"
#include <xen/hvm/e820.h>
+#include "cpu.h"
static MemoryRegion ram_640k, ram_lo, ram_hi;
static MemoryRegion *framebuffer;
--
2.41.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 15/19] hw/xen: Reduce inclusion of 'cpu.h' to target-specific sources
2023-11-14 14:38 ` [PATCH-for-9.0 v2 15/19] hw/xen: Reduce inclusion of 'cpu.h' to target-specific sources Philippe Mathieu-Daudé
@ 2024-03-27 17:13 ` Anthony PERARD
0 siblings, 0 replies; 50+ messages in thread
From: Anthony PERARD @ 2024-03-27 17:13 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: David Woodhouse, qemu-devel, Alex Bennée, Paul Durrant,
qemu-arm, Paolo Bonzini, David Woodhouse, Stefano Stabellini,
Richard Henderson, xen-devel, qemu-block, kvm, Thomas Huth,
Peter Maydell, Eduardo Habkost, Michael S. Tsirkin,
Marcel Apfelbaum
On Tue, Nov 14, 2023 at 03:38:11PM +0100, Philippe Mathieu-Daudé wrote:
> We rarely need to include "cpu.h" in headers. Including it
> 'taint' headers to be target-specific. Here only the i386/arm
> implementations requires "cpu.h", so include it there and
> remove from the "hw/xen/xen-hvm-common.h" *common* header.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH-for-9.0 v2 16/19] hw/xen/xen_pt: Add missing license
2023-11-14 14:37 [PATCH-for-9.0 v2 00/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
` (14 preceding siblings ...)
2023-11-14 14:38 ` [PATCH-for-9.0 v2 15/19] hw/xen: Reduce inclusion of 'cpu.h' to target-specific sources Philippe Mathieu-Daudé
@ 2023-11-14 14:38 ` Philippe Mathieu-Daudé
2023-11-14 15:54 ` David Woodhouse
2024-03-27 17:27 ` Anthony PERARD
2023-11-14 14:38 ` [PATCH-for-9.0 v2 17/19] hw/xen: Extract 'xen_igd.h' from 'xen_pt.h' Philippe Mathieu-Daudé
` (2 subsequent siblings)
18 siblings, 2 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 14:38 UTC (permalink / raw)
To: David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
David Woodhouse, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, Anthony Perard, kvm, Thomas Huth,
Philippe Mathieu-Daudé
Commit eaab4d60d3 ("Introduce Xen PCI Passthrough, qdevice")
introduced both xen_pt.[ch], but only added the license to
xen_pt.c. Use the same license for xen_pt.h.
Suggested-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/xen/xen_pt.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 31bcfdf705..d3180bb134 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -1,3 +1,13 @@
+/*
+ * Copyright (c) 2007, Neocleus Corporation.
+ * Copyright (c) 2007, Intel Corporation.
+ *
+ * SPDX-License-Identifier: GPL-2.0-only
+ *
+ * Alex Novik <alex@neocleus.com>
+ * Allen Kay <allen.m.kay@intel.com>
+ * Guy Zana <guy@neocleus.com>
+ */
#ifndef XEN_PT_H
#define XEN_PT_H
--
2.41.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 16/19] hw/xen/xen_pt: Add missing license
2023-11-14 14:38 ` [PATCH-for-9.0 v2 16/19] hw/xen/xen_pt: Add missing license Philippe Mathieu-Daudé
@ 2023-11-14 15:54 ` David Woodhouse
2024-03-27 17:27 ` Anthony PERARD
1 sibling, 0 replies; 50+ messages in thread
From: David Woodhouse @ 2023-11-14 15:54 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
Stefano Stabellini, Richard Henderson, xen-devel, qemu-block,
Anthony Perard, kvm, Thomas Huth
On 14 November 2023 09:38:12 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>Commit eaab4d60d3 ("Introduce Xen PCI Passthrough, qdevice")
>introduced both xen_pt.[ch], but only added the license to
>xen_pt.c. Use the same license for xen_pt.h.
>
>Suggested-by: David Woodhouse <dwmw@amazon.co.uk>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 16/19] hw/xen/xen_pt: Add missing license
2023-11-14 14:38 ` [PATCH-for-9.0 v2 16/19] hw/xen/xen_pt: Add missing license Philippe Mathieu-Daudé
2023-11-14 15:54 ` David Woodhouse
@ 2024-03-27 17:27 ` Anthony PERARD
1 sibling, 0 replies; 50+ messages in thread
From: Anthony PERARD @ 2024-03-27 17:27 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: David Woodhouse, qemu-devel, Alex Bennée, Paul Durrant,
qemu-arm, Paolo Bonzini, David Woodhouse, Stefano Stabellini,
Richard Henderson, xen-devel, qemu-block, kvm, Thomas Huth
On Tue, Nov 14, 2023 at 03:38:12PM +0100, Philippe Mathieu-Daudé wrote:
> Commit eaab4d60d3 ("Introduce Xen PCI Passthrough, qdevice")
> introduced both xen_pt.[ch], but only added the license to
> xen_pt.c. Use the same license for xen_pt.h.
>
> Suggested-by: David Woodhouse <dwmw@amazon.co.uk>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Fine by me. Looks like there was a license header before:
https://xenbits.xen.org/gitweb/?p=qemu-xen-unstable.git;a=blob;f=hw/pass-through.h;h=0b5822414e24d199a064abccc4d378dcaf569bd6;hb=HEAD
I don't know why I didn't copied it over here.
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH-for-9.0 v2 17/19] hw/xen: Extract 'xen_igd.h' from 'xen_pt.h'
2023-11-14 14:37 [PATCH-for-9.0 v2 00/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
` (15 preceding siblings ...)
2023-11-14 14:38 ` [PATCH-for-9.0 v2 16/19] hw/xen/xen_pt: Add missing license Philippe Mathieu-Daudé
@ 2023-11-14 14:38 ` Philippe Mathieu-Daudé
2023-11-14 14:38 ` [PATCH-for-9.0 v2 18/19] hw/i386/xen: Compile 'xen-hvm.c' with Xen CPPFLAGS Philippe Mathieu-Daudé
2023-11-14 14:38 ` [PATCH-for-9.0 v2 19/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
18 siblings, 0 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 14:38 UTC (permalink / raw)
To: David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
David Woodhouse, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, Anthony Perard, kvm, Thomas Huth,
Philippe Mathieu-Daudé,
Eduardo Habkost, Michael S. Tsirkin, Marcel Apfelbaum
"hw/xen/xen_pt.h" requires "hw/xen/xen_native.h" which is target
specific. It also declares IGD methods, which are not target
specific.
Target-agnostic code can use IGD methods. To allow that, extract
these methos into a new "hw/xen/xen_igd.h" header.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
---
hw/xen/xen_pt.h | 14 --------------
include/hw/xen/xen_igd.h | 33 +++++++++++++++++++++++++++++++++
accel/xen/xen-all.c | 1 +
hw/i386/pc_piix.c | 1 +
hw/xen/xen_pt.c | 3 ++-
hw/xen/xen_pt_config_init.c | 3 ++-
hw/xen/xen_pt_graphics.c | 3 ++-
hw/xen/xen_pt_stub.c | 2 +-
8 files changed, 42 insertions(+), 18 deletions(-)
create mode 100644 include/hw/xen/xen_igd.h
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index d3180bb134..095a0f0365 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -15,9 +15,6 @@
#include "xen-host-pci-device.h"
#include "qom/object.h"
-bool xen_igd_gfx_pt_enabled(void);
-void xen_igd_gfx_pt_set(bool value, Error **errp);
-
void xen_pt_log(const PCIDevice *d, const char *f, ...) G_GNUC_PRINTF(2, 3);
#define XEN_PT_ERR(d, _f, _a...) xen_pt_log(d, "%s: Error: "_f, __func__, ##_a)
@@ -62,12 +59,6 @@ typedef struct XenPTDeviceClass {
XenPTQdevRealize pci_qdev_realize;
} XenPTDeviceClass;
-uint32_t igd_read_opregion(XenPCIPassthroughState *s);
-void xen_igd_reserve_slot(PCIBus *pci_bus);
-void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val);
-void xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
- XenHostPCIDevice *dev);
-
/* function type for config reg */
typedef int (*xen_pt_conf_reg_init)
(XenPCIPassthroughState *, XenPTRegInfo *, uint32_t real_offset,
@@ -353,11 +344,6 @@ static inline bool xen_pt_has_msix_mapping(XenPCIPassthroughState *s, int bar)
void *pci_assign_dev_load_option_rom(PCIDevice *dev, int *size,
unsigned int domain, unsigned int bus,
unsigned int slot, unsigned int function);
-static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev)
-{
- return (xen_igd_gfx_pt_enabled()
- && ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA));
-}
int xen_pt_register_vga_regions(XenHostPCIDevice *dev);
int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev);
void xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev,
diff --git a/include/hw/xen/xen_igd.h b/include/hw/xen/xen_igd.h
new file mode 100644
index 0000000000..7ffca06c10
--- /dev/null
+++ b/include/hw/xen/xen_igd.h
@@ -0,0 +1,33 @@
+/*
+ * Copyright (c) 2007, Neocleus Corporation.
+ * Copyright (c) 2007, Intel Corporation.
+ *
+ * SPDX-License-Identifier: GPL-2.0-only
+ *
+ * Alex Novik <alex@neocleus.com>
+ * Allen Kay <allen.m.kay@intel.com>
+ * Guy Zana <guy@neocleus.com>
+ */
+#ifndef XEN_IGD_H
+#define XEN_IGD_H
+
+#include "hw/xen/xen-host-pci-device.h"
+
+typedef struct XenPCIPassthroughState XenPCIPassthroughState;
+
+bool xen_igd_gfx_pt_enabled(void);
+void xen_igd_gfx_pt_set(bool value, Error **errp);
+
+uint32_t igd_read_opregion(XenPCIPassthroughState *s);
+void xen_igd_reserve_slot(PCIBus *pci_bus);
+void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val);
+void xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
+ XenHostPCIDevice *dev);
+
+static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev)
+{
+ return (xen_igd_gfx_pt_enabled()
+ && ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA));
+}
+
+#endif
diff --git a/accel/xen/xen-all.c b/accel/xen/xen-all.c
index 5ff0cb8bd9..0bdefce537 100644
--- a/accel/xen/xen-all.c
+++ b/accel/xen/xen-all.c
@@ -15,6 +15,7 @@
#include "hw/xen/xen_native.h"
#include "hw/xen/xen-legacy-backend.h"
#include "hw/xen/xen_pt.h"
+#include "hw/xen/xen_igd.h"
#include "chardev/char.h"
#include "qemu/accel.h"
#include "sysemu/cpus.h"
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index eace854335..a607dcb56c 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -56,6 +56,7 @@
#ifdef CONFIG_XEN
#include <xen/hvm/hvm_info_table.h>
#include "hw/xen/xen_pt.h"
+#include "hw/xen/xen_igd.h"
#endif
#include "hw/xen/xen-x86.h"
#include "hw/xen/xen.h"
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index 36e6f93c37..a8edabdabc 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -59,7 +59,8 @@
#include "hw/pci/pci.h"
#include "hw/qdev-properties.h"
#include "hw/qdev-properties-system.h"
-#include "xen_pt.h"
+#include "hw/xen/xen_pt.h"
+#include "hw/xen/xen_igd.h"
#include "hw/xen/xen.h"
#include "hw/xen/xen-legacy-backend.h"
#include "qemu/range.h"
diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index 2b8680b112..ba4cd78238 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -15,7 +15,8 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "qemu/timer.h"
-#include "xen_pt.h"
+#include "hw/xen/xen_pt.h"
+#include "hw/xen/xen_igd.h"
#include "hw/xen/xen-legacy-backend.h"
#define XEN_PT_MERGE_VALUE(value, data, val_mask) \
diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
index 0aed3bb6fd..6c2e3f4840 100644
--- a/hw/xen/xen_pt_graphics.c
+++ b/hw/xen/xen_pt_graphics.c
@@ -3,7 +3,8 @@
*/
#include "qemu/osdep.h"
#include "qapi/error.h"
-#include "xen_pt.h"
+#include "hw/xen/xen_pt.h"
+#include "hw/xen/xen_igd.h"
#include "xen-host-pci-device.h"
static unsigned long igd_guest_opregion;
diff --git a/hw/xen/xen_pt_stub.c b/hw/xen/xen_pt_stub.c
index 5c108446a8..72feebeb20 100644
--- a/hw/xen/xen_pt_stub.c
+++ b/hw/xen/xen_pt_stub.c
@@ -6,7 +6,7 @@
*/
#include "qemu/osdep.h"
-#include "hw/xen/xen_pt.h"
+#include "hw/xen/xen_igd.h"
#include "qapi/error.h"
bool xen_igd_gfx_pt_enabled(void)
--
2.41.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH-for-9.0 v2 18/19] hw/i386/xen: Compile 'xen-hvm.c' with Xen CPPFLAGS
2023-11-14 14:37 [PATCH-for-9.0 v2 00/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
` (16 preceding siblings ...)
2023-11-14 14:38 ` [PATCH-for-9.0 v2 17/19] hw/xen: Extract 'xen_igd.h' from 'xen_pt.h' Philippe Mathieu-Daudé
@ 2023-11-14 14:38 ` Philippe Mathieu-Daudé
2023-11-14 15:56 ` David Woodhouse
2023-11-14 14:38 ` [PATCH-for-9.0 v2 19/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
18 siblings, 1 reply; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 14:38 UTC (permalink / raw)
To: David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
David Woodhouse, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, Anthony Perard, kvm, Thomas Huth,
Philippe Mathieu-Daudé,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost
xen-hvm.c calls xc_set_hvm_param() from <xenctrl.h>,
so better compile it with Xen CPPFLAGS.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/i386/xen/meson.build | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/i386/xen/meson.build b/hw/i386/xen/meson.build
index 3dc4c4f106..3f0df8bc07 100644
--- a/hw/i386/xen/meson.build
+++ b/hw/i386/xen/meson.build
@@ -1,8 +1,10 @@
i386_ss.add(when: 'CONFIG_XEN', if_true: files(
- 'xen-hvm.c',
'xen_apic.c',
'xen_pvdevice.c',
))
+i386_ss.add(when: ['CONFIG_XEN', xen], if_true: files(
+ 'xen-hvm.c',
+))
i386_ss.add(when: 'CONFIG_XEN_BUS', if_true: files(
'xen_platform.c',
--
2.41.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 18/19] hw/i386/xen: Compile 'xen-hvm.c' with Xen CPPFLAGS
2023-11-14 14:38 ` [PATCH-for-9.0 v2 18/19] hw/i386/xen: Compile 'xen-hvm.c' with Xen CPPFLAGS Philippe Mathieu-Daudé
@ 2023-11-14 15:56 ` David Woodhouse
0 siblings, 0 replies; 50+ messages in thread
From: David Woodhouse @ 2023-11-14 15:56 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
Stefano Stabellini, Richard Henderson, xen-devel, qemu-block,
Anthony Perard, kvm, Thomas Huth, Michael S. Tsirkin,
Marcel Apfelbaum, Eduardo Habkost
On 14 November 2023 09:38:14 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>xen-hvm.c calls xc_set_hvm_param() from <xenctrl.h>,
>so better compile it with Xen CPPFLAGS.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH-for-9.0 v2 19/19] hw/xen: Have most of Xen files become target-agnostic
2023-11-14 14:37 [PATCH-for-9.0 v2 00/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
` (17 preceding siblings ...)
2023-11-14 14:38 ` [PATCH-for-9.0 v2 18/19] hw/i386/xen: Compile 'xen-hvm.c' with Xen CPPFLAGS Philippe Mathieu-Daudé
@ 2023-11-14 14:38 ` Philippe Mathieu-Daudé
2024-03-28 15:13 ` Anthony PERARD
18 siblings, 1 reply; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-14 14:38 UTC (permalink / raw)
To: David Woodhouse, qemu-devel
Cc: Alex Bennée, Paul Durrant, qemu-arm, Paolo Bonzini,
David Woodhouse, Stefano Stabellini, Richard Henderson,
xen-devel, qemu-block, Anthony Perard, kvm, Thomas Huth,
Philippe Mathieu-Daudé,
Stefan Hajnoczi, Kevin Wolf, Hanna Reitz
Previous commits re-organized the target-specific bits
from Xen files. We can now build the common files once
instead of per-target.
Only 4 files call libxen API (thus its CPPFLAGS):
- xen-hvm-common.c,
- xen_pt.c, xen_pt_graphics.c, xen_pt_msi.c
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
Reworked since v1 so dropping David's R-b tag.
---
accel/xen/meson.build | 2 +-
hw/block/dataplane/meson.build | 2 +-
hw/xen/meson.build | 21 ++++++++++-----------
3 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/accel/xen/meson.build b/accel/xen/meson.build
index 002bdb03c6..455ad5d6be 100644
--- a/accel/xen/meson.build
+++ b/accel/xen/meson.build
@@ -1 +1 @@
-specific_ss.add(when: 'CONFIG_XEN', if_true: files('xen-all.c'))
+system_ss.add(when: 'CONFIG_XEN', if_true: files('xen-all.c'))
diff --git a/hw/block/dataplane/meson.build b/hw/block/dataplane/meson.build
index 025b3b061b..4d8bcb0bb9 100644
--- a/hw/block/dataplane/meson.build
+++ b/hw/block/dataplane/meson.build
@@ -1,2 +1,2 @@
system_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c'))
-specific_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xen-block.c'))
+system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xen-block.c'))
diff --git a/hw/xen/meson.build b/hw/xen/meson.build
index d887fa9ba4..403cab49cf 100644
--- a/hw/xen/meson.build
+++ b/hw/xen/meson.build
@@ -7,26 +7,25 @@ system_ss.add(when: ['CONFIG_XEN_BUS'], if_true: files(
'xen_pvdev.c',
))
-system_ss.add(when: ['CONFIG_XEN', xen], if_true: files(
+system_ss.add(when: ['CONFIG_XEN'], if_true: files(
'xen-operations.c',
-))
-
-xen_specific_ss = ss.source_set()
-xen_specific_ss.add(files(
'xen-mapcache.c',
+))
+system_ss.add(when: ['CONFIG_XEN', xen], if_true: files(
'xen-hvm-common.c',
))
+
if have_xen_pci_passthrough
- xen_specific_ss.add(files(
+ system_ss.add(when: ['CONFIG_XEN'], if_true: files(
'xen-host-pci-device.c',
- 'xen_pt.c',
'xen_pt_config_init.c',
- 'xen_pt_graphics.c',
'xen_pt_load_rom.c',
+ ))
+ system_ss.add(when: ['CONFIG_XEN', xen], if_true: files(
+ 'xen_pt.c',
+ 'xen_pt_graphics.c',
'xen_pt_msi.c',
))
else
- xen_specific_ss.add(files('xen_pt_stub.c'))
+ system_ss.add(when: ['CONFIG_XEN'], if_true: files('xen_pt_stub.c'))
endif
-
-specific_ss.add_all(when: ['CONFIG_XEN', xen], if_true: xen_specific_ss)
--
2.41.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH-for-9.0 v2 19/19] hw/xen: Have most of Xen files become target-agnostic
2023-11-14 14:38 ` [PATCH-for-9.0 v2 19/19] hw/xen: Have most of Xen files become target-agnostic Philippe Mathieu-Daudé
@ 2024-03-28 15:13 ` Anthony PERARD
0 siblings, 0 replies; 50+ messages in thread
From: Anthony PERARD @ 2024-03-28 15:13 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: David Woodhouse, qemu-devel, Alex Bennée, Paul Durrant,
qemu-arm, Paolo Bonzini, David Woodhouse, Stefano Stabellini,
Richard Henderson, xen-devel, qemu-block, kvm, Thomas Huth,
Stefan Hajnoczi, Kevin Wolf, Hanna Reitz
On Tue, Nov 14, 2023 at 03:38:15PM +0100, Philippe Mathieu-Daudé wrote:
> Previous commits re-organized the target-specific bits
> from Xen files. We can now build the common files once
> instead of per-target.
>
> Only 4 files call libxen API (thus its CPPFLAGS):
> - xen-hvm-common.c,
> - xen_pt.c, xen_pt_graphics.c, xen_pt_msi.c
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> Reworked since v1 so dropping David's R-b tag.
> ---
> accel/xen/meson.build | 2 +-
> hw/block/dataplane/meson.build | 2 +-
> hw/xen/meson.build | 21 ++++++++++-----------
> 3 files changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/accel/xen/meson.build b/accel/xen/meson.build
> index 002bdb03c6..455ad5d6be 100644
> --- a/accel/xen/meson.build
> +++ b/accel/xen/meson.build
> @@ -1 +1 @@
> -specific_ss.add(when: 'CONFIG_XEN', if_true: files('xen-all.c'))
> +system_ss.add(when: 'CONFIG_XEN', if_true: files('xen-all.c'))
> diff --git a/hw/block/dataplane/meson.build b/hw/block/dataplane/meson.build
> index 025b3b061b..4d8bcb0bb9 100644
> --- a/hw/block/dataplane/meson.build
> +++ b/hw/block/dataplane/meson.build
> @@ -1,2 +1,2 @@
> system_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c'))
> -specific_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xen-block.c'))
> +system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xen-block.c'))
> diff --git a/hw/xen/meson.build b/hw/xen/meson.build
> index d887fa9ba4..403cab49cf 100644
> --- a/hw/xen/meson.build
> +++ b/hw/xen/meson.build
> @@ -7,26 +7,25 @@ system_ss.add(when: ['CONFIG_XEN_BUS'], if_true: files(
> 'xen_pvdev.c',
> ))
>
> -system_ss.add(when: ['CONFIG_XEN', xen], if_true: files(
> +system_ss.add(when: ['CONFIG_XEN'], if_true: files(
> 'xen-operations.c',
> -))
> -
> -xen_specific_ss = ss.source_set()
> -xen_specific_ss.add(files(
> 'xen-mapcache.c',
> +))
> +system_ss.add(when: ['CONFIG_XEN', xen], if_true: files(
> 'xen-hvm-common.c',
> ))
> +
> if have_xen_pci_passthrough
> - xen_specific_ss.add(files(
> + system_ss.add(when: ['CONFIG_XEN'], if_true: files(
> 'xen-host-pci-device.c',
> - 'xen_pt.c',
> 'xen_pt_config_init.c',
> - 'xen_pt_graphics.c',
> 'xen_pt_load_rom.c',
> + ))
> + system_ss.add(when: ['CONFIG_XEN', xen], if_true: files(
> + 'xen_pt.c',
> + 'xen_pt_graphics.c',
How is it useful to separate those source files? In the commit
description, there's a talk about "CPPFLAGS", but having `when: [xen]`
doesn't change the flags used to build those objects, so the talk about
"CPPFLAGS" is confusing.
Second, if for some reason the dependency `xen` is false, but
`CONFIG_XEN` is true, then we wouldn't be able to build QEMU. Try
linking a binary with "xen_pt_config_init.o" but without "xen_pt.o",
that's not going to work. So even if that first source file doesn't
directly depend on the Xen libraries, it depends on "xen_pt.o" which
depends on the Xen libraries. So ultimately, I think all those source
files should have the same condition: ['CONFIG_XEN', xen].
I've only checked the xen_pt* source files, I don't know if the same
applies to "xen-operations.c" or "xen-mapcache.c".
Beside this, QEMU built with Xen support still seems to works fine, so
adding the objects to `system_ss` instead of `specific_ss` seems
alright.
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 50+ messages in thread