Re: Status update for maintainers file

2021-04-01 Thread Sarah Harris
On Thu, 1 Apr 2021 09:53:38 +0200
Philippe Mathieu-Daudé  wrote:

> Thank you Sarah for your AVR reviews the last years, they
> have been of great value for the project and community.

I'm glad my work has been of use, much appreciated!

Kind regards,
Sarah Harris



Re: [PATCH] MAINTAINERS: Drop the lines with Sarah Harris

2021-04-01 Thread Sarah Harris
On Thu, 1 Apr 2021 08:26:41 +0200
Thomas Huth  wrote:

> This is done by sending a patch to the list that remove the entries. I just 
> did that now - would be great if you could reply to that mail on the list 
> with "Reviewed-by: ..." or "Acked-by: ...".

On Thu, 1 Apr 2021 08:24:26 +0200
Thomas Huth  wrote:

> Signed-off-by: Thomas Huth 
> ---
>  MAINTAINERS | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 554be84b32..e7b54372c8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -174,7 +174,6 @@ F: include/hw/arm/smmu*
>  
>  AVR TCG CPUs
>  M: Michael Rolnik 
> -R: Sarah Harris 
>  S: Maintained
>  F: docs/system/target-avr.rst
>  F: gdb-xml/avr-cpu.xml
> @@ -1045,7 +1044,6 @@ AVR Machines
>  
>  AVR MCUs
>  M: Michael Rolnik 
> -R: Sarah Harris 
>  S: Maintained
>  F: default-configs/*/avr-softmmu.mak
>  F: hw/avr/
> @@ -1058,7 +1056,6 @@ F: hw/misc/avr_power.c
>  
>  Arduino
>  M: Philippe Mathieu-Daudé 
> -R: Sarah Harris 
>  S: Maintained
>  F: hw/avr/arduino.c
>  
> -- 
> 2.27.0

Thanks for explaining and submitting this patch!
Reviewed-by: Sarah Harris 



Status update for maintainers file

2021-03-31 Thread Sarah Harris
Hi all,

I was added as a reviewer (in MAINTAINERS) for the AVR target for the duration 
of my research work using it.
The funding for my project expires in the middle of April, so I will not be 
able to provide time for reviewing patches from that point.

I'm not sure what the process is for removing my reviewer entry, but I assume 
if someone needs to be notified they'll be a member of this list.

Kind regards,
Sarah Harris



Re: [PATCH 03/11] hw/avr: Add limited support for avr gpio registers

2021-03-15 Thread Sarah Harris
s set output-high or input-pull-up read high).

> +
> +static const MemoryRegionOps avr_gpio_ops = {
> +.read = avr_gpio_read,
> +.write = avr_gpio_write,
> +.endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static void avr_gpio_init(Object *obj)
> +{
> +AVRGPIOState *s = AVR_GPIO(obj);
> +
> +qdev_init_gpio_out(DEVICE(obj), s->out, ARRAY_SIZE(s->out));
> +memory_region_init_io(>mmio, obj, _gpio_ops, s, TYPE_AVR_GPIO, 3);
> +sysbus_init_mmio(SYS_BUS_DEVICE(obj), >mmio);
> +}
> +static void avr_gpio_realize(DeviceState *dev, Error **errp)
> +{
> +/* Do nothing currently */
> +}
> +
> +
> +static void avr_gpio_class_init(ObjectClass *klass, void *data)
> +{
> +DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +dc->reset = avr_gpio_reset;
> +dc->realize = avr_gpio_realize;
> +}
> +
> +static const TypeInfo avr_gpio_info = {
> +.name  = TYPE_AVR_GPIO,
> +.parent= TYPE_SYS_BUS_DEVICE,
> +.instance_size = sizeof(AVRGPIOState),
> +.instance_init = avr_gpio_init,
> +.class_init= avr_gpio_class_init,
> +};
> +
> +static void avr_gpio_register_types(void)
> +{
> +type_register_static(_gpio_info);
> +}
> +
> +type_init(avr_gpio_register_types)
> diff --git a/hw/avr/Kconfig b/hw/avr/Kconfig
> index d31298c3cce..16a57ced11f 100644
> --- a/hw/avr/Kconfig
> +++ b/hw/avr/Kconfig
> @@ -3,6 +3,7 @@ config AVR_ATMEGA_MCU
>  select AVR_TIMER16
>  select AVR_USART
>  select AVR_POWER
> +select AVR_GPIO
>  
>  config ARDUINO
>  select AVR_ATMEGA_MCU
> diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
> index f0e7405f6e6..fde7019b2ba 100644
> --- a/hw/gpio/Kconfig
> +++ b/hw/gpio/Kconfig
> @@ -13,3 +13,6 @@ config GPIO_PWR
>  
>  config SIFIVE_GPIO
>  bool
> +
> +config AVR_GPIO
> +bool
> diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build
> index 79568f00ce3..366aca52ca2 100644
> --- a/hw/gpio/meson.build
> +++ b/hw/gpio/meson.build
> @@ -13,3 +13,4 @@
>  softmmu_ss.add(when: 'CONFIG_RASPI', if_true: files('bcm2835_gpio.c'))
>  softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_gpio.c'))
>  softmmu_ss.add(when: 'CONFIG_SIFIVE_GPIO', if_true: files('sifive_gpio.c'))
> +softmmu_ss.add(when: 'CONFIG_AVR_GPIO', if_true: files('avr_gpio.c'))
> -- 
> 2.26.2

Regards,
Sarah Harris



Re: [PATCH rc3 02/30] target/avr: Introduce AVR CPU class object

2020-01-29 Thread Sarah Harris
Hi,

I think I've found a minor bug: the stack pointer should be initialised to the 
size of SRAM in some or most cases.
Currently, SP is initialised to zero.

It seems modern AVRs set SP to the size of SRAM (RAMEND) at power-on, though a 
few older ones initialise to zero.
The ATmega328 (from 2009) [1], ATmega2560 (from 2005) [2], ATtiny2313 (from 
2003) [6], and ATtiny85 (from 2005) [3] all use RAMEND.
The ATmega8 (from 2001) [4], ATmega8535 (from 2002) [5], and AT90S8535 (from 
1998) [7] use zero.
I haven't found a list of which AVRs use which value (other than reading every 
datasheet).

Given that GCC performs this initialisation in software anyway (so what the 
hardware does doesn't matter), I think this is a minor issue.
It will only affect hand written assembly programs that don't do their own 
initialisation (which seems to be discouraged as not all resets are power-on 
events).
I'm not sure what, if anything, needs to be done about it but it might be worth 
fixing now we're emulating specific chips.

Kind regards,
Sarah Harris

[1] 
http://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-7810-Automotive-Microcontrollers-ATmega328P_Datasheet.pdf
 (section 6.5.1)
[2] 
http://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-2549-8-bit-AVR-Microcontroller-ATmega640-1280-1281-2560-2561_datasheet.pdf
 (section 7.6)
[3] 
http://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-2586-AVR-8-bit-Microcontroller-ATtiny25-ATtiny45-ATtiny85_Datasheet.pdf
 (section 4.6.1)
[4] 
http://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-2486-8-bit-AVR-microcontroller-ATmega8_L_datasheet.pdf
 (page 13)
[5] http://ww1.microchip.com/downloads/en/DeviceDoc/doc2502.pdf (page 12)
[6] 
http://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-2543-AVR-ATtiny2313_Datasheet.pdf
 (page 11)
[7] http://ww1.microchip.com/downloads/en/DeviceDoc/doc1041.pdf (page 20)

On Sun, 26 Jan 2020 23:54:43 +0100
Aleksandar Markovic  wrote:

> +static void avr_cpu_reset(CPUState *cs)
> +{
> +AVRCPU *cpu = AVR_CPU(cs);
> +AVRCPUClass *mcc = AVR_CPU_GET_CLASS(cpu);
> +CPUAVRState *env = >env;
> +
> +mcc->parent_reset(cs);
> +
> +env->pc_w = 0;
> +env->sregI = 1;
> +env->sregC = 0;
> +env->sregZ = 0;
> +env->sregN = 0;
> +env->sregV = 0;
> +env->sregS = 0;
> +env->sregH = 0;
> +env->sregT = 0;
> +
> +env->rampD = 0;
> +env->rampX = 0;
> +env->rampY = 0;
> +env->rampZ = 0;
> +env->eind = 0;
> +env->sp = 0;
> +
> +env->skip = 0;
> +
> +memset(env->r, 0, sizeof(env->r));
> +
> +tlb_flush(cs);
> +}



Re: [PATCH rc2 12/25] hw/timer: Add limited support for Atmel 16 bit timer peripheral

2020-01-24 Thread Sarah Harris
Hi,

Do I understand correctly that you need Ed to email a "Signed-off-by: Ed 
Robbins " himself?
Ed's cc'ed already, but I'll email him directly to make sure he's seen this 
discussion.

Sarah

On Fri, 24 Jan 2020 11:51:13 +0100
Philippe Mathieu-Daudé  wrote:

> Hello Sarah,
> 
> On 1/24/20 11:42 AM, Alex Bennée wrote:
> > 
> > Philippe Mathieu-Daudé  writes:
> > 
> >> From: Michael Rolnik 
> >>
> >> These were designed to facilitate testing but should provide enough
> >> function to be useful in other contexts.  Only a subset of the functions
> >> of each peripheral is implemented, mainly due to the lack of a standard
> >> way to handle electrical connections (like GPIO pins).
> >>
> >> Signed-off-by: Sarah Harris 
> >> Message-Id: <20200118191416.19934-13-mrol...@gmail.com>
> >> Signed-off-by: Philippe Mathieu-Daudé 
> >> [rth: Squash info mtree fixes and a file rename from f4bug, which was:]
> >> Suggested-by: Aleksandar Markovic 
> >> Signed-off-by: Richard Henderson 
> >> [PMD: Use qemu_log_mask(LOG_UNIMP), replace goto by return]
> >> Signed-off-by: Philippe Mathieu-Daudé 
> >> ---
> > 
> >> --- /dev/null
> >> +++ b/include/hw/timer/atmel_timer16.h
> >> @@ -0,0 +1,94 @@
> >> +/*
> >> + * Atmel AVR 16 bit timer
> >> + *
> >> + * Copyright (c) 2018 University of Kent
> >> + * Author: Ed Robbins
> > 
> > No sign off from the author here?
> 
> Hmmm I Sarah Harris's one, who is from the University of Kent, isn't it 
> enough? (I remember patched from Xilinx with Edgar S-o-b but from other 
> authors, Edgar vouched for Xilinx).
> 
> Sarah, can you get Ed Signed-off-by?
> 
> >> --- /dev/null
> >> +++ b/hw/timer/atmel_timer16.c
> >> @@ -0,0 +1,605 @@
> > 
> >> +
> >> +/* Helper macros */
> >> +#define VAL16(l, h) ((h << 8) | l)
> >> +#define DB_PRINT(fmt, args...) /* Nothing */
> >> +/*#define DB_PRINT(fmt, args...) printf("%s: " fmt "\n", __func__, ##
> >> args)*/
> > 
> > Format strings are likely to bitrot. Either use a if (GATE) or
> > tracepoints.
> 
> Yes...
> 
> > 
> > 
> > Otherwise:
> > 
> > Reviewed-by: Alex Bennée 
> 
> Thanks!
> 



Re: [PATCH v37 00/17] QEMU AVR 8 bit cores

2019-11-29 Thread Sarah Harris
> Sarah,
> do you mind if use the same license I use for my code?
I'm happy to use the same license.

Kind regards,
Sarah Harris


On Thu, 28 Nov 2019 14:28:19 +0200
Michael Rolnik  wrote:

> On Wed, Nov 27, 2019 at 11:06 PM Aleksandar Markovic <
> aleksandar.m.m...@gmail.com> wrote:
> 
> > On Wed, Nov 27, 2019 at 6:53 PM Michael Rolnik  wrote:
> > >
> > > This series of patches adds 8bit AVR cores to QEMU.
> > > All instruction, except BREAK/DES/SPM/SPMX, are implemented. Not fully
> > tested yet.
> > > However I was able to execute simple code with functions. e.g fibonacci
> > calculation.
> > > This series of patches include a non real, sample board.
> > > No fuses support yet. PC is set to 0 at reset.
> > >
> >
> > I have a couple of general remarks, so I am responding to the cover
> > letter, not individual patches.
> >
> > 1) The licenses for Sarah devices differ than the rest - shouldn't all
> > licenses be harmonized?
> 
> Sarah,
> do you mind if use the same license I use for my code?
> 
> 
> >
> 
> 
> > 2) There is an architectural problem with peripherals. It is possible
> > that they evolve over time, so, for example, USART could not be the
> > same for older and newer CPUs (in principle, newer peripheral is
> > expected to be o sort of "superset" of the older). How do you solve
> > that problem? Right now, it may not looks serious to you, but if you
> > don;t think about that right now, from the outset, soon the code will
> > become so entangled, ti woudl be almost very difficult to fix it.
> > Please think about that, how would you solve it, is there a way to
> > pass the information on the currently emulated CPU to the code
> > covering a peripheral, and provide a different behaviour?
> >
> Hi Aleksandar,
> 
> Please explain. I don't see any problem from CPU's perspective.
> as for the sample board is just a sample, I hope other people will create
> real models or real hw.
> there was no way I could provide a CPU alone, that's why there is sample.
> 
> 
> 
> >
> > > Following are examples of possible usages, assuming program.elf is
> > compiled for AVR cpu
> > > 1.  Continious non interrupted execution
> > > run `qemu-system-avr -kernel program.elf`
> > > 2.  Continious non interrupted execution with serial output into telnet
> > window
> > > run `qemu-system-avr -kernel program.elf -serial
> > tcp::5678,server,nowait -nographic `
> > > run `telent localhost 5678`
> > > 3.  Continious non interrupted execution with serial output into stdout
> > > run `qemu-system-avr -kernel program.elf -serial stdio`
> > > 4.  Debugging wit GDB debugger
> > > run `qemu-system-avr -kernel program.elf -s -S`
> > > run `avr-gdb program.elf` and then within GDB shell `target remote
> > :1234`
> > > 5.  Print out executed instructions
> > > run `qemu-system-avr -kernel program.elf -d in_asm`
> > >
> >
> > Thank you so much for these examples!
> >
> > Aleksandar
> >
> >
> > >
> > > the patches include the following
> > > 1. just a basic 8bit AVR CPU, without instruction decoding or translation
> > > 2. CPU features which allow define the following 8bit AVR cores
> > >  avr1
> > >  avr2 avr25
> > >  avr3 avr31 avr35
> > >  avr4
> > >  avr5 avr51
> > >  avr6
> > >  xmega2 xmega4 xmega5 xmega6 xmega7
> > > 3. a definition of sample machine with SRAM, FLASH and CPU which allows
> > to execute simple code
> > > 4. encoding for all AVR instructions
> > > 5. interrupt handling
> > > 6. helpers for IN, OUT, SLEEP, WBR & unsupported instructions
> > > 7. a decoder which given an opcode decides what istruction it is
> > > 8. translation of AVR instruction into TCG
> > > 9. all features together
> > >
> > > changes since v3
> > > 1. rampD/X/Y/Z registers are encoded as 0x00ff (instead of
> > 0x00ff) for faster address manipulaton
> > > 2. ffs changed to ctz32
> > > 3. duplicate code removed at avr_cpu_do_interrupt
> > > 4. using andc instead of not + and
> > > 5. fixing V flag calculation in varios instructions
> > > 6. freeing local variables in PUSH
> > > 7. tcg_const_local_i32 -> tcg_const_i32
> > > 8. using sextract32 instead of my implementation
> > > 9. fixing BLD instruction
> > > 10.xor(r) instead of 0xff - r at CO

