[PATCH v2] iommu/arm-smmu-v3: Match Stall behaviour for S2

[PATCH v2] iommu/arm-smmu-v3: Match Stall behaviour for S2

[Mostafa Saleh]

According to the spec (ARM IHI 0070 F.b), in
"5.5 Fault configuration (A, R, S bits)":
    A STE with stage 2 translation enabled and STE.S2S == 0 is
    considered ILLEGAL if SMMU_IDR0.STALL_MODEL == 0b10.

Also described in the pseudocode “SteIllegal()”
    if eff_idr0_stall_model == '10' && STE.S2S == '0' then
        // stall_model forcing stall, but S2S == 0
        return TRUE;

Which means, S2S must be set when stall model is
"ARM_SMMU_FEAT_STALL_FORCE", but at the moment the driver ignores that.

Although, the driver can do the minimum and only set S2S for
“ARM_SMMU_FEAT_STALL_FORCE”, it is more consistent to match S1
behaviour, which also sets it for “ARM_SMMU_FEAT_STALL” if the
master has requested stalls.

One caveat, is that S2S is not compatible with ATS, this mentioned
in “5.2 Stream Table Entry”:
    In implementations of SMMUv3.1 and later, this configuration is
    ILLEGAL if EATS is not IGNORED and Config[1] == 1 and S2S == 1.

    In SMMUv3.0 implementations, this configuration is ILLEGAL if EATS
    is not IGNORED and S2S == 1

This is also described in the pseudocode “SteIllegal()”

Also, since S2 stalls are enabled now, report them to the IOMMU layer
and for VFIO devices it will fail anyway as VFIO doesn’t register an
iopf handler.

Signed-off-by: Mostafa Saleh <smostafa@google.com>

---
v2:
- Fix index of the STE
- Fix conflict with ATS
- Squash the 2 patches and drop enable_nesting
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 12 ++++++++----
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  1 +
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index a31460f9f3d4..3b70816a2b81 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1601,6 +1601,14 @@ void arm_smmu_make_cdtable_ste(struct arm_smmu_ste *target,
 		target->data[2] =
 			cpu_to_le64(FIELD_PREP(STRTAB_STE_2_S2VMID, 0));
 	}
+
+	/*
+	 * S2S is ignored if stage-2 exists but not enabled.
+	 * S2S is not compatible with ATS.
+	 */
+	if (master->stall_enabled && !ats_enabled &&
+	    smmu->features & ARM_SMMU_FEAT_TRANS_S2)
+		target->data[2] |= STRTAB_STE_2_S2S;
 }
 EXPORT_SYMBOL_IF_KUNIT(arm_smmu_make_cdtable_ste);
 
@@ -1739,10 +1747,6 @@ static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt)
 		return -EOPNOTSUPP;
 	}
 
