[Qemu-devel] [PATCH 0/4] Xen cleanups

[Qemu-devel] [PATCH 0/4] Xen cleanups

[Jan Kiszka]

I came across some Xen oddities I couldn't help to clean up. These
patches
 - simplify the build
 - remove a win32 breakage from xen-mapcache.h
 - move the map cache functions out of the generic name space

Jan Kiszka (4):
  xen: Clean up build system
  xen: Uninline xen_unmap_block
  xen: Clean up map cache API naming
  xen: Fold CONFIG_XEN_MAPCACHE into CONFIG_XEN

 Makefile.objs       |    4 +-
 Makefile.target     |   14 +-----------
 configure           |    5 +---
 exec.c              |   25 ++++++++++++-----------
 hw/xen.h            |    9 --------
 trace-events        |    6 ++--
 xen-all.c           |    4 +-
 xen-mapcache-stub.c |   44 -------------------------------------------
 xen-mapcache.c      |   52 ++++++++++++++++++++++++++++++++------------------
 xen-mapcache.h      |   30 +++++++---------------------
 xen-stub.c          |   29 ++++++++++++++++++++++++++++
 11 files changed, 93 insertions(+), 129 deletions(-)
 delete mode 100644 xen-mapcache-stub.c

[Qemu-devel] [PATCH 1/4] xen: Clean up build system

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Introduce CONFIG_XEN_BACKEND so that this new config solely controls the
target-independent backend build and CONFIG_XEN can focus on per-target
building.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 Makefile.objs   |    4 ++--
 Makefile.target |   13 +++----------
 configure       |    2 +-
 3 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index cea15e4..1635df6 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -155,8 +155,8 @@ slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o
 common-obj-$(CONFIG_SLIRP) += $(addprefix slirp/, $(slirp-obj-y))
 
 # xen backend driver support
-common-obj-$(CONFIG_XEN) += xen_backend.o xen_devconfig.o
-common-obj-$(CONFIG_XEN) += xen_console.o xenfb.o xen_disk.o xen_nic.o
+common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o
+common-obj-$(CONFIG_XEN_BACKEND) += xen_console.o xenfb.o xen_disk.o xen_nic.o
 
 ######################################################################
 # libuser
diff --git a/Makefile.target b/Makefile.target
index b1a0f6d..f325554 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -3,6 +3,8 @@
 GENERATED_HEADERS = config-target.h
 CONFIG_NO_PCI = $(if $(subst n,,$(CONFIG_PCI)),n,y)
 CONFIG_NO_KVM = $(if $(subst n,,$(CONFIG_KVM)),n,y)
+CONFIG_NO_XEN = $(if $(subst n,,$(CONFIG_XEN)),n,y)
+CONFIG_NO_XEN_MAPCACHE = $(if $(subst n,,$(CONFIG_XEN_MAPCACHE)),n,y)
 
 include ../config-host.mak
 include config-devices.mak
@@ -203,17 +205,8 @@ QEMU_CFLAGS += $(VNC_SASL_CFLAGS)
 QEMU_CFLAGS += $(VNC_JPEG_CFLAGS)
 QEMU_CFLAGS += $(VNC_PNG_CFLAGS)
 
-# xen backend driver support
-obj-i386-$(CONFIG_XEN) += xen_machine_pv.o xen_domainbuild.o
-
-ifeq ($(TARGET_BASE_ARCH), i386)
-  CONFIG_NO_XEN = $(if $(subst n,,$(CONFIG_XEN)),n,y)
-else
-  CONFIG_NO_XEN = y
-endif
 # xen support
-CONFIG_NO_XEN_MAPCACHE = $(if $(subst n,,$(CONFIG_XEN_MAPCACHE)),n,y)
-obj-i386-$(CONFIG_XEN) += xen-all.o
+obj-$(CONFIG_XEN) += xen-all.o xen_machine_pv.o xen_domainbuild.o
 obj-$(CONFIG_NO_XEN) += xen-stub.o
 obj-i386-$(CONFIG_XEN_MAPCACHE) += xen-mapcache.o
 obj-$(CONFIG_NO_XEN_MAPCACHE) += xen-mapcache-stub.o
diff --git a/configure b/configure
index 44c092a..7991489 100755
--- a/configure
+++ b/configure
@@ -2944,7 +2944,7 @@ if test "$bluez" = "yes" ; then
   echo "BLUEZ_CFLAGS=$bluez_cflags" >> $config_host_mak
 fi
 if test "$xen" = "yes" ; then
-  echo "CONFIG_XEN=y" >> $config_host_mak
+  echo "CONFIG_XEN_BACKEND=y" >> $config_host_mak
   echo "CONFIG_XEN_CTRL_INTERFACE_VERSION=$xen_ctrl_version" >> $config_host_mak
 fi
 if test "$io_thread" = "yes" ; then
-- 
1.7.1

Re: [Qemu-devel] [PATCH 1/4] xen: Clean up build system

[Alexander Graf]

On 21.06.2011, at 08:26, Jan Kiszka wrote:

> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Introduce CONFIG_XEN_BACKEND so that this new config solely controls the
> target-independent backend build and CONFIG_XEN can focus on per-target
> building.

What's the incentive here? I very much doubt we'll ever want to support xen on !x86. In fact, the better solution would probably be to just move every build component to x86 specific code.


Alex

Re: [Qemu-devel] [PATCH 1/4] xen: Clean up build system

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 630 bytes --]

On 2011-06-21 09:21, Alexander Graf wrote:
> 
> On 21.06.2011, at 08:26, Jan Kiszka wrote:
> 
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Introduce CONFIG_XEN_BACKEND so that this new config solely controls the
>> target-independent backend build and CONFIG_XEN can focus on per-target
>> building.
> 
> What's the incentive here? I very much doubt we'll ever want to support xen on !x86. In fact, the better solution would probably be to just move every build component to x86 specific code.

