diff mbox series

[2/5] x86/lbr: enable hypervisor LER with arch LBR

Message ID 20220520133746.66142-3-roger.pau@citrix.com (mailing list archive)
State New
Headers show
Series x86/lbr: handle lack of model-specific LBRs | expand

Commit Message

Roger Pau Monné May 20, 2022, 1:37 p.m. UTC
CPUs having no model-specific LBRs don't implement DEBUGCTLMSR.LBR
and LBRs can only be enabled if the processor supports architectural
LBRs.

Split the logic to enable LBRs into a separate function and expand the
logic to also implement support for arch LBRs if model-specific LBRs
are not supported.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/include/asm/msr-index.h        | 18 +++++++++++++
 xen/arch/x86/traps.c                        | 29 ++++++++++++++++++---
 xen/include/public/arch-x86/cpufeatureset.h |  1 +
 3 files changed, 45 insertions(+), 3 deletions(-)

Comments

Jan Beulich May 30, 2022, 3:31 p.m. UTC | #1
On 20.05.2022 15:37, Roger Pau Monne wrote:
> --- a/xen/arch/x86/include/asm/msr-index.h
> +++ b/xen/arch/x86/include/asm/msr-index.h
> @@ -139,6 +139,24 @@
>  #define  PASID_PASID_MASK                   0x000fffff
>  #define  PASID_VALID                        (_AC(1, ULL) << 31)
>  
> +#define MSR_ARCH_LBR_CTL                    0x000014ce
> +#define  ARCH_LBR_CTL_LBREN                 (_AC(1, ULL) <<  0)
> +#define  ARCH_LBR_CTL_OS                    (_AC(1, ULL) <<  1)

Bits 2 and 3 also have meaning (USR and CALL_STACK) according to
ISE version 44. If it was intentional that you omitted those
(perhaps you intended to add only the bits you actually use right
away), it would have been nice if you said so in the description.

> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -1963,6 +1963,29 @@ void do_device_not_available(struct cpu_user_regs *regs)
>  #endif
>  }
>  
> +static bool enable_lbr(void)
> +{
> +    uint64_t debugctl;
> +
> +    wrmsrl(MSR_IA32_DEBUGCTLMSR, IA32_DEBUGCTLMSR_LBR);
> +    rdmsrl(MSR_IA32_DEBUGCTLMSR, debugctl);
> +    if ( !(debugctl & IA32_DEBUGCTLMSR_LBR) )
> +    {
> +        /*
> +         * CPUs with no model-specific LBRs always return DEBUGCTLMSR.LBR
> +         * == 0, attempt to set arch LBR if available.
> +         */
> +        if ( !boot_cpu_has(X86_FEATURE_ARCH_LBR) )
> +            return false;
> +
> +        /* Note that LASTINT{FROMIP,TOIP} matches LER_{FROM_IP,TO_IP} */
> +        wrmsrl(MSR_ARCH_LBR_CTL, ARCH_LBR_CTL_LBREN | ARCH_LBR_CTL_OS |
> +                                 ARCH_LBR_CTL_RECORD_ALL);
> +    }
> +
> +    return true;
> +}

Would it make sense to try architectural LBRs first?

> @@ -1997,7 +2020,7 @@ void do_debug(struct cpu_user_regs *regs)
>  
>      /* #DB automatically disabled LBR.  Reinstate it if debugging Xen. */
>      if ( cpu_has_xen_lbr )
> -        wrmsrl(MSR_IA32_DEBUGCTLMSR, IA32_DEBUGCTLMSR_LBR);
> +        enable_lbr();
>  
>      if ( !guest_mode(regs) )
>      {
> @@ -2179,8 +2202,8 @@ void percpu_traps_init(void)
>      if ( !ler_msr && (ler_msr = calc_ler_msr()) )
>          setup_force_cpu_cap(X86_FEATURE_XEN_LBR);
>  
> -    if ( cpu_has_xen_lbr )
> -        wrmsrl(MSR_IA32_DEBUGCTLMSR, IA32_DEBUGCTLMSR_LBR);
> +    if ( cpu_has_xen_lbr && !enable_lbr() )
> +        printk(XENLOG_ERR "CPU#%u: failed to enable LBR\n", smp_processor_id());
>  }