-	/* Stage-2 is always pinned at the moment */
-	if (evt[1] & EVTQ_1_S2)
-		return -EFAULT;
-
 	if (!(evt[1] & EVTQ_1_STALL))
 		return -EOPNOTSUPP;
 
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 14bca41a981b..0dc7ad43c64c 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -267,6 +267,7 @@ struct arm_smmu_ste {
 #define STRTAB_STE_2_S2AA64		(1UL << 51)
 #define STRTAB_STE_2_S2ENDI		(1UL << 52)
 #define STRTAB_STE_2_S2PTW		(1UL << 54)
+#define STRTAB_STE_2_S2S		(1UL << 57)
 #define STRTAB_STE_2_S2R		(1UL << 58)
 
 #define STRTAB_STE_3_S2TTB_MASK		GENMASK_ULL(51, 4)
-- 
2.46.0.76.ge559c4bf1a-goog

Re: [PATCH v2] iommu/arm-smmu-v3: Match Stall behaviour for S2

[Jason Gunthorpe]

On Wed, Aug 14, 2024 at 02:56:33PM +0000, Mostafa Saleh wrote:

> Also described in the pseudocode “SteIllegal()”
>     if eff_idr0_stall_model == '10' && STE.S2S == '0' then
>         // stall_model forcing stall, but S2S == 0
>         return TRUE;

This clips out an important bit:

if STE.Config == '11x' then
  [..]
  if eff_idr0_stall_model == '10' && STE.S2S == '0' then
      // stall_model forcing stall, but S2S == 0
      return TRUE;

And here we are using STRTAB_STE_0_CFG_S1_TRANS which is 101 and won't
match the STE.Config qualification.

The plain text language said the S2S is only required if the S2 is
translating, STRTAB_STE_0_CFG_S1_TRANS puts it in bypass.

> +	/*
> +	 * S2S is ignored if stage-2 exists but not enabled.
> +	 * S2S is not compatible with ATS.
> +	 */
> +	if (master->stall_enabled && !ats_enabled &&
> +	    smmu->features & ARM_SMMU_FEAT_TRANS_S2)
> +		target->data[2] |= STRTAB_STE_2_S2S;

We can't ignore ATS if it was requested here.

I think that does point to an issue, ATS should be fixed up here:

--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2492,6 +2492,9 @@ static bool arm_smmu_ats_supported(struct arm_smmu_master *master)
        if (!(fwspec->flags & IOMMU_FWSPEC_PCI_RC_ATS))
                return false;
 
+       if (master->stall_enabled)
+               return false;
+
        return dev_is_pci(dev) && pci_ats_supported(to_pci_dev(dev));
 }

And your hunk above should be placed in arm_smmu_make_s2_domain_ste()
not arm_smmu_make_cdtable_ste()

Not ignoring the event still makes sense to me, but I didn't check it
carefully. We can decode the S2 event right?

Jason

Re: [PATCH v2] iommu/arm-smmu-v3: Match Stall behaviour for S2

[Mostafa Saleh]

Hi Jason,

On Wed, Aug 14, 2024 at 12:51:51PM -0300, Jason Gunthorpe wrote:
> On Wed, Aug 14, 2024 at 02:56:33PM +0000, Mostafa Saleh wrote:
> 
> > Also described in the pseudocode “SteIllegal()”
> >     if eff_idr0_stall_model == '10' && STE.S2S == '0' then
> >         // stall_model forcing stall, but S2S == 0
> >         return TRUE;
> 
> This clips out an important bit:
> 
> if STE.Config == '11x' then
>   [..]
>   if eff_idr0_stall_model == '10' && STE.S2S == '0' then
>       // stall_model forcing stall, but S2S == 0
>       return TRUE;
> 
> And here we are using STRTAB_STE_0_CFG_S1_TRANS which is 101 and won't
> match the STE.Config qualification.
> 
> The plain text language said the S2S is only required if the S2 is
> translating, STRTAB_STE_0_CFG_S1_TRANS puts it in bypass.

Yes, my bad, this should be for stage-2 only which is populated in
arm_smmu_make_s2_domain_ste()

> 
> > +	/*
> > +	 * S2S is ignored if stage-2 exists but not enabled.
> > +	 * S2S is not compatible with ATS.
> > +	 */
> > +	if (master->stall_enabled && !ats_enabled &&
> > +	    smmu->features & ARM_SMMU_FEAT_TRANS_S2)
> > +		target->data[2] |= STRTAB_STE_2_S2S;
> 
> We can't ignore ATS if it was requested here.
> 
> I think that does point to an issue, ATS should be fixed up here:
> 
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2492,6 +2492,9 @@ static bool arm_smmu_ats_supported(struct arm_smmu_master *master)
>         if (!(fwspec->flags & IOMMU_FWSPEC_PCI_RC_ATS))
>                 return false;
>  
> +       if (master->stall_enabled)
> +               return false;
> +
>         return dev_is_pci(dev) && pci_ats_supported(to_pci_dev(dev));
>  }

Makes sense, I will add that to the patch instead of checking at STE
creation time.

> 
> And your hunk above should be placed in arm_smmu_make_s2_domain_ste()
> not arm_smmu_make_cdtable_ste()
> 
> Not ignoring the event still makes sense to me, but I didn't check it
> carefully. We can decode the S2 event right?
> 
Yes, s2 translation fault events are the same but with an extra bit
set (S2), and with a new field for IPA which is not relevant here as
we only report the IOVA.