I doubt that as well. But keeping the backend centrals prevent building
them twice (for i386 and x86_64).

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [Qemu-devel] [PATCH 1/4] xen: Clean up build system

[Alexander Graf]

On 21.06.2011, at 09:23, Jan Kiszka wrote:

> On 2011-06-21 09:21, Alexander Graf wrote:
>> 
>> On 21.06.2011, at 08:26, Jan Kiszka wrote:
>> 
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>> 
>>> Introduce CONFIG_XEN_BACKEND so that this new config solely controls the
>>> target-independent backend build and CONFIG_XEN can focus on per-target
>>> building.
>> 
>> What's the incentive here? I very much doubt we'll ever want to support xen on !x86. In fact, the better solution would probably be to just move every build component to x86 specific code.
> 
> I doubt that as well. But keeping the backend centrals prevent building
> them twice (for i386 and x86_64).

How can we not build them twice without risking that target_ulong breaks somewhere along the way?


Alex

Re: [Qemu-devel] [PATCH 1/4] xen: Clean up build system

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 964 bytes --]

On 2011-06-21 09:26, Alexander Graf wrote:
> 
> On 21.06.2011, at 09:23, Jan Kiszka wrote:
> 
>> On 2011-06-21 09:21, Alexander Graf wrote:
>>>
>>> On 21.06.2011, at 08:26, Jan Kiszka wrote:
>>>
>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>
>>>> Introduce CONFIG_XEN_BACKEND so that this new config solely controls the
>>>> target-independent backend build and CONFIG_XEN can focus on per-target
>>>> building.
>>>
>>> What's the incentive here? I very much doubt we'll ever want to support xen on !x86. In fact, the better solution would probably be to just move every build component to x86 specific code.
>>
>> I doubt that as well. But keeping the backend centrals prevent building
>> them twice (for i386 and x86_64).
> 
> How can we not build them twice without risking that target_ulong breaks somewhere along the way?

Because the backends don't depend on that. And we already build them
only once, your your logs. :)

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [Qemu-devel] [PATCH 1/4] xen: Clean up build system

[Alexander Graf]

On 21.06.2011, at 09:27, Jan Kiszka wrote:

> On 2011-06-21 09:26, Alexander Graf wrote:
>> 
>> On 21.06.2011, at 09:23, Jan Kiszka wrote:
>> 
>>> On 2011-06-21 09:21, Alexander Graf wrote:
>>>> 
>>>> On 21.06.2011, at 08:26, Jan Kiszka wrote:
>>>> 
>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>> 
>>>>> Introduce CONFIG_XEN_BACKEND so that this new config solely controls the
>>>>> target-independent backend build and CONFIG_XEN can focus on per-target
>>>>> building.
>>>> 
>>>> What's the incentive here? I very much doubt we'll ever want to support xen on !x86. In fact, the better solution would probably be to just move every build component to x86 specific code.
>>> 
>>> I doubt that as well. But keeping the backend centrals prevent building
>>> them twice (for i386 and x86_64).
>> 
>> How can we not build them twice without risking that target_ulong breaks somewhere along the way?
> 
> Because the backends don't depend on that. And we already build them
> only once, your your logs. :)

So what does the patch change then if it's already only built once?


Alex

Re: [Qemu-devel] [PATCH 1/4] xen: Clean up build system

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1333 bytes --]

On 2011-06-21 09:46, Alexander Graf wrote:
> 
> On 21.06.2011, at 09:27, Jan Kiszka wrote:
> 
>> On 2011-06-21 09:26, Alexander Graf wrote:
>>>
>>> On 21.06.2011, at 09:23, Jan Kiszka wrote:
>>>
>>>> On 2011-06-21 09:21, Alexander Graf wrote:
>>>>>
>>>>> On 21.06.2011, at 08:26, Jan Kiszka wrote:
>>>>>
>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>
>>>>>> Introduce CONFIG_XEN_BACKEND so that this new config solely controls the
>>>>>> target-independent backend build and CONFIG_XEN can focus on per-target
>>>>>> building.
>>>>>
>>>>> What's the incentive here? I very much doubt we'll ever want to support xen on !x86. In fact, the better solution would probably be to just move every build component to x86 specific code.
>>>>
>>>> I doubt that as well. But keeping the backend centrals prevent building
>>>> them twice (for i386 and x86_64).
>>>
>>> How can we not build them twice without risking that target_ulong breaks somewhere along the way?
>>
>> Because the backends don't depend on that. And we already build them
>> only once, your your logs. :)
> 
> So what does the patch change then if it's already only built once?

It allows to clean up Makefile.target like I did as CONFIG_XEN is no
longer defined for all targets but only for those that actually support Xen.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [Qemu-devel] [PATCH 1/4] xen: Clean up build system

[Alexander Graf]

On 21.06.2011, at 09:57, Jan Kiszka wrote:

> On 2011-06-21 09:46, Alexander Graf wrote:
>> 
>> On 21.06.2011, at 09:27, Jan Kiszka wrote:
>> 
>>> On 2011-06-21 09:26, Alexander Graf wrote:
>>>> 
>>>> On 21.06.2011, at 09:23, Jan Kiszka wrote:
>>>> 
>>>>> On 2011-06-21 09:21, Alexander Graf wrote:
>>>>>> 
>>>>>> On 21.06.2011, at 08:26, Jan Kiszka wrote:
>>>>>> 
>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>> 
>>>>>>> Introduce CONFIG_XEN_BACKEND so that this new config solely controls the
>>>>>>> target-independent backend build and CONFIG_XEN can focus on per-target
>>>>>>> building.
>>>>>> 
>>>>>> What's the incentive here? I very much doubt we'll ever want to support xen on !x86. In fact, the better solution would probably be to just move every build component to x86 specific code.
>>>>> 
>>>>> I doubt that as well. But keeping the backend centrals prevent building
>>>>> them twice (for i386 and x86_64).
>>>> 
>>>> How can we not build them twice without risking that target_ulong breaks somewhere along the way?
>>> 
>>> Because the backends don't depend on that. And we already build them
>>> only once, your your logs. :)
>> 
>> So what does the patch change then if it's already only built once?
> 
> It allows to clean up Makefile.target like I did as CONFIG_XEN is no
> longer defined for all targets but only for those that actually support Xen.