Re: [PATCH v35 10/13] target/avr: Add limited support for USART and 16 bit timer peripherals

2019-11-29 Thread Sarah Harris
Hi Aleksandar,

Yes, adding a note about the limitations of the USART emulation sounds like a 
good idea.

Yes, I'm happy with switching to the (L)GPL license that's being used elsewhere.

Kind regards,
Sarah Harris


On Thu, 28 Nov 2019 12:02:38 +0100
Aleksandar Markovic  wrote:

> On Thursday, November 28, 2019, Aleksandar Markovic <
> aleksandar.m.m...@gmail.com> wrote:
> 
> >
> >
> > On Thursday, November 28, 2019, Sarah Harris  wrote:
> >
> >> Hi Aleksandar,
> >>
> >> > Sarah, thanks for taking your tome to respond!
> >> No problem! :)
> >>
> >> > do we fully support what is said in:
> >> > * 22.6.2 Sending Frames with 9 Data Bit
> >> > * 22.7.2 Receiving Frames with 9 Data Bits
> >> No, QEMU's character device system only supports 8 bit characters.
> >> Shorter characters can be padded easily, but longer is a problem.
> >> At the moment we just emit a warning and ignore the extra bit in UCSRnB
> >> (i.e. behave as if 8 bits was selected).
> >>
> >> > And the same question for section:
> >> > * 22.9 Multi-processor Communication Mode
> >> No, this was out of scope for testing use.
> >> This case is checked when writing to the UCSRnA register, `if (value &
> >> USART_CSRA_MPCM)`, and causes a warning.
> >> I don't know if we should crash instead, but at the moment we just log
> >> the warning and continue.
> >> (USART emulation will be incorrect from when this happens and until MPCM
> >> is disabled)
> >>
> >>
> > OK. Thanks. All this sounds reasonable to me. Do you agree that we insert:
> >
> > /*
> >  * Limitation of this emulation:
> >  *
> >  *   * Sending and receiving frames with 9 data bits sre not supported
> >  *   * Multi-processor communication mode is not supported
> >  */
> >
> > or a similar comment, close to the top of the file?
> >
> >
> One more question, Sarah, Michael left the license preambles the same as
> originals, however this is not a good license (there are some legal
> nuances) for QEMU, do you agree that the license preambles for your
> implementations are changed to LGPL 2.1 (with wording "or later (at your
> option)") that Michael used elsewhere?
> 
> Best regards,
> 
>  Aleksandar
> 
> 
> > Yours,
> > Aleksandar
> >
> >
> > Kind regards,
> >> Sarah Harris
> >>
> >>
> >> On Mon, 25 Nov 2019 19:57:48 +0100
> >> Aleksandar Markovic  wrote:
> >>
> >> > On Mon, Nov 25, 2019 at 4:57 PM Sarah Harris  wrote:
> >> > >
> >> > > Hi Aleksandar,
> >> > >
> >> > > > - Is there a place in docs that explain its implementation in
> >> general?
> >> > > This implementation was based on the datasheet for the ATmega2560
> >> ("ATmega640/1280/1281/2560/2561 datasheet" available from Microchip's
> >> website).
> >> > > (I'm not sure if posting a URL will trigger any spam filters, so I'll
> >> leave it for now)
> >> > > See section 22.10, "USART - Register Description".
> >> > >
> >> >
> >> > OK.
> >> >
> >> > > > - Why do cases 4, 5, 6 issue relatively unclear error message
> >> > > > ""update_char_mask(): Reserved character size "? Is there a
> >> > > > better wording perhaps? Where is justification in the doc for these
> >> > > > cases?
> >> > > The hardware can send/receive characters of various lengths,
> >> specified by settings in these configuration registers.
> >> > > The cases are defined in table 22-7, "UCSZn Bits Settings", which
> >> specifies that modes 4, 5, and 6 are reserved and should not be used.
> >> > > I'm not sure how better to explain this fault to the user; this is an
> >> edge case that I'd expect only an AVR developer testing their own program
> >> to see, so describing it in the same way as the datasheet seems a good 
> >> idea.
> >> > >
> >> >
> >> > OK. I somehow missed table 22-7 while comparing the code and specs - my
> >> bad.
> >> >
> >> > > > - What would be the docs justification for case 7? Why is an error
> >> > > > message issued, but still "char_mask" is set, and I guess, further
> >> > > > processing will go on? Why the error message says &qu

Re: [PATCH v36 17/17] target/avr: Update MAINTAINERS file

2019-11-28 Thread Sarah Harris
Yes, I don't have time to do maintenance, but I can manage reviewing.

Kind regards,
Sarah Harris


On Tue, 26 Nov 2019 22:41:32 +0200
Michael Rolnik  wrote:

> Ah. I think Sarah was ok with reviewer role.
> 
> On Tue, Nov 26, 2019 at 9:39 PM Aleksandar Markovic <
> aleksandar.m.m...@gmail.com> wrote:
> 
> > On Tue, Nov 26, 2019 at 8:06 PM Michael Rolnik  wrote:
> > >
> > > Aleksandar,
> > >
> > > there was an email from Sarah, stating that she does not want to be a
> > maintainer.
> > >
> >
> > But this is for "reviewer" role, not "maintainer".
> >
> > Sarah?
> >
> > > On Tue, Nov 26, 2019 at 5:17 AM Aleksandar Markovic <
> > aleksandar.m.m...@gmail.com> wrote:
> > >>
> > >>
> > >>
> > >> On Sunday, November 24, 2019, Michael Rolnik  wrote:
> > >>>
> > >>> Include AVR maintaners in MAINTAINERS file
> > >>>
> > >>> Signed-off-by: Michael Rolnik 
> > >>> ---
> > >>>  MAINTAINERS | 9 +
> > >>>  1 file changed, 9 insertions(+)
> > >>>
> > >>> diff --git a/MAINTAINERS b/MAINTAINERS
> > >>> index 5e5e3e52d6..ad2d9dd04a 100644
> > >>> --- a/MAINTAINERS
> > >>> +++ b/MAINTAINERS
> > >>> @@ -163,6 +163,15 @@ S: Maintained
> > >>>  F: hw/arm/smmu*
> > >>>  F: include/hw/arm/smmu*
> > >>>
> > >>> +AVR TCG CPUs
> > >>> +M: Michael Rolnik 
> > >>> +S: Maintained
> > >>> +F: target/avr/
> > >>> +F: hw/misc/avr_mask.c
> > >>> +F: hw/char/avr_usart.c
> > >>> +F: hw/timer/avr_timer16.c
> > >>> +F: hw/avr/
> > >>> +
> > >>
> > >>
> > >> I had a strange feeling that something is missing here, and I finally
> > realized what that was:
> > >>
> > >> R: Sarah Harris 
> > >>
> > >> https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg04225.html
> > >>
> > >>
> > >>
> > >>>  CRIS TCG CPUs
> > >>>  M: Edgar E. Iglesias 
> > >>>  S: Maintained
> > >>> --
> > >>> 2.17.2 (Apple Git-113)
> > >>>
> > >
> > >
> > > --
> > > Best Regards,
> > > Michael Rolnik
> >
> 
> 
> -- 
> Best Regards,
> Michael Rolnik



