Re: [Xen-devel] [PATCH 2/2] xen/arm: Make HAS_PCI compilable on ARM by adding place-holder code

2015-04-16 Thread Ian Campbell 
On Mon, 2015-04-13 at 13:18 +0530, Manish Jaggi wrote:
 uint8_t pci_conf_read8(
 +uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func,
 +uint32_t reg)

Shouldn't this (and the other functions) match the prototype in
xen/pci.h, which is:
uint8_t pci_conf_read8(
unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
unsigned int reg);
?

We generally try and use precisely sized types in the ARM code, but
where we interact with generic/common interfaces we should either follow
what they do or make an argument (via a preparatory patch) for changing
them.

Ian.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 2/2] xen/arm: Make HAS_PCI compilable on ARM by adding place-holder code

2015-04-14 Thread Stefano Stabellini 
On Tue, 14 Apr 2015, Jaggi, Manish wrote:
  diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h
  index de13359..b8ec882 100644
  --- a/xen/include/asm-arm/pci.h
  +++ b/xen/include/asm-arm/pci.h
  @@ -1,7 +1,8 @@
  -#ifndef __X86_PCI_H__
  -#define __X86_PCI_H__
  +#ifndef __ARM_PCI_H__
  +#define __ARM_PCI_H__
 
   struct arch_pci_dev {
  +void *dev;
 
 void * is error-prone. Why can't you use the use the real structure?
 
 [manish]Will change it.  I believe dev_archdata structure has also a void *  
 (in asm-arm/device.h). So all void * are error prone in xen ?
 

As you know void* works around the type system, so it prevents the
compiler from making many type safety checks. We should try to avoid
them if we can.

I think that you are right, the void *iommu in dev_archdata should
actually be struct arm_smmu_xen_device *iommu.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 2/2] xen/arm: Make HAS_PCI compilable on ARM by adding place-holder code

2015-04-14 Thread Julien Grall 
On 14/04/15 10:28, Stefano Stabellini wrote:
 On Tue, 14 Apr 2015, Jaggi, Manish wrote:
 diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h
 index de13359..b8ec882 100644
 --- a/xen/include/asm-arm/pci.h
 +++ b/xen/include/asm-arm/pci.h
 @@ -1,7 +1,8 @@
 -#ifndef __X86_PCI_H__
 -#define __X86_PCI_H__
 +#ifndef __ARM_PCI_H__
 +#define __ARM_PCI_H__

  struct arch_pci_dev {
 +void *dev;

 void * is error-prone. Why can't you use the use the real structure?

 [manish]Will change it.  I believe dev_archdata structure has also a void *  
 (in asm-arm/device.h). So all void * are error prone in xen ?

 
 As you know void* works around the type system, so it prevents the
 compiler from making many type safety checks. We should try to avoid
 them if we can.

In this case, the pointer add more management (allocation...).

As for the device tree solution, the field should be a struct device.

 I think that you are right, the void *iommu in dev_archdata should
 actually be struct arm_smmu_xen_device *iommu.

I agree that void* should be void in most of the case when we know the
underlaying type. Although there is some place where the number of type
of unknown because it could be used to store driver specific case.

This is actually the case of this field. It contains private data for
IOMMU driver. When a new driver will be implemented, it will likely use
a different structure.

Furthermore, the SMMU drivers is self contained in a file, using struct
arm_smmu_xen_device* would require to export/define some part of the
driver in an header.

So, I don't think this is the right things to do.

Regards,

-- 
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 2/2] xen/arm: Make HAS_PCI compilable on ARM by adding place-holder code

