On Fri, May 09, 2014 at 10:15:29PM +0100, James Hogan wrote:
> Hi Andreas,
>
> On 06/05/14 16:51, Andreas Herrmann wrote:
> > From: David Daney <david.daney@xxxxxxxxxx>
> >
> > So far this was tested with host running KVM using MIPS-VZ (on Cavium
> > Octeon3). A paravirtualized mips kernel was used for the guest.
> >
> > [andreas.herrmann:
> > * Renamed kvm__arch_periodic_poll to kvm__arch_read_term
> > because of commit fa817d892508b6d3a90f478dbeedbe5583b14da7
> > (kvm tools: remove periodic tick in favour of a polling thread)
> > * Added ioport__map_irq skeleton to fix build problem.
> > * Rely on TERM_MAX_DEVS instead of using other macros
> > * Adaptions for MMIO support
> > * Set coalesc offset
> > * Fixed compile warnings]
> >
> > Signed-off-by: David Daney <david.daney@xxxxxxxxxx>
> > Signed-off-by: Andreas Herrmann <andreas.herrmann@xxxxxxxxxxxxxxxxxx>
>
>
> > +static bool kvm_cpu__hypercall_write_cons(struct kvm_cpu *vcpu)
> > +{
> > + int term = (int)vcpu->kvm_run->hypercall.args[0];
> > + u64 addr = vcpu->kvm_run->hypercall.args[1];
> > + int len = (int)vcpu->kvm_run->hypercall.args[2];
> > + char *host_addr;
> > +
> > + if (term < 0 || term >= TERM_MAX_DEVS) {
> > + pr_warning("hypercall_write_cons term out of range <%d>", term);
> > + return false;
> > + }
> > + if (len <= 0) {
> > + pr_warning("hypercall_write_cons len out of range <%d>", len);
> > + return false;
> > + }
> > +
> > + if ((addr & 0xffffffffc0000000ull) == 0xffffffff80000000ull)
> > + addr &= 0x1ffffffful; /* Convert KSEG{0,1} to physical. */
> > + if ((addr & 0xc000000000000000ull) == 0x8000000000000000ull)
> > + addr &= 0x07ffffffffffffffull; /* Convert XKPHYS to pysical */
> > +
> > + host_addr = guest_flat_to_host(vcpu->kvm, addr);
> > + if (!host_addr) {
> > + pr_warning("hypercall_write_cons unmapped physaddr %llx", (unsigned long long)addr);
> > + return false;
> > + }
> > +
> > + term_putc(host_addr, len, term);
>
> Does len need to be range checked?
len <= 0 is checked above.
I don't think an upper boundery check is required.
term_putc (using write) should be able to handle it.
No?
> > +void kvm_cpu__show_registers(struct kvm_cpu *vcpu)
> > +{
> > + struct kvm_regs regs;
> > +
> > + if (ioctl(vcpu->vcpu_fd, KVM_GET_REGS, ®s) < 0)
> > + die("KVM_GET_REGS failed");
> > + dprintf(debug_fd, "\n Registers:\n");
> > + dprintf(debug_fd, " ----------\n");
> > + dprintf(debug_fd, "$0 : %016lx %016lx %016lx %016lx\n",
> > + (unsigned long)regs.gpr[0], (unsigned long)regs.gpr[1],
> > + (unsigned long)regs.gpr[2], (unsigned long)regs.gpr[3]);
>
> Presumably there's nothing stopping a 32-bit userland from creating a
> 64-bit guest?
Yes, that can be run.
> If that's the case should this all use unsigned long longs?
... and yes it creates wrong register dump.
Will fix this.
> > + dprintf(debug_fd, "$4 : %016lx %016lx %016lx %016lx\n",
> > + (unsigned long)regs.gpr[4], (unsigned long)regs.gpr[5],
> > + (unsigned long)regs.gpr[6], (unsigned long)regs.gpr[7]);
> > + dprintf(debug_fd, "$8 : %016lx %016lx %016lx %016lx\n",
> > + (unsigned long)regs.gpr[8], (unsigned long)regs.gpr[9],
> > + (unsigned long)regs.gpr[10], (unsigned long)regs.gpr[11]);
> > + dprintf(debug_fd, "$12 : %016lx %016lx %016lx %016lx\n",
> > + (unsigned long)regs.gpr[12], (unsigned long)regs.gpr[13],
> > + (unsigned long)regs.gpr[14], (unsigned long)regs.gpr[15]);
> > + dprintf(debug_fd, "$16 : %016lx %016lx %016lx %016lx\n",
> > + (unsigned long)regs.gpr[16], (unsigned long)regs.gpr[17],
> > + (unsigned long)regs.gpr[18], (unsigned long)regs.gpr[19]);
> > + dprintf(debug_fd, "$20 : %016lx %016lx %016lx %016lx\n",
> > + (unsigned long)regs.gpr[20], (unsigned long)regs.gpr[21],
> > + (unsigned long)regs.gpr[22], (unsigned long)regs.gpr[23]);
> > + dprintf(debug_fd, "$24 : %016lx %016lx %016lx %016lx\n",
> > + (unsigned long)regs.gpr[24], (unsigned long)regs.gpr[25],
> > + (unsigned long)regs.gpr[26], (unsigned long)regs.gpr[27]);
> > + dprintf(debug_fd, "$28 : %016lx %016lx %016lx %016lx\n",
> > + (unsigned long)regs.gpr[28], (unsigned long)regs.gpr[29],
> > + (unsigned long)regs.gpr[30], (unsigned long)regs.gpr[31]);
> > +
> > + dprintf(debug_fd, "hi : %016lx\n", (unsigned long)regs.hi);
> > + dprintf(debug_fd, "lo : %016lx\n", (unsigned long)regs.lo);
> > + dprintf(debug_fd, "epc : %016lx\n", (unsigned long)regs.pc);
> > +
> > + dprintf(debug_fd, "\n");
> > +}
>
> Cheers
> James
Thanks,
Andreas