Re: [PATCH v35 10/13] target/avr: Add limited support for USART and 16 bit timer peripherals

2019-11-28 Thread Sarah Harris
Hi Aleksandar,

> Sarah, thanks for taking your tome to respond!
No problem! :)

> do we fully support what is said in:
> * 22.6.2 Sending Frames with 9 Data Bit
> * 22.7.2 Receiving Frames with 9 Data Bits
No, QEMU's character device system only supports 8 bit characters.
Shorter characters can be padded easily, but longer is a problem.
At the moment we just emit a warning and ignore the extra bit in UCSRnB (i.e. 
behave as if 8 bits was selected).

> And the same question for section:
> * 22.9 Multi-processor Communication Mode
No, this was out of scope for testing use.
This case is checked when writing to the UCSRnA register, `if (value & 
USART_CSRA_MPCM)`, and causes a warning.
I don't know if we should crash instead, but at the moment we just log the 
warning and continue.
(USART emulation will be incorrect from when this happens and until MPCM is 
disabled)

Kind regards,
Sarah Harris


On Mon, 25 Nov 2019 19:57:48 +0100
Aleksandar Markovic  wrote:

> On Mon, Nov 25, 2019 at 4:57 PM Sarah Harris  wrote:
> >
> > Hi Aleksandar,
> >
> > > - Is there a place in docs that explain its implementation in general?
> > This implementation was based on the datasheet for the ATmega2560 
> > ("ATmega640/1280/1281/2560/2561 datasheet" available from Microchip's 
> > website).
> > (I'm not sure if posting a URL will trigger any spam filters, so I'll leave 
> > it for now)
> > See section 22.10, "USART - Register Description".
> >
> 
> OK.
> 
> > > - Why do cases 4, 5, 6 issue relatively unclear error message
> > > ""update_char_mask(): Reserved character size "? Is there a
> > > better wording perhaps? Where is justification in the doc for these
> > > cases?
> > The hardware can send/receive characters of various lengths, specified by 
> > settings in these configuration registers.
> > The cases are defined in table 22-7, "UCSZn Bits Settings", which specifies 
> > that modes 4, 5, and 6 are reserved and should not be used.
> > I'm not sure how better to explain this fault to the user; this is an edge 
> > case that I'd expect only an AVR developer testing their own program to 
> > see, so describing it in the same way as the datasheet seems a good idea.
> >
> 
> OK. I somehow missed table 22-7 while comparing the code and specs - my bad.
> 
> > > - What would be the docs justification for case 7? Why is an error
> > > message issued, but still "char_mask" is set, and I guess, further
> > > processing will go on? Why the error message says "Nine bit character
> > > requested"? Who said that (that *nine* bit characters were requested?
> > > :-)
> > Case 7 also comes from table 22-7, and specifies that the USART should 
> > send/receive 9 bits per character.
> > For characters <= 8 bits it's easy to pad them to the 8 bit bytes that the 
> > character device subsystem operates on.
> > For characters of 9 bits we'd have to throw away one bit, which seems like 
> > a bad thing to do.
> > I decided it wasn't enough to justify crashing, but the user should be made 
> > aware that data is being lost and the output might not be what they would 
> > otherwise expect.
> >
> 
> Sarah, thanks for taking your tome to respond! Could you just explain
> to me do we fully support what is said in:
> 
> * 22.6.2 Sending Frames with 9 Data Bit
> * 22.7.2 Receiving Frames with 9 Data Bits
> 
> or perhaps there are some limitations?
> 
> And the same question for section:
> 
> * 22.9 Multi-processor Communication Mode
> 
> Please note that I don't suggest amending or extending your
> implementation, I just want to understand it better.
> 
> Best regards,
> Aleksandar
> 
> 
> > Kind regards,
> > Sarah Harris
> >
> >
> > On Fri, 22 Nov 2019 16:10:02 +0100
> > Aleksandar Markovic  wrote:
> >
> > > On Tue, Oct 29, 2019 at 10:25 PM Michael Rolnik  wrote:
> > > >
> > > > From: Sarah Harris 
> > > >
> > > > These were designed to facilitate testing but should provide enough 
> > > > function to be useful in other contexts.
> > > > Only a subset of the functions of each peripheral is implemented, 
> > > > mainly due to the lack of a standard way to handle electrical 
> > > > connections (like GPIO pins).
> > > >
> > > > Signed-off-by: Sarah Harris 
> > > > ---
> > > >  hw/char/Kconfig|   3 +
> > > >  hw/char/Makefile.objs  |   1 +
> > > >  hw/char/avr_usart.c| 324 

Re: [PATCH v35 10/13] target/avr: Add limited support for USART and 16 bit timer peripherals

2019-11-25 Thread Sarah Harris
Hi Aleksandar,

> - Is there a place in docs that explain its implementation in general?
This implementation was based on the datasheet for the ATmega2560 
("ATmega640/1280/1281/2560/2561 datasheet" available from Microchip's website).
(I'm not sure if posting a URL will trigger any spam filters, so I'll leave it 
for now)
See section 22.10, "USART - Register Description".

> - Why do cases 4, 5, 6 issue relatively unclear error message
> ""update_char_mask(): Reserved character size "? Is there a
> better wording perhaps? Where is justification in the doc for these
> cases?
The hardware can send/receive characters of various lengths, specified by 
settings in these configuration registers.
The cases are defined in table 22-7, "UCSZn Bits Settings", which specifies 
that modes 4, 5, and 6 are reserved and should not be used.
I'm not sure how better to explain this fault to the user; this is an edge case 
that I'd expect only an AVR developer testing their own program to see, so 
describing it in the same way as the datasheet seems a good idea.

> - What would be the docs justification for case 7? Why is an error
> message issued, but still "char_mask" is set, and I guess, further
> processing will go on? Why the error message says "Nine bit character
> requested"? Who said that (that *nine* bit characters were requested?
> :-)
Case 7 also comes from table 22-7, and specifies that the USART should 
send/receive 9 bits per character.
For characters <= 8 bits it's easy to pad them to the 8 bit bytes that the 
character device subsystem operates on.
For characters of 9 bits we'd have to throw away one bit, which seems like a 
bad thing to do.
I decided it wasn't enough to justify crashing, but the user should be made 
aware that data is being lost and the output might not be what they would 
otherwise expect.

Kind regards,
Sarah Harris


On Fri, 22 Nov 2019 16:10:02 +0100
Aleksandar Markovic  wrote:

> On Tue, Oct 29, 2019 at 10:25 PM Michael Rolnik  wrote:
> >
> > From: Sarah Harris 
> >
> > These were designed to facilitate testing but should provide enough 
> > function to be useful in other contexts.
> > Only a subset of the functions of each peripheral is implemented, mainly 
> > due to the lack of a standard way to handle electrical connections (like 
> > GPIO pins).
> >
> > Signed-off-by: Sarah Harris 
> > ---
> >  hw/char/Kconfig|   3 +
> >  hw/char/Makefile.objs  |   1 +
> >  hw/char/avr_usart.c| 324 ++
> >  hw/misc/Kconfig|   3 +
> >  hw/misc/Makefile.objs  |   2 +
> >  hw/misc/avr_mask.c | 112 ++
> >  hw/timer/Kconfig   |   3 +
> >  hw/timer/Makefile.objs |   2 +
> >  hw/timer/avr_timer16.c | 605 +
> >  include/hw/char/avr_usart.h|  97 ++
> >  include/hw/misc/avr_mask.h |  47 +++
> >  include/hw/timer/avr_timer16.h |  97 ++
> >  12 files changed, 1296 insertions(+)
> >  create mode 100644 hw/char/avr_usart.c
> >  create mode 100644 hw/misc/avr_mask.c
> >  create mode 100644 hw/timer/avr_timer16.c
> >  create mode 100644 include/hw/char/avr_usart.h
> >  create mode 100644 include/hw/misc/avr_mask.h
> >  create mode 100644 include/hw/timer/avr_timer16.h
> >
> > diff --git a/hw/char/Kconfig b/hw/char/Kconfig
> > index 40e7a8b8bb..331b20983f 100644
> > --- a/hw/char/Kconfig
> > +++ b/hw/char/Kconfig
> > @@ -46,3 +46,6 @@ config SCLPCONSOLE
> >
> >  config TERMINAL3270
> >  bool
> > +
> > +config AVR_USART
> > +bool
> > diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
> > index 02d8a66925..f05c1f5667 100644
> > --- a/hw/char/Makefile.objs
> > +++ b/hw/char/Makefile.objs
> > @@ -21,6 +21,7 @@ obj-$(CONFIG_PSERIES) += spapr_vty.o
> >  obj-$(CONFIG_DIGIC) += digic-uart.o
> >  obj-$(CONFIG_STM32F2XX_USART) += stm32f2xx_usart.o
> >  obj-$(CONFIG_RASPI) += bcm2835_aux.o
> > +common-obj-$(CONFIG_AVR_USART) += avr_usart.o
> >
> >  common-obj-$(CONFIG_CMSDK_APB_UART) += cmsdk-apb-uart.o
> >  common-obj-$(CONFIG_ETRAXFS) += etraxfs_ser.o
> > diff --git a/hw/char/avr_usart.c b/hw/char/avr_usart.c
> > new file mode 100644
> > index 00..9ca3c2a1cd
> > --- /dev/null
> > +++ b/hw/char/avr_usart.c
> > @@ -0,0 +1,324 @@
> > +/*
> > + * AVR USART
> > + *
> > + * Copyright (c) 2018 University of Kent
> > + * Author: Sarah Harris
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a 
>

Re: [PATCH v35 10/13] target/avr: Add limited support for USART and 16 bit timer peripherals

2019-11-25 Thread Sarah Harris
Hi Aleksandar,

I think returning immediately should be ok, it just happened to make sense to 
me to think in terms of this being a special case of a normal read.
The else handles the case in which no data has been received, but the user's 
program reads the incoming buffer anyway.
(As far as I'm aware this is undefined behaviour, so returning zero is 
reasonable)

Kind regards,
Sarah Harris


On Fri, 22 Nov 2019 17:48:56 +0100
Aleksandar Markovic  wrote:

> On Tue, Oct 29, 2019 at 10:25 PM Michael Rolnik  wrote:
> >
> > From: Sarah Harris 
> >
> > These were designed to facilitate testing but should provide enough 
> > function to be useful in other contexts.
> > Only a subset of the functions of each peripheral is implemented, mainly 
> > due to the lack of a standard way to handle electrical connections (like 
> > GPIO pins).
> >
> > Signed-off-by: Sarah Harris 
> > ---
> >  hw/char/Kconfig|   3 +
> >  hw/char/Makefile.objs  |   1 +
> >  hw/char/avr_usart.c| 324 ++
> >  hw/misc/Kconfig|   3 +
> >  hw/misc/Makefile.objs  |   2 +
> >  hw/misc/avr_mask.c | 112 ++
> >  hw/timer/Kconfig   |   3 +
> >  hw/timer/Makefile.objs |   2 +
> >  hw/timer/avr_timer16.c | 605 +
> >  include/hw/char/avr_usart.h|  97 ++
> >  include/hw/misc/avr_mask.h |  47 +++
> >  include/hw/timer/avr_timer16.h |  97 ++
> >  12 files changed, 1296 insertions(+)
> >  create mode 100644 hw/char/avr_usart.c
> >  create mode 100644 hw/misc/avr_mask.c
> >  create mode 100644 hw/timer/avr_timer16.c
> >  create mode 100644 include/hw/char/avr_usart.h
> >  create mode 100644 include/hw/misc/avr_mask.h
> >  create mode 100644 include/hw/timer/avr_timer16.h
> >
> > diff --git a/hw/char/Kconfig b/hw/char/Kconfig
> > index 40e7a8b8bb..331b20983f 100644
> > --- a/hw/char/Kconfig
> > +++ b/hw/char/Kconfig
> > @@ -46,3 +46,6 @@ config SCLPCONSOLE
> >
> >  config TERMINAL3270
> >  bool
> > +
> > +config AVR_USART
> > +bool
> > diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
> > index 02d8a66925..f05c1f5667 100644
> > --- a/hw/char/Makefile.objs
> > +++ b/hw/char/Makefile.objs
> > @@ -21,6 +21,7 @@ obj-$(CONFIG_PSERIES) += spapr_vty.o
> >  obj-$(CONFIG_DIGIC) += digic-uart.o
> >  obj-$(CONFIG_STM32F2XX_USART) += stm32f2xx_usart.o
> >  obj-$(CONFIG_RASPI) += bcm2835_aux.o
> > +common-obj-$(CONFIG_AVR_USART) += avr_usart.o
> >
> >  common-obj-$(CONFIG_CMSDK_APB_UART) += cmsdk-apb-uart.o
> >  common-obj-$(CONFIG_ETRAXFS) += etraxfs_ser.o
> > diff --git a/hw/char/avr_usart.c b/hw/char/avr_usart.c
> > new file mode 100644
> > index 00..9ca3c2a1cd
> > --- /dev/null
> > +++ b/hw/char/avr_usart.c
> > @@ -0,0 +1,324 @@
> > +/*
> > + * AVR USART
> > + *
> > + * Copyright (c) 2018 University of Kent
> > + * Author: Sarah Harris
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a 
> > copy
> > + * of this software and associated documentation files (the "Software"), 
> > to deal
> > + * in the Software without restriction, including without limitation the 
> > rights
> > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or 
> > sell
> > + * copies of the Software, and to permit persons to whom the Software is
> > + * furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be included 
> > in
> > + * all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
> > OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
> > OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> > FROM,
> > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS 
> > IN
> > + * THE SOFTWARE.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/char/avr_usart.h"
> > +#include "qemu/log.h"
> > +#include "hw/irq.h"
> > +#include "hw/qdev-properties.h"
> > +
> >

Re: [PATCH v35 10/13] target/avr: Add limited support for USART and 16 bit timer peripherals

2019-11-25 Thread Sarah Harris
Hi Aleksandar,

In avr_usart_receive():
The two assertions check that we get only what avr_usart_can_receive() asked 
for.
It always requests zero or one byte and I must have assumed zero bytes isn't a 
valid read (so assert size==1).
It only requests data when !usart->data_valid (so assert !usart->data_valid).
(I think this is what Philippe already said)

In avr_usart_read() and avr_usart_write():
I assumed that accesses would only ever be a single byte at a time; I don't 
think the AVR has any multi-byte memory access instructions.
I wasn't convinced I understood QEMU's memory model in enough depth to be 
certain of this so I left an assertion to document and check my assumption.
Sorry for the lack of explanatory comments, I was thinking of this as test code 
at the time so I wasn't being as thorough as I probably would have been 
otherwise!

All of these functions use existing QEMU APIs (the read and write functions are 
passed via the MemoryRegionOps struct, the receive function is passed to 
qemu_chr_fe_set_handlers), so the size parameters are required to match the 
existing interfaces.

Kind regards,
Sarah Harris


On Fri, 22 Nov 2019 15:41:03 +0100
Aleksandar Markovic  wrote:

> On Tue, Oct 29, 2019 at 10:25 PM Michael Rolnik  wrote:
> >
> > From: Sarah Harris 
> >
> > These were designed to facilitate testing but should provide enough 
> > function to be useful in other contexts.
> > Only a subset of the functions of each peripheral is implemented, mainly 
> > due to the lack of a standard way to handle electrical connections (like 
> > GPIO pins).
> >
> > Signed-off-by: Sarah Harris 
> > ---
> >  hw/char/Kconfig|   3 +
> >  hw/char/Makefile.objs  |   1 +
> >  hw/char/avr_usart.c| 324 ++
> >  hw/misc/Kconfig|   3 +
> >  hw/misc/Makefile.objs  |   2 +
> >  hw/misc/avr_mask.c | 112 ++
> >  hw/timer/Kconfig   |   3 +
> >  hw/timer/Makefile.objs |   2 +
> >  hw/timer/avr_timer16.c | 605 +
> >  include/hw/char/avr_usart.h|  97 ++
> >  include/hw/misc/avr_mask.h |  47 +++
> >  include/hw/timer/avr_timer16.h |  97 ++
> >  12 files changed, 1296 insertions(+)
> >  create mode 100644 hw/char/avr_usart.c
> >  create mode 100644 hw/misc/avr_mask.c
> >  create mode 100644 hw/timer/avr_timer16.c
> >  create mode 100644 include/hw/char/avr_usart.h
> >  create mode 100644 include/hw/misc/avr_mask.h
> >  create mode 100644 include/hw/timer/avr_timer16.h
> >
> > diff --git a/hw/char/Kconfig b/hw/char/Kconfig
> > index 40e7a8b8bb..331b20983f 100644
> > --- a/hw/char/Kconfig
> > +++ b/hw/char/Kconfig
> > @@ -46,3 +46,6 @@ config SCLPCONSOLE
> >
> >  config TERMINAL3270
> >  bool
> > +
> > +config AVR_USART
> > +bool
> > diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
> > index 02d8a66925..f05c1f5667 100644
> > --- a/hw/char/Makefile.objs
> > +++ b/hw/char/Makefile.objs
> > @@ -21,6 +21,7 @@ obj-$(CONFIG_PSERIES) += spapr_vty.o
> >  obj-$(CONFIG_DIGIC) += digic-uart.o
> >  obj-$(CONFIG_STM32F2XX_USART) += stm32f2xx_usart.o
> >  obj-$(CONFIG_RASPI) += bcm2835_aux.o
> > +common-obj-$(CONFIG_AVR_USART) += avr_usart.o
> >
> >  common-obj-$(CONFIG_CMSDK_APB_UART) += cmsdk-apb-uart.o
> >  common-obj-$(CONFIG_ETRAXFS) += etraxfs_ser.o
> > diff --git a/hw/char/avr_usart.c b/hw/char/avr_usart.c
> > new file mode 100644
> > index 00..9ca3c2a1cd
> > --- /dev/null
> > +++ b/hw/char/avr_usart.c
> > @@ -0,0 +1,324 @@
> > +/*
> > + * AVR USART
> > + *
> > + * Copyright (c) 2018 University of Kent
> > + * Author: Sarah Harris
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a 
> > copy
> > + * of this software and associated documentation files (the "Software"), 
> > to deal
> > + * in the Software without restriction, including without limitation the 
> > rights
> > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or 
> > sell
> > + * copies of the Software, and to permit persons to whom the Software is
> > + * furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be included 
> > in
> > + * all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
> > OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO 

Re: [Qemu-devel] [PATCH v1 8/8] target/avr: Register AVR support with the rest of QEMU, the build system, and the MAINTAINERS file

2019-10-17 Thread Sarah Harris
Hi Philippe,

I've checked and yes, I can make time to review patches.
This is part of my current paid work, so when my contract eventually ends I 
will likely have to step down as reviewer.
I'll have notice of that happening some time before, so I can pass that along 
when it comes.

This message comes from my work email so the details should be correct, i.e. 
Sarah Harris 

Kind regards,
Sarah Harris

On Sat, 12 Oct 2019 09:31:07 +0200
Philippe Mathieu-Daudé  wrote:

> Hi Sarah,
> 
> On 5/10/19 1:17 PM, Sarah Harris wrote:
> > Hi Richard,
> > 
> > Having discussed with my colleagues, we don't have the resources to 
> > maintain this.
> > If you wanted to take up Michael's offer then I'm happy to respond to 
> > queries and provide minor fixes.
> > Otherwise, we will make our repository publicly available in the near 
> > future for anyone who wants to use it.
> 
> Would you agree to be listed as reviewer for AVR (Michael being the 
> maintainer)?
> 
>M: Mail patches to: FullName 
>   Maintainers are looking after a certain area and must be CCed on
>   patches. They are considered the main contact point.
>R: Designated reviewer: FullName 
>   These reviewers should be CCed on patches.
>   Reviewers are familiar with the subject matter and provide feedback
>   even though they are not maintainers.
> 
> > Kind regards,
> > Sarah Harris
> > 
> > On Sun, 5 May 2019 09:10:00 -0700
> > Michael Rolnik  wrote:
> > 
> >> Hi Richard.
> >>
> >> I can maintain it
> >>
> >> Sent from my cell phone, please ignore typos
> >>
> >> On Sun, May 5, 2019, 8:57 AM Richard Henderson 
> >> 
> >> wrote:
> >>
> >>> On 5/4/19 1:36 AM, Sarah Harris wrote:
> >>>> Signed-off-by: Sarah Harris 
> >>> ...
> >>>>
> >>>> +AVR
> >>>> +M: Michael Rolnik 
> >>>> +S: Odd Fixes
> >>>> +F: target-avr/
> >>>> +F: hw/avr/
> >>>> +
> >>>
> >>> This is not how things work.  Michael wasn't up to maintaining the code 2
> >>> years
> >>> ago; that's why it was never committed.
> >>>
> >>> You would need to maintain this yourself, and for more than "Odd Fixes".
> >>>
> >>>
> >>> r~
> >>>
> > 
> 



Re: [Qemu-devel] [PATCH v29 1/8] target/avr: Add outward facing interfaces and core CPU logic

2019-08-27 Thread Sarah Harris
I don't mind if you want to drop my signed-off-by for commits that were based 
on Michael's work.
You probably want to keep my sign-off for the USART/timer commit though as that 
was new code.

Regards,
Sarah Harris

On Mon, 26 Aug 2019 10:00:51 +0200
Thomas Huth  wrote:

> On 26/08/2019 09.53, Michael Rolnik wrote:
> > the commit was originally mine. Then Sarah rearranged it, signed and
> > submitted. She no longer maintains it. So' I believe I can remove her sob.
> > what do you think?.
> 
> Fine for me, but maybe you should mention her in the patch description
> if she made significant changes?
> 
> Sarah, what do you think?
> 
> Anyway, if the patch is originally from you, you should also remove the
> "From: Sarah ..." line from the patch. For this, you likely have to
> change the author of the patch in your git tree with "git commit --amend
> --reset-author".
> 
>  Thomas



Re: [Qemu-devel] [PATCH v27 5/8] target/avr: Add limited support for USART and 16 bit timer peripherals

2019-07-26 Thread Sarah Harris
Hi Michael and Pavel,

The USART was based on the ATMega2560.
It was designed for testing so its functionality is somewhat limited.

Peripherals seem to vary between AVR chips so the configuration in the 2560 may 
not match other chips, especially the older ones.
>From memory, the only shared registers in the 2560 USART are the PRR's, which 
>we implemented by adding single byte memory regions during board 
>initialisation (so that the memory region wasn't part of any one device).
I expect there are cleaner ways to do it.

Kind regards,
Sarah Harris

On Thu, 25 Jul 2019 20:52:33 +0300
Michael Rolnik  wrote:

> Hi Pavel.
> 
> Please see my answers below.
> 
> On Thu, Jul 25, 2019 at 1:00 PM Pavel Dovgalyuk  wrote:
> 
> > > From: Qemu-devel [mailto:qemu-devel-bounces+patchwork-qemu-
> > > devel=patchwork.kernel@nongnu.org] On Behalf Of Michael Rolnik
> > > From: Sarah Harris 
> > >
> > > These were designed to facilitate testing but should provide enough
> > function to be useful in
> > > other contexts.
> >
> > USART is very useful for testing, but to which model of AVR is belongs?
> > We also started implementation of USART and other devices in our
> > internship program,
> > using prior version of your patches.
> > There were other register addresses for the registers and some of them
> > even intersect
> > making read/write logic more complex (we looked at Atmega8).
> >
> 
> This is a question for Sara as she built it. (I think it was ATmega2560)
> 
> 
> >
> > You also mix the board and the SoC into one file, making hardware-on-chip
> > harder to reuse.
> >
> > I think that the structure can be revised in the following way:
> > Board -> SoC -> Devices
> >
> > Board includes SoC, loads the firmware, and adds some external peripheral
> > devices, if needed.
> >
> > SoC includes embedded peripherals. It dispatches IO memory accesses and
> > passes them
> > to the devices. In this case you can have different register addresses for
> > different SoCs, but
> > the embedded device emulation code can be mostly the same for simple
> > devices like USART.
> >
> > > Only a subset of the functions of each peripheral is implemented, mainly
> > due to the lack of a
> > > standard way to handle electrical connections (like GPIO pins).
> >
> You are right.
> 
> >
> > We did not got too much results, you can check for our changes here:
> > https://github.com/Dovgalyuk/qemu/tree/avr8
> >
> > But we can help you in development of better version of the patches and
> > split the work
> > for making this platform more usable.
> >
> 
> Gladly.
> You are welcome.
> My initial intent was to provide CPU only and I hoped that others will
> model the devices.
> 
> *Richard, Phil & all,*
> what would be the right mechanism / procedure to split the work?
> 
> 
> >
> > Pavel Dovgalyuk
> >
> >
> 
> -- 
> Best Regards,
> Michael Rolnik



Re: [Qemu-devel] [PATCH v1 8/8] target/avr: Register AVR support with the rest of QEMU, the build system, and the MAINTAINERS file

2019-05-10 Thread Sarah Harris
Hi Richard,

Having discussed with my colleagues, we don't have the resources to maintain 
this.
If you wanted to take up Michael's offer then I'm happy to respond to queries 
and provide minor fixes.
Otherwise, we will make our repository publicly available in the near future 
for anyone who wants to use it.

Kind regards,
Sarah Harris

On Sun, 5 May 2019 09:10:00 -0700
Michael Rolnik  wrote:

> Hi Richard.
> 
> I can maintain it
> 
> Sent from my cell phone, please ignore typos
> 
> On Sun, May 5, 2019, 8:57 AM Richard Henderson 
> wrote:
> 
> > On 5/4/19 1:36 AM, Sarah Harris wrote:
> > > Signed-off-by: Sarah Harris 
> > ...
> > >
> > > +AVR
> > > +M: Michael Rolnik 
> > > +S: Odd Fixes
> > > +F: target-avr/
> > > +F: hw/avr/
> > > +
> >
> > This is not how things work.  Michael wasn't up to maintaining the code 2
> > years
> > ago; that's why it was never committed.
> >
> > You would need to maintain this yourself, and for more than "Odd Fixes".
> >
> >
> > r~
> >



Re: [Qemu-devel] [PATCH v1 8/8] target/avr: Register AVR support with the rest of QEMU, the build system, and the MAINTAINERS file

2019-05-07 Thread Sarah Harris
Hi Richard,

I've raised this with my colleagues, we'll discuss tomorrow whether we have the 
funding and person resources to provide maintenance and get back to you.

I've noted the issues you and Eric raised, I'll wait until I know where things 
are going before I look at fixing them.

Kind regards,
Sarah Harris

On Sun, 5 May 2019 09:10:00 -0700
Michael Rolnik  wrote:

> Hi Richard.
> 
> I can maintain it
> 
> Sent from my cell phone, please ignore typos
> 
> On Sun, May 5, 2019, 8:57 AM Richard Henderson 
> wrote:
> 
> > On 5/4/19 1:36 AM, Sarah Harris wrote:
> > > Signed-off-by: Sarah Harris 
> > ...
> > >
> > > +AVR
> > > +M: Michael Rolnik 
> > > +S: Odd Fixes
> > > +F: target-avr/
> > > +F: hw/avr/
> > > +
> >
> > This is not how things work.  Michael wasn't up to maintaining the code 2
> > years
> > ago; that's why it was never committed.
> >
> > You would need to maintain this yourself, and for more than "Odd Fixes".
> >
> >
> > r~
> >



[Qemu-devel] [PATCH v1 5/8] target/avr: Add instruction translation

2019-05-04 Thread Sarah Harris
This includes:
- table of instruction bit patterns
- intermediate translation functions that extract parameters from opcodes
- TCG translations for each instruction

Signed-off-by: Sarah Harris 
---
 target/avr/translate-inst.h |  695 
 target/avr/translate.c  | 3013 +++
 2 files changed, 3708 insertions(+)
 create mode 100644 target/avr/translate-inst.h
 create mode 100644 target/avr/translate.c

diff --git a/target/avr/translate-inst.h b/target/avr/translate-inst.h
new file mode 100644
index 00..e27f403ba7
--- /dev/null
+++ b/target/avr/translate-inst.h
@@ -0,0 +1,695 @@
+/*
+ *  QEMU AVR CPU
+ *
+ *  Copyright (c) 2016 Michael Rolnik
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library 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
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, see
+ *  <http://www.gnu.org/licenses/lgpl-2.1.html>
+ */
+
+/*
+ * Functions for extracting instruction parameters from raw opcodes.
+ */
+
+#ifndef AVR_TRANSLATE_INST_H_
+#define AVR_TRANSLATE_INST_H_
+
+static inline uint32_t MOVW_Rr(uint32_t opcode)
+{
+return extract32(opcode, 0, 4);
+}
+
+static inline uint32_t MOVW_Rd(uint32_t opcode)
+{
+return extract32(opcode, 4, 4);
+}
+
+static inline uint32_t MULS_Rr(uint32_t opcode)
+{
+return extract32(opcode, 0, 4);
+}
+
+static inline uint32_t MULS_Rd(uint32_t opcode)
+{
+return extract32(opcode, 4, 4);
+}
+
+static inline uint32_t MULSU_Rr(uint32_t opcode)
+{
+return extract32(opcode, 0, 3);
+}
+
+static inline uint32_t MULSU_Rd(uint32_t opcode)
+{
+return extract32(opcode, 4, 3);
+}
+
+static inline uint32_t FMUL_Rr(uint32_t opcode)
+{
+return extract32(opcode, 0, 3);
+}
+
+static inline uint32_t FMUL_Rd(uint32_t opcode)
+{
+return extract32(opcode, 4, 3);
+}
+
+static inline uint32_t FMULS_Rr(uint32_t opcode)
+{
+return extract32(opcode, 0, 3);
+}
+
+static inline uint32_t FMULS_Rd(uint32_t opcode)
+{
+return extract32(opcode, 4, 3);
+}
+
+static inline uint32_t FMULSU_Rr(uint32_t opcode)
+{
+return extract32(opcode, 0, 3);
+}
+
+static inline uint32_t FMULSU_Rd(uint32_t opcode)
+{
+return extract32(opcode, 4, 3);
+}
+
+static inline uint32_t CPC_Rd(uint32_t opcode)
+{
+return extract32(opcode, 4, 5);
+}
+
+static inline uint32_t CPC_Rr(uint32_t opcode)
+{
+return (extract32(opcode, 9, 1) << 4) |
+(extract32(opcode, 0, 4));
+}
+
+static inline uint32_t SBC_Rd(uint32_t opcode)
+{
+return extract32(opcode, 4, 5);
+}
+
+static inline uint32_t SBC_Rr(uint32_t opcode)
+{
+return (extract32(opcode, 9, 1) << 4) |
+(extract32(opcode, 0, 4));
+}
+
+static inline uint32_t ADD_Rd(uint32_t opcode)
+{
+return extract32(opcode, 4, 5);
+}
+
+static inline uint32_t ADD_Rr(uint32_t opcode)
+{
+return (extract32(opcode, 9, 1) << 4) |
+(extract32(opcode, 0, 4));
+}
+
+static inline uint32_t AND_Rd(uint32_t opcode)
+{
+return extract32(opcode, 4, 5);
+}
+
+static inline uint32_t AND_Rr(uint32_t opcode)
+{
+return (extract32(opcode, 9, 1) << 4) |
+(extract32(opcode, 0, 4));
+}
+
+static inline uint32_t EOR_Rd(uint32_t opcode)
+{
+return extract32(opcode, 4, 5);
+}
+
+static inline uint32_t EOR_Rr(uint32_t opcode)
+{
+return (extract32(opcode, 9, 1) << 4) |
+(extract32(opcode, 0, 4));
+}
+
+static inline uint32_t OR_Rd(uint32_t opcode)
+{
+return extract32(opcode, 4, 5);
+}
+
+static inline uint32_t OR_Rr(uint32_t opcode)
+{
+return (extract32(opcode, 9, 1) << 4) |
+(extract32(opcode, 0, 4));
+}
+
+static inline uint32_t MOV_Rd(uint32_t opcode)
+{
+return extract32(opcode, 4, 5);
+}
+
+static inline uint32_t MOV_Rr(uint32_t opcode)
+{
+return (extract32(opcode, 9, 1) << 4) |
+(extract32(opcode, 0, 4));
+}
+
+static inline uint32_t CPSE_Rd(uint32_t opcode)
+{
+return extract32(opcode, 4, 5);
+}
+
+static inline uint32_t CPSE_Rr(uint32_t opcode)
+{
+return (extract32(opcode, 9, 1) << 4) |
+(extract32(opcode, 0, 4));
+}
+
+static inline uint32_t CP_Rd(uint32_t opcode)
+{
+return extract32(opcode, 4, 5);
+}
+
+static inline uint32_t CP_Rr(uint32_t opcode)
+{
+return (extract32(opcode, 9, 1) << 4) |
+(extract32(opcode, 0, 4));
+}
+
+static inline uint32_t SUB_Rd(uint32_t opcode)
+{
+return extract32(opcode, 4, 5);
+}
+
+static inline uint32_t SUB_Rr(u

[Qemu-devel] [PATCH v1 3/8] target/avr: Add outward facing interfaces and core CPU logic

2019-05-04 Thread Sarah Harris
This includes:
- CPU data structures
- object model classes and functions
- migration functions
- GDB hooks

Signed-off-by: Sarah Harris 
---
 target/avr/cpu-qom.h |  83 +++
 target/avr/cpu.c | 570 +++
 target/avr/cpu.h | 238 ++
 target/avr/gdbstub.c |  85 +++
 target/avr/machine.c | 122 +
 5 files changed, 1098 insertions(+)
 create mode 100644 target/avr/cpu-qom.h
 create mode 100644 target/avr/cpu.c
 create mode 100644 target/avr/cpu.h
 create mode 100644 target/avr/gdbstub.c
 create mode 100644 target/avr/machine.c

diff --git a/target/avr/cpu-qom.h b/target/avr/cpu-qom.h
new file mode 100644
index 00..8085567b87
--- /dev/null
+++ b/target/avr/cpu-qom.h
@@ -0,0 +1,83 @@
+/*
+ * QEMU AVR CPU
+ *
+ * Copyright (c) 2016 Michael Rolnik
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
+ * <http://www.gnu.org/licenses/lgpl-2.1.html>
+ */
+
+#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.
+ */
+typedef struct AVRCPUClass {
+CPUClass parent_class;
+
+DeviceRealize parent_realize;
+void (*parent_reset)(CPUState *cpu);
+} AVRCPUClass;
+
+/**
+ *  AVRCPU:
+ *  @env: #CPUAVRState
+ *
+ *  A AVR CPU.
+ */
+typedef struct AVRCPU {
+/*< private >*/
+CPUState parent_obj;
+/*< public >*/
+
+CPUAVRState env;
+} AVRCPU;
+
+static inline AVRCPU *avr_env_get_cpu(CPUAVRState *env)
+{
+return container_of(env, AVRCPU, env);
+}
+
+#define ENV_GET_CPU(e) CPU(avr_env_get_cpu(e))
+#define ENV_OFFSET offsetof(AVRCPU, env)
+
+#ifndef CONFIG_USER_ONLY
+extern const struct VMStateDescription vms_avr_cpu;
+#endif
+
+void avr_cpu_do_interrupt(CPUState *cpu);
+bool avr_cpu_exec_interrupt(CPUState *cpu, int int_req);
+void avr_cpu_dump_state(CPUState *cs, FILE *f, int flags);
+hwaddr avr_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
+int avr_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
+int avr_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
+
+#endif
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
new file mode 100644
index 00..19ce1d7386
--- /dev/null
+++ b/target/avr/cpu.c
@@ -0,0 +1,570 @@
+/*
+ * QEMU AVR CPU
+ *
+ * Copyright (c) 2016 Michael Rolnik
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
+ * <http://www.gnu.org/licenses/lgpl-2.1.html>
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/qemu-print.h"
+#include "qapi/error.h"
+#include "cpu.h"
+#include "qemu-common.h"
+#include "migration/vmstate.h"
+
+static void avr_cpu_set_pc(CPUState *cs, vaddr value)
+{
+AVRCPU *cpu = AVR_CPU(cs);
+
+cpu->env.pc_w = value / 2; /* internally PC points to words */
+}
+
+static bool avr_cpu_has_work(CPUState *cs)
+{
+AVRCPU *cpu = AVR_CPU(cs);
+CPUAVRState *env = >env;
+
+return (cs->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_RESET))
+&& cpu_interrupts_enabled(env);
+}
+
+static void avr_cpu_synchronize_from_tb(CPUState *cs, TranslationBlock *tb)
+{
+AVRCPU *cpu = AVR_CPU(cs);
+CPUAVRState *env = >env;
+
+env->pc_w = tb->pc / 2; /* internally PC points to

[Qemu-devel] [PATCH v1 7/8] target/avr: Add example board configuration

2019-05-04 Thread Sarah Harris
A simple board setup that configures an AVR CPU to run a given firmware image.
This is all that's useful to implement without peripheral emulation as AVR CPUs 
include a lot of on-board peripherals.

Signed-off-by: Sarah Harris 
---
 hw/Kconfig   |   1 +
 hw/avr/Kconfig   |   4 +
 hw/avr/Makefile.objs |   1 +
 hw/avr/sample.c  | 177 +++
 4 files changed, 183 insertions(+)
 create mode 100644 hw/avr/Kconfig
 create mode 100644 hw/avr/Makefile.objs
 create mode 100644 hw/avr/sample.c

diff --git a/hw/Kconfig b/hw/Kconfig
index 88b9f15007..dc5eb7a038 100644
--- a/hw/Kconfig
+++ b/hw/Kconfig
@@ -41,6 +41,7 @@ source watchdog/Kconfig
 # arch Kconfig
 source arm/Kconfig
 source alpha/Kconfig
+source avr/Kconfig
 source cris/Kconfig
 source hppa/Kconfig
 source i386/Kconfig
diff --git a/hw/avr/Kconfig b/hw/avr/Kconfig
new file mode 100644
index 00..c6ca8fe775
--- /dev/null
+++ b/hw/avr/Kconfig
@@ -0,0 +1,4 @@
+config AVR_SAMPLE
+bool
+select AVR_TIMER16
+select AVR_USART
diff --git a/hw/avr/Makefile.objs b/hw/avr/Makefile.objs
new file mode 100644
index 00..626b7064b3
--- /dev/null
+++ b/hw/avr/Makefile.objs
@@ -0,0 +1 @@
+obj-y += sample.o
diff --git a/hw/avr/sample.c b/hw/avr/sample.c
new file mode 100644
index 00..21b384b3b3
--- /dev/null
+++ b/hw/avr/sample.c
@@ -0,0 +1,177 @@
+/*
+ * QEMU AVR CPU
+ *
+ * Copyright (c) 2016 Michael Rolnik
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
+ * <http://www.gnu.org/licenses/lgpl-2.1.html>
+ */
+
+/*
+ *  NOTE:
+ *  This is not a real AVR board, this is an example!
+ *  The CPU is an approximation of an ATmega2560, but is missing various
+ *  built-in peripherals.
+ *
+ *  This example board loads provided binary file into flash memory and
+ *  executes it from 0x address in the code memory space.
+ *
+ *  Currently used for AVR CPU validation
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu-common.h"
+#include "cpu.h"
+#include "hw/hw.h"
+#include "sysemu/sysemu.h"
+#include "sysemu/qtest.h"
+#include "ui/console.h"
+#include "hw/boards.h"
+#include "hw/loader.h"
+#include "qemu/error-report.h"
+#include "exec/address-spaces.h"
+#include "include/hw/sysbus.h"
+#include "include/hw/char/avr_usart.h"
+#include "include/hw/timer/avr_timer16.h"
+#include "elf.h"
+
+#define SIZE_FLASH 0x0004
+#define SIZE_SRAM 0x2200
+/*
+ * Size of additional "external" memory, as if the AVR were configured to use
+ * an external RAM chip.
+ * Note that the configuration registers that normally enable this feature are
+ * unimplemented.
+ */
+#define SIZE_EXMEM 0x
+
+/* Offsets of periphals in emulated memory space (i.e. not host addresses)  */
+#define PRR0 0x64
+#define PRR1 0x65
+#define USART_BASE 0xc0
+#define USART_PRR PRR0
+#define USART_PRR_MASK 0b0010
+#define TIMER1_BASE 0x80
+#define TIMER1_IMSK_BASE 0x6f
+#define TIMER1_IFR_BASE 0x36
+#define TIMER1_PRR PRR0
+#define TIMER1_PRR_MASK 0b0100
+
+/* Interrupt numbers used by peripherals */
+#define TIMER1_CAPT_IRQ 15
+#define TIMER1_COMPA_IRQ 16
+#define TIMER1_COMPB_IRQ 17
+#define TIMER1_COMPC_IRQ 18
+#define TIMER1_OVF_IRQ 19
+
+static void sample_init(MachineState *machine)
+{
+MemoryRegion *address_space_mem;
+MemoryRegion *ram;
+MemoryRegion *flash;
+AVRCPU *cpu_avr;
+const char *firmware = NULL;
+const char *filename;
+int bytes_loaded;
+AVRUsartState *usart0;
+AVRTimer16State *timer1;
+SysBusDevice *busdev;
+
+address_space_mem = get_system_memory();
+ram = g_new(MemoryRegion, 1);
+flash = g_new(MemoryRegion, 1);
+
+/* ATmega2560. */
+cpu_avr = AVR_CPU(cpu_create("avr6-avr"));
+
+memory_region_allocate_system_memory(
+ram, NULL, "avr.ram", SIZE_SRAM + SIZE_EXMEM);
+memory_region_add_subregion(address_space_mem, OFFSET_DATA, ram);
+
+memory_region_init_rom(flash, NULL, "avr.flash", SIZE_FLASH, _fatal);
+memory_region_add_subregion(address_space_mem, OFFSET_CODE, flash);
+
+/* USART 0 built-in peripheral */
+usart0 = AVR_USART(object_new(TYPE_AVR_

[Qemu-devel] [PATCH v1 4/8] target/avr: Add instruction helpers

2019-05-04 Thread Sarah Harris
Stubs for unimplemented instructions and helpers for instructions that need to 
interact with QEMU.
SPM and WDR are unimplemented because they require emulation of complex 
peripherals.
The implementation of SLEEP is very limited due to the lack of peripherals to 
generate wake interrupts.
Memory access instructions are implemented here because some address ranges 
actually refer to CPU registers.

Signed-off-by: Sarah Harris 
---
 target/avr/helper.c | 343 
 target/avr/helper.h |  28 
 2 files changed, 371 insertions(+)
 create mode 100644 target/avr/helper.c
 create mode 100644 target/avr/helper.h

diff --git a/target/avr/helper.c b/target/avr/helper.c
new file mode 100644
index 00..29b8bfce02
--- /dev/null
+++ b/target/avr/helper.c
@@ -0,0 +1,343 @@
+/*
+ * QEMU AVR CPU
+ *
+ * Copyright (c) 2016 Michael Rolnik
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
+ * <http://www.gnu.org/licenses/lgpl-2.1.html>
+ */
+
+#include "qemu/osdep.h"
+
+#include "cpu.h"
+#include "hw/irq.h"
+#include "hw/sysbus.h"
+#include "sysemu/sysemu.h"
+#include "exec/exec-all.h"
+#include "exec/cpu_ldst.h"
+#include "exec/helper-proto.h"
+#include "exec/ioport.h"
+#include "qemu/host-utils.h"
+#include "qemu/error-report.h"
+
+bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
+{
+bool ret = false;
+CPUClass *cc = CPU_GET_CLASS(cs);
+AVRCPU *cpu = AVR_CPU(cs);
+CPUAVRState *env = >env;
+
+if (interrupt_request & CPU_INTERRUPT_RESET) {
+if (cpu_interrupts_enabled(env)) {
+cs->exception_index = EXCP_RESET;
+cc->do_interrupt(cs);
+
+cs->interrupt_request &= ~CPU_INTERRUPT_RESET;
+
+ret = true;
+}
+}
+if (interrupt_request & CPU_INTERRUPT_HARD) {
+if (cpu_interrupts_enabled(env) && env->intsrc != 0) {
+int index = ctz32(env->intsrc);
+cs->exception_index = EXCP_INT(index);
+cc->do_interrupt(cs);
+
+env->intsrc &= env->intsrc - 1; /* clear the interrupt */
+cs->interrupt_request &= ~CPU_INTERRUPT_HARD;
+
+ret = true;
+}
+}
+return ret;
+}
+
+void avr_cpu_do_interrupt(CPUState *cs)
+{
+AVRCPU *cpu = AVR_CPU(cs);
+CPUAVRState *env = >env;
+
+uint32_t ret = env->pc_w;
+int vector = 0;
+int size = avr_feature(env, AVR_FEATURE_JMP_CALL) ? 2 : 1;
+int base = 0;
+
+if (cs->exception_index == EXCP_RESET) {
+vector = 0;
+} else if (env->intsrc != 0) {
+vector = ctz32(env->intsrc) + 1;
+}
+
+if (avr_feature(env, AVR_FEATURE_3_BYTE_PC)) {
+cpu_stb_data(env, env->sp--, (ret & 0xff));
+cpu_stb_data(env, env->sp--, (ret & 0x00ff00) >> 8);
+cpu_stb_data(env, env->sp--, (ret & 0xff) >> 16);
+} else if (avr_feature(env, AVR_FEATURE_2_BYTE_PC)) {
+cpu_stb_data(env, env->sp--, (ret & 0xff));
+cpu_stb_data(env, env->sp--, (ret & 0x00ff00) >> 8);
+} else {
+cpu_stb_data(env, env->sp--, (ret & 0xff));
+}
+
+env->pc_w = base + vector * size;
+env->sregI = 0; /* clear Global Interrupt Flag */
+
+cs->exception_index = -1;
+}
+
+int avr_cpu_memory_rw_debug(CPUState *cs, vaddr addr, uint8_t *buf,
+int len, bool is_write)
+{
+return cpu_memory_rw_debug(cs, addr, buf, len, is_write);
+}
+
+hwaddr avr_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
+{
+return addr; /* I assume 1:1 address correspondance */
+}
+
+int avr_cpu_handle_mmu_fault(
+CPUState *cs, vaddr address, int size, int rw, int mmu_idx)
+{
+/* currently it's assumed that this will never happen */
+cs->exception_index = EXCP_DEBUG;
+cpu_dump_state(cs, stderr, 0);
+return 1;
+}
+
+void tlb_fill(CPUState *cs, target_ulong vaddr, int size,
+  MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
+{
+int prot = 0;
+MemTxAttrs attrs = {};
+uint32_t paddr;
+
+vaddr &= TARGET_PAGE_MASK;
+
+if (mmu_idx == MMU_CODE_IDX) {
+/* access to code in flas

[Qemu-devel] [PATCH v1 6/8] target/avr: Add limited support for USART and 16 bit timer peripherals

2019-05-04 Thread Sarah Harris
These were designed to facilitate testing but should provide enough function to 
be useful in other contexts.
Only a subset of the functions of each peripheral is implemented, mainly due to 
the lack of a standard way to handle electrical connections (like GPIO pins).

Signed-off-by: Sarah Harris 
---
 hw/char/Kconfig|   3 +
 hw/char/Makefile.objs  |   1 +
 hw/char/avr_usart.c| 316 ++
 hw/timer/Kconfig   |   3 +
 hw/timer/Makefile.objs |   1 +
 hw/timer/avr_timer16.c | 587 +
 include/hw/char/avr_usart.h|  99 ++
 include/hw/timer/avr_timer16.h |  99 ++
 8 files changed, 1109 insertions(+)
 create mode 100644 hw/char/avr_usart.c
 create mode 100644 hw/timer/avr_timer16.c
 create mode 100644 include/hw/char/avr_usart.h
 create mode 100644 include/hw/timer/avr_timer16.h

diff --git a/hw/char/Kconfig b/hw/char/Kconfig
index 6360c9fffa..7af2c6dd21 100644
--- a/hw/char/Kconfig
+++ b/hw/char/Kconfig
@@ -40,3 +40,6 @@ config SCLPCONSOLE
 
 config TERMINAL3270
 bool
+
+config AVR_USART
+bool
diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
index cf086e7114..962556d00d 100644
--- a/hw/char/Makefile.objs
+++ b/hw/char/Makefile.objs
@@ -20,6 +20,7 @@ obj-$(CONFIG_PSERIES) += spapr_vty.o
 obj-$(CONFIG_DIGIC) += digic-uart.o
 obj-$(CONFIG_STM32F2XX_USART) += stm32f2xx_usart.o
 obj-$(CONFIG_RASPI) += bcm2835_aux.o
+obj-$(CONFIG_AVR_USART) += avr_usart.o
 
 common-obj-$(CONFIG_CMSDK_APB_UART) += cmsdk-apb-uart.o
 common-obj-$(CONFIG_ETRAXFS) += etraxfs_ser.o
diff --git a/hw/char/avr_usart.c b/hw/char/avr_usart.c
new file mode 100644
index 00..26c711336b
--- /dev/null
+++ b/hw/char/avr_usart.c
@@ -0,0 +1,316 @@
+/*
+ * AVR USART
+ *
+ * Copyright (c) 2018 University of Kent
+ * Author: Sarah Harris
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/char/avr_usart.h"
+#include "qemu/log.h"
+
+static int avr_usart_can_receive(void *opaque)
+{
+AVRUsartState *usart = opaque;
+
+if (usart->data_valid || !(usart->csrb & USART_CSRB_RXEN)) {
+return 0;
+}
+return 1;
+}
+
+static void avr_usart_receive(void *opaque, const uint8_t *buffer, int size)
+{
+AVRUsartState *usart = opaque;
+assert(size == 1);
+assert(!usart->data_valid);
+usart->data = buffer[0];
+usart->data_valid = true;
+usart->csra |= USART_CSRA_RXC;
+if (usart->csrb & USART_CSRB_RXCIE) {
+qemu_set_irq(usart->rxc_irq, 1);
+}
+}
+
+static void update_char_mask(AVRUsartState *usart)
+{
+uint8_t mode = ((usart->csrc & USART_CSRC_CSZ0) ? 1 : 0) |
+((usart->csrc & USART_CSRC_CSZ1) ? 2 : 0) |
+((usart->csrb & USART_CSRB_CSZ2) ? 4 : 0);
+switch (mode) {
+case 0:
+usart->char_mask = 0b1;
+break;
+case 1:
+usart->char_mask = 0b11;
+break;
+case 2:
+usart->char_mask = 0b111;
+break;
+case 3:
+usart->char_mask = 0b;
+break;
+case 4:
+/* Fallthrough. */
+case 5:
+/* Fallthrough. */
+case 6:
+qemu_log_mask(
+LOG_GUEST_ERROR,
+"%s: Reserved character size 0x%x\n",
+__func__,
+mode);
+break;
+case 7:
+qemu_log_mask(
+LOG_GUEST_ERROR,
+"%s: Nine bit character size not supported (forcing eight)\n",
+__func__);
+usart->char_mask = 0b;
+break;
+default:
+assert(0);
+}
+}
+
+static void avr_usart_reset(DeviceState *dev)
+{
+AVRUsartState *usart = AVR_USART(dev);
+usart->data_valid = false;
+usart->csra = 0b0010;
+usart->csrb = 0b000

[Qemu-devel] [PATCH v1 1/8] target/avr: Add instruction decoder

2019-05-04 Thread Sarah Harris
This utility module builds a decision tree to decode instructions, starting 
from a human readable list of instruction bit patterns.
Automatic tree generation will hopefully be more efficient and more 
maintainable than a hand-designed opcode parser.

Tree generation happens at startup because this seemed simpler to implement 
than adding a new build step.

Signed-off-by: Sarah Harris 
---
 target/avr/decode.c | 441 
 target/avr/decode.h |  68 +++
 2 files changed, 509 insertions(+)
 create mode 100644 target/avr/decode.c
 create mode 100644 target/avr/decode.h

diff --git a/target/avr/decode.c b/target/avr/decode.c
new file mode 100644
index 00..a984806d96
--- /dev/null
+++ b/target/avr/decode.c
@@ -0,0 +1,441 @@
+/*
+ * AVR instruction decoder.
+ *
+ * Copyright (c) 2019 University of Kent
+ * Author: Sarah Harris
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+/*
+ * # Why is this here?
+ * This decoder takes a list of human readable descriptions of instructions
+ * and uses it to build a binary decision tree used to choose translation
+ * functions for opcodes.
+ * It's built like this because figuring out the structure of AVR instructions
+ * was too hard and writing a Big Nested Switch by hand seemed too painful.
+ * This seems to be the simplest answer that doesn't use loads (>0.5MB) of RAM.
+ *
+ * # How does it work?
+ * This is based J. R. Quinlan's ID3 algorithm, tweaked to add weights to each
+ * instruction.
+ * Having a binary tree branch on opcode bits seems obvious, but the awkward
+ * part is deciding which order to test the bits.
+ * Getting the order right means that redundant bits can be ignored and fewer
+ * branches are needed; i.e. less memory and faster lookups.
+ * Here, the tests are ordered by an estimate of information gain based on
+ * Shannon Entropy.
+ * In short, we guess how much each bit tells us and pick the one that gives
+ * us most progress toward knowing which instruction we're seeing.
+ * The weights are currently only used to prioritise legal opcodes over
+ * illegal opcodes, which significantly reduces the tree size.
+ *
+ * # Why are you doing this at run time?
+ * It was easier than building and running a special purpose tool during
+ * QEMU's build process.
+ * The tree is only built once, during startup, and hopefully doesn't take long
+ * enough to be noticeable.
+ */
+
+#include 
+#include "qemu/osdep.h"
+#include "qemu/bitops.h"
+#include "qemu/error-report.h"
+#include "decode.h"
+
+/* #define DEBUG_DECODER */
+
+/* Wide enough for the largest AVR instruction. */
+#define OPCODE_T uint16_t
+#define OPCODE_SIZE 16
+
+/*
+ * Probability estimate for each instruction.
+ * Larger values mean higher priority.
+ */
+#define WEIGHT_T uint64_t
+#define WEIGHT_LEGAL (1 << 16)
+#define WEIGHT_ILLEGAL 1
+
+typedef union Tree Tree;
+
+typedef struct {
+bool is_leaf;
+/* Bit to test */
+uint bit;
+const Tree *zero;
+const Tree *one;
+} Branch;
+
+typedef struct {
+bool is_leaf;
+TranslateFn decoder;
+/* Instruction length in bits */
+uint32_t length;
+const char *name;
+} Leaf;
+
+union Tree {
+bool is_leaf;
+Branch branch;
+Leaf leaf;
+};
+
+/* Additional (generated) instruction data */
+typedef struct {
+const Instruction *instruction;
+/* Instruction length in bits */
+uint32_t length;
+WEIGHT_T weight;
+/*
+ * Bit pattern matched in opcodes.
+ * For each 1 in mask, the same bit in the opcode must match that from 
bits.
+ */
+OPCODE_T bits;
+OPCODE_T mask;
+} Pattern;
+
+/* Cached decoding tree */
+Tree *cache;
+
+/* Return calculated bit pattern and length for an instruction */
+static Pattern get_info(const Instruction *instruction)
+{
+OPCODE_T bit = 1 << (OPCODE_SIZE - 1);
+OPCODE_T bits = 0;
+OP

[Qemu-devel] [PATCH v1 0/8] DRAFT AVR Patches

2019-05-04 Thread Sarah Harris
This series of patches adds support for 8 bit Atmel (Microchip) AVR 
microcontrollers.
All documented instructions except DES, SPM, and WDR are implemented.
These patches include very incomplete peripheral emulation, and only a single 
example board definition.

All instructions except LAC, LAS, LAT, XCH, BREAK, and SLEEP have been tested 
by comparing their behaviours against hardware.
The test programs used were designed specifically to exercise as many 
instruction variants as possible.
More details, source code, and results are available here: 
https://github.com/seharris/qemu-avr-tests/tree/master/instruction-tests

Additionally, it has been confirmed that this emulation can run FreeRTOS (an 
open source realtime operating system).
AVRs don't have memory management hardware and typically only have a few 
kilobytes of RAM so booting something more standard, e.g. Linux, wasn't 
feasible.
Two peripherals were needed (USART and 16 bit timer) for this test and are 
included in these patches.
The implementations are somewhat limited, mostly because QEMU doesn't seem to 
have much in the way of facilities to handle low-level electrical interfaces 
like GPIO pins.
A simple LED indicator peripheral was also used, but is not included because it 
isn't likely to be generally useful.
The FreeRTOS build and LED patch are available here: 
https://github.com/seharris/qemu-avr-tests/tree/master/free-rtos

These patches are based on work by Michael Rolnik, last discussed here in 2017: 
https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg02290.html
This series is derived from the version found in this repository: 
https://github.com/michaelrolnik/qemu-avr
Changes from that version:
- rebase onto current master
- fixes for some accumulated bitrot (including a crash at startup)
- minor improvements to sample board firmware loading
- fixes for bugs in instruction translations (POP, ASR, LSR, ROR, FMUL, FMULS, 
FMULSU, MUL, MULS, MULSU, OR, SBC, SBCI)
- new instruction decoder to avoid some awkward dependencies
- general cleanup (style fixes, fixing unclear comments, making code easier to 
follow)

On a personal note, I'm unfamiliar with this style of submission so I hope I 
haven't broken anything!

Sarah Harris (8):
  target/avr: Add instruction decoder
  target/avr: Add mechanism to check for active debugger connection
  target/avr: Add outward facing interfaces and core CPU logic
  target/avr: Add instruction helpers
  target/avr: Add instruction translation
  target/avr: Add limited support for USART and 16 bit timer peripherals
  target/avr: Add example board configuration
  target/avr: Register AVR support with the rest of QEMU, the build
system, and the MAINTAINERS file

 MAINTAINERS |6 +
 arch_init.c |2 +
 configure   |6 +
 default-configs/avr-softmmu.mak |5 +
 gdbstub.c   |5 +
 hw/Kconfig  |1 +
 hw/avr/Kconfig  |4 +
 hw/avr/Makefile.objs|1 +
 hw/avr/sample.c |  177 ++
 hw/char/Kconfig |3 +
 hw/char/Makefile.objs   |1 +
 hw/char/avr_usart.c |  316 
 hw/timer/Kconfig|3 +
 hw/timer/Makefile.objs  |1 +
 hw/timer/avr_timer16.c  |  587 ++
 include/disas/dis-asm.h |6 +
 include/exec/gdbstub.h  |4 +
 include/hw/char/avr_usart.h |   99 +
 include/hw/timer/avr_timer16.h  |   99 +
 include/sysemu/arch_init.h  |1 +
 qapi/common.json|2 +-
 target/avr/Makefile.objs|   23 +
 target/avr/cpu-qom.h|   83 +
 target/avr/cpu.c|  570 ++
 target/avr/cpu.h|  238 +++
 target/avr/decode.c |  441 +
 target/avr/decode.h |   68 +
 target/avr/gdbstub.c|   85 +
 target/avr/helper.c |  343 
 target/avr/helper.h |   28 +
 target/avr/machine.c|  122 ++
 target/avr/translate-inst.h |  695 +++
 target/avr/translate.c  | 3013 +++
 tests/machine-none-test.c   |1 +
 34 files changed, 7038 insertions(+), 1 deletion(-)
 create mode 100644 default-configs/avr-softmmu.mak
 create mode 100644 hw/avr/Kconfig
 create mode 100644 hw/avr/Makefile.objs
 create mode 100644 hw/avr/sample.c
 create mode 100644 hw/char/avr_usart.c
 create mode 100644 hw/timer/avr_timer16.c
 create mode 100644 include/hw/char/avr_usart.h
 create mode 100644 include/hw/timer/avr_timer16.h
 create mode 100644 target/avr/Makefile.objs
 create mode 100644 target/avr/cpu-qom.h
 create mode 100644 target/avr/cpu.c
 create mode 100644 target/avr/cpu.h
 create mode 100644 target/avr/decode.c
 create mode 100644 target/avr/decode.h
 create mode 100644 target/avr/gdbstub.c
 create mode 100644 target/avr/helper.c
 create mode 100644 target/avr/helper.h
 create mode 100644 target

[Qemu-devel] [PATCH v1 8/8] target/avr: Register AVR support with the rest of QEMU, the build system, and the MAINTAINERS file

2019-05-04 Thread Sarah Harris
Signed-off-by: Sarah Harris 
---
 MAINTAINERS |  6 ++
 arch_init.c |  2 ++
 configure   |  6 ++
 default-configs/avr-softmmu.mak |  5 +
 include/disas/dis-asm.h |  6 ++
 include/sysemu/arch_init.h  |  1 +
 qapi/common.json|  2 +-
 target/avr/Makefile.objs| 23 +++
 tests/machine-none-test.c   |  1 +
 9 files changed, 51 insertions(+), 1 deletion(-)
 create mode 100644 default-configs/avr-softmmu.mak
 create mode 100644 target/avr/Makefile.objs

diff --git a/MAINTAINERS b/MAINTAINERS
index 7dd71e0a2d..859ceb2d08 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -162,6 +162,12 @@ S: Maintained
 F: hw/arm/smmu*
 F: include/hw/arm/smmu*
 
+AVR
+M: Michael Rolnik 
+S: Odd Fixes
+F: target-avr/
+F: hw/avr/
+
 CRIS
 M: Edgar E. Iglesias 
 S: Maintained
diff --git a/arch_init.c b/arch_init.c
index f4f3f610c8..184cdca6dd 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -86,6 +86,8 @@ int graphic_depth = 32;
 #define QEMU_ARCH QEMU_ARCH_UNICORE32
 #elif defined(TARGET_XTENSA)
 #define QEMU_ARCH QEMU_ARCH_XTENSA
+#elif defined(TARGET_AVR)
+#define QEMU_ARCH QEMU_ARCH_AVR
 #endif
 
 const uint32_t arch_type = QEMU_ARCH;
diff --git a/configure b/configure
index 60719ddcc5..b8843059b5 100755
--- a/configure
+++ b/configure
@@ -7451,6 +7451,9 @@ case "$target_name" in
 target_compiler=$cross_cc_aarch64
 eval "target_compiler_cflags=\$cross_cc_cflags_${target_name}"
   ;;
+  avr)
+target_compiler=$cross_cc_avr
+  ;;
   cris)
 target_compiler=$cross_cc_cris
   ;;
@@ -7726,6 +7729,9 @@ for i in $ARCH $TARGET_BASE_ARCH ; do
   disas_config "ARM_A64"
 fi
   ;;
+  avr)
+disas_config "AVR"
+  ;;
   cris)
 disas_config "CRIS"
   ;;
diff --git a/default-configs/avr-softmmu.mak b/default-configs/avr-softmmu.mak
new file mode 100644
index 00..d1e1c28118
--- /dev/null
+++ b/default-configs/avr-softmmu.mak
@@ -0,0 +1,5 @@
+# Default configuration for avr-softmmu
+
+# Boards:
+#
+CONFIG_AVR_SAMPLE=y
diff --git a/include/disas/dis-asm.h b/include/disas/dis-asm.h
index 9240ec32c2..a7d230ba66 100644
--- a/include/disas/dis-asm.h
+++ b/include/disas/dis-asm.h
@@ -211,6 +211,12 @@ enum bfd_architecture
 #define bfd_mach_m32r  0  /* backwards compatibility */
   bfd_arch_mn10200,/* Matsushita MN10200 */
   bfd_arch_mn10300,/* Matsushita MN10300 */
+  bfd_arch_avr,   /* Atmel AVR microcontrollers.  */
+#define bfd_mach_avr1  1
+#define bfd_mach_avr2  2
+#define bfd_mach_avr3  3
+#define bfd_mach_avr4  4
+#define bfd_mach_avr5  5
   bfd_arch_cris,   /* Axis CRIS */
 #define bfd_mach_cris_v0_v10   255
 #define bfd_mach_cris_v32  32
diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index 10cbafe970..aff57bfe61 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -25,6 +25,7 @@ enum {
 QEMU_ARCH_NIOS2 = (1 << 17),
 QEMU_ARCH_HPPA = (1 << 18),
 QEMU_ARCH_RISCV = (1 << 19),
+QEMU_ARCH_AVR = (1 << 20),
 };
 
 extern const uint32_t arch_type;
diff --git a/qapi/common.json b/qapi/common.json
index 99d313ef3b..eeacd0e3c2 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -187,7 +187,7 @@
 # Since: 3.0
 ##
 { 'enum' : 'SysEmuTarget',
-  'data' : [ 'aarch64', 'alpha', 'arm', 'cris', 'hppa', 'i386', 'lm32',
+  'data' : [ 'aarch64', 'alpha', 'arm', 'avr', 'cris', 'hppa', 'i386', 'lm32',
  'm68k', 'microblaze', 'microblazeel', 'mips', 'mips64',
  'mips64el', 'mipsel', 'moxie', 'nios2', 'or1k', 'ppc',
  'ppc64', 'riscv32', 'riscv64', 's390x', 'sh4',
diff --git a/target/avr/Makefile.objs b/target/avr/Makefile.objs
new file mode 100644
index 00..41355dea1e
--- /dev/null
+++ b/target/avr/Makefile.objs
@@ -0,0 +1,23 @@
+#
+#  QEMU AVR CPU
+#
+#  Copyright (c) 2016 Michael Rolnik
+#
+#  This library is free software; you can redistribute it and/or
+#  modify it under the terms of the GNU Lesser General Public
+#  License as published by the Free Software Foundation; either
+#  version 2.1 of the License, or (at your option) any later version.
+#
+#  This library 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
+#  Lesser General Public License for more details.
+#
+#  You should have received a copy of the GNU Lesser General Public
+#  License along with this library; if not, see
+#  <http://www.gnu.org/licenses/lgpl-2.1.html>
+#
+
+obj-y += translate.o cpu.o helper.o decode.o
+obj-y += gdbstub.o
+obj-$(CONFIG_SOFTMMU) += machine.o
diff --git a/tests/machine-none-test.c b/tests/machine-none-test.c
index 4c6d470798..361927bb76 100644
--- a/tests/machine-none-test.c
+++ b/tests/machine-none-test.c
@@ 

[Qemu-devel] [PATCH v1 2/8] target/avr: Add mechanism to check for active debugger connection

2019-05-04 Thread Sarah Harris
AVR CPUs have a BREAK instruction which behaves differently depending on 
whether debugging is enabled.
Since the hardware fuses that normally control this are difficult to emulate, 
and the BREAK instruction is useful for testing, the BREAK instruction is 
instead enabled/disabled depending on whether a GDB session is attached.

Signed-off-by: Sarah Harris 
---
 gdbstub.c  | 5 +
 include/exec/gdbstub.h | 4 
 2 files changed, 9 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index d54abd17cc..a254a364e6 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1793,6 +1793,11 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 return RS_IDLE;
 }
 
+bool gdb_is_active(void)
+{
+return gdbserver_state != NULL;
+}
+
 void gdb_set_stop_cpu(CPUState *cpu)
 {
 GDBProcess *p = gdb_get_cpu_process(gdbserver_state, cpu);
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 08363969c1..d059bf5339 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -45,6 +45,10 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char 
*fmt, ...);
  */
 void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va);
 int use_gdb_syscalls(void);
+/**
+ * gdb_is_active: return true if debugging in progress
+ */
+bool gdb_is_active(void);
 void gdb_set_stop_cpu(CPUState *cpu);
 void gdb_exit(CPUArchState *, int);
 #ifdef CONFIG_USER_ONLY
-- 
2.21.0