Re: [PATCH kernel v2 4/4] powerpc/pseries/svm: Allow IOMMU to work in SVM

[David Gibson <david@gibson.dropbear.id.au>]

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

On Thu, Jan 02, 2020 at 02:21:06PM -0800, Ram Pai wrote:
> On Mon, Dec 16, 2019 at 03:19:24PM +1100, Alexey Kardashevskiy wrote:
> > H_PUT_TCE_INDIRECT uses a shared page to send up to 512 TCE to
> > a hypervisor in a single hypercall.
> 
> Actually H_PUT_TCE_INDIRECT never used shared page.  It would have
> used shared pages if the 'shared-page' solution was accepted. :)

Well, it depends what you mean by "shared".  In the non-PEF case we do
use a shared page in the sense that it is accessed by both guest and
hypervisor.  It's just not shared in the PEF sense.

> > This does not work for secure VMs
> > as the page needs to be shared or the VM should use H_PUT_TCE instead.
> 
> Maybe you should say something like this.. ?
> 
> H_PUT_TCE_INDIRECT does not work for secure VMs, since the page
> containing the TCE entries is not accessible to the hypervisor.
> 
> > 
> > This disables H_PUT_TCE_INDIRECT by clearing the FW_FEATURE_PUT_TCE_IND
> > feature bit so SVMs will map TCEs using H_PUT_TCE.
> > 
> > This is not a part of init_svm() as it is called too late after FW
> > patching is done and may result in a warning like this:
> > 
> > [    3.727716] Firmware features changed after feature patching!
> > [    3.727965] WARNING: CPU: 0 PID: 1 at (...)arch/powerpc/lib/feature-fixups.c:466 check_features+0xa4/0xc0
> > 
> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> 
> Reviewed-by: Ram Pai <linuxram@us.ibm.com>
> 
> 
> > ---
> > Changes:
> > v2
> > * new in the patchset
> > ---
> >  arch/powerpc/platforms/pseries/firmware.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/arch/powerpc/platforms/pseries/firmware.c b/arch/powerpc/platforms/pseries/firmware.c
> > index d3acff23f2e3..3e49cc23a97a 100644
> > --- a/arch/powerpc/platforms/pseries/firmware.c
> > +++ b/arch/powerpc/platforms/pseries/firmware.c
> > @@ -22,6 +22,7 @@
> >  #include <asm/firmware.h>
> >  #include <asm/prom.h>
> >  #include <asm/udbg.h>
> > +#include <asm/svm.h>
> > 
> >  #include "pseries.h"
> > 
> > @@ -101,6 +102,12 @@ static void __init fw_hypertas_feature_init(const char *hypertas,
> >  		}
> >  	}
> > 
> > +	if (is_secure_guest() &&
> > +	    (powerpc_firmware_features & FW_FEATURE_PUT_TCE_IND)) {
> > +		powerpc_firmware_features &= ~FW_FEATURE_PUT_TCE_IND;
> > +		pr_debug("SVM: disabling PUT_TCE_IND firmware feature\n");
> > +	}
> > +
> >  	pr_debug(" <- fw_hypertas_feature_init()\n");
> >  }
> > 
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]