From: KONRAD Frederic <fred.konrad@greensocs.com>

This patch-set allows to execute code from mmio areas.
The main goal of this is to be able to run code for example from an SPI device.

The three first patch fixes the way get_page_addr_code fills the TLB.

The fourth patch implements the mmio execution helpers: the device must
implement the request_ptr callback of the MemoryRegion and will be notified when
the guest wants to execute code from it.

The fifth patch implements the execution from the SPI memories in the
xilinx_spips model.

Thanks,
Fred

KONRAD Frederic (5):
  cputlb: cleanup get_page_addr_code to use VICTIM_TLB_HIT
  cputlb: move get_page_addr_code
  cputlb: fix the way get_page_addr_code fills the tlb
  exec: allow to get a pointer for some mmio memory region
  xilinx_spips: allow mmio execution

 cputlb.c              | 81 ++++++++++++++++++++++++++++-----------------------
 hw/ssi/xilinx_spips.c | 74 ++++++++++++++++++++++++++++++++++------------
 include/exec/memory.h | 35 ++++++++++++++++++++++
 memory.c              | 45 ++++++++++++++++++++++++++++
 4 files changed, 180 insertions(+), 55 deletions(-)

-- 
1.8.3.1

On 3 February 2017 at 17:06,  <fred.konrad@greensocs.com> wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
>
> This patch-set allows to execute code from mmio areas.
> The main goal of this is to be able to run code for example from an SPI device.
>
> The three first patch fixes the way get_page_addr_code fills the TLB.
>
> The fourth patch implements the mmio execution helpers: the device must
> implement the request_ptr callback of the MemoryRegion and will be notified when
> the guest wants to execute code from it.
>
> The fifth patch implements the execution from the SPI memories in the
> xilinx_spips model.

I like the general idea, but there's an awkward issue:
at the moment our translation system assumes that when we're
translating code then if the first instruction in the TB
can be read OK then we won't ever get a fault trying to
read subsequent bytes up to the end of the page. If we
move from "we only translate code out of whole pages of
RAM" to "we might translate code out of devices that
are in subpages" then this assumption gets broken.
(The symptom would be that we would report the fault
in the wrong place, for the PC at the start of the TB.)

thanks
-- PMM

On 02/04/2017 01:33 PM, Peter Maydell wrote:
> On 3 February 2017 at 17:06,  <fred.konrad@greensocs.com> wrote:
>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>>
>> This patch-set allows to execute code from mmio areas.
>> The main goal of this is to be able to run code for example from an SPI device.
>>
>> The three first patch fixes the way get_page_addr_code fills the TLB.
>>
>> The fourth patch implements the mmio execution helpers: the device must
>> implement the request_ptr callback of the MemoryRegion and will be notified when
>> the guest wants to execute code from it.
>>
>> The fifth patch implements the execution from the SPI memories in the
>> xilinx_spips model.
> 
> I like the general idea, but there's an awkward issue:
> at the moment our translation system assumes that when we're
> translating code then if the first instruction in the TB
> can be read OK then we won't ever get a fault trying to
> read subsequent bytes up to the end of the page. If we
> move from "we only translate code out of whole pages of
> RAM" to "we might translate code out of devices that
> are in subpages" then this assumption gets broken.
> (The symptom would be that we would report the fault
> in the wrong place, for the PC at the start of the TB.)
> 
> thanks
> -- PMM
> 

Hi Peter,

I think I see your point.
Is that the case that we might get a Bad RAM address error or some such
if we are not on a page boundary (or too small as you say)?
I guess this is a limitation. Mapping on a page boundary shouldn't be
too much restrictive.

Thanks,
Fred

On 4 February 2017 at 12:52, Frederic Konrad <fred.konrad@greensocs.com> wrote:
> Is that the case that we might get a Bad RAM address error or some such
> if we are not on a page boundary (or too small as you say)?
> I guess this is a limitation. Mapping on a page boundary shouldn't be
> too much restrictive.

Yeah. I really ought to look more closely at what the flow of
execution is here, because I think how it works right now
is a bit weird and works as much by luck as by judgement
(we can longjump out of the middle of translating code
right back to the cpu-exec.c loop, and in some cases
I think what happens is that we try to translate code,
and as part of the "load didn't work" code path we
nestedly try to translate the same thing again which
of course fails again, only the second time around we
realize and longjump out.

(At the moment for linux-user mode this is causing us to
assert about taking the tb lock twice, because we hold
the tb lock during translation and then try to grab it
again to do the cpu_restore_state() in the signal handler.)

thanks
-- PMM

On 02/04/2017 02:17 PM, Peter Maydell wrote:
> On 4 February 2017 at 12:52, Frederic Konrad <fred.konrad@greensocs.com> wrote:
>> Is that the case that we might get a Bad RAM address error or some such
>> if we are not on a page boundary (or too small as you say)?
>> I guess this is a limitation. Mapping on a page boundary shouldn't be
>> too much restrictive.
> 
> Yeah. I really ought to look more closely at what the flow of
> execution is here, because I think how it works right now
> is a bit weird and works as much by luck as by judgement
> (we can longjump out of the middle of translating code
> right back to the cpu-exec.c loop, and in some cases
> I think what happens is that we try to translate code,
> and as part of the "load didn't work" code path we
> nestedly try to translate the same thing again which
> of course fails again, only the second time around we
> realize and longjump out.
> 
> (At the moment for linux-user mode this is causing us to
> assert about taking the tb lock twice, because we hold
> the tb lock during translation and then try to grab it
> again to do the cpu_restore_state() in the signal handler.)
> 

Yes it seems there are some scary things happening there.

Fred

> thanks
> -- PMM
>