Seems to make sense. Let's hope it doesn't break anything :)


Alex

[Qemu-devel] [Xen-devel] Re: [PATCH 1/4] xen: Clean up build system

[Ian Jackson]

Alexander Graf writes ("[Xen-devel] Re: [PATCH 1/4] xen: Clean up build system"):
> On 21.06.2011, at 08:26, Jan Kiszka wrote:
> > From: Jan Kiszka <jan.kiszka@siemens.com>
> > 
> > Introduce CONFIG_XEN_BACKEND so that this new config solely controls the
> > target-independent backend build and CONFIG_XEN can focus on per-target
> > building.
> 
> What's the incentive here? I very much doubt we'll ever want to
> support xen on !x86. In fact, the better solution would probably be
> to just move every build component to x86 specific code.

In the past Xen has supported ppc too and there is a half-congealed
arm port.  It's not a good assumption that Xen is x86-specific.

Ian.

Re: [Qemu-devel] [Xen-devel] Re: [PATCH 1/4] xen: Clean up build system

[Jan Kiszka]

On 2011-06-21 15:01, Ian Jackson wrote:
> Alexander Graf writes ("[Xen-devel] Re: [PATCH 1/4] xen: Clean up build system"):
>> On 21.06.2011, at 08:26, Jan Kiszka wrote:
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> Introduce CONFIG_XEN_BACKEND so that this new config solely controls the
>>> target-independent backend build and CONFIG_XEN can focus on per-target
>>> building.
>>
>> What's the incentive here? I very much doubt we'll ever want to
>> support xen on !x86. In fact, the better solution would probably be
>> to just move every build component to x86 specific code.
> 
> In the past Xen has supported ppc too and there is a half-congealed
> arm port.  It's not a good assumption that Xen is x86-specific.

We aren't setting x86 dependency into stone here, we are just trying to
keep support for what is there today clean and simple. The build system
can become more advanced again once there is support for modern ARM or
PowerPC. But until other Xen archs materialize into a usable form, it's
futile to design for their past or possible future requirements. KVM is
also no longer considering IA64 when cleaning up.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [Xen-devel] Re: [PATCH 1/4] xen: Clean up build system

[Alexander Graf]

On 21.06.2011, at 15:01, Ian Jackson wrote:

> Alexander Graf writes ("[Xen-devel] Re: [PATCH 1/4] xen: Clean up build system"):
>> On 21.06.2011, at 08:26, Jan Kiszka wrote:
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>> 
>>> Introduce CONFIG_XEN_BACKEND so that this new config solely controls the
>>> target-independent backend build and CONFIG_XEN can focus on per-target
>>> building.
>> 
>> What's the incentive here? I very much doubt we'll ever want to
>> support xen on !x86. In fact, the better solution would probably be
>> to just move every build component to x86 specific code.
> 
> In the past Xen has supported ppc too and there is a half-congealed
> arm port.  It's not a good assumption that Xen is x86-specific.

Well, is there a realistic chance that we'll ever get non-x86 Xen code into upstream Qemu? I don't know how well the arm port's been doing, but the PPC one is long dead and was always just a toy. I'm fairly sure nobody really cares about the IA64 port either these days.


Alex

Re: [Qemu-devel] [Xen-devel] Re: [PATCH 1/4] xen: Clean up build system

[Stefano Stabellini]

On Tue, 21 Jun 2011, Alexander Graf wrote:
> On 21.06.2011, at 15:01, Ian Jackson wrote:
> 
> > Alexander Graf writes ("[Xen-devel] Re: [PATCH 1/4] xen: Clean up build system"):
> >> On 21.06.2011, at 08:26, Jan Kiszka wrote:
> >>> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>> 
> >>> Introduce CONFIG_XEN_BACKEND so that this new config solely controls the
> >>> target-independent backend build and CONFIG_XEN can focus on per-target
> >>> building.
> >> 
> >> What's the incentive here? I very much doubt we'll ever want to
> >> support xen on !x86. In fact, the better solution would probably be
> >> to just move every build component to x86 specific code.
> > 
> > In the past Xen has supported ppc too and there is a half-congealed
> > arm port.  It's not a good assumption that Xen is x86-specific.
> 
> Well, is there a realistic chance that we'll ever get non-x86 Xen code into upstream Qemu? I don't know how well the arm port's been doing, but the PPC one is long dead and was always just a toy. I'm fairly sure nobody really cares about the IA64 port either these days.

There is a realistic chance we'll get ARM Xen code into upstream Qemu.
That said, I don't think this patch would prevent it from happening.
Also I tested it and it works OK, so it is fine by me.

[Qemu-devel] [PATCH 2/4] xen: Uninline xen_unmap_block

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

No reason to inline this helper. Also, cleaning up the header unbreaks
non-UNIX building where sys/mman.h is not available.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 exec.c              |    1 +
 xen-mapcache-stub.c |    4 ++++
 xen-mapcache.c      |    9 +++++++++
 xen-mapcache.h      |   14 +-------------
 4 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/exec.c b/exec.c
index 09928a3..41ba883 100644
--- a/exec.c
+++ b/exec.c
@@ -34,6 +34,7 @@
 #include "kvm.h"
 #include "hw/xen.h"
 #include "qemu-timer.h"
+#include "trace.h"
 #if defined(CONFIG_USER_ONLY)
 #include <qemu.h>
 #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
diff --git a/xen-mapcache-stub.c b/xen-mapcache-stub.c
index 7c14b3d..dfecbdd 100644
--- a/xen-mapcache-stub.c
+++ b/xen-mapcache-stub.c
@@ -42,3 +42,7 @@ uint8_t *xen_map_block(target_phys_addr_t phys_addr, target_phys_addr_t size)
 {
     return NULL;
 }
+
+void xen_unmap_block(void *addr, ram_addr_t size)
+{
+}
diff --git a/xen-mapcache.c b/xen-mapcache.c
index 349cc62..4c663e6 100644
--- a/xen-mapcache.c
+++ b/xen-mapcache.c
@@ -373,3 +373,12 @@ uint8_t *xen_map_block(target_phys_addr_t phys_addr, target_phys_addr_t size)
 
     return vaddr_base;
 }
+
+void xen_unmap_block(void *addr, ram_addr_t size)
+{
+    trace_xen_unmap_block(addr, size);
+
+    if (munmap(addr, size) != 0) {
+        hw_error("xen_unmap_block: %s", strerror(errno));
+    }
+}
diff --git a/xen-mapcache.h b/xen-mapcache.h
index 339444c..f7b4c04 100644
--- a/xen-mapcache.h
+++ b/xen-mapcache.h
@@ -9,9 +9,6 @@
 #ifndef XEN_MAPCACHE_H
 #define XEN_MAPCACHE_H
 
-#include <sys/mman.h>
-#include "trace.h"
-
 void     qemu_map_cache_init(void);
 uint8_t  *qemu_map_cache(target_phys_addr_t phys_addr, target_phys_addr_t size, uint8_t lock);
 void     qemu_map_cache_unlock(void *phys_addr);
@@ -20,16 +17,7 @@ void     qemu_invalidate_entry(uint8_t *buffer);
 void     qemu_invalidate_map_cache(void);
 
 uint8_t *xen_map_block(target_phys_addr_t phys_addr, target_phys_addr_t size);
-
-static inline void xen_unmap_block(void *addr, ram_addr_t size)
-{
-    trace_xen_unmap_block(addr, size);
-
-    if (munmap(addr, size) != 0) {
-        hw_error("xen_unmap_block: %s", strerror(errno));
-    }
-}
-
+void xen_unmap_block(void *addr, ram_addr_t size);
 
 #define mapcache_lock()   ((void)0)
 #define mapcache_unlock() ((void)0)
-- 
1.7.1

Re: [Qemu-devel] [PATCH 2/4] xen: Uninline xen_unmap_block

[Alexander Graf]

On 21.06.2011, at 08:26, Jan Kiszka wrote:

> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> No reason to inline this helper. Also, cleaning up the header unbreaks
> non-UNIX building where sys/mman.h is not available.

Looks good to me. Stefano, please ack.

Alex

Re: [Qemu-devel] [PATCH 2/4] xen: Uninline xen_unmap_block

[Stefano Stabellini]

On Tue, 21 Jun 2011, Alexander Graf wrote:
> On 21.06.2011, at 08:26, Jan Kiszka wrote:
> 
> > From: Jan Kiszka <jan.kiszka@siemens.com>
> > 
> > No reason to inline this helper. Also, cleaning up the header unbreaks
> > non-UNIX building where sys/mman.h is not available.
> 
> Looks good to me. Stefano, please ack.
 

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

[Qemu-devel] [PATCH 3/4] xen: Clean up map cache API naming

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

The map cache is a Xen thing, so its API should make this clear.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 exec.c              |   10 +++++-----
 trace-events        |    6 +++---
 xen-all.c           |    4 ++--
 xen-mapcache-stub.c |   13 +++++++------
 xen-mapcache.c      |   43 ++++++++++++++++++++++++-------------------
 xen-mapcache.h      |   16 +++++++---------
 6 files changed, 48 insertions(+), 44 deletions(-)

diff --git a/exec.c b/exec.c
index 41ba883..91751c5 100644
--- a/exec.c
+++ b/exec.c
@@ -2990,7 +2990,7 @@ void qemu_ram_free(ram_addr_t addr)
                 munmap(block->host, block->length);
 #else
                 if (xen_mapcache_enabled()) {
-                    qemu_invalidate_entry(block->host);
+                    xen_invalidate_map_cache_entry(block->host);
                 } else {
                     qemu_vfree(block->host);
                 }
@@ -3087,7 +3087,7 @@ void *qemu_get_ram_ptr(ram_addr_t addr)
                  * because we don't want to map the entire memory in QEMU.
                  */
                 if (block->offset == 0) {
-                    return qemu_map_cache(addr, 0, 1);
+                    return xen_map_cache(addr, 0, 1);
                 } else if (block->host == NULL) {
                     block->host = xen_map_block(block->offset, block->length);
                 }
@@ -3116,7 +3116,7 @@ void *qemu_safe_ram_ptr(ram_addr_t addr)
                  * because we don't want to map the entire memory in QEMU.
                  */
                 if (block->offset == 0) {
-                    return qemu_map_cache(addr, 0, 1);
+                    return xen_map_cache(addr, 0, 1);
                 } else if (block->host == NULL) {
                     block->host = xen_map_block(block->offset, block->length);
                 }
@@ -3147,7 +3147,7 @@ void qemu_put_ram_ptr(void *addr)
             xen_unmap_block(block->host, block->length);
             block->host = NULL;
         } else {
-            qemu_map_cache_unlock(addr);
+            xen_map_cache_unlock(addr);
         }
     }
 }
@@ -3169,7 +3169,7 @@ int qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr)
     }
 
     if (xen_mapcache_enabled()) {
-        *ram_addr = qemu_ram_addr_from_mapcache(ptr);
+        *ram_addr = xen_ram_addr_from_mapcache(ptr);
         return 0;
     }
 
diff --git a/trace-events b/trace-events
index f1230f1..a2839dd 100644
--- a/trace-events
+++ b/trace-events
@@ -398,9 +398,9 @@ disable milkymist_vgafb_memory_write(uint32_t addr, uint32_t value) "addr %08x v
 disable xen_ram_alloc(unsigned long ram_addr, unsigned long size) "requested: %#lx, size %#lx"
 
 # xen-mapcache.c
