LinuxPPC-Dev Archive on lore.kernel.org
 help / color / Atom feed
From: Claudio Carvalho <cclaudio@linux.ibm.com>
To: Michael Ellerman <mpe@ellerman.id.au>, linuxppc-dev@ozlabs.org
Cc: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>,
	Michael Anderson <andmike@linux.ibm.com>,
	Ram Pai <linuxram@us.ibm.com>,
	kvm-ppc@vger.kernel.org, Bharata B Rao <bharata@linux.ibm.com>,
	Ryan Grimm <grimm@linux.ibm.com>,
	Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>,
	Thiago Bauermann <bauerman@linux.ibm.com>,
	Anshuman Khandual <khandual@linux.vnet.ibm.com>
Subject: Re: [PATCH v4 2/8] powerpc: Introduce FW_FEATURE_ULTRAVISOR
Date: Fri, 12 Jul 2019 15:01:56 -0300
Message-ID: <4da093e5-14ea-963b-9e8d-a6ba2aa4678f@linux.ibm.com> (raw)
In-Reply-To: <87k1cog250.fsf@concordia.ellerman.id.au>

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


On 7/11/19 9:57 AM, Michael Ellerman wrote:
> Claudio Carvalho <cclaudio@linux.ibm.com> writes:
>> diff --git a/arch/powerpc/include/asm/ultravisor.h b/arch/powerpc/include/asm/ultravisor.h
>> new file mode 100644
>> index 000000000000..e5009b0d84ea
>> --- /dev/null
>> +++ b/arch/powerpc/include/asm/ultravisor.h
>> @@ -0,0 +1,15 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Ultravisor definitions
>> + *
>> + * Copyright 2019, IBM Corporation.
>> + *
>> + */
>> +#ifndef _ASM_POWERPC_ULTRAVISOR_H
>> +#define _ASM_POWERPC_ULTRAVISOR_H
>> +
>> +/* Internal functions */
>> +extern int early_init_dt_scan_ultravisor(unsigned long node, const char *uname,
>> +					 int depth, void *data);
> Please don't use extern in new headers.
>
>> diff --git a/arch/powerpc/kernel/ultravisor.c b/arch/powerpc/kernel/ultravisor.c
>> new file mode 100644
>> index 000000000000..dc6021f63c97
>> --- /dev/null
>> +++ b/arch/powerpc/kernel/ultravisor.c
> Is there a reason this (and other later files) aren't in platforms/powernv ?

Yes, there is.
https://www.spinics.net/lists/kvm-ppc/msg14998.html

We also need to do ucalls from a secure guest and its kernel may not have CONFIG_PPC_POWERNV=y. I can make it clear in the commit message.