When full nesting is supported, as iopf_fault doesn’t understand
nesting and only have the IOVA in addr, we would need to filter the
event by stage.

Thanks,
Mostafa


> Jason

Re: [PATCH v2] iommu/arm-smmu-v3: Match Stall behaviour for S2

[Jason Gunthorpe]

On Thu, Aug 15, 2024 at 11:30:41AM +0000, Mostafa Saleh wrote:

> Yes, s2 translation fault events are the same but with an extra bit
> set (S2), and with a new field for IPA which is not relevant here as
> we only report the IOVA.

Okay, as long as the IOVA is decoded and passed into report_fault it
should be fine

We don't yet have support for specifying the IPA on nesting..

Jason

Re: [PATCH v2] iommu/arm-smmu-v3: Match Stall behaviour for S2

[Robin Murphy]

On 15/08/2024 12:30 pm, Mostafa Saleh wrote:
> Hi Jason,
> 
> On Wed, Aug 14, 2024 at 12:51:51PM -0300, Jason Gunthorpe wrote:
>> On Wed, Aug 14, 2024 at 02:56:33PM +0000, Mostafa Saleh wrote:
>>
>>> Also described in the pseudocode “SteIllegal()”
>>>      if eff_idr0_stall_model == '10' && STE.S2S == '0' then
>>>          // stall_model forcing stall, but S2S == 0
>>>          return TRUE;
>>
>> This clips out an important bit:
>>
>> if STE.Config == '11x' then
>>    [..]
>>    if eff_idr0_stall_model == '10' && STE.S2S == '0' then
>>        // stall_model forcing stall, but S2S == 0
>>        return TRUE;
>>
>> And here we are using STRTAB_STE_0_CFG_S1_TRANS which is 101 and won't
>> match the STE.Config qualification.
>>
>> The plain text language said the S2S is only required if the S2 is
>> translating, STRTAB_STE_0_CFG_S1_TRANS puts it in bypass.
> 
> Yes, my bad, this should be for stage-2 only which is populated in
> arm_smmu_make_s2_domain_ste()
> 
>>
>>> +	/*
>>> +	 * S2S is ignored if stage-2 exists but not enabled.
>>> +	 * S2S is not compatible with ATS.
>>> +	 */
>>> +	if (master->stall_enabled && !ats_enabled &&
>>> +	    smmu->features & ARM_SMMU_FEAT_TRANS_S2)
>>> +		target->data[2] |= STRTAB_STE_2_S2S;
>>
>> We can't ignore ATS if it was requested here.

I don't see much value in adding effectively-dead checks for something 
which is already forbidden by the architecture. The definition of 
STALL_MODEL explicitly states:

"An SMMU associated with a PCI system must not have STALL_MODEL == 0b10".

Thanks,
Robin.

Re: [PATCH v2] iommu/arm-smmu-v3: Match Stall behaviour for S2

[Mostafa Saleh]

Hi Robin,