-disable qemu_map_cache(uint64_t phys_addr) "want %#"PRIx64""
-disable qemu_remap_bucket(uint64_t index) "index %#"PRIx64""
-disable qemu_map_cache_return(void* ptr) "%p"
+disable xen_map_cache(uint64_t phys_addr) "want %#"PRIx64""
+disable xen_remap_bucket(uint64_t index) "index %#"PRIx64""
+disable xen_map_cache_return(void* ptr) "%p"
 disable xen_map_block(uint64_t phys_addr, uint64_t size) "%#"PRIx64", size %#"PRIx64""
 disable xen_unmap_block(void* addr, unsigned long size) "%p, size %#lx"
 
diff --git a/xen-all.c b/xen-all.c
index 0eac202..7346751 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -368,7 +368,7 @@ static void handle_ioreq(ioreq_t *req)
         case IOREQ_TYPE_TIMEOFFSET:
             break;
         case IOREQ_TYPE_INVALIDATE:
-            qemu_invalidate_map_cache();
+            xen_invalidate_map_cache();
             break;
         default:
             hw_error("Invalid ioreq type 0x%x\n", req->type);
@@ -576,7 +576,7 @@ int xen_hvm_init(void)
     }
 
     /* Init RAM management */
-    qemu_map_cache_init();
+    xen_map_cache_init();
     xen_ram_init(ram_size);
 
     qemu_add_vm_change_state_handler(xen_vm_change_state_handler, state);
diff --git a/xen-mapcache-stub.c b/xen-mapcache-stub.c
index dfecbdd..6ee0dc8 100644
--- a/xen-mapcache-stub.c
+++ b/xen-mapcache-stub.c
@@ -13,29 +13,30 @@
 #include "cpu-common.h"
 #include "xen-mapcache.h"
 
-void qemu_map_cache_init(void)
+void xen_map_cache_init(void)
 {
 }
 
-uint8_t *qemu_map_cache(target_phys_addr_t phys_addr, target_phys_addr_t size, uint8_t lock)
+uint8_t *xen_map_cache(target_phys_addr_t phys_addr, target_phys_addr_t size,
+                       uint8_t lock)
 {
     return qemu_get_ram_ptr(phys_addr);
 }
 
-void qemu_map_cache_unlock(void *buffer)
+void xen_map_cache_unlock(void *buffer)
 {
 }
 
-ram_addr_t qemu_ram_addr_from_mapcache(void *ptr)
+ram_addr_t xen_ram_addr_from_mapcache(void *ptr)
 {
     return -1;
 }
 
-void qemu_invalidate_map_cache(void)
+void xen_invalidate_map_cache(void)
 {
 }
 
-void qemu_invalidate_entry(uint8_t *buffer)
+void xen_invalidate_map_cache_entry(uint8_t *buffer)
 {
 }
 uint8_t *xen_map_block(target_phys_addr_t phys_addr, target_phys_addr_t size)
diff --git a/xen-mapcache.c b/xen-mapcache.c
index 4c663e6..e823246 100644
--- a/xen-mapcache.c
+++ b/xen-mapcache.c
@@ -40,6 +40,9 @@
 #endif
 #define MCACHE_BUCKET_SIZE (1UL << MCACHE_BUCKET_SHIFT)
 
