Re: [Qemu-devel] [PATCH 25/34] scripts/kvm/kvm_stat: Group arch specific data

[Paolo Bonzini]

On 10/12/2015 13:12, Janosch Frank wrote:
> +        self.exit_reasons = None
> +        self.sc_perf_evt_open = None
> +        self.ioctl_numbers = {
> +            'SET_FILTER':  0x40082406,
> +            'ENABLE':      0x00002400,
> +            'DISABLE':     0x00002401,
> +            'RESET':       0x00002403,
> +        }
> +        self.set_arch_data()
>  
> -    SC_PERF_EVT_OPEN = 298
> -    EXIT_REASONS = X86_EXIT_REASONS[flag]
> +    def set_arch_data(self):
> +        machine = os.uname()[4]
>  
> -def s390_init():
> -    global SC_PERF_EVT_OPEN
> +        if machine.startswith('ppc'):
> +            self.sc_perf_evt_open = 319
> +            self.ioctl_numbers['ENABLE'] = 0x20002400
> +            self.ioctl_numbers['DISABLE'] = 0x20002401

Please keep the separate foo_init() methods.  Even better, change them
to __init__ of Arch subclasses, and add a static get_arch() method that
instantiates the appropriate subclass.  Then get_arch() can be used to
initialize ARCH.

Paolo

>  
> -    SC_PERF_EVT_OPEN = 331
> +            # PPC comes in 32 and 64 bit and some generated ioctl
> +            # numbers depend on the wordsize.
> +            char_ptr_size = ctypes.sizeof(ctypes.c_char_p)
> +            self.ioctl_numbers['SET_FILTER'] = 0x80002406 | char_ptr_size << 
> 16
>  
> -def ppc_init():
> -    global SC_PERF_EVT_OPEN
> -    global IOCTL_NUMBERS
> +        elif machine.startswith('aarch64'):
> +            self.sc_perf_evt_open = 241
> +            self.exit_reasons = AARCH64_EXIT_REASONS
>  
> -    SC_PERF_EVT_OPEN = 319
> +        elif machine.startswith('s390'):
> +            self.sc_perf_evt_open = 331
>  
> -    IOCTL_NUMBERS['ENABLE'] = 0x20002400
> -    IOCTL_NUMBERS['DISABLE'] = 0x20002401
> -    IOCTL_NUMBERS['SET_FILTER'] = 0x80002406 | 
> (ctypes.sizeof(ctypes.c_char_p)
> -                                                << 16)
> +        else:
> +            # X86_64
> +            for line in open('/proc/cpuinfo'):
> +                if not line.startswith('flags'):
> +                    continue
>  
> -def aarch64_init():
> -    global SC_PERF_EVT_OPEN
> -    global EXIT_REASONS
> +                self.sc_perf_evt_open = 298
>  
> -    SC_PERF_EVT_OPEN = 241
> -    EXIT_REASONS = AARCH64_EXIT_REASONS
> +                flags = line.split()
> +                if 'vmx' in flags:
> +                    self.exit_reasons = VMX_EXIT_REASONS
> +                if 'svm' in flags:
> +                    self.exit_reasons = SVM_EXIT_REASONS
> +                return
>  
> -def detect_platform():