Isn't enable_lbr() failing a strong indication that we shouldn't have
set XEN_LBR just before this? IOW doesn't this want re-arranging such
that the feature bit and maybe also ler_msr (albeit some care would
be required there; in fact I think this is broken for the case of
running on non-{Intel,AMD,Hygon} CPUs [or unrecognized models] but
opt_ler being true) remain unset in that case?

As there's no good place to ask the VMX-related question, it needs to
go here: Aiui with this patch in place VMX guests will be run with
Xen's choice of LBR_CTL. That's different from DebugCtl, which - being
part of the VMCS - would be loaded by the CPU. Such a difference, if
intended, would imo again want pointing out in the description.

Jan
Roger Pau Monné July 1, 2022, 3:39 p.m. UTC | #2
On Mon, May 30, 2022 at 05:31:18PM +0200, Jan Beulich wrote:
> On 20.05.2022 15:37, Roger Pau Monne wrote:
> > --- a/xen/arch/x86/include/asm/msr-index.h
> > +++ b/xen/arch/x86/include/asm/msr-index.h
> > @@ -139,6 +139,24 @@
> >  #define  PASID_PASID_MASK                   0x000fffff
> >  #define  PASID_VALID                        (_AC(1, ULL) << 31)
> >  
> > +#define MSR_ARCH_LBR_CTL                    0x000014ce
> > +#define  ARCH_LBR_CTL_LBREN                 (_AC(1, ULL) <<  0)
> > +#define  ARCH_LBR_CTL_OS                    (_AC(1, ULL) <<  1)
> 
> Bits 2 and 3 also have meaning (USR and CALL_STACK) according to
> ISE version 44. If it was intentional that you omitted those
> (perhaps you intended to add only the bits you actually use right
> away), it would have been nice if you said so in the description.

Yes, I've only added the bits used.  I could add all if that's better.

> > --- a/xen/arch/x86/traps.c
> > +++ b/xen/arch/x86/traps.c
> > @@ -1963,6 +1963,29 @@ void do_device_not_available(struct cpu_user_regs *regs)
> >  #endif
> >  }
> >  
> > +static bool enable_lbr(void)
> > +{
> > +    uint64_t debugctl;
> > +
> > +    wrmsrl(MSR_IA32_DEBUGCTLMSR, IA32_DEBUGCTLMSR_LBR);
> > +    rdmsrl(MSR_IA32_DEBUGCTLMSR, debugctl);
> > +    if ( !(debugctl & IA32_DEBUGCTLMSR_LBR) )
> > +    {
> > +        /*
> > +         * CPUs with no model-specific LBRs always return DEBUGCTLMSR.LBR
> > +         * == 0, attempt to set arch LBR if available.
> > +         */
> > +        if ( !boot_cpu_has(X86_FEATURE_ARCH_LBR) )
> > +            return false;
> > +
> > +        /* Note that LASTINT{FROMIP,TOIP} matches LER_{FROM_IP,TO_IP} */
> > +        wrmsrl(MSR_ARCH_LBR_CTL, ARCH_LBR_CTL_LBREN | ARCH_LBR_CTL_OS |
> > +                                 ARCH_LBR_CTL_RECORD_ALL);
> > +    }
> > +
> > +    return true;
> > +}
> 
> Would it make sense to try architectural LBRs first?

I didn't want to change behavior for existing platforms that have
both architectural and model specific LBRs.

> > @@ -1997,7 +2020,7 @@ void do_debug(struct cpu_user_regs *regs)
> >  
> >      /* #DB automatically disabled LBR.  Reinstate it if debugging Xen. */
> >      if ( cpu_has_xen_lbr )
> > -        wrmsrl(MSR_IA32_DEBUGCTLMSR, IA32_DEBUGCTLMSR_LBR);
> > +        enable_lbr();
> >  
> >      if ( !guest_mode(regs) )
> >      {
> > @@ -2179,8 +2202,8 @@ void percpu_traps_init(void)
> >      if ( !ler_msr && (ler_msr = calc_ler_msr()) )
> >          setup_force_cpu_cap(X86_FEATURE_XEN_LBR);
> >  
> > -    if ( cpu_has_xen_lbr )
> > -        wrmsrl(MSR_IA32_DEBUGCTLMSR, IA32_DEBUGCTLMSR_LBR);
> > +    if ( cpu_has_xen_lbr && !enable_lbr() )
> > +        printk(XENLOG_ERR "CPU#%u: failed to enable LBR\n", smp_processor_id());
> >  }
> 
> Isn't enable_lbr() failing a strong indication that we shouldn't have
> set XEN_LBR just before this?