+#define mapcache_lock()   ((void)0)
+#define mapcache_unlock() ((void)0)
+
 typedef struct MapCacheEntry {
     target_phys_addr_t paddr_index;
     uint8_t *vaddr_base;
@@ -68,7 +71,7 @@ typedef struct MapCache {
 
 static MapCache *mapcache;
 
-void qemu_map_cache_init(void)
+void xen_map_cache_init(void)
 {
     unsigned long size;
     struct rlimit rlimit_as;
@@ -95,13 +98,14 @@ void qemu_map_cache_init(void)
 
     size = mapcache->nr_buckets * sizeof (MapCacheEntry);
     size = (size + XC_PAGE_SIZE - 1) & ~(XC_PAGE_SIZE - 1);
-    DPRINTF("qemu_map_cache_init, nr_buckets = %lx size %lu\n", mapcache->nr_buckets, size);
+    DPRINTF("%s, nr_buckets = %lx size %lu\n", __func__,
+            mapcache->nr_buckets, size);
     mapcache->entry = qemu_mallocz(size);
 }
 
-static void qemu_remap_bucket(MapCacheEntry *entry,
-                              target_phys_addr_t size,
-                              target_phys_addr_t address_index)
+static void xen_remap_bucket(MapCacheEntry *entry,
+                             target_phys_addr_t size,
+                             target_phys_addr_t address_index)
 {
     uint8_t *vaddr_base;
     xen_pfn_t *pfns;
@@ -109,7 +113,7 @@ static void qemu_remap_bucket(MapCacheEntry *entry,
     unsigned int i;
     target_phys_addr_t nb_pfn = size >> XC_PAGE_SHIFT;
 
-    trace_qemu_remap_bucket(address_index);
+    trace_xen_remap_bucket(address_index);
 
     pfns = qemu_mallocz(nb_pfn * sizeof (xen_pfn_t));
     err = qemu_mallocz(nb_pfn * sizeof (int));
@@ -146,16 +150,17 @@ static void qemu_remap_bucket(MapCacheEntry *entry,
     qemu_free(err);
 }
 
-uint8_t *qemu_map_cache(target_phys_addr_t phys_addr, target_phys_addr_t size, uint8_t lock)
+uint8_t *xen_map_cache(target_phys_addr_t phys_addr, target_phys_addr_t size,
+                       uint8_t lock)
 {
     MapCacheEntry *entry, *pentry = NULL;
     target_phys_addr_t address_index  = phys_addr >> MCACHE_BUCKET_SHIFT;
     target_phys_addr_t address_offset = phys_addr & (MCACHE_BUCKET_SIZE - 1);
 
-    trace_qemu_map_cache(phys_addr);
+    trace_xen_map_cache(phys_addr);
 
     if (address_index == mapcache->last_address_index && !lock) {
-        trace_qemu_map_cache_return(mapcache->last_address_vaddr + address_offset);
+        trace_xen_map_cache_return(mapcache->last_address_vaddr + address_offset);
         return mapcache->last_address_vaddr + address_offset;
     }
 
@@ -168,17 +173,17 @@ uint8_t *qemu_map_cache(target_phys_addr_t phys_addr, target_phys_addr_t size, u
     if (!entry) {
         entry = qemu_mallocz(sizeof (MapCacheEntry));
         pentry->next = entry;
-        qemu_remap_bucket(entry, size ? : MCACHE_BUCKET_SIZE, address_index);
+        xen_remap_bucket(entry, size ? : MCACHE_BUCKET_SIZE, address_index);
     } else if (!entry->lock) {
         if (!entry->vaddr_base || entry->paddr_index != address_index ||
             !test_bit(address_offset >> XC_PAGE_SHIFT, entry->valid_mapping)) {
-            qemu_remap_bucket(entry, size ? : MCACHE_BUCKET_SIZE, address_index);
+            xen_remap_bucket(entry, size ? : MCACHE_BUCKET_SIZE, address_index);
         }
     }
 
     if (!test_bit(address_offset >> XC_PAGE_SHIFT, entry->valid_mapping)) {
         mapcache->last_address_index = -1;
-        trace_qemu_map_cache_return(NULL);
+        trace_xen_map_cache_return(NULL);
         return NULL;
     }
 
@@ -192,11 +197,11 @@ uint8_t *qemu_map_cache(target_phys_addr_t phys_addr, target_phys_addr_t size, u
         QTAILQ_INSERT_HEAD(&mapcache->locked_entries, reventry, next);
     }
 
-    trace_qemu_map_cache_return(mapcache->last_address_vaddr + address_offset);
+    trace_xen_map_cache_return(mapcache->last_address_vaddr + address_offset);
     return mapcache->last_address_vaddr + address_offset;
 }
 
-void qemu_map_cache_unlock(void *buffer)
+void xen_map_cache_unlock(void *buffer)
 {
     MapCacheEntry *entry = NULL, *pentry = NULL;
     MapCacheRev *reventry;
@@ -229,7 +234,7 @@ void qemu_map_cache_unlock(void *buffer)
     }
 }
 
-ram_addr_t qemu_ram_addr_from_mapcache(void *ptr)
+ram_addr_t xen_ram_addr_from_mapcache(void *ptr)
 {
     MapCacheRev *reventry;
     target_phys_addr_t paddr_index;
@@ -243,7 +248,7 @@ ram_addr_t qemu_ram_addr_from_mapcache(void *ptr)
         }
     }
     if (!found) {
-        fprintf(stderr, "qemu_ram_addr_from_mapcache, could not find %p\n", ptr);
+        fprintf(stderr, "%s, could not find %p\n", __func__, ptr);
         QTAILQ_FOREACH(reventry, &mapcache->locked_entries, next) {
             DPRINTF("   "TARGET_FMT_plx" -> %p is present\n", reventry->paddr_index,
                     reventry->vaddr_req);
@@ -255,7 +260,7 @@ ram_addr_t qemu_ram_addr_from_mapcache(void *ptr)
     return paddr_index << MCACHE_BUCKET_SHIFT;
 }
 
-void qemu_invalidate_entry(uint8_t *buffer)
+void xen_invalidate_map_cache_entry(uint8_t *buffer)
 {
     MapCacheEntry *entry = NULL, *pentry = NULL;
     MapCacheRev *reventry;
@@ -274,7 +279,7 @@ void qemu_invalidate_entry(uint8_t *buffer)
         }
     }
     if (!found) {
-        DPRINTF("qemu_invalidate_entry, could not find %p\n", buffer);
+        DPRINTF("%s, could not find %p\n", __func__, buffer);
         QTAILQ_FOREACH(reventry, &mapcache->locked_entries, next) {
             DPRINTF("   "TARGET_FMT_plx" -> %p is present\n", reventry->paddr_index, reventry->vaddr_req);
         }
@@ -305,7 +310,7 @@ void qemu_invalidate_entry(uint8_t *buffer)
     qemu_free(entry);
 }
 
-void qemu_invalidate_map_cache(void)
+void xen_invalidate_map_cache(void)
 {
     unsigned long i;
     MapCacheRev *reventry;
diff --git a/xen-mapcache.h b/xen-mapcache.h
index f7b4c04..b46c971 100644
--- a/xen-mapcache.h
+++ b/xen-mapcache.h
@@ -9,17 +9,15 @@
 #ifndef XEN_MAPCACHE_H
 #define XEN_MAPCACHE_H
 
-void     qemu_map_cache_init(void);
-uint8_t  *qemu_map_cache(target_phys_addr_t phys_addr, target_phys_addr_t size, uint8_t lock);
-void     qemu_map_cache_unlock(void *phys_addr);
-ram_addr_t qemu_ram_addr_from_mapcache(void *ptr);
-void     qemu_invalidate_entry(uint8_t *buffer);
-void     qemu_invalidate_map_cache(void);
+void xen_map_cache_init(void);
+uint8_t *xen_map_cache(target_phys_addr_t phys_addr, target_phys_addr_t size,
+                       uint8_t lock);
+void xen_map_cache_unlock(void *phys_addr);
+ram_addr_t xen_ram_addr_from_mapcache(void *ptr);
+void xen_invalidate_map_cache_entry(uint8_t *buffer);
+void xen_invalidate_map_cache(void);
 
 uint8_t *xen_map_block(target_phys_addr_t phys_addr, target_phys_addr_t size);
 void xen_unmap_block(void *addr, ram_addr_t size);
 
-#define mapcache_lock()   ((void)0)
-#define mapcache_unlock() ((void)0)
-
 #endif /* !XEN_MAPCACHE_H */
-- 
1.7.1

Re: [Qemu-devel] [PATCH 3/4] xen: Clean up map cache API naming

[Alexander Graf]

On 21.06.2011, at 08:26, Jan Kiszka wrote:

> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> The map cache is a Xen thing, so its API should make this clear.

Also looks good. Stefano, please ack.

Alex

Re: [Qemu-devel] [PATCH 3/4] xen: Clean up map cache API naming

[Stefano Stabellini]

On Tue, 21 Jun 2011, Alexander Graf wrote:
> On 21.06.2011, at 08:26, Jan Kiszka wrote:
> 
> > From: Jan Kiszka <jan.kiszka@siemens.com>
> > 
> > The map cache is a Xen thing, so its API should make this clear.
> 
> Also looks good. Stefano, please ack.
 
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

[Qemu-devel] [PATCH 4/4] xen: Fold CONFIG_XEN_MAPCACHE into CONFIG_XEN

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Xen won't be enabled if there is no backend support available for the
host. And that also means the map cache will work. So drop the separate
config switch and move the required stubs over to xen-stub.c.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 Makefile.target     |    5 +----
 configure           |    3 ---
 exec.c              |   14 +++++++-------
 hw/xen.h            |    9 ---------
 xen-mapcache-stub.c |   49 -------------------------------------------------
 xen-stub.c          |   29 +++++++++++++++++++++++++++++
 6 files changed, 37 insertions(+), 72 deletions(-)
 delete mode 100644 xen-mapcache-stub.c

diff --git a/Makefile.target b/Makefile.target
index f325554..2507466 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -4,7 +4,6 @@ GENERATED_HEADERS = config-target.h
 CONFIG_NO_PCI = $(if $(subst n,,$(CONFIG_PCI)),n,y)
 CONFIG_NO_KVM = $(if $(subst n,,$(CONFIG_KVM)),n,y)
 CONFIG_NO_XEN = $(if $(subst n,,$(CONFIG_XEN)),n,y)
-CONFIG_NO_XEN_MAPCACHE = $(if $(subst n,,$(CONFIG_XEN_MAPCACHE)),n,y)
 
 include ../config-host.mak
 include config-devices.mak
@@ -206,10 +205,8 @@ QEMU_CFLAGS += $(VNC_JPEG_CFLAGS)
 QEMU_CFLAGS += $(VNC_PNG_CFLAGS)
 
 # xen support
-obj-$(CONFIG_XEN) += xen-all.o xen_machine_pv.o xen_domainbuild.o
+obj-$(CONFIG_XEN) += xen-all.o xen_machine_pv.o xen_domainbuild.o xen-mapcache.o
 obj-$(CONFIG_NO_XEN) += xen-stub.o
-obj-i386-$(CONFIG_XEN_MAPCACHE) += xen-mapcache.o
-obj-$(CONFIG_NO_XEN_MAPCACHE) += xen-mapcache-stub.o
 
 # Inter-VM PCI shared memory
 CONFIG_IVSHMEM =
diff --git a/configure b/configure
index 7991489..ef4453a 100755
--- a/configure
+++ b/configure
@@ -3329,9 +3329,6 @@ case "$target_arch2" in
     if test "$xen" = "yes" -a "$target_softmmu" = "yes" ; then
       target_phys_bits=64
       echo "CONFIG_XEN=y" >> $config_target_mak
-      if test "$cpu" = "i386" -o "$cpu" = "x86_64"; then
-          echo "CONFIG_XEN_MAPCACHE=y" >> $config_target_mak
-      fi
     fi
 esac
 case "$target_arch2" in
diff --git a/exec.c b/exec.c
index 91751c5..dd2faa3 100644
--- a/exec.c
+++ b/exec.c
@@ -2923,7 +2923,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name,
                 abort();
             }
 #else
-            if (xen_mapcache_enabled()) {
+            if (xen_enabled()) {
                 xen_ram_alloc(new_block->offset, size);
             } else {
                 new_block->host = qemu_vmalloc(size);
@@ -2989,7 +2989,7 @@ void qemu_ram_free(ram_addr_t addr)
 #if defined(TARGET_S390X) && defined(CONFIG_KVM)
                 munmap(block->host, block->length);
 #else
-                if (xen_mapcache_enabled()) {
+                if (xen_enabled()) {
                     xen_invalidate_map_cache_entry(block->host);
                 } else {
                     qemu_vfree(block->host);
@@ -3082,7 +3082,7 @@ void *qemu_get_ram_ptr(ram_addr_t addr)
                 QLIST_REMOVE(block, next);
                 QLIST_INSERT_HEAD(&ram_list.blocks, block, next);
             }
-            if (xen_mapcache_enabled()) {
+            if (xen_enabled()) {
                 /* We need to check if the requested address is in the RAM
                  * because we don't want to map the entire memory in QEMU.
                  */
@@ -3111,7 +3111,7 @@ void *qemu_safe_ram_ptr(ram_addr_t addr)
 
     QLIST_FOREACH(block, &ram_list.blocks, next) {
         if (addr - block->offset < block->length) {
-            if (xen_mapcache_enabled()) {
+            if (xen_enabled()) {
                 /* We need to check if the requested address is in the RAM
                  * because we don't want to map the entire memory in QEMU.
                  */
@@ -3135,7 +3135,7 @@ void qemu_put_ram_ptr(void *addr)
 {
     trace_qemu_put_ram_ptr(addr);
 
-    if (xen_mapcache_enabled()) {
+    if (xen_enabled()) {
         RAMBlock *block;
 
         QLIST_FOREACH(block, &ram_list.blocks, next) {
@@ -3168,7 +3168,7 @@ int qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr)
         }
     }
 
-    if (xen_mapcache_enabled()) {
+    if (xen_enabled()) {
         *ram_addr = xen_ram_addr_from_mapcache(ptr);
         return 0;
     }
@@ -4080,7 +4080,7 @@ void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len,
                 access_len -= l;
             }
         }
-        if (xen_mapcache_enabled()) {
+        if (xen_enabled()) {
             uint8_t *buffer1 = buffer;
             uint8_t *end_buffer = buffer + len;
 
diff --git a/hw/xen.h b/hw/xen.h
index d435ca0..95029bb 100644
--- a/hw/xen.h
+++ b/hw/xen.h
@@ -31,15 +31,6 @@ static inline int xen_enabled(void)
 #endif
 }
 
-static inline int xen_mapcache_enabled(void)
-{
-#ifdef CONFIG_XEN_MAPCACHE
-    return xen_enabled();
-#else
-    return 0;
-#endif
-}
-
 int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
 void xen_piix3_set_irq(void *opaque, int irq_num, int level);
 void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len);
diff --git a/xen-mapcache-stub.c b/xen-mapcache-stub.c
deleted file mode 100644
index 6ee0dc8..0000000
--- a/xen-mapcache-stub.c
+++ /dev/null
@@ -1,49 +0,0 @@
-/*
- * Copyright (C) 2011       Citrix Ltd.
- *
- * This work is licensed under the terms of the GNU GPL, version 2.  See
- * the COPYING file in the top-level directory.
- *
- */
-
-#include "config.h"
-
-#include "exec-all.h"
-#include "qemu-common.h"
-#include "cpu-common.h"
-#include "xen-mapcache.h"
-
-void xen_map_cache_init(void)
-{
-}
-
-uint8_t *xen_map_cache(target_phys_addr_t phys_addr, target_phys_addr_t size,
-                       uint8_t lock)
-{
-    return qemu_get_ram_ptr(phys_addr);
-}
-
-void xen_map_cache_unlock(void *buffer)
-{
-}
-
-ram_addr_t xen_ram_addr_from_mapcache(void *ptr)
-{
-    return -1;
-}
-
-void xen_invalidate_map_cache(void)
-{
-}
-
-void xen_invalidate_map_cache_entry(uint8_t *buffer)
-{
-}
-uint8_t *xen_map_block(target_phys_addr_t phys_addr, target_phys_addr_t size)
-{
-    return NULL;
-}
-
-void xen_unmap_block(void *addr, ram_addr_t size)
-{
-}
diff --git a/xen-stub.c b/xen-stub.c
index a4f35a1..d38b41a 100644
--- a/xen-stub.c
+++ b/xen-stub.c
@@ -8,6 +8,7 @@
 
 #include "qemu-common.h"
 #include "hw/xen.h"
+#include "xen-mapcache.h"
 
 int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
 {
@@ -39,3 +40,31 @@ int xen_init(void)
 {
     return -ENOSYS;
 }
+
+uint8_t *xen_map_cache(target_phys_addr_t phys_addr, target_phys_addr_t size,
+                       uint8_t lock)
+{
+    return NULL;
+}
+
+void xen_map_cache_unlock(void *buffer)
+{
+}
+
+ram_addr_t xen_ram_addr_from_mapcache(void *ptr)
+{
+    return -1;
+}
+
+void xen_invalidate_map_cache_entry(uint8_t *buffer)
+{
+}
+
+uint8_t *xen_map_block(target_phys_addr_t phys_addr, target_phys_addr_t size)
+{
+    return NULL;
+}
+
+void xen_unmap_block(void *addr, ram_addr_t size)
+{
+}
-- 
1.7.1

Re: [Qemu-devel] [PATCH 4/4] xen: Fold CONFIG_XEN_MAPCACHE into CONFIG_XEN

[Alexander Graf]

On 21.06.2011, at 08:26, Jan Kiszka wrote:

> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Xen won't be enabled if there is no backend support available for the
> host. And that also means the map cache will work. So drop the separate
> config switch and move the required stubs over to xen-stub.c.

Anthony, Stefano, what was the reason for the separate config option?


Alex

Re: [Qemu-devel] [PATCH 4/4] xen: Fold CONFIG_XEN_MAPCACHE into CONFIG_XEN

[Stefano Stabellini]

On Tue, 21 Jun 2011, Alexander Graf wrote:
> On 21.06.2011, at 08:26, Jan Kiszka wrote:
> 
> > From: Jan Kiszka <jan.kiszka@siemens.com>
> > 
> > Xen won't be enabled if there is no backend support available for the
> > host. And that also means the map cache will work. So drop the separate
> > config switch and move the required stubs over to xen-stub.c.
> 
> Anthony, Stefano, what was the reason for the separate config option?

Even though I think it is nice to be able to completely disable the
mapcache from a compile time option, in practice we don't use it and we
don't even have it in qemu-xen.
Also I am pretty sure that disabling the mapcache at the moment would
prevent qemu from working on xen completely.
So I think it is OK to remove the option.

Re: [Qemu-devel] [PATCH 0/4] Xen cleanups

[Stefano Stabellini]

On Tue, 21 Jun 2011, Jan Kiszka wrote:
> I came across some Xen oddities I couldn't help to clean up. These
> patches
>  - simplify the build
>  - remove a win32 breakage from xen-mapcache.h
>  - move the map cache functions out of the generic name space
> 

thanks for the patches, cleanups are always welcome :)

Re: [Qemu-devel] [PATCH 0/4] Xen cleanups

[Alexander Graf]

[-- Attachment #1: Type: text/plain, Size: 370 bytes --]


On 21.06.2011, at 08:26, Jan Kiszka wrote:

> I came across some Xen oddities I couldn't help to clean up. These
> patches
> - simplify the build
> - remove a win32 breakage from xen-mapcache.h
> - move the map cache functions out of the generic name space

Patches 2-4 don't apply. Please rebase against xen-next at:

  git://repo.or.cz/qemu/agraf.git xen-next

Alex


[-- Attachment #2: Type: text/html, Size: 759 bytes --]