On Thu, Aug 15, 2024 at 01:16:19PM +0100, Robin Murphy wrote:
> On 15/08/2024 12:30 pm, Mostafa Saleh wrote:
> > Hi Jason,
> > 
> > On Wed, Aug 14, 2024 at 12:51:51PM -0300, Jason Gunthorpe wrote:
> > > On Wed, Aug 14, 2024 at 02:56:33PM +0000, Mostafa Saleh wrote:
> > > 
> > > > Also described in the pseudocode “SteIllegal()”
> > > >      if eff_idr0_stall_model == '10' && STE.S2S == '0' then
> > > >          // stall_model forcing stall, but S2S == 0
> > > >          return TRUE;
> > > 
> > > This clips out an important bit:
> > > 
> > > if STE.Config == '11x' then
> > >    [..]
> > >    if eff_idr0_stall_model == '10' && STE.S2S == '0' then
> > >        // stall_model forcing stall, but S2S == 0
> > >        return TRUE;
> > > 
> > > And here we are using STRTAB_STE_0_CFG_S1_TRANS which is 101 and won't
> > > match the STE.Config qualification.
> > > 
> > > The plain text language said the S2S is only required if the S2 is
> > > translating, STRTAB_STE_0_CFG_S1_TRANS puts it in bypass.
> > 
> > Yes, my bad, this should be for stage-2 only which is populated in
> > arm_smmu_make_s2_domain_ste()
> > 
> > > 
> > > > +	/*
> > > > +	 * S2S is ignored if stage-2 exists but not enabled.
> > > > +	 * S2S is not compatible with ATS.
> > > > +	 */
> > > > +	if (master->stall_enabled && !ats_enabled &&
> > > > +	    smmu->features & ARM_SMMU_FEAT_TRANS_S2)
> > > > +		target->data[2] |= STRTAB_STE_2_S2S;
> > > 
> > > We can't ignore ATS if it was requested here.
> 
> I don't see much value in adding effectively-dead checks for something which
> is already forbidden by the architecture. The definition of STALL_MODEL
> explicitly states:
> 
> "An SMMU associated with a PCI system must not have STALL_MODEL == 0b10".
> 

Ah, I was expecting that as otherwise it's contradiction, but couldn't
find it while searching. Thanks for pointing it out, I will drop all
references to ATS then.

Thanks,
Mostafa

> Thanks,
> Robin.

Re: [PATCH v2] iommu/arm-smmu-v3: Match Stall behaviour for S2

[Jason Gunthorpe]

On Thu, Aug 15, 2024 at 12:26:46PM +0000, Mostafa Saleh wrote:
> Hi Robin,
> 
> On Thu, Aug 15, 2024 at 01:16:19PM +0100, Robin Murphy wrote:
> > On 15/08/2024 12:30 pm, Mostafa Saleh wrote:
> > > Hi Jason,
> > > 
> > > On Wed, Aug 14, 2024 at 12:51:51PM -0300, Jason Gunthorpe wrote:
> > > > On Wed, Aug 14, 2024 at 02:56:33PM +0000, Mostafa Saleh wrote:
> > > > 
> > > > > Also described in the pseudocode “SteIllegal()”
> > > > >      if eff_idr0_stall_model == '10' && STE.S2S == '0' then
> > > > >          // stall_model forcing stall, but S2S == 0
> > > > >          return TRUE;
> > > > 
> > > > This clips out an important bit:
> > > > 
> > > > if STE.Config == '11x' then
> > > >    [..]
> > > >    if eff_idr0_stall_model == '10' && STE.S2S == '0' then
> > > >        // stall_model forcing stall, but S2S == 0
> > > >        return TRUE;
> > > > 
> > > > And here we are using STRTAB_STE_0_CFG_S1_TRANS which is 101 and won't
> > > > match the STE.Config qualification.
> > > > 
> > > > The plain text language said the S2S is only required if the S2 is
> > > > translating, STRTAB_STE_0_CFG_S1_TRANS puts it in bypass.
> > > 
> > > Yes, my bad, this should be for stage-2 only which is populated in
> > > arm_smmu_make_s2_domain_ste()
> > > 
> > > > 
> > > > > +	/*
> > > > > +	 * S2S is ignored if stage-2 exists but not enabled.
> > > > > +	 * S2S is not compatible with ATS.
> > > > > +	 */
> > > > > +	if (master->stall_enabled && !ats_enabled &&
> > > > > +	    smmu->features & ARM_SMMU_FEAT_TRANS_S2)
> > > > > +		target->data[2] |= STRTAB_STE_2_S2S;
> > > > 
> > > > We can't ignore ATS if it was requested here.
> > 
> > I don't see much value in adding effectively-dead checks for something which
> > is already forbidden by the architecture. The definition of STALL_MODEL
> > explicitly states:
> > 
> > "An SMMU associated with a PCI system must not have STALL_MODEL == 0b10".
> > 
> 
> Ah, I was expecting that as otherwise it's contradiction, but couldn't
> find it while searching. Thanks for pointing it out, I will drop all
> references to ATS then.

I was thinking this was also protecting against buggy FW since
stall_enable can be set by:
    device_property_read_bool(dev, "dma-can-stall"))

Alternatively we could directly prevent the clash even earlier:

@@ -3292,8 +3292,13 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev)
 
        if ((smmu->features & ARM_SMMU_FEAT_STALLS &&
             device_property_read_bool(dev, "dma-can-stall")) ||
-           smmu->features & ARM_SMMU_FEAT_STALL_FORCE)
-               master->stall_enabled = true;
+           smmu->features & ARM_SMMU_FEAT_STALL_FORCE) {
+               if (!dev_is_pci(dev))
+                       master->stall_enabled = true;
+               else
+                       dev_err(dev, FW_BUG
+                               "A SMMUv3 is required to run in stall mode for a PCI device\n");
+           }
 
        if (dev_is_pci(dev)) {

Though I have no idea how the GPU driver that wants to use this
works - it doesn't seem to be intree :\

Jason

Re: [PATCH v2] iommu/arm-smmu-v3: Match Stall behaviour for S2

[Robin Murphy]

On 15/08/2024 1:59 pm, Jason Gunthorpe wrote:
> On Thu, Aug 15, 2024 at 12:26:46PM +0000, Mostafa Saleh wrote:
>> Hi Robin,
>>
>> On Thu, Aug 15, 2024 at 01:16:19PM +0100, Robin Murphy wrote:
>>> On 15/08/2024 12:30 pm, Mostafa Saleh wrote:
>>>> Hi Jason,
>>>>
>>>> On Wed, Aug 14, 2024 at 12:51:51PM -0300, Jason Gunthorpe wrote:
>>>>> On Wed, Aug 14, 2024 at 02:56:33PM +0000, Mostafa Saleh wrote:
>>>>>
>>>>>> Also described in the pseudocode “SteIllegal()”
>>>>>>       if eff_idr0_stall_model == '10' && STE.S2S == '0' then
>>>>>>           // stall_model forcing stall, but S2S == 0
>>>>>>           return TRUE;
>>>>>
>>>>> This clips out an important bit:
>>>>>
>>>>> if STE.Config == '11x' then
>>>>>     [..]
>>>>>     if eff_idr0_stall_model == '10' && STE.S2S == '0' then
>>>>>         // stall_model forcing stall, but S2S == 0
>>>>>         return TRUE;
>>>>>
>>>>> And here we are using STRTAB_STE_0_CFG_S1_TRANS which is 101 and won't
>>>>> match the STE.Config qualification.
>>>>>
>>>>> The plain text language said the S2S is only required if the S2 is
>>>>> translating, STRTAB_STE_0_CFG_S1_TRANS puts it in bypass.
>>>>
>>>> Yes, my bad, this should be for stage-2 only which is populated in
>>>> arm_smmu_make_s2_domain_ste()
>>>>
>>>>>
>>>>>> +	/*
>>>>>> +	 * S2S is ignored if stage-2 exists but not enabled.
>>>>>> +	 * S2S is not compatible with ATS.
>>>>>> +	 */
>>>>>> +	if (master->stall_enabled && !ats_enabled &&
>>>>>> +	    smmu->features & ARM_SMMU_FEAT_TRANS_S2)
>>>>>> +		target->data[2] |= STRTAB_STE_2_S2S;
>>>>>
>>>>> We can't ignore ATS if it was requested here.
>>>
>>> I don't see much value in adding effectively-dead checks for something which
>>> is already forbidden by the architecture. The definition of STALL_MODEL
>>> explicitly states:
>>>
>>> "An SMMU associated with a PCI system must not have STALL_MODEL == 0b10".
>>>
>>
>> Ah, I was expecting that as otherwise it's contradiction, but couldn't
>> find it while searching. Thanks for pointing it out, I will drop all
>> references to ATS then.
> 
> I was thinking this was also protecting against buggy FW since
> stall_enable can be set by:
>      device_property_read_bool(dev, "dma-can-stall"))