2015-04-14 Thread Julien Grall 
On 14/04/15 11:33, Julien Grall wrote:
 On 14/04/15 10:28, Stefano Stabellini wrote:
 On Tue, 14 Apr 2015, Jaggi, Manish wrote:
 diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h
 index de13359..b8ec882 100644
 --- a/xen/include/asm-arm/pci.h
 +++ b/xen/include/asm-arm/pci.h
 @@ -1,7 +1,8 @@
 -#ifndef __X86_PCI_H__
 -#define __X86_PCI_H__
 +#ifndef __ARM_PCI_H__
 +#define __ARM_PCI_H__

  struct arch_pci_dev {
 +void *dev;

 void * is error-prone. Why can't you use the use the real structure?

 [manish]Will change it.  I believe dev_archdata structure has also a void * 
  (in asm-arm/device.h). So all void * are error prone in xen ?


 As you know void* works around the type system, so it prevents the
 compiler from making many type safety checks. We should try to avoid
 them if we can.
 
 In this case, the pointer add more management (allocation...).
 
 As for the device tree solution, the field should be a struct device.
 
 I think that you are right, the void *iommu in dev_archdata should
 actually be struct arm_smmu_xen_device *iommu.
 
 I agree that void* should be void in most of the case when we know the
 underlaying type. Although there is some place where the number of type
 of unknown because it could be used to store driver specific case.
 
 This is actually the case of this field. It contains private data for
 IOMMU driver. When a new driver will be implemented, it will likely use
 a different structure.
 
 Furthermore, the SMMU drivers is self contained in a file, using struct
 arm_smmu_xen_device* would require to export/define some part of the
 driver in an header.

A similar example would be the scheduler coder (see sched_data in
include/xen/sched-if.h). We don't want to expose the underlying
structure out of the file.

Regards,


-- 
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 2/2] xen/arm: Make HAS_PCI compilable on ARM by adding place-holder code

2015-04-13 Thread Jaggi, Manish 
Hi Julien,


From: xen-devel-boun...@lists.xen.org xen-devel-boun...@lists.xen.org on 
behalf of Julien Grall julien.gr...@citrix.com
Sent: Monday, April 13, 2015 4:12 PM
To: Jaggi, Manish; Xen Devel; Stefano Stabellini; Julien Grall; Ian Campbell; 
Kumar, Vijaya; prasun.kap...@cavium.com
Subject: Re: [Xen-devel] [PATCH 2/2] xen/arm: Make HAS_PCI compilable on ARM by 
adding place-holder code

Hi Manish,

On 13/04/15 08:48, Manish Jaggi wrote:
 Add ARM PCI Support
 ---
 a) Place holder functions are added for pci_conf_read/write calls.
 b) Macros dev_is_pci, pci_to_dev are implemented in
 drivers/passthrough/pci/arm code

 Signed-off-by: Manish Jaggi manish.ja...@caviumnetworks.com
 ---
  xen/arch/arm/Makefile|1 +
  xen/arch/arm/pci.c   |   60 +++
  xen/drivers/passthrough/arm/Makefile |1 +
  xen/drivers/passthrough/arm/pci.c|   88
 ++
  xen/drivers/passthrough/arm/smmu.c   |1 -
  xen/drivers/passthrough/pci.c|9 ++--
  xen/include/asm-arm/device.h |   33 +
  xen/include/asm-arm/domain.h |3 ++
  xen/include/asm-arm/pci.h|7 +--
  9 files changed, 186 insertions(+), 17 deletions(-)

[...]
 diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
 index 004aba9..68c292b 100644
 --- a/xen/drivers/passthrough/pci.c
 +++ b/xen/drivers/passthrough/pci.c
 @@ -1252,9 +1252,12 @@ static int assign_device(struct domain *d, u16
 seg, u8 bus, u8 devfn)
  /* Prevent device assign if mem paging or mem sharing have been
   * enabled for this domain */
  if ( unlikely(!need_iommu(d) 
 -(d-arch.hvm_domain.mem_sharing_enabled ||
 - d-mem_event-paging.ring_page ||
 - p2m_get_hostp2m(d)-global_logdirty)) )
 +(d-mem_event-paging.ring_page
 +#ifdef CONFIG_X86
 + || d-arch.hvm_domain.mem_sharing_enabled ||
 + p2m_get_hostp2m(d)-global_logdirty
 +#endif
 +)) )

Please avoid #ifdef CONFIG_* and introduce an arch macro. 
[Manish] ok
  return -EXDEV;

  if ( !spin_trylock(pcidevs_lock) )
 diff --git a/xen/include/asm-arm/device.h b/xen/include/asm-arm/device.h
 index a72f7c9..009ff0a 100644
 --- a/xen/include/asm-arm/device.h
 +++ b/xen/include/asm-arm/device.h
 @@ -6,6 +6,17 @@
  enum device_type
  {
  DEV_DT,
 +DEV_ENUMERATED,
 +};
 +
 +enum device_class
 +{
 +DEVICE_SERIAL,
 +DEVICE_IOMMU,
 +DEVICE_GIC,
 +DEVICE_PCI,
 +/* Use for error */
 +DEVICE_UNKNOWN,
  };

Why? It will be very unlikely that we have to create a structure device
for the IOMMU, GIC and SERIAL.

