Re: [Qemu-devel] [PATCH v14 1/9] target-avr: AVR cores support is added.

[Peter Maydell]

On 29 July 2016 at 16:32, Michael Rolnik <address@hidden> wrote:
>     1. basic CPU structure
>     2. registers
>     3. no instructions
>     4. saving sreg, rampD, rampX, rampY, rampD, eind in HW representation
>
> Signed-off-by: Michael Rolnik <address@hidden>

> diff --git a/configure b/configure
> index f57fcc6..c4d58b4 100755
> --- a/configure
> +++ b/configure
> @@ -5641,6 +5641,8 @@ case "$target_name" in
>    x86_64)
>      TARGET_BASE_ARCH=i386
>    ;;
> +  avr)
> +  ;;

I asked you to move this in my review of v12, but you haven't.

>    alpha)
>    ;;
>    arm|armeb)
> @@ -5837,6 +5839,9 @@ disas_config() {
>
>  for i in $ARCH $TARGET_BASE_ARCH ; do
>    case "$i" in
> +  avr)
> +    disas_config "AVR"
> +  ;;

Ditto.

>    alpha)
>      disas_config "ALPHA"
>    ;;

> +#ifndef QEMU_AVR_CPU_QOM_H
> +#define QEMU_AVR_CPU_QOM_H
> +
> +#include "qom/cpu.h"
> +
> +#define TYPE_AVR_CPU "avr"
> +
> +#define AVR_CPU_CLASS(klass) \
> +                    OBJECT_CLASS_CHECK(AVRCPUClass, (klass), TYPE_AVR_CPU)
> +#define AVR_CPU(obj) \
> +                    OBJECT_CHECK(AVRCPU, (obj), TYPE_AVR_CPU)
> +#define AVR_CPU_GET_CLASS(obj) \
> +                    OBJECT_GET_CLASS(AVRCPUClass, (obj), TYPE_AVR_CPU)
> +
> +/**
> +*  AVRCPUClass:
> +*  @parent_realize: The parent class' realize handler.
> +*  @parent_reset: The parent class' reset handler.
> +*  @vr: Version Register value.
> +*
> +*  A AVR CPU model.
> + */

The spacing here is still wrong.

> +typedef struct AVRCPUClass {
> +    CPUClass parent_class;
> +
> +    DeviceRealize parent_realize;
> +    void (*parent_reset)(CPUState *cpu);
> +} AVRCPUClass;
> +
> +/**
> +*  AVRCPU:
> +*  @env: #CPUAVRState
> +*
> +*  A AVR CPU.
> +*/

Ditto.

> +typedef struct AVRCPU {
> +    /* < private > */
> +    CPUState parent_obj;
> +    /* < public > */
> +
> +    CPUAVRState env;
> +} AVRCPU;

> +static bool avr_cpu_has_work(CPUState *cs)
> +{
> +    AVRCPU *cpu = AVR_CPU(cs);
> +    CPUAVRState *env = &cpu->env;
> +
> +    return (cs->interrupt_request
> +                &   (CPU_INTERRUPT_HARD
> +                    | CPU_INTERRUPT_RESET))
> +            &&  cpu_interrupts_enabled(env);

I asked you to fix this spacing, but it's still all over the place.

If you could check that you've addressed all the comments
from previous patch review rounds that would make things
faster.

thanks
-- PMM