>
>> @@ -0,0 +1,24 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Ultravisor high level interfaces
>> + *
>> + * Copyright 2019, IBM Corporation.
>> + *
>> + */
>> +#include <linux/init.h>
>> +#include <linux/printk.h>
>> +#include <linux/string.h>
>> +
>> +#include <asm/ultravisor.h>
>> +#include <asm/firmware.h>
>> +
>> +int __init early_init_dt_scan_ultravisor(unsigned long node, const char *uname,
>> +					 int depth, void *data)
>> +{
>> +	if (depth != 1 || strcmp(uname, "ibm,ultravisor") != 0)
>> +		return 0;
> I know you're following the example of OPAL, but this is not the best
> way to search for the ultravisor node.
>
> It makes the location and name of the node part of the ABI, when there's
> no need for it to be.
>
> If instead you just scan the tree for a node that is *compatible* with
> "ibm,ultravisor" (or whatever compatible string) then the node can be
> placed any where in the tree and have any name, which gives us the most
> flexibility in future to change the location of the device tree node.

I will do that in the next version.

Thanks,
Claudio


>
> cheers
>

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

<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <p><br>
    </p>
    <div class="moz-cite-prefix">On 7/11/19 9:57 AM, Michael Ellerman
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:87k1cog250.fsf@concordia.ellerman.id.au">
      <pre class="moz-quote-pre" wrap="">Claudio Carvalho <a class="moz-txt-link-rfc2396E" href="mailto:cclaudio@linux.ibm.com">&lt;cclaudio@linux.ibm.com&gt;</a> writes:
</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">diff --git a/arch/powerpc/include/asm/ultravisor.h b/arch/powerpc/include/asm/ultravisor.h
new file mode 100644
index 000000000000..e5009b0d84ea
--- /dev/null
+++ b/arch/powerpc/include/asm/ultravisor.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Ultravisor definitions
+ *
+ * Copyright 2019, IBM Corporation.
+ *
+ */
+#ifndef _ASM_POWERPC_ULTRAVISOR_H
+#define _ASM_POWERPC_ULTRAVISOR_H
+
+/* Internal functions */
+extern int early_init_dt_scan_ultravisor(unsigned long node, const char *uname,
+					 int depth, void *data);
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
Please don't use extern in new headers.

</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">diff --git a/arch/powerpc/kernel/ultravisor.c b/arch/powerpc/kernel/ultravisor.c
new file mode 100644
index 000000000000..dc6021f63c97
--- /dev/null
+++ b/arch/powerpc/kernel/ultravisor.c
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
Is there a reason this (and other later files) aren't in platforms/powernv ?</pre>
    </blockquote>
    <pre style="white-space: pre-wrap; color: rgb(0, 0, 0); font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration-style: initial; text-decoration-color: initial;">Yes, there is.
<a href="https://www.spinics.net/lists/kvm-ppc/msg14998.html">https://www.spinics.net/lists/kvm-ppc/msg14998.html</a>

We also need to do ucalls from a secure guest and its kernel may not have CONFIG_PPC_POWERNV=y. I can make it clear in the commit message.


</pre>
    <blockquote type="cite"
      cite="mid:87k1cog250.fsf@concordia.ellerman.id.au">
      <pre class="moz-quote-pre" wrap="">

</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Ultravisor high level interfaces
+ *
+ * Copyright 2019, IBM Corporation.
+ *
+ */
+#include &lt;linux/init.h&gt;
+#include &lt;linux/printk.h&gt;
+#include &lt;linux/string.h&gt;
+
+#include &lt;asm/ultravisor.h&gt;
+#include &lt;asm/firmware.h&gt;
+
+int __init early_init_dt_scan_ultravisor(unsigned long node, const char *uname,
+					 int depth, void *data)
+{
+	if (depth != 1 || strcmp(uname, "ibm,ultravisor") != 0)
+		return 0;
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
I know you're following the example of OPAL, but this is not the best
way to search for the ultravisor node.

It makes the location and name of the node part of the ABI, when there's
no need for it to be.

If instead you just scan the tree for a node that is *compatible* with
"ibm,ultravisor" (or whatever compatible string) then the node can be
placed any where in the tree and have any name, which gives us the most
flexibility in future to change the location of the device tree node.</pre>
    </blockquote>
    <p>I will do that in the next version.</p>
    <p>Thanks,<br>
      Claudio<br>
    </p>
    <p><br>
    </p>
    <blockquote type="cite"
      cite="mid:87k1cog250.fsf@concordia.ellerman.id.au">
      <pre class="moz-quote-pre" wrap="">

cheers

</pre>
    </blockquote>
  </body>
</html>

  reply index

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-28 20:08 [PATCH v4 0/8] kvmppc: Paravirtualize KVM to support ultravisor Claudio Carvalho
2019-06-28 20:08 ` [PATCH v4 1/8] KVM: PPC: Ultravisor: Introduce the MSR_S bit Claudio Carvalho
2019-07-08 17:38   ` janani
2019-07-11 12:57   ` Michael Ellerman
2019-07-12  0:59     ` Nicholas Piggin
2019-07-12  0:57   ` Nicholas Piggin
2019-07-12  6:29     ` Michael Ellerman
2019-07-12 21:07     ` Claudio Carvalho
2019-06-28 20:08 ` [PATCH v4 2/8] powerpc: Introduce FW_FEATURE_ULTRAVISOR Claudio Carvalho
2019-07-08 17:40   ` janani
2019-07-11 12:57   ` Michael Ellerman
2019-07-12 18:01     ` Claudio Carvalho [this message]
2019-07-15  4:10       ` Michael Ellerman
2019-06-28 20:08 ` [PATCH v4 3/8] KVM: PPC: Ultravisor: Add generic ultravisor call handler Claudio Carvalho
2019-07-08 17:55   ` janani
2019-07-11 12:57   ` Michael Ellerman
2019-07-13 17:42     ` Claudio Carvalho
2019-07-15  4:46       ` Michael Ellerman
2019-07-12  1:18   ` Nicholas Piggin
2019-06-28 20:08 ` [PATCH v4 4/8] KVM: PPC: Ultravisor: Use UV_WRITE_PATE ucall to register a PATE Claudio Carvalho
2019-07-08 17:57   ` janani
2019-07-11 12:57   ` Michael Ellerman
2019-06-28 20:08 ` [PATCH v4 5/8] KVM: PPC: Ultravisor: Restrict flush of the partition tlb cache Claudio Carvalho
2019-07-01  5:54   ` Alexey Kardashevskiy
2019-07-08 20:05     ` Claudio Carvalho
2019-07-08 19:54   ` janani
2019-07-10 17:09     ` Ram Pai
2019-06-28 20:08 ` [PATCH v4 6/8] KVM: PPC: Ultravisor: Restrict LDBAR access Claudio Carvalho
2019-07-01  5:54   ` Alexey Kardashevskiy
2019-07-01  6:17     ` maddy
2019-07-01  6:30       ` Alexey Kardashevskiy
2019-07-01  6:46         ` Ram Pai
2019-07-13 17:56           ` Claudio Carvalho
2019-07-08 20:22   ` janani
2019-07-11 12:57   ` Michael Ellerman
2019-07-15  0:38     ` Claudio Carvalho
2019-06-28 20:08 ` [PATCH v4 7/8] KVM: PPC: Ultravisor: Enter a secure guest Claudio Carvalho
2019-07-08 20:53   ` janani
2019-07-08 20:52     ` Claudio Carvalho
2019-07-11 12:57   ` Michael Ellerman
2019-07-12  2:03   ` Nicholas Piggin
2019-06-28 20:08 ` [PATCH v4 8/8] KVM: PPC: Ultravisor: Check for MSR_S during hv_reset_msr Claudio Carvalho
2019-07-08 20:54   ` janani
2019-07-11 12:57   ` Michael Ellerman

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4da093e5-14ea-963b-9e8d-a6ba2aa4678f@linux.ibm.com \
    --to=cclaudio@linux.ibm.com \
    --cc=andmike@linux.ibm.com \
    --cc=bauerman@linux.ibm.com \
    --cc=bharata@linux.ibm.com \
    --cc=grimm@linux.ibm.com \
    --cc=khandual@linux.vnet.ibm.com \
    --cc=kvm-ppc@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=linuxram@us.ibm.com \
    --cc=maddy@linux.vnet.ibm.com \
    --cc=mpe@ellerman.id.au \
    --cc=sukadev@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LinuxPPC-Dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linuxppc-dev/0 linuxppc-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linuxppc-dev linuxppc-dev/ https://lore.kernel.org/linuxppc-dev \
		linuxppc-dev@lists.ozlabs.org linuxppc-dev@ozlabs.org linuxppc-dev@archiver.kernel.org
	public-inbox-index linuxppc-dev


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linuxppc-dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox