[PATCH 04/15] ia64/pv_ops: introduce pv_info which describes some random info.

[PATCH 04/15] ia64/pv_ops: introduce pv_info which describes some random info.

[Isaku Yamahata]

introduce pv_info which describes some randome info about
underlying execution environment.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 arch/ia64/kernel/Makefile   |    2 +
 arch/ia64/kernel/paravirt.c |   41 ++++++++++++++++++++++++++++
 include/asm-ia64/paravirt.h |   62 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 105 insertions(+), 0 deletions(-)
 create mode 100644 arch/ia64/kernel/paravirt.c
 create mode 100644 include/asm-ia64/paravirt.h

diff --git a/arch/ia64/kernel/Makefile b/arch/ia64/kernel/Makefile
index 33e5a59..40a10e7 100644
--- a/arch/ia64/kernel/Makefile
+++ b/arch/ia64/kernel/Makefile
@@ -36,6 +36,8 @@ obj-$(CONFIG_PCI_MSI)		+= msi_ia64.o
 mca_recovery-y			+= mca_drv.o mca_drv_asm.o
 obj-$(CONFIG_IA64_MC_ERR_INJECT)+= err_inject.o
 
+obj-$(CONFIG_PARAVIRT)		+= paravirt.o
+
 obj-$(CONFIG_IA64_ESI)		+= esi.o
 ifneq ($(CONFIG_IA64_ESI),)
 obj-y				+= esi_stub.o	# must be in kernel proper
diff --git a/arch/ia64/kernel/paravirt.c b/arch/ia64/kernel/paravirt.c
new file mode 100644
index 0000000..d295ea5
--- /dev/null
+++ b/arch/ia64/kernel/paravirt.c
@@ -0,0 +1,41 @@
+/******************************************************************************
+ * arch/ia64/kernel/paravirt.c
+ *
+ * Copyright (c) 2008 Isaku Yamahata <yamahata at valinux co jp>
+ *                    VA Linux Systems Japan K.K.
+ *     Yaozu (Eddie) Dong <eddie.dong@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ */
+
+#include <linux/init.h>
+
+#include <linux/compiler.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/types.h>
+
+#include <asm/iosapic.h>
+#include <asm/paravirt.h>
+
+/***************************************************************************
+ * general info
+ */
+struct pv_info pv_info = {
+	.kernel_rpl = 0,
+	.paravirt_enabled = 0,
+	.name = "bare hardware"
+};
diff --git a/include/asm-ia64/paravirt.h b/include/asm-ia64/paravirt.h
new file mode 100644
index 0000000..26b4334
--- /dev/null
+++ b/include/asm-ia64/paravirt.h
@@ -0,0 +1,62 @@
+/******************************************************************************
+ * include/asm-ia64/paravirt.h
+ *
+ * Copyright (c) 2008 Isaku Yamahata <yamahata at valinux co jp>
+ *                    VA Linux Systems Japan K.K.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ */
+
+
+#ifndef __ASM_PARAVIRT_H
+#define __ASM_PARAVIRT_H
+
+#ifdef CONFIG_PARAVIRT_GUEST
+
+#ifndef __ASSEMBLY__
+
+#include <asm/hw_irq.h>
+#include <asm/meminit.h>
+
+/******************************************************************************
+ * general info
+ */
+struct pv_info {
+	unsigned int kernel_rpl;
+	int paravirt_enabled;
+	const char *name;
+};
+
+extern struct pv_info pv_info;
+
+static inline int paravirt_enabled(void)
+{
+	return pv_info.paravirt_enabled;
+}
+
+static inline unsigned int get_kernel_rpl(void)
+{
+	return pv_info.kernel_rpl;
+}
+
+#endif /* !__ASSEMBLY__ */
+
+#else
+/* fallback for native case */
+
+#endif /* CONFIG_PARAVIRT_GUEST */
+
+#endif /* __ASM_PARAVIRT_H */
-- 
1.5.3

Re: [PATCH 04/15] ia64/pv_ops: introduce pv_info which describes some random info.

[Jes Sorensen]

>>>>> "Isaku" = Isaku Yamahata <yamahata@valinux.co.jp> writes:

Isaku> introduce pv_info which describes some randome info about
Isaku> underlying execution environment.

Hi Isaku,

I am missing some of the patches in this series for some reason.
However, I'd like to open the discussion and ask why you are
implementing the pv-ops seperately instead of adding them to the
machine vectors and using the machine vector interface to handle the
direct implementation?

It seems to me that pvops really are just an advanced level of the
ia64 machine vectors and building on top of the interface we already
have would make it easier to support different machine types through
pvops.

Cheers,
Jes

Re: [PATCH 04/15] ia64/pv_ops: introduce pv_info which describes some random info.

[Isaku Yamahata]

On Tue, Apr 22, 2008 at 05:16:41AM -0400, Jes Sorensen wrote:
> >>>>> "Isaku" = Isaku Yamahata <yamahata@valinux.co.jp> writes:
> 
> Isaku> introduce pv_info which describes some randome info about
> Isaku> underlying execution environment.
> 
> Hi Isaku,

Hi. I'm very glad to get reply.


> I am missing some of the patches in this series for some reason.
> However, I'd like to open the discussion and ask why you are
> implementing the pv-ops seperately instead of adding them to the
> machine vectors and using the machine vector interface to handle the
> direct implementation?
>
> It seems to me that pvops really are just an advanced level of the
> ia64 machine vectors and building on top of the interface we already
> have would make it easier to support different machine types through
> pvops.

Okay. Your question is quite reasonable and very good one.
In fact the current xen domU implementation is using both 
pv_ops and machine vector because it simplifies the code.
pv_ops somewhat overlaps with machine vector.

Our justification is as follows.
The difference is its scope. pv_ops for virtualization and
machine vector is for platform difference.

- pv_ops does cover the area which shouldn't belong to machine vector.
  For example, ia64 intrinsics paravirtualization.
  It shouldn't belong to the machine vector.
  It must be initialized very early before platform detection.

- pv_ops covers some performance critical part (e.g. ia64 intrinsics)
  so that in the future they should be optimized with binary patch like x86.
  We had the experimental patch to do that, but they are dropped for
  the merge. It reduced patch size greatly.
  After merging the first patch series, we're planning to optimize
  pv_ops with binary patch.
  The optimization with binary patch is out of the machine vector scope.

- The current pv_ops implements only one for only domU, but in future
  pv_ops will support dom0. It means dom0 linux would run with
  the underlying machine vector + pv_ops, i.e.
  {dig, hpzx1, hpzx1_swiotlb, ...} machine vector + xen pv_ops

Probably some hooks of pv_ops could be replaced with
enhancing machine vector. But from the above separating pv_ops from
machine vector looks reasonable.
Integrating pv_ops functionality to machine vector would complicate
the machine vector implementation unnecessary, I think.

thanks,
-- 
yamahata

Re: [PATCH 04/15] ia64/pv_ops: introduce pv_info which describes

[Jes Sorensen]

Isaku Yamahata wrote:
> Our justification is as follows.
> The difference is its scope. pv_ops for virtualization and
> machine vector is for platform difference.
> 
> - pv_ops does cover the area which shouldn't belong to machine vector.
>   For example, ia64 intrinsics paravirtualization.
>   It shouldn't belong to the machine vector.
>   It must be initialized very early before platform detection.

Hi Isaku,

Ok this is a good point.

> - pv_ops covers some performance critical part (e.g. ia64 intrinsics)
>   so that in the future they should be optimized with binary patch like x86.
>   We had the experimental patch to do that, but they are dropped for
>   the merge. It reduced patch size greatly.
>   After merging the first patch series, we're planning to optimize
>   pv_ops with binary patch.
>   The optimization with binary patch is out of the machine vector scope.

Rather than making these binary patches, why not make them fast syscalls
and using a vdso page. Some of the priviledged instructions are simply
reads and we could have that information in a read-only data page, so
there is no need to do a context switch at all. Others could benefit
from a fast system call that doesn't do a full context switch.

It would be nice if we could come up with a generic implementation for
such a vdso style interface that could be shared between xen/kvm/lguest.


> - The current pv_ops implements only one for only domU, but in future
>   pv_ops will support dom0. It means dom0 linux would run with
>   the underlying machine vector + pv_ops, i.e.
>   {dig, hpzx1, hpzx1_swiotlb, ...} machine vector + xen pv_ops
> 
> Probably some hooks of pv_ops could be replaced with
> enhancing machine vector. But from the above separating pv_ops from
> machine vector looks reasonable.

Would it make sense to make the pv_ops pointer part of the machine
vector?

Cheers,
Jes

RE: [PATCH 04/15] ia64/pv_ops: introduce pv_info which describes some random info.

[Dong, Eddie]

> 
> Rather than making these binary patches, why not make them fast
> syscalls and using a vdso page. Some of the priviledged instructions
> are simply reads and we could have that information in a read-only
> data page, so there is no need to do a context switch at all. Others
> could benefit from a fast system call that doesn't do a full context
> switch. 

The issue is we don't want to change Linux code a lot, otherwise it
won't be accepted. If we use same logic with original Linux,
but it is using indirect function call now, binary patching could help
in performance.

> 
> It would be nice if we could come up with a generic implementation for
> such a vdso style interface that could be shared between
> xen/kvm/lguest. 
> 
Introducing a new mechanism to use it to replace Linux code to use them
is another different story.

Eddie

Re: [PATCH 04/15] ia64/pv_ops: introduce pv_info which describes some random info.

[Isaku Yamahata]

On Tue, Apr 22, 2008 at 12:37:06PM +0200, Jes Sorensen wrote:
> >- pv_ops covers some performance critical part (e.g. ia64 intrinsics)
> >  so that in the future they should be optimized with binary patch like 
> >  x86.
> >  We had the experimental patch to do that, but they are dropped for
> >  the merge. It reduced patch size greatly.
> >  After merging the first patch series, we're planning to optimize
> >  pv_ops with binary patch.
> >  The optimization with binary patch is out of the machine vector scope.
> 
> Rather than making these binary patches, why not make them fast syscalls
> and using a vdso page. Some of the priviledged instructions are simply
> reads and we could have that information in a read-only data page, so
> there is no need to do a context switch at all. Others could benefit
> from a fast system call that doesn't do a full context switch.
> 
> It would be nice if we could come up with a generic implementation for
> such a vdso style interface that could be shared between xen/kvm/lguest.

Yes, the above is possible.
But what if native case? Probably the calling stub code should be
binary patched to eliminate the calling overhead completely.
Anyway binary patch doesn't prevent from implementing the above your proposal.
With binary patch, we can implement vdso page infrastructure in future.


> >- The current pv_ops implements only one for only domU, but in future
> >  pv_ops will support dom0. It means dom0 linux would run with
> >  the underlying machine vector + pv_ops, i.e.
> >  {dig, hpzx1, hpzx1_swiotlb, ...} machine vector + xen pv_ops
> >
> >Probably some hooks of pv_ops could be replaced with
> >enhancing machine vector. But from the above separating pv_ops from
> >machine vector looks reasonable.
> 
> Would it make sense to make the pv_ops pointer part of the machine
> vector?

I doubt it because it introduces just one more indirection
pv_ops->ops => machine_vector->pv_ops ->ops.
But it can be done if you like..

-- 
yamahata

Re: [PATCH 04/15] ia64/pv_ops: introduce pv_info which describes

[Jes Sorensen]

Dong, Eddie wrote:
>> Rather than making these binary patches, why not make them fast
>> syscalls and using a vdso page. Some of the priviledged instructions
>> are simply reads and we could have that information in a read-only
>> data page, so there is no need to do a context switch at all. Others
>> could benefit from a fast system call that doesn't do a full context
>> switch. 
> 
> The issue is we don't want to change Linux code a lot, otherwise it
> won't be accepted. If we use same logic with original Linux,
> but it is using indirect function call now, binary patching could help
> in performance.

Hi Eddie,

Sorry but this is a wrong assumption. If the code is correct then there
is no reason why it will not be accepted. It's far more important to
avoid ugly clutter that makes the code hard to maintain.

>> It would be nice if we could come up with a generic implementation for
>> such a vdso style interface that could be shared between
>> xen/kvm/lguest. 
>>
> Introducing a new mechanism to use it to replace Linux code to use them
> is another different story.

If someone starts doing a fast syscall implementation for hypercalls, it
is obvious the person(s) will be asked to make it generic on ia64.
Anything else would be silly, but it would definitely be a good thing to
do.

Cheers,
Jes

RE: [PATCH 04/15] ia64/pv_ops: introduce pv_info which describes some random info.

[Dong, Eddie]

Jes Sorensen wrote:
> Dong, Eddie wrote:
>>> Rather than making these binary patches, why not make them fast
>>> syscalls and using a vdso page. Some of the priviledged instructions
>>> are simply reads and we could have that information in a read-only
>>> data page, so there is no need to do a context switch at all. Others
>>> could benefit from a fast system call that doesn't do a full context
>>> switch.
>> 
>> The issue is we don't want to change Linux code a lot, otherwise it
>> won't be accepted. If we use same logic with original Linux,
>> but it is using indirect function call now, binary patching could
>> help in performance.
> 
> Hi Eddie,
> 
> Sorry but this is a wrong assumption. If the code is correct then
> there is no reason why it will not be accepted. It's far more
> important to avoid ugly clutter that makes the code hard to maintain.
> 

My understanding is that code such as IVT table are well tuned and you
are really 
difficult to pursuade people to replace those privilege resource access
instruction
to use vdso or something equalvalent such as mov GRx=CRy.  For those C
code
previlige resource access, like Isaku mentioned, we need to consider
native too.