It would make more sense to add a DEV_PCI directly to device_type.
[manish] ok. 


  struct dev_archdata {
 @@ -16,28 +27,30 @@ struct dev_archdata {
  struct device
  {
  enum device_type type;
 +enum device_class class;
  #ifdef HAS_DEVICE_TREE
  struct dt_device_node *of_node; /* Used by drivers imported from
 Linux */
  #endif
  struct dev_archdata archdata;
 +#ifdef HAS_PCI
 +void *pci_dev;

This field is not necessary. I've added one for the DT for keeping
compatibility with Linux.
[Manish] pci_dev is not in struct device currently. Didn't get you
 I have added as It is required  for to_pci_dev call.

 +#endif

[..]

 +int dev_is_pci(device_t *dev);

This could easily be a macro.
[manish] ok


  struct device_desc {
  /* Device name */
 diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
 index 9e0419e..41ae847 100644
 --- a/xen/include/asm-arm/domain.h
 +++ b/xen/include/asm-arm/domain.h
 @@ -42,6 +42,8 @@ struct vtimer {
  uint64_t cval;
  };

 +#define has_arch_pdevs(d)(!list_empty((d)-arch.pdev_list))
 +
  struct arch_domain
  {
  #ifdef CONFIG_ARM_64
 @@ -56,6 +58,7 @@ struct arch_domain
  xen_pfn_t *grant_table_gpfn;

  struct io_handler io_handlers;
 +struct list_head pdev_list;
  /* Continuable domain_relinquish_resources(). */
  enum {
  RELMEM_not_started,
 diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h
 index de13359..b8ec882 100644
 --- a/xen/include/asm-arm/pci.h
 +++ b/xen/include/asm-arm/pci.h
 @@ -1,7 +1,8 @@
 -#ifndef __X86_PCI_H__
 -#define __X86_PCI_H__
 +#ifndef __ARM_PCI_H__
 +#define __ARM_PCI_H__

  struct arch_pci_dev {
 +void *dev;

void * is error-prone. Why can't you use the use the real structure?

[manish]Will change it.  I believe dev_archdata structure has also a void *  
(in asm-arm/device.h). So all void * are error prone in xen ?

Regards,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 2/2] xen/arm: Make HAS_PCI compilable on ARM by adding place-holder code

2015-04-13 Thread Manish Jaggi 


On Monday 13 April 2015 03:44 PM, Stefano Stabellini wrote:

On Mon, 13 Apr 2015, Manish Jaggi wrote:

Add ARM PCI Support
---
a) Place holder functions are added for pci_conf_read/write calls.
b) Macros dev_is_pci, pci_to_dev are implemented in
drivers/passthrough/pci/arm code

Signed-off-by: Manish Jaggi manish.ja...@caviumnetworks.com
---
  xen/arch/arm/Makefile|1 +
  xen/arch/arm/pci.c   |   60 +++
  xen/drivers/passthrough/arm/Makefile |1 +
  xen/drivers/passthrough/arm/pci.c|   88
++
  xen/drivers/passthrough/arm/smmu.c   |1 -
  xen/drivers/passthrough/pci.c|9 ++--
  xen/include/asm-arm/device.h |   33 +
  xen/include/asm-arm/domain.h |3 ++
  xen/include/asm-arm/pci.h|7 +--
  9 files changed, 186 insertions(+), 17 deletions(-)

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 935999e..aaf5d88 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -39,6 +39,7 @@ obj-y += device.o
  obj-y += decode.o
  obj-y += processor.o
  obj-y += smc.o
+obj-$(HAS_PCI) += pci.o
   #obj-bin-y += o
  diff --git a/xen/arch/arm/pci.c b/xen/arch/arm/pci.c
new file mode 100644
index 000..9438f41
--- /dev/null
+++ b/xen/arch/arm/pci.c
@@ -0,0 +1,60 @@
+#include xen/pci.h
+
+void _pci_conf_write(
+uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func,
+uint32_t reg, uint32_t data, int bytes)
+{
+}
+
+uint32_t _pci_conf_read(
+uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func,
+uint32_t reg, int bytes)
+{
+return 0;
+}

I guess that they are going to be implemented with platform specific
code? Although I thought that the mmcfg stuff is somewhat standard across
architectures.


yes.
_pci_conf_read/write will call pcihb_conf_read/write (pcihb = 
pci_host_bridge).

ref: http://lists.xen.org/archives/html/xen-devel/2015-02/msg02717.html

+uint8_t pci_conf_read8(
+uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func,
+uint32_t reg)
+{
+return (uint8_t)_pci_conf_read(seg, bus, dev, func, reg, 1);
+}
+
+uint16_t pci_conf_read16(
+uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func,
+uint32_t reg)
+{
+return (uint8_t)_pci_conf_read(seg, bus, dev, func, reg, 2);
+}
+
+uint32_t pci_conf_read32(
+uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func,
+uint32_t reg)
+{
+return (uint8_t)_pci_conf_read(seg, bus, dev, func, reg, 4);
+}
+
+void pci_conf_write8(
+uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func,
+uint32_t reg, uint8_t data)
+{
+ _pci_conf_write(seg, bus, dev, func, reg, data, 1);
+}
+
+void pci_conf_write16(
+uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func,
+uint32_t reg, uint16_t data)
+{
+ _pci_conf_write(seg, bus, dev, func, reg, data, 2);
+}
+
+void pci_conf_write32(
+uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func,
+uint32_t reg, uint32_t data)
+{
+ _pci_conf_write(seg, bus, dev, func, reg, data, 4);
+}
+
+void arch_pci_ro_device(int seg, int bdf)
+{
+}

This is also missing an implementation. Maybe you should add /* TODO */
here too

ok.




diff --git a/xen/drivers/passthrough/arm/Makefile
b/xen/drivers/passthrough/arm/Makefile
index f4cd26e..1a41549 100644
--- a/xen/drivers/passthrough/arm/Makefile
+++ b/xen/drivers/passthrough/arm/Makefile
@@ -1,2 +1,3 @@
  obj-y += iommu.o
  obj-y += smmu.o
+obj-$(HAS_PCI) += pci.o
diff --git a/xen/drivers/passthrough/arm/pci.c
b/xen/drivers/passthrough/arm/pci.c
new file mode 100644
index 000..07a5a78
--- /dev/null
+++ b/xen/drivers/passthrough/arm/pci.c
@@ -0,0 +1,88 @@
+/*
+ * PCI helper functions for arm for passthrough support.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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.
+ *
+ * Copyright (C) 2015 Cavium Networks
+ *
+ * Author: Manish Jaggi manish.ja...@cavium.com
+ */
+#include xen/pci.h
+#include asm/device.h
+#include xen/sched.h
+
+int _dump_pci_devices(struct pci_seg *pseg, void *arg)
+{
+struct pci_dev *pdev;
+
+printk( segment %04x \n, pseg-nr);
+
+list_for_each_entry ( pdev, pseg-alldevs_list, alldevs_list )
+{
+printk(%04x:%02x:%02x.%u - dom %-3d - node %-3d - MSIs  ,
+   pseg-nr, pdev-bus,
+   PCI_SLOT(pdev-devfn), PCI_FUNC(pdev-devfn),
+

Re: [Xen-devel] [PATCH 2/2] xen/arm: Make HAS_PCI compilable on ARM by adding place-holder code

2015-04-13 Thread Julien Grall 
Hi Manish,

On 13/04/15 08:48, Manish Jaggi wrote:
 Add ARM PCI Support
 ---
 a) Place holder functions are added for pci_conf_read/write calls.
 b) Macros dev_is_pci, pci_to_dev are implemented in
 drivers/passthrough/pci/arm code
 
 Signed-off-by: Manish Jaggi manish.ja...@caviumnetworks.com
 ---
  xen/arch/arm/Makefile|1 +
  xen/arch/arm/pci.c   |   60 +++
  xen/drivers/passthrough/arm/Makefile |1 +
  xen/drivers/passthrough/arm/pci.c|   88
 ++
  xen/drivers/passthrough/arm/smmu.c   |1 -
  xen/drivers/passthrough/pci.c|9 ++--
  xen/include/asm-arm/device.h |   33 +
  xen/include/asm-arm/domain.h |3 ++
  xen/include/asm-arm/pci.h|7 +--
  9 files changed, 186 insertions(+), 17 deletions(-)
 
 diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
 index 935999e..aaf5d88 100644
 --- a/xen/arch/arm/Makefile
 +++ b/xen/arch/arm/Makefile
 @@ -39,6 +39,7 @@ obj-y += device.o
  obj-y += decode.o
  obj-y += processor.o
  obj-y += smc.o
 +obj-$(HAS_PCI) += pci.o
  
  #obj-bin-y += o
  
 diff --git a/xen/arch/arm/pci.c b/xen/arch/arm/pci.c
 new file mode 100644
 index 000..9438f41
 --- /dev/null
 +++ b/xen/arch/arm/pci.c
 @@ -0,0 +1,60 @@
 +#include xen/pci.h
 +
 +void _pci_conf_write(

The _ is not necessary here. Please name the function pci_conf_write.

 +uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func,
 +uint32_t reg, uint32_t data, int bytes)

unsigned int bytes?

 +{
 +}
 +
 +uint32_t _pci_conf_read(
 +uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func,
 +uint32_t reg, int bytes)
 +{
 +return 0;
 +}

Same here.

 +
 +uint8_t pci_conf_read8(
 +uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func,
 +uint32_t reg)
 +{
 +return (uint8_t)_pci_conf_read(seg, bus, dev, func, reg, 1);
 +}
 +
 +uint16_t pci_conf_read16(
 +uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func,
 +uint32_t reg)
 +{
 +return (uint8_t)_pci_conf_read(seg, bus, dev, func, reg, 2);

Wrong cast.

 +}
 +
 +uint32_t pci_conf_read32(
 +uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func,
 +uint32_t reg)
 +{
 +return (uint8_t)_pci_conf_read(seg, bus, dev, func, reg, 4);

Wrong cast.

[..]

 diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
 index 004aba9..68c292b 100644
 --- a/xen/drivers/passthrough/pci.c
 +++ b/xen/drivers/passthrough/pci.c
 @@ -1252,9 +1252,12 @@ static int assign_device(struct domain *d, u16
 seg, u8 bus, u8 devfn)
  /* Prevent device assign if mem paging or mem sharing have been
   * enabled for this domain */
  if ( unlikely(!need_iommu(d) 
 -(d-arch.hvm_domain.mem_sharing_enabled ||
 - d-mem_event-paging.ring_page ||
 - p2m_get_hostp2m(d)-global_logdirty)) )
 +(d-mem_event-paging.ring_page
 +#ifdef CONFIG_X86
 + || d-arch.hvm_domain.mem_sharing_enabled ||
 + p2m_get_hostp2m(d)-global_logdirty
 +#endif
 +)) )

Please avoid #ifdef CONFIG_* and introduce an arch macro.

  return -EXDEV;
  
  if ( !spin_trylock(pcidevs_lock) )
 diff --git a/xen/include/asm-arm/device.h b/xen/include/asm-arm/device.h
 index a72f7c9..009ff0a 100644
 --- a/xen/include/asm-arm/device.h
 +++ b/xen/include/asm-arm/device.h
 @@ -6,6 +6,17 @@
  enum device_type
  {
  DEV_DT,
 +DEV_ENUMERATED,
 +};
 +
 +enum device_class
 +{
 +DEVICE_SERIAL,
 +DEVICE_IOMMU,
 +DEVICE_GIC,
 +DEVICE_PCI,
 +/* Use for error */
 +DEVICE_UNKNOWN,
  };

Why? It will be very unlikely that we have to create a structure device
for the IOMMU, GIC and SERIAL.

It would make more sense to add a DEV_PCI directly to device_type.

  
  struct dev_archdata {
 @@ -16,28 +27,30 @@ struct dev_archdata {
  struct device
  {
  enum device_type type;
 +enum device_class class;
  #ifdef HAS_DEVICE_TREE
  struct dt_device_node *of_node; /* Used by drivers imported from
 Linux */
  #endif
  struct dev_archdata archdata;
 +#ifdef HAS_PCI
 +void *pci_dev;

This field is not necessary. I've added one for the DT for keeping
compatibility with Linux.

 +#endif

[..]

 +int dev_is_pci(device_t *dev);

This could easily be a macro.

  
  struct device_desc {
  /* Device name */
 diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
 index 9e0419e..41ae847 100644
 --- a/xen/include/asm-arm/domain.h
 +++ b/xen/include/asm-arm/domain.h
 @@ -42,6 +42,8 @@ struct vtimer {
  uint64_t cval;
  };
  
 +#define has_arch_pdevs(d)(!list_empty((d)-arch.pdev_list))
 +
  struct arch_domain
  {
  #ifdef CONFIG_ARM_64
 @@ -56,6 +58,7 @@ struct arch_domain
  xen_pfn_t *grant_table_gpfn;
  
  struct io_handler io_handlers;
 +struct list_head pdev_list;
  /* Continuable domain_relinquish_resources(). */
  enum {