If firmware goes out of its way to describe the system incorrectly then 
I'd consider that all bets are off, and there's little point in us 
trying to guess how to cope with a contradiction. For all we know, it 
could be that the stall property is in fact genuine and it's the ATS 
capability that was advertised erroneously.

> Alternatively we could directly prevent the clash even earlier:
> 
> @@ -3292,8 +3292,13 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev)
>   
>          if ((smmu->features & ARM_SMMU_FEAT_STALLS &&
>               device_property_read_bool(dev, "dma-can-stall")) ||
> -           smmu->features & ARM_SMMU_FEAT_STALL_FORCE)
> -               master->stall_enabled = true;
> +           smmu->features & ARM_SMMU_FEAT_STALL_FORCE) {
> +               if (!dev_is_pci(dev))
> +                       master->stall_enabled = true;
> +               else
> +                       dev_err(dev, FW_BUG
> +                               "A SMMUv3 is required to run in stall mode for a PCI device\n");

Unfortunately we can't do that, because there *are* RCiEP devices whose 
data interfaces are native AMBA, and thus for whom stalling is not 
actually a protocol violation as it would be on a real PCIe transport 
layer; correspondingly, it's *because* they are not true PCIe devices 
that they can't support ATS, and thus need stall support in order to do 
SVA, so things should still work out OK in practice.

> +           }
>   
>          if (dev_is_pci(dev)) {
> 
> Though I have no idea how the GPU driver that wants to use this
> works - it doesn't seem to be intree :\

It's not a GPU: drivers/crypto/hisilicon/zip/

Thanks,
Robin.

Re: [PATCH v2] iommu/arm-smmu-v3: Match Stall behaviour for S2

[Jason Gunthorpe]

On Thu, Aug 15, 2024 at 02:41:52PM +0100, Robin Murphy wrote:

> Unfortunately we can't do that, because there *are* RCiEP devices whose data
> interfaces are native AMBA, and thus for whom stalling is not actually a
> protocol violation as it would be on a real PCIe transport layer;
> correspondingly, it's *because* they are not true PCIe devices that they
> can't support ATS, and thus need stall support in order to do SVA, so things
> should still work out OK in practice.

I wondered if that would be the case.

Looks like if we want to do anything then arm_smmu_ats_supported()
would be the right place.

> >          if (dev_is_pci(dev)) {
> > 
> > Though I have no idea how the GPU driver that wants to use this
> > works - it doesn't seem to be intree :\
> 
> It's not a GPU: drivers/crypto/hisilicon/zip/

Ohhhh.. so it goes through uacce, I see.

Jason

Re: [PATCH v2] iommu/arm-smmu-v3: Match Stall behaviour for S2

[Mostafa Saleh]

On Thu, Aug 15, 2024 at 10:59:51AM -0300, Jason Gunthorpe wrote:
> On Thu, Aug 15, 2024 at 02:41:52PM +0100, Robin Murphy wrote:
> 
> > Unfortunately we can't do that, because there *are* RCiEP devices whose data
> > interfaces are native AMBA, and thus for whom stalling is not actually a
> > protocol violation as it would be on a real PCIe transport layer;
> > correspondingly, it's *because* they are not true PCIe devices that they
> > can't support ATS, and thus need stall support in order to do SVA, so things
> > should still work out OK in practice.
> 
> I wondered if that would be the case.
> 
> Looks like if we want to do anything then arm_smmu_ats_supported()
> would be the right place.

Yes, I guess that might be the place, although the spec seems more
relaxed regarding stage-1 stall.

Anyway I will drop it from this patch, as Robin mentioned this should
only happen with buggy firmware and we can have another patch to
harden that if required.

Thanks,
Mostafa

> 
> > >          if (dev_is_pci(dev)) {
> > > 
> > > Though I have no idea how the GPU driver that wants to use this
> > > works - it doesn't seem to be intree :\
> > 
> > It's not a GPU: drivers/crypto/hisilicon/zip/
> 
> Ohhhh.. so it goes through uacce, I see.
> 
> Jason