Anyway binary patching is just an optimization that X86 used and there
is no 
reason IA64 can't take. At least replacing indirect function with direct
function
call. 

Eddie

Re: [PATCH 04/15] ia64/pv_ops: introduce pv_info which describes

[Jes Sorensen]

Dong, Eddie wrote:
> My understanding is that code such as IVT table are well tuned and you
> are really 
> difficult to pursuade people to replace those privilege resource access
> instruction
> to use vdso or something equalvalent such as mov GRx=CRy.  For those C
> code
> previlige resource access, like Isaku mentioned, we need to consider
> native too.

Hi Eddie,

I think binary patching of the ivt makes a lot of sense, I am certainly
not opposed to that. I was more thinking in terms of calls out of the
regular C code.

Cheers,
Jes

[PATCH 04/15] ia64/pv_ops: introduce pv_info which describes some random info.

[Isaku Yamahata]

introduce pv_info which describes some randome info about
underlying execution environment.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 arch/ia64/kernel/Makefile   |    2 +
 arch/ia64/kernel/paravirt.c |   41 ++++++++++++++++++++++++++++
 include/asm-ia64/paravirt.h |   62 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 105 insertions(+), 0 deletions(-)
 create mode 100644 arch/ia64/kernel/paravirt.c
 create mode 100644 include/asm-ia64/paravirt.h

diff --git a/arch/ia64/kernel/Makefile b/arch/ia64/kernel/Makefile
index 13fd10e..10a4ddb 100644
--- a/arch/ia64/kernel/Makefile
+++ b/arch/ia64/kernel/Makefile
@@ -36,6 +36,8 @@ obj-$(CONFIG_PCI_MSI)		+= msi_ia64.o
 mca_recovery-y			+= mca_drv.o mca_drv_asm.o
 obj-$(CONFIG_IA64_MC_ERR_INJECT)+= err_inject.o
 
+obj-$(CONFIG_PARAVIRT)		+= paravirt.o
+
 obj-$(CONFIG_IA64_ESI)		+= esi.o
 ifneq ($(CONFIG_IA64_ESI),)
 obj-y				+= esi_stub.o	# must be in kernel proper
diff --git a/arch/ia64/kernel/paravirt.c b/arch/ia64/kernel/paravirt.c
new file mode 100644
index 0000000..d295ea5
--- /dev/null
+++ b/arch/ia64/kernel/paravirt.c
@@ -0,0 +1,41 @@
+/******************************************************************************
+ * arch/ia64/kernel/paravirt.c
+ *
+ * Copyright (c) 2008 Isaku Yamahata <yamahata at valinux co jp>
+ *                    VA Linux Systems Japan K.K.
+ *     Yaozu (Eddie) Dong <eddie.dong@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ */
+
+#include <linux/init.h>
+
+#include <linux/compiler.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/types.h>
+
+#include <asm/iosapic.h>
+#include <asm/paravirt.h>
+
+/***************************************************************************
+ * general info
+ */
+struct pv_info pv_info = {
+	.kernel_rpl = 0,
+	.paravirt_enabled = 0,
+	.name = "bare hardware"
+};
diff --git a/include/asm-ia64/paravirt.h b/include/asm-ia64/paravirt.h
new file mode 100644
index 0000000..26b4334
--- /dev/null
+++ b/include/asm-ia64/paravirt.h
@@ -0,0 +1,62 @@
+/******************************************************************************
+ * include/asm-ia64/paravirt.h
+ *
+ * Copyright (c) 2008 Isaku Yamahata <yamahata at valinux co jp>
+ *                    VA Linux Systems Japan K.K.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ */
+
+
+#ifndef __ASM_PARAVIRT_H
+#define __ASM_PARAVIRT_H
+
+#ifdef CONFIG_PARAVIRT_GUEST
+
+#ifndef __ASSEMBLY__
+
+#include <asm/hw_irq.h>
+#include <asm/meminit.h>
+
+/******************************************************************************
+ * general info
+ */
+struct pv_info {
+	unsigned int kernel_rpl;
+	int paravirt_enabled;
+	const char *name;
+};
+
+extern struct pv_info pv_info;
+
+static inline int paravirt_enabled(void)
+{
+	return pv_info.paravirt_enabled;
+}
+
+static inline unsigned int get_kernel_rpl(void)
+{
+	return pv_info.kernel_rpl;
+}
+
+#endif /* !__ASSEMBLY__ */
+
+#else
+/* fallback for native case */
+
+#endif /* CONFIG_PARAVIRT_GUEST */
+
+#endif /* __ASM_PARAVIRT_H */
-- 
1.5.3