So I've now added extra checks in calc_ler_msr() so that it only
returns != 0 when there's LBR support (either model specific or
architectural).

> IOW doesn't this want re-arranging such
> that the feature bit and maybe also ler_msr (albeit some care would
> be required there; in fact I think this is broken for the case of
> running on non-{Intel,AMD,Hygon} CPUs [or unrecognized models] but
> opt_ler being true) remain unset in that case?

opt_ler will be set to false if calc_ler_msr() return 0, which is the
case for non-{Intel,AMD,Hygon} or unrecognized models.

> As there's no good place to ask the VMX-related question, it needs to
> go here: Aiui with this patch in place VMX guests will be run with
> Xen's choice of LBR_CTL. That's different from DebugCtl, which - being
> part of the VMCS - would be loaded by the CPU. Such a difference, if
> intended, would imo again want pointing out in the description.

LBR_CTL will only be set by Xen when the CPU only supports
architectural LBRs (no model specific LBR support at all), and in that
case LBR support won't be exposed to guests, as the ARCH_LBR CPUID is
not exposed, neither are guests allowed access to ARCH_LBR_CTL.

Note that in such scenario also setting DebugCtl.LBR has not effect, as
there's no model specific LBR support, and the hardware will just
ignore the bit.  Also none of the LBR MSRs are exposed to guests
either.

I can try to clarify all the above in the commit message.

Thanks, Roger.
Jan Beulich July 4, 2022, 6:15 a.m. UTC | #3
On 01.07.2022 17:39, Roger Pau Monné wrote:
> On Mon, May 30, 2022 at 05:31:18PM +0200, Jan Beulich wrote:
>> On 20.05.2022 15:37, Roger Pau Monne wrote:
>>> --- a/xen/arch/x86/include/asm/msr-index.h
>>> +++ b/xen/arch/x86/include/asm/msr-index.h
>>> @@ -139,6 +139,24 @@
>>>  #define  PASID_PASID_MASK                   0x000fffff
>>>  #define  PASID_VALID                        (_AC(1, ULL) << 31)
>>>  
>>> +#define MSR_ARCH_LBR_CTL                    0x000014ce
>>> +#define  ARCH_LBR_CTL_LBREN                 (_AC(1, ULL) <<  0)
>>> +#define  ARCH_LBR_CTL_OS                    (_AC(1, ULL) <<  1)
>>
>> Bits 2 and 3 also have meaning (USR and CALL_STACK) according to
>> ISE version 44. If it was intentional that you omitted those
>> (perhaps you intended to add only the bits you actually use right
>> away), it would have been nice if you said so in the description.
> 
> Yes, I've only added the bits used.  I could add all if that's better.

Personally I'd slightly prefer if you added all. But if you don't, which
is also okay, please make this explicit in the description.

>>> --- a/xen/arch/x86/traps.c
>>> +++ b/xen/arch/x86/traps.c
>>> @@ -1963,6 +1963,29 @@ void do_device_not_available(struct cpu_user_regs *regs)
>>>  #endif
>>>  }
>>>  
>>> +static bool enable_lbr(void)
>>> +{
>>> +    uint64_t debugctl;
>>> +
>>> +    wrmsrl(MSR_IA32_DEBUGCTLMSR, IA32_DEBUGCTLMSR_LBR);
>>> +    rdmsrl(MSR_IA32_DEBUGCTLMSR, debugctl);
>>> +    if ( !(debugctl & IA32_DEBUGCTLMSR_LBR) )
>>> +    {
>>> +        /*
>>> +         * CPUs with no model-specific LBRs always return DEBUGCTLMSR.LBR
>>> +         * == 0, attempt to set arch LBR if available.
>>> +         */
>>> +        if ( !boot_cpu_has(X86_FEATURE_ARCH_LBR) )
>>> +            return false;
>>> +
>>> +        /* Note that LASTINT{FROMIP,TOIP} matches LER_{FROM_IP,TO_IP} */
>>> +        wrmsrl(MSR_ARCH_LBR_CTL, ARCH_LBR_CTL_LBREN | ARCH_LBR_CTL_OS |
>>> +                                 ARCH_LBR_CTL_RECORD_ALL);
>>> +    }
>>> +
>>> +    return true;
>>> +}
>>
>> Would it make sense to try architectural LBRs first?
> 
> I didn't want to change behavior for existing platforms that have
> both architectural and model specific LBRs.

Are there such platforms? While it may not be said explicitly, so far
I took it that the LBR format indicator being 0x3f was connected to
arch LBR being available.

>> As there's no good place to ask the VMX-related question, it needs to
>> go here: Aiui with this patch in place VMX guests will be run with
>> Xen's choice of LBR_CTL. That's different from DebugCtl, which - being
>> part of the VMCS - would be loaded by the CPU. Such a difference, if
>> intended, would imo again want pointing out in the description.
> 
> LBR_CTL will only be set by Xen when the CPU only supports
> architectural LBRs (no model specific LBR support at all), and in that
> case LBR support won't be exposed to guests, as the ARCH_LBR CPUID is
> not exposed, neither are guests allowed access to ARCH_LBR_CTL.
> 
> Note that in such scenario also setting DebugCtl.LBR has not effect, as
> there's no model specific LBR support, and the hardware will just
> ignore the bit.  Also none of the LBR MSRs are exposed to guests
> either.

My question wasn't about guest support, but about us (perhaps mistakenly)
running guest code with the Xen setting in place. It cannot be excluded
that running with LBR enabled has a performance impact, after all.

> I can try to clarify all the above in the commit message.

Thanks.

Jan
Roger Pau Monné July 4, 2022, 8:23 a.m. UTC | #4
On Mon, Jul 04, 2022 at 08:15:15AM +0200, Jan Beulich wrote:
> On 01.07.2022 17:39, Roger Pau Monné wrote:
> > On Mon, May 30, 2022 at 05:31:18PM +0200, Jan Beulich wrote:
> >> On 20.05.2022 15:37, Roger Pau Monne wrote:
> >>> --- a/xen/arch/x86/include/asm/msr-index.h
> >>> +++ b/xen/arch/x86/include/asm/msr-index.h
> >>> @@ -139,6 +139,24 @@
> >>>  #define  PASID_PASID_MASK                   0x000fffff
> >>>  #define  PASID_VALID                        (_AC(1, ULL) << 31)
> >>>  
> >>> +#define MSR_ARCH_LBR_CTL                    0x000014ce
> >>> +#define  ARCH_LBR_CTL_LBREN                 (_AC(1, ULL) <<  0)
> >>> +#define  ARCH_LBR_CTL_OS                    (_AC(1, ULL) <<  1)
> >>
> >> Bits 2 and 3 also have meaning (USR and CALL_STACK) according to
> >> ISE version 44. If it was intentional that you omitted those
> >> (perhaps you intended to add only the bits you actually use right
> >> away), it would have been nice if you said so in the description.
> > 
> > Yes, I've only added the bits used.  I could add all if that's better.
> 
> Personally I'd slightly prefer if you added all. But if you don't, which
> is also okay, please make this explicit in the description.
> 
> >>> --- a/xen/arch/x86/traps.c
> >>> +++ b/xen/arch/x86/traps.c
> >>> @@ -1963,6 +1963,29 @@ void do_device_not_available(struct cpu_user_regs *regs)
> >>>  #endif
> >>>  }
> >>>  
> >>> +static bool enable_lbr(void)
> >>> +{
> >>> +    uint64_t debugctl;
> >>> +
> >>> +    wrmsrl(MSR_IA32_DEBUGCTLMSR, IA32_DEBUGCTLMSR_LBR);
> >>> +    rdmsrl(MSR_IA32_DEBUGCTLMSR, debugctl);
> >>> +    if ( !(debugctl & IA32_DEBUGCTLMSR_LBR) )
> >>> +    {
> >>> +        /*
> >>> +         * CPUs with no model-specific LBRs always return DEBUGCTLMSR.LBR
> >>> +         * == 0, attempt to set arch LBR if available.
> >>> +         */
> >>> +        if ( !boot_cpu_has(X86_FEATURE_ARCH_LBR) )
> >>> +            return false;
> >>> +
> >>> +        /* Note that LASTINT{FROMIP,TOIP} matches LER_{FROM_IP,TO_IP} */
> >>> +        wrmsrl(MSR_ARCH_LBR_CTL, ARCH_LBR_CTL_LBREN | ARCH_LBR_CTL_OS |
> >>> +                                 ARCH_LBR_CTL_RECORD_ALL);
> >>> +    }
> >>> +
> >>> +    return true;
> >>> +}
> >>
> >> Would it make sense to try architectural LBRs first?
> > 
> > I didn't want to change behavior for existing platforms that have
> > both architectural and model specific LBRs.
> 
> Are there such platforms? While it may not be said explicitly, so far
> I took it that the LBR format indicator being 0x3f was connected to
> arch LBR being available.

IIRC Ice Lake has both model-specific and architectural LBRs.

While format being 0x3f could indicate the likely presence of arch
LBRs, the CPUID bit need to be checked.

> >> As there's no good place to ask the VMX-related question, it needs to
> >> go here: Aiui with this patch in place VMX guests will be run with
> >> Xen's choice of LBR_CTL. That's different from DebugCtl, which - being
> >> part of the VMCS - would be loaded by the CPU. Such a difference, if
> >> intended, would imo again want pointing out in the description.
> > 
> > LBR_CTL will only be set by Xen when the CPU only supports
> > architectural LBRs (no model specific LBR support at all), and in that
> > case LBR support won't be exposed to guests, as the ARCH_LBR CPUID is
> > not exposed, neither are guests allowed access to ARCH_LBR_CTL.
> > 
> > Note that in such scenario also setting DebugCtl.LBR has not effect, as
> > there's no model specific LBR support, and the hardware will just
> > ignore the bit.  Also none of the LBR MSRs are exposed to guests
> > either.
> 
> My question wasn't about guest support, but about us (perhaps mistakenly)
> running guest code with the Xen setting in place. It cannot be excluded
> that running with LBR enabled has a performance impact, after all.

It's possible.  'ler' option already states that it should be used for
debugging purposes only, so I think it's fine if this results in
slower guest performance, as it's not a general purpose option after
all.

Thanks, Roger.
diff mbox series

Patch

diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
index 3e038db618..7b08e1804b 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -139,6 +139,24 @@ 
 #define  PASID_PASID_MASK                   0x000fffff
 #define  PASID_VALID                        (_AC(1, ULL) << 31)
 
+#define MSR_ARCH_LBR_CTL                    0x000014ce
+#define  ARCH_LBR_CTL_LBREN                 (_AC(1, ULL) <<  0)
+#define  ARCH_LBR_CTL_OS                    (_AC(1, ULL) <<  1)
+#define  ARCH_LBR_CTL_COND                  (_AC(1, ULL) << 16)
+#define  ARCH_LBR_CTL_NEAR_REL_JMP          (_AC(1, ULL) << 17)
+#define  ARCH_LBR_CTL_NEAR_IND_JMP          (_AC(1, ULL) << 18)
+#define  ARCH_LBR_CTL_NEAR_REL_CALL         (_AC(1, ULL) << 19)
+#define  ARCH_LBR_CTL_NEAR_IND_CALL         (_AC(1, ULL) << 20)
+#define  ARCH_LBR_CTL_NEAR_RET              (_AC(1, ULL) << 21)
+#define  ARCH_LBR_CTL_OTHER_BRANCH          (_AC(1, ULL) << 22)
+#define  ARCH_LBR_CTL_RECORD_ALL            (ARCH_LBR_CTL_COND | \
+                                             ARCH_LBR_CTL_NEAR_REL_JMP | \
+                                             ARCH_LBR_CTL_NEAR_IND_JMP | \
+                                             ARCH_LBR_CTL_NEAR_REL_CALL | \
+                                             ARCH_LBR_CTL_NEAR_IND_CALL | \
+                                             ARCH_LBR_CTL_NEAR_RET | \
+                                             ARCH_LBR_CTL_OTHER_BRANCH)
+
 #define MSR_EFER                            0xc0000080 /* Extended Feature Enable Register */
 #define  EFER_SCE                           (_AC(1, ULL) <<  0) /* SYSCALL Enable */
 #define  EFER_LME                           (_AC(1, ULL) <<  8) /* Long Mode Enable */
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 4c38f6c015..133348d9f9 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1963,6 +1963,29 @@  void do_device_not_available(struct cpu_user_regs *regs)
 #endif
 }
 
+static bool enable_lbr(void)
+{
+    uint64_t debugctl;
+
+    wrmsrl(MSR_IA32_DEBUGCTLMSR, IA32_DEBUGCTLMSR_LBR);
+    rdmsrl(MSR_IA32_DEBUGCTLMSR, debugctl);
+    if ( !(debugctl & IA32_DEBUGCTLMSR_LBR) )
+    {
+        /*
+         * CPUs with no model-specific LBRs always return DEBUGCTLMSR.LBR
+         * == 0, attempt to set arch LBR if available.
+         */
+        if ( !boot_cpu_has(X86_FEATURE_ARCH_LBR) )
+            return false;
+
+        /* Note that LASTINT{FROMIP,TOIP} matches LER_{FROM_IP,TO_IP} */
+        wrmsrl(MSR_ARCH_LBR_CTL, ARCH_LBR_CTL_LBREN | ARCH_LBR_CTL_OS |
+                                 ARCH_LBR_CTL_RECORD_ALL);
+    }
+
+    return true;
+}
+
 void do_debug(struct cpu_user_regs *regs)
 {
     unsigned long dr6;
@@ -1997,7 +2020,7 @@  void do_debug(struct cpu_user_regs *regs)
 
     /* #DB automatically disabled LBR.  Reinstate it if debugging Xen. */
     if ( cpu_has_xen_lbr )
-        wrmsrl(MSR_IA32_DEBUGCTLMSR, IA32_DEBUGCTLMSR_LBR);
+        enable_lbr();
 
     if ( !guest_mode(regs) )
     {
@@ -2179,8 +2202,8 @@  void percpu_traps_init(void)
     if ( !ler_msr && (ler_msr = calc_ler_msr()) )
         setup_force_cpu_cap(X86_FEATURE_XEN_LBR);
 
-    if ( cpu_has_xen_lbr )
-        wrmsrl(MSR_IA32_DEBUGCTLMSR, IA32_DEBUGCTLMSR_LBR);
+    if ( cpu_has_xen_lbr && !enable_lbr() )
+        printk(XENLOG_ERR "CPU#%u: failed to enable LBR\n", smp_processor_id());
 }
 
 void __init init_idt_traps(void)
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index 9cee4b439e..cd6409f9f3 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -280,6 +280,7 @@  XEN_CPUFEATURE(RTM_ALWAYS_ABORT, 9*32+11) /*! June 2021 TSX defeaturing in micro
 XEN_CPUFEATURE(TSX_FORCE_ABORT, 9*32+13) /* MSR_TSX_FORCE_ABORT.RTM_ABORT */
 XEN_CPUFEATURE(SERIALIZE,     9*32+14) /*A  SERIALIZE insn */
 XEN_CPUFEATURE(TSXLDTRK,      9*32+16) /*a  TSX load tracking suspend/resume insns */
+XEN_CPUFEATURE(ARCH_LBR,      9*32+19) /*   Intel ARCH LBR */
 XEN_CPUFEATURE(CET_IBT,       9*32+20) /*   CET - Indirect Branch Tracking */
 XEN_CPUFEATURE(IBRSB,         9*32+26) /*A  IBRS and IBPB support (used by Intel) */
 XEN_CPUFEATURE(STIBP,         9*32+27) /*A  STIBP */