Am Thu, 2 May 2019 16:34:35 +0200
schrieb Ralf Ramsauer <[email protected]>:

> On 5/2/19 4:14 PM, Henning Schild wrote:
> > Am Thu, 2 May 2019 12:19:10 +0200
> > schrieb Ralf Ramsauer <[email protected]>:
> >   
> >> Hi,
> >>
> >> On 5/2/19 10:31 AM, Henning Schild wrote:  
> >>> Hi Ralf,
> >>>
> >>> redefining the "Enum" seems not too elegant. Did you look into
> >>> ways to use the header from python?    
> >>
> >> Duplicating things is in deed not the most elegant way, but it's
> >> the way how we handle other magic constants as well.
> >>
> >> Didn't yet look at any alternatives.
> >>  
> >>>
> >>> The "defines" should be really easy to parse without even using a
> >>> special python library. The only real problem might be locating
> >>> the header, it would need to be installed when running
> >>> "installed" or relative when running "local".    
> >>
> >> We could create a pyjailhouse/pcicaps.py during compilation phase.
> >> Make/sed magic could create the python file from a stub. This is
> >> basically the same how we create generated C headers.
> >>
> >> This would a) autocreate the "Enum" and make it easy to maintain
> >> and b) solve the problem when being installed.
> >>
> >> What do you think about this?  
> > 
> > Not sure the extra make before the first use would be nice or
> > acceptable. The python code could be generated inside pip, in which
> > case you want to have a solution for non-pip users.  
> 
> Maybe we're not talking about the same thing.
> 
> I'd simply use a small template for the skeleton of the python file,
> use sed and friends to fill its content based on C headers and copy
> it over to its final destination (e.g., pyjailhouse/pci_caps.py).

I see, have a generated copy in the tree instead of just shipping the
generator. Reminds me of autotools and make ;).

Henning

> There's no overlap with Pip. Pip won't care how this file was
> generated.
> 
> > Maybe generate the python code from the header with pip, if the
> > python code is not there fall back to using magic numbers.
> > 
> > Or we stick with the copy and wait for the first inconsistency to
> > make us follow up ;).  
> 
> Yeah, that's the easiest way. But maybe it's worth sourcing out those
> classes to a separate python file in any case.
> 
> BTW, there's one nice thing about those python Enums:
> 
> They will raise an expection if we get an unknown PCI cap id (which
> should never happen, though). If it happens, this gives a hint that we
> either lack a definition, or something really odd is going on.
> 
>   Ralf
> 
> > 
> > Henning
> >   
> >>   Ralf
> >>  
> >>>
> >>> Henning
> >>>
> >>> Am Tue, 30 Apr 2019 23:45:04 +0200
> >>> schrieb Ralf Ramsauer <[email protected]>:
> >>>     
> >>>> Definitions on C-side are in place, so let the generator produce
> >>>> those definitions.
> >>>>
> >>>> Signed-off-by: Ralf Ramsauer <[email protected]>
> >>>> ---
> >>>>  pyjailhouse/sysfs_parser.py   | 79
> >>>> +++++++++++++++++++++++++++++++----
> >>>> tools/root-cell-config.c.tmpl | 6 +-- 2 files changed, 72
> >>>> insertions(+), 13 deletions(-)
> >>>>
> >>>> diff --git a/pyjailhouse/sysfs_parser.py
> >>>> b/pyjailhouse/sysfs_parser.py index 4bb50605..368714b0 100644
> >>>> --- a/pyjailhouse/sysfs_parser.py
> >>>> +++ b/pyjailhouse/sysfs_parser.py
> >>>> @@ -22,6 +22,8 @@ import struct
> >>>>  import os
> >>>>  import fnmatch
> >>>>  
> >>>> +from enum import Enum
> >>>> +
> >>>>  root_dir = "/"
> >>>>  
> >>>>  def set_root_dir(dir):
> >>>> @@ -542,6 +544,65 @@ class PCIBARs:
> >>>>          f.close()
> >>>>  
> >>>>  
> >>>> +class PCI_CAP_ID(Enum):
> >>>> +    PM     = 0x01 # Power Management
> >>>> +    VPD    = 0x03 # Vital Product Data
> >>>> +    MSI    = 0x05 # Message Signalled Interrupts
> >>>> +    HT     = 0x08 # HyperTransport
> >>>> +    VNDR   = 0x09 # Vendor-Specific
> >>>> +    DBG    = 0x0A # Debug port
> >>>> +    SSVID  = 0x0D # Bridge subsystem vendor/device ID
> >>>> +    SECDEV = 0x0F # Secure Device
> >>>> +    EXP    = 0x10 # PCI Express
> >>>> +    MSIX   = 0x11 # MSI-X
> >>>> +    SATA   = 0x12 # SATA Data/Index Conf.
> >>>> +    AF     = 0x13 # PCI Advanced Features
> >>>> +
> >>>> +    def __str__(self):
> >>>> +        return "PCI_CAP_ID_" + self.name
> >>>> +
> >>>> +
> >>>> +class PCI_EXT_CAP_ID(Enum):
> >>>> +    ZERO    = 0x00 # ???
> >>>> +
> >>>> +    ERR     = 0x01 # Advanced Error Reporting
> >>>> +    VC      = 0x02 # Virtual Channel Capability
> >>>> +    DSN     = 0x03 # Device Serial Number
> >>>> +    PWR     = 0x04 # Power Budgeting
> >>>> +    RCLD    = 0x05 # Root Complex Link Declaration
> >>>> +    RCILC   = 0x06 # Root Complex Internal Link Control
> >>>> +    RCEC    = 0x07 # Root Complex Event Collector
> >>>> +    MFVC    = 0x08 # Multi-Function VC Capability
> >>>> +    VC9     = 0x09 # same as _VC
> >>>> +    RCRB    = 0x0A # Root Complex RB?
> >>>> +    VNDR    = 0x0B # Vendor-Specific
> >>>> +    CAC     = 0x0C # Config Access - obsolete
> >>>> +    ACS     = 0x0D # Access Control Services
> >>>> +    ARI     = 0x0E # Alternate Routing ID
> >>>> +    ATS     = 0x0F # Address Translation Services
> >>>> +    SRIOV   = 0x10 # Single Root I/O Virtualization
> >>>> +    MRIOV   = 0x11 # Multi Root I/O Virtualization
> >>>> +    MCAST   = 0x12 # Multicast
> >>>> +    PRI     = 0x13 # Page Request Interface
> >>>> +    AMD_XXX = 0x14 # Reserved for AMD
> >>>> +    REBAR   = 0x15 # Resizable BAR
> >>>> +    DPA     = 0x16 # Dynamic Power Allocation
> >>>> +    TPH     = 0x17 # TPH Requester
> >>>> +    LTR     = 0x18 # Latency Tolerance Reporting
> >>>> +    SECPCI  = 0x19 # Secondary PCIe Capability
> >>>> +    PMUX    = 0x1A # Protocol Multiplexing
> >>>> +    PASID   = 0x1B # Process Address Space ID
> >>>> +    DPC     = 0x1D # Downstream Port Containment
> >>>> +    L1SS    = 0x1E # L1 PM Substates
> >>>> +    PTM     = 0x1F # Precision Time Measurement
> >>>> +
> >>>> +    def __str__(self):
> >>>> +        id = "0x00"
> >>>> +        if self.value != 0:
> >>>> +            id = "PCI_EXT_CAP_ID_" + self.name
> >>>> +        return id + " | JAILHOUSE_PCI_EXT_CAP"
> >>>> +
> >>>> +
> >>>>  class PCICapability:
> >>>>      def __init__(self, id, start, len, flags, content,
> >>>> msix_address): self.id = id
> >>>> @@ -580,11 +641,12 @@ class PCICapability:
> >>>>              msix_address = 0
> >>>>              f.seek(cap)
> >>>>              (id, next) = struct.unpack('<BB', f.read(2))
> >>>> -            if id == 0x01:  # Power Management
> >>>> +            id = PCI_CAP_ID(id)
> >>>> +            if id == PCI_CAP_ID.PM:
> >>>>                  # this cap can be handed out completely
> >>>>                  len = 8
> >>>>                  flags = PCICapability.RW
> >>>> -            elif id == 0x05:  # MSI
> >>>> +            elif id == PCI_CAP_ID.MSI:
> >>>>                  # access will be moderated by hypervisor
> >>>>                  len = 10
> >>>>                  (msgctl,) = struct.unpack('<H', f.read(2))
> >>>> @@ -593,7 +655,7 @@ class PCICapability:
> >>>>                  if (msgctl & (1 << 8)) != 0:  # per-vector
> >>>> masking support len += 10
> >>>>                  flags = PCICapability.RW
> >>>> -            elif id == 0x10:  # Express
> >>>> +            elif id == PCI_CAP_ID.EXP:
> >>>>                  len = 20
> >>>>                  (cap_reg,) = struct.unpack('<H', f.read(2))
> >>>>                  if (cap_reg & 0xf) >= 2:  # v2 capability
> >>>> @@ -601,7 +663,7 @@ class PCICapability:
> >>>>                  # access side effects still need to be analyzed
> >>>>                  flags = PCICapability.RD
> >>>>                  has_extended_caps = True
> >>>> -            elif id == 0x11:  # MSI-X
> >>>> +            elif id == PCI_CAP_ID.MSIX:
> >>>>                  # access will be moderated by hypervisor
> >>>>                  len = 12
> >>>>                  (table,) = struct.unpack('<xxI', f.read(6))
> >>>> @@ -637,8 +699,9 @@ class PCICapability:
> >>>>                            'Extended Capability ID %x' % id)
> >>>>                      continue
> >>>>  
> >>>> +                id = PCI_EXT_CAP_ID(id)
> >>>>                  next = version_next >> 4
> >>>> -                if id == 0x0010:  # SR-IOV
> >>>> +                if id == PCI_EXT_CAP_ID.SRIOV:
> >>>>                      len = 64
> >>>>                      # access side effects still need to be
> >>>> analyzed flags = PCICapability.RD
> >>>> @@ -648,7 +711,6 @@ class PCICapability:
> >>>>                      flags = PCICapability.RD
> >>>>                  f.seek(cap + 4)
> >>>>                  content = f.read(len - 4)
> >>>> -                id |= PCICapability.JAILHOUSE_PCI_EXT_CAP
> >>>>                  caps.append(PCICapability(id, cap, len, flags,
> >>>> content, 0)) 
> >>>>          f.close()
> >>>> @@ -674,9 +736,10 @@ class PCIDevice:
> >>>>          self.msix_region_size = 0
> >>>>          self.msix_address = 0
> >>>>          for c in caps:
> >>>> -            if c.id in (0x05, 0x11):
> >>>> +            if isinstance(c.id, PCI_CAP_ID) and \
> >>>> +               c.id in (PCI_CAP_ID.MSI, PCI_CAP_ID.MSIX):
> >>>>                  msg_ctrl = struct.unpack('<H', c.content[:2])[0]
> >>>> -                if c.id == 0x05:  # MSI
> >>>> +                if c.id == PCI_CAP_ID.MSI:
> >>>>                      self.num_msi_vectors = 1 << ((msg_ctrl >>
> >>>> 1) & 0x7) self.msi_64bits = (msg_ctrl >> 7) & 1
> >>>>                  else:  # MSI-X
> >>>> diff --git a/tools/root-cell-config.c.tmpl
> >>>> b/tools/root-cell-config.c.tmpl index b80d7889..b4d64adf 100644
> >>>> --- a/tools/root-cell-config.c.tmpl
> >>>> +++ b/tools/root-cell-config.c.tmpl
> >>>> @@ -196,11 +196,7 @@ struct {
> >>>>                  /* ${comment} */
> >>>>                  % endfor
> >>>>                  {
> >>>> -                        % if (c.id & 0x8000) != 0:
> >>>> -                        .id = ${hex(c.id & 0x7fff)} |
> >>>> JAILHOUSE_PCI_EXT_CAP,
> >>>> -                        % else:
> >>>> -                        .id = ${hex(c.id)},
> >>>> -                        % endif
> >>>> +                        .id = ${c.id},
> >>>>                          .start = ${hex(c.start)},
> >>>>                          .len = ${c.len},
> >>>>                          .flags = ${c.flags},    
> >>>     
> >   

-- 
You received this message because you are subscribed to the Google